Repository navigation
fix: count pooled blocks from the restored keys, not the donor table - #650
Open
goodbadwolf wants to merge 1 commit into
Open
goodbadwolf wants to merge 1 commit into
goodbadwolf wants to merge 1 commit into
Conversation
A conversation restored from the SSD tier below its end kept the donor's whole pooled indexer table, and the cache counted its valid blocks from that table's length. The indexer then computed no blocks for the new tokens and scored attention against another conversation's keys, so the model lost the request (youssofal#635).
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
This branch has not been deployed
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
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.
Summary
A conversation restored from the SSD tier below its end loses the user's request on Flash-Next. The sub-prefix decoder cuts the keys and values to the restore point, but the indexer's raw and pooled tensors are stored whole, so they arrive at the donor entry's full length, and
QSACache.statetook its count of valid pooled blocks from that length. The indexer then found every block of the new tokens already pooled, computed none, and scored attention against the donor conversation's block keys, so the model answered from facts that are not in the prompt (#635). A RAM restore does not show this because it loads the full state and trims it, which clamps the count.Changes
Verification
Three new tests in
tests/test_qsa_state_restore_frontier.py: the prefix-restore case fails on v2.12.2 and passes on this branch, and the full-state round trip and the missing-table case pass on both.On the real stored entry that failed (58,073 tokens, restore point 2,048), the old setter gave
pooled_len=14518with the keys at 2,048 rows, and the next 4,096-token chunk recomputed 0 pooled blocks where 1,024 were needed. With this changepooled_len=512, the value a RAM restore and trim gives.End to end on
Youssofal/Qwen3.8-Flash-Next-MTPLX-Optimized-Speedat34ae535f, MacBook Pro M5 Max with 128 GB on macOS 27.0.1, 2026-10-20: a Claude Code request at temperature 0 with thinking on, restored from the SSD tier at 8,192 tokens from an entry holding a different conversation, with the RAM bank cleared before each run. Built from this branch, 5 of 5 runs found the task, at a first-depth draft acceptance of 0.89. On v2.12.2, 2 of 2 runs answered that there was no task, at 0.64, and the same request withx-mtplx-cache-mode: bypassfound it at 0.85.The 20 existing test files that mention
QSACacheorpooled_lengive the same results before and after: one failure in both,tests/test_one_copy_reserve_resize.py::test_the_resize_allocates_its_bill_and_no_more. The three files named in CONTRIBUTING.md show the same 14 failures on v2.12.2 and on this branch in my environment, which runs them through a custom launcher.python -m buildandscripts/fresh_venv_smoke.shwere not run.Not examined: the cross-session entry sharing described in the second comment on #635, and cache families other than QSA.
Benchmark Evidence
This does not change runtime performance. A restore that previously skipped the pooled-block work now does it for the new tokens only.