fix(claude-code): count turn boundaries by conversational content, not user-role records - #3000
Open
merlinr68 wants to merge 1 commit into
Open
Conversation
Turn selection counted every user-role record as a boundary, including records
the conversation extractor already discards. `_extract_text_content()` documents
tool results as "operational results, not conversation", and the recall composer
additionally strips channel envelopes and injected memory blocks — so a window
could be filled entirely by user-role records contributing nothing, exhausting
the requested turn count before reaching any actual conversation.
In Claude Code this is the common case: tool results are stored as role="user"
messages. Measured on a real 1,639-message transcript with 566 user-role records,
514 of them (91%) tool results:
recallContextTurns=3 -> 1 conversational turn, 82 chars (before)
recallContextTurns=3 -> 3 conversational turns, 8,081 chars (after)
recallContextTurns=5 -> 1 conversational turn, 3,019 chars (before)
recallContextTurns=5 -> 5 conversational turns, 14,152 chars (after)
Multi-turn recall context was effectively inert on agentic transcripts.
Rather than special-casing tool_result blocks, define boundaries with the same
cleaned representation the composer already uses: extract conversational blocks,
strip the channel envelope, strip injected memory, trim. That is factored into
one private helper used by both boundary selection and recall composition, so
the two cannot disagree about what counts as content. A record carrying both a
tool result and genuine text still counts; an empty or memory-only record does
not.
Boundary selection is all that changes. Records inside the selected window are
returned untouched, so retention and formatting policy still decide what is kept
from them.
Both call sites want the same conversational unit — recall context selection and
the chunked-retention window — so the fix belongs in the shared helper.
Reverting the predicate fails 5 of the 12 new tests; 213 pass with it.
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.
The inconsistency
slice_last_turns_by_user_boundary()counts everyrole == "user"record as a turn boundary:But the conversation extractor in the same file already treats many of those records as non-conversational.
_extract_text_content()'s own docstring says it excludes:and
compose_recall_query()additionally strips channel envelopes and injected<hindsight_memories>blocks.So turn selection counts records that formatting then discards. A window can be exhausted by user-role transport records before reaching any actual conversation.
Impact
Two call sites:
content.py—compose_recall_query(), controllingrecallContextTurns(runs on every user prompt)retain.py— the chunked-retention windowIn Claude Code this is the common case rather than an edge case, because tool results are stored as
role: "user"messages. Measured on a real 1,639-message transcript with 566 user-role records, 514 of them (91%) tool results:recallContextTurns=3recallContextTurns=5Asking for three turns of recall context returned 82 characters. The multi-turn feature was effectively inert on agentic transcripts.
Fix
Rather than special-casing
tool_resultblocks, define a boundary using the same cleaned representation the composer already applies — extract conversational blocks, strip channel envelope, strip injected memory, trim — factored into one private helper:used by both boundary selection and recall composition, so the two cannot disagree about what counts as content.
This is preferable to structural
tool_resultdetection because it handles the mixed cases correctly without duplicating knowledge that already lives in_extract_text_content():Boundary selection is the only thing that changes. Records inside the selected window are returned untouched, so retention and formatting policy still decide what is kept from them. Tool results remain excluded from output by
_extract_text_content()exactly as before.Both call sites want the same conversational unit, so the fix belongs in the shared helper rather than being duplicated — otherwise the two definitions would drift.
Intentional consequences
Recall windows traverse more records. Finding N conversational turns may now walk across many tool records. Only extractable conversational content enters the query, and
recallMaxQueryChars(default 800) bounds the final query — note it does not bound the intermediate composed string or the amount of transcript traversed. At the scale measured here that cost is negligible next to reading the transcript and making the recall request.Chunked retention documents get larger. A window of N real turns now includes the tool activity between those prompts, where before it often spanned only the last few records. That is the point of a turn window, and the previously small payloads were a symptom of selecting the wrong boundaries — but it is a visible change, so calling it out.
Not included
recallContextTurnsshould count the latest prompt as one of its N. The selector counts it andcompose_recall_query()then drops it as a duplicate, so N can mean "latest plus N−1 prior". That predates this change and is left alone.Tests
12 tests in
tests/test_turn_boundary_semantics.py— 10 on boundary qualification, 2 on recall composition end-to-end. The key assertions check not just the size of the returned window but that it starts at the Nth qualifying record and still contains the intervening tool results.ruff checkandruff format --checkcleanRelated
Same family as #2999 (retain dropped the unprocessed suffix), #2974 (retain never sent
MemoryItem.timestamp) and #2869 (SessionEnd retain cancelled at shutdown).