test: keep the summary content index out of the reply's reach - #562
Open
Brandon-Tate wants to merge 1 commit into
Open
Brandon-Tate wants to merge 1 commit into
Brandon-Tate wants to merge 1 commit into
Conversation
A compaction checkpoint is only worth producing if the host can read it back, and that depends on one thing: whether the summarize node's run step keeps the content index it was given. The node builds its own run step and dispatches it itself, so nothing else in the graph decides that slot. A reply that lands on the same index replaces the checkpoint before the message is saved, and the next turn starts with no memory of what was compacted. Cover both shapes of the guarantee against the real graph and the real content aggregator: a summarize-only run must leave the summary on the saved message, and a run that answers as well must keep the checkpoint and the reply on separate indexes.
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.
What this covers
A compaction checkpoint is only worth producing if the host can read it back
after the turn. That depends on one thing: the summarize node's run step keeps
the content index it was given. The node builds its own run step and dispatches
it itself, so the graph's index allocation is the only thing standing between
the checkpoint and the assistant's own reply landing on the same slot.
A host that reported a lost checkpoint on an older release saw exactly this
shape: the summary was produced,
on_summarize_completecarried a well-formedblock with a boundary, and the saved message contained only the reply text.
Nothing in the suite asserted the guarantee, so nothing would have caught a
regression of it.
The test
src/graphs/__tests__/Graph.summaryContentIndex.test.tsruns a realRunwithFakeChatModel-free recording models and the realcreateContentAggregator,then asserts:
indexes, with every dispatched run step on a distinct index
Both cases were verified green on
main(v3.9.3), and the wholesrc/graphs+src/summarizationsurface passes with the file added(18 suites, 287 tests).
Notes
npx eslint,npx tsc --noEmitandsort-imports:checkare clean for thenew file.