Repository navigation
fix(codemode): send catalog deltas instead of restating the whole catalog - #60129
Closed
Neko-Yukari wants to merge 1 commit into
Closed
Neko-Yukari wants to merge 1 commit into
Neko-Yukari wants to merge 1 commit into
Conversation
Contributor
|
Thanks for this, and for the before/after measurements. They're really useful. I think this duplicates #57090, which tackles the same problem: when the tool inventory changes,
Your approach goes further: real deltas, and the inline sample no longer counts as a change. If you think #57090 is missing that, it would help to say so on #57090 or #55529, with your MCP toggle numbers, so the maintainers can decide which direction to take. |
3 of 6 tasks
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.
Issue for this PR
Closes #60128
Type of change
What does this PR do?
The catalog identity is now the inventory instead of the inline sample.
snapshot()keeps the sorted listings plus a fingerprint of them,select()still picks what to inline for rendering, andupdate()describes the difference: a new namespace with its tool count, a changed namespace inventory, new or changed listing lines, and removed tools named by their call expression. The whole block is still sent when a delta would not be shorter: no previous state, an emptied catalog, or a delta that loses on length.Because the sample is no longer part of the identity, rotation between steps stops counting as a change by itself.
Stored state changes shape from
SummarytoSnapshot. An older value fails to decode and is treated as absent, so the first change after upgrading sends one full block and then continues with deltas. No migration is needed.How did you verify your code works?
Two builds, one session each, the same six MCP disconnect/connect toggles after a 15 s warm-up so the baseline catalog is the full inventory:
From
packages/core:bun test test/codemode(33 pass),bun test test/instructions(17 pass),bun run typecheck:tsc(clean).New cases cover an added namespace, a changed namespace inventory, a changed listing that must not be reported as a removal, a description-only change, the "never longer than the whole block" invariant, and sample independence — the same change under
budget: 0and the default budget produces identical text. Two assertions were rewritten for the new semantics: a partial catalog now announces a new namespace instead of inlining its tools.Visual
Checklist