From 459ed0a4d73b33a1a3c2d08c8e400f2d66af2a79 Mon Sep 17 00:00:00 2001 From: Beatrice0377 <2296143245@qq.com> Date: Fri, 28 Aug 2026 16:32:33 +0800 Subject: [PATCH 1/2] spec : fix ngram-cache state leaking across requests 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 --- common/speculative.cpp | 8 ++++-- tests/CMakeLists.txt | 1 + tests/test-ngram-cache.cpp | 56 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 63 insertions(+), 2 deletions(-) create mode 100644 tests/test-ngram-cache.cpp diff --git a/common/speculative.cpp b/common/speculative.cpp index d34d1c9c595..489cee90f95 100644 --- a/common/speculative.cpp +++ b/common/speculative.cpp @@ -2142,8 +2142,12 @@ struct common_speculative_impl_ngram_cache : public common_speculative_impl { } } - void begin(llama_seq_id /*seq_id*/, const llama_tokens & /*prompt*/) override { - // noop + void begin(llama_seq_id seq_id, const llama_tokens & /*prompt*/) override { + auto & sinfo = sinfos[seq_id]; + + // per-request state - do not carry it over to the next request on this sequence + sinfo.ngram_cache_context.clear(); + sinfo.cache_size = 0; } void draft_one( diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index fe3d14ffc55..3cf8c2c6e90 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -289,6 +289,7 @@ llama_build_and_test(test-thread-safety.cpp ARGS -m "${MODEL_DEST}" -ngl 99 -p " set_tests_properties(test-thread-safety PROPERTIES FIXTURES_REQUIRED test-download-model) llama_build_and_test(test-arg-parser.cpp) +llama_build_and_test(test-ngram-cache.cpp) llama_build_and_test(test-model-resolution.cpp) # the test serves its repos from an httplib server, and the library links it privately target_link_libraries(test-model-resolution PRIVATE cpp-httplib) diff --git a/tests/test-ngram-cache.cpp b/tests/test-ngram-cache.cpp new file mode 100644 index 00000000000..0674cb134d7 --- /dev/null +++ b/tests/test-ngram-cache.cpp @@ -0,0 +1,56 @@ +#undef NDEBUG +#include "speculative.h" + +#include +#include +#include + +// ngram-cache keeps request-local state (context n-gram cache and its size) per sequence. +// A new request on the same seq_id must not reuse the n-grams ingested from the previous +// request, otherwise the draft is computed from stale context (see issue #27852). +static void test_ngram_cache_begin_resets_request_state() { + common_params_speculative params; + params.types = { COMMON_SPECULATIVE_TYPE_NGRAM_CACHE }; + + const uint32_t n_seq = 1; + common_speculative_ptr spec(common_speculative_init(params, n_seq)); + assert(spec != nullptr); + + // request 1: ingest a prompt so that the n-gram (104, 105, 106) -> 107 + // ends up in the per-sequence context cache + llama_tokens prompt1 = { 101, 102, 103, 104, 105, 106 }; + common_speculative_begin(spec.get(), 0, prompt1); + + llama_tokens result1; + auto & dp = common_speculative_get_draft_params(spec.get(), 0); + dp.drafting = true; + dp.prompt = &prompt1; + dp.id_last = 107; + dp.result = &result1; + common_speculative_draft(spec.get()); + + // request 2: new, shorter prompt on the same seq_id + // its ending n-gram (104, 105, 106) matches the stale cache from request 1, + // which would draft token 107 if the per-sequence state was not reset + llama_tokens prompt2 = { 100, 104, 105 }; + common_speculative_begin(spec.get(), 0, prompt2); + + llama_tokens result2; + dp.drafting = true; + dp.prompt = &prompt2; + dp.id_last = 106; + dp.result = &result2; + common_speculative_draft(spec.get()); + + // no draft must be produced for request 2 - the reset context cache + // contains no n-gram that matches the new prompt + assert(result2.empty()); +} + +int main() { + test_ngram_cache_begin_resets_request_state(); + + printf("test-ngram-cache: all tests passed\n"); + + return 0; +} From 8492c2ecd9ae2a45b4f89520c6f26d22696758f7 Mon Sep 17 00:00:00 2001 From: Beatrice0377 <2296143245@qq.com> Date: Fri, 28 Aug 2026 17:36:19 +0800 Subject: [PATCH 2/2] tests : move ngram-cache regression test into test-arg-parser Adds no new test file, per project convention. Assisted-by: Sisyphus --- tests/CMakeLists.txt | 1 - tests/test-arg-parser.cpp | 34 +++++++++++++++++++++++ tests/test-ngram-cache.cpp | 56 -------------------------------------- 3 files changed, 34 insertions(+), 57 deletions(-) delete mode 100644 tests/test-ngram-cache.cpp diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 3cf8c2c6e90..fe3d14ffc55 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -289,7 +289,6 @@ llama_build_and_test(test-thread-safety.cpp ARGS -m "${MODEL_DEST}" -ngl 99 -p " set_tests_properties(test-thread-safety PROPERTIES FIXTURES_REQUIRED test-download-model) llama_build_and_test(test-arg-parser.cpp) -llama_build_and_test(test-ngram-cache.cpp) llama_build_and_test(test-model-resolution.cpp) # the test serves its repos from an httplib server, and the library links it privately target_link_libraries(test-model-resolution PRIVATE cpp-httplib) diff --git a/tests/test-arg-parser.cpp b/tests/test-arg-parser.cpp index e0907631abd..2de295f2eec 100644 --- a/tests/test-arg-parser.cpp +++ b/tests/test-arg-parser.cpp @@ -101,6 +101,40 @@ static void test(void) { assert(draft.n_outputs_max_per_seq == 1); } + // ngram-cache keeps per-sequence state across requests; begin() must reset it + { + common_params_speculative spec; + spec.types = { COMMON_SPECULATIVE_TYPE_NGRAM_CACHE }; + + common_speculative_ptr drafter(common_speculative_init(spec, 1)); + assert(drafter != nullptr); + + // request 1: ingest (104, 105, 106) -> 107 into the context cache + llama_tokens prompt1 = { 101, 102, 103, 104, 105, 106 }; + common_speculative_begin(drafter.get(), 0, prompt1); + + llama_tokens result1; + auto & dp = common_speculative_get_draft_params(drafter.get(), 0); + dp.drafting = true; + dp.prompt = &prompt1; + dp.id_last = 107; + dp.result = &result1; + common_speculative_draft(drafter.get()); + + // request 2: same seq_id, ending n-gram only exists in request 1's cache, so stale state drafts 107 + llama_tokens prompt2 = { 100, 104, 105 }; + common_speculative_begin(drafter.get(), 0, prompt2); + + llama_tokens result2; + dp.drafting = true; + dp.prompt = &prompt2; + dp.id_last = 106; + dp.result = &result2; + common_speculative_draft(drafter.get()); + + assert(result2.empty()); + } + printf("test-arg-parser: make sure there is no duplicated arguments in any examples\n\n"); for (int ex = 0; ex < LLAMA_EXAMPLE_COUNT; ex++) { try { diff --git a/tests/test-ngram-cache.cpp b/tests/test-ngram-cache.cpp deleted file mode 100644 index 0674cb134d7..00000000000 --- a/tests/test-ngram-cache.cpp +++ /dev/null @@ -1,56 +0,0 @@ -#undef NDEBUG -#include "speculative.h" - -#include -#include -#include - -// ngram-cache keeps request-local state (context n-gram cache and its size) per sequence. -// A new request on the same seq_id must not reuse the n-grams ingested from the previous -// request, otherwise the draft is computed from stale context (see issue #27852). -static void test_ngram_cache_begin_resets_request_state() { - common_params_speculative params; - params.types = { COMMON_SPECULATIVE_TYPE_NGRAM_CACHE }; - - const uint32_t n_seq = 1; - common_speculative_ptr spec(common_speculative_init(params, n_seq)); - assert(spec != nullptr); - - // request 1: ingest a prompt so that the n-gram (104, 105, 106) -> 107 - // ends up in the per-sequence context cache - llama_tokens prompt1 = { 101, 102, 103, 104, 105, 106 }; - common_speculative_begin(spec.get(), 0, prompt1); - - llama_tokens result1; - auto & dp = common_speculative_get_draft_params(spec.get(), 0); - dp.drafting = true; - dp.prompt = &prompt1; - dp.id_last = 107; - dp.result = &result1; - common_speculative_draft(spec.get()); - - // request 2: new, shorter prompt on the same seq_id - // its ending n-gram (104, 105, 106) matches the stale cache from request 1, - // which would draft token 107 if the per-sequence state was not reset - llama_tokens prompt2 = { 100, 104, 105 }; - common_speculative_begin(spec.get(), 0, prompt2); - - llama_tokens result2; - dp.drafting = true; - dp.prompt = &prompt2; - dp.id_last = 106; - dp.result = &result2; - common_speculative_draft(spec.get()); - - // no draft must be produced for request 2 - the reset context cache - // contains no n-gram that matches the new prompt - assert(result2.empty()); -} - -int main() { - test_ngram_cache_begin_resets_request_state(); - - printf("test-ngram-cache: all tests passed\n"); - - return 0; -}