spec : reset ngram-cache state between requests - #27866
Open
Beatrice0377 wants to merge 2 commits into
Open
Conversation
|
Hi @Beatrice0377, thanks for your contribution! Per our contribution guidelines, the automated PR checker found the following issue(s) that need your attention:
Please note that maintainers reserve the right to make final decisions on PRs. If you believe there is a mistake, please comment below. |
ggml-gh-bot
Bot
added
the
draft
github-actions
Bot
removed
the
draft
Beatrice0377
force-pushed
the
fix-ngram-cache-request-state
branch
from
August 28, 2026 09:44
e67c784 to
2e61742
Compare
Reset the per-sequence context n-gram cache and its size when a new request begins (begin). Previously begin was a no-op, so a sequence reused by a new request kept the n-grams ingested by the previous request. This made the draft for the new request depend on stale context, e.g. dropping acceptance from 86% to 11% in server scenarios that reuse slots across requests. The static and dynamic caches are intentionally shared across requests and are left untouched. Assisted-by: Sisyphus
Adds no new test file, per project convention. Assisted-by: Sisyphus
Beatrice0377
force-pushed
the
fix-ngram-cache-request-state
branch
from
August 28, 2026 09:54
2e61742 to
8492c2e
Compare
Beatrice0377
marked this pull request as ready for review
August 28, 2026 09:56
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
common_speculative_impl_ngram_cache::begin()was a no-op, so a new request on a reused slot kept the previous request's per-sequence context n-gram cache and itscache_sizeoffset. The new prompt was then either never re-ingested (shorter prompt) or ingested from the wrong offset, and drafts came from stale n-grams (acceptance dropped from 86% to 11% in the report).begin()now clearssinfo.ngram_cache_contextand resetssinfo.cache_sizeto 0. The static and dynamic caches are shared across requests by design and are not touched.The regression test in
tests/test-arg-parser.cppruns two requests on the sameseq_id: the second one ends with an n-gram that only exists in the first request's context cache, and it asserts the second request produces no draft. It fails before the fix and passes after.Requirements