fix(#288): derive-and-drop sub-agent state (stop retaining raw entries — 85% of heap)#322
Merged
Merged
Conversation
…es in SubAgentWatcher (85% of heap) SubAgentWatcher retained up to 500 fully-parsed sub-agent JSONL entries per agent (`entriesByAgent: Map<string, RawEntry[]>`). A dominator-tree heap analysis (scripts/analyze-heapsnapshot.mjs --owners) proved that map was ~85% of a 227 MB main-process heap, because each retained entry pins its full multi-MB tool-result/Read body. Prior fixes only SHRANK retained entries (PR #300 count cap of 500; PR #320/#321 per-field byte clamps); none questioned why we retained entries at all. We didn't need to: buildSubAgentState was a pure left-to-right fold over the entries, re-run on every emit, so the only reason to keep them was to re-fold them. ACCUMULATOR (added to subagentState.ts, next to the logic it mirrors): a small per-agent SubAgentAccumulator folded incrementally as each line streams in, then the raw entry is dropped. createAccumulator() / accumulateSubAgentEntry(acc, entry) (the per-entry step of buildSubAgentState's loop) / buildSubAgentStateFromAccumulator(acc, …) (the post-loop tail). Memory is now O(open tool calls), not O(transcript length). Mirrors the Codex twin (codexSubagentState.ts, PR #317). The 6 invariants an adversarial review derived, and how each is honored: 1. openToolCallIds is a STANDALONE Set, persisted independently of the toolCalls ring, so a tool_result flips status / decrements the open count even if the call's display object was already evicted from the ring. 2. Ring cap = 60. Measured max simultaneously-open tool calls across 318 transcripts = 8; 60 is 7× headroom (ring ≥ max tool_use→tool_result gap or a status flip is lost FROM THE RING). Past 60 we evict the OLDEST (front) but keep its id in openToolCallIds if still unresolved. 3. turnCount and droppedToolCalls are MONOTONIC fold counters, never derived from ring contents. turnCount ++ per assistant entry. A true totalToolUses is maintained; droppedToolCalls = max(0, totalToolUses - SUBAGENT_TOOL_CALLS_MAX), matching the array builder's total-minus-40. 4. currentActivity is literal LAST-WRITE-WINS per block in arrival order: set to `running <tool>` on tool_use, cleared to null on tool_result, set to `thinking` on a thinking block. Not reinterpreted as "last open call". 5. startedAt = FIRST valid timestamp (set once); lastActivityAt = LAST valid timestamp (last-write-wins, NOT Math.max — 21 out-of-order cases in real data match the original builder's plain assignment). 6. Terminal done/error and the status/currentActivity gating stay computed at BUILD time from done/error (which come from parentResult), NOT stored in the accumulator. emit() reads the live accumulator each call so a late parentResult flip still re-renders. WIRING (SubAgentWatcher.ts): entriesByAgent → accByAgent. readAppended folds each parsed line via accumulateSubAgentEntry then lets it die — no truncate, no intern, no array, no 500-cap splice. emit() builds via buildSubAgentStateFromAccumulator. stop() clears accByAgent. DEAD CODE REMOVED from SubAgentWatcher.ts: truncateEntryBodies + clampDeep + clampString (the #320/#321 byte-cap machinery, only ever used at the retention site) and the internEntryFields/makeStringPool usage + the intern field + MAX_ENTRY_FIELD_BYTES / MAX_RETAINED_ENTRIES_PER_AGENT / ENTRY_CLAMP_DEPTH constants. internEntryFields/makeStringPool STAY in internEntry.ts (still used by jsonlCoalescer.ts and historyLoader.ts — untouched). CONSCIOUS BEHAVIOR DELTA: removing the 500-entry cap makes droppedToolCalls and turnCount TRUE running totals rather than relative-to-a-500-tail — higher for very long agents (6/318 transcripts exceed 60 tool calls; max 79). That is MORE correct: the displayed timeline is still capped to SUBAGENT_TOOL_CALLS_MAX, only the "+N earlier" count grows to the real total. The SubAgentState/SubAgentToolCall IPC contract is unchanged. TEST: SubAgentWatcher.test.ts updated — the 521-tool_use fixture now asserts droppedToolCalls === 481 (true total 521 - 40 visible) instead of 460 (500 retained - 40), with a comment explaining the delta. Passes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…nd resolvedToolCallIds — review fixes Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
Problem (dominator evidence)
SubAgentWatcherretained up to 500 fully-parsed sub-agent JSONL entries per agent inentriesByAgent: Map<string, RawEntry[]>. A dominator-tree heap analysis (scripts/analyze-heapsnapshot.mjs --owners) proved that map was ~85% of a 227 MB main-process heap — each retained entry pins its full multi-MB tool-result / Read body viaentry → message.content → huge string.Prior fixes only shrank the retained entries: PR #300 capped the count at 500; PR #320/#321 added per-field byte clamps (
truncateEntryBodies). None asked why we retained entries at all. We didn't need to —buildSubAgentStatewas a pure left-to-right fold over the entries, re-run on every emit, so the only reason to keep them was to re-fold them.Fix: derive-and-drop accumulator
Added to
subagentState.ts(next to the fold it mirrors):SubAgentAccumulator—startedAt,lastActivityAt,turnCount,totalToolUses,currentActivity, a boundedtoolCallsring (cap 60),openToolCallIds: Set,resolvedToolCallIds: Set.createAccumulator()accumulateSubAgentEntry(acc, entry)— the per-entry step ofbuildSubAgentState's loop, run as each line streams in.buildSubAgentStateFromAccumulator(acc, toolUseId, agentId, meta, done, error)— the post-loop tail; yields the SAMESubAgentStatethe array builder did.Memory is now O(open tool calls), not O(transcript length). This mirrors the Codex twin (
codexSubagentState.ts, PR #317).The 6 invariants (an adversarial review derived them)
openToolCallIdsis a standalone Set, persisted independently of the ring — atool_resultflips status / decrements the open count even if its display object was already evicted.tool_use→tool_resultgap or a status flip is lost from the ring). Past 60 we evict the oldest (front) but keep its id inopenToolCallIdsif unresolved.turnCount/droppedToolCallsare monotonic fold counters, never derived from ring contents.turnCount++ perassistantentry; a truetotalToolUsesis kept;droppedToolCalls = max(0, totalToolUses − SUBAGENT_TOOL_CALLS_MAX).currentActivityis literal last-write-wins per block in arrival order:running <tool>ontool_use, cleared tonullontool_result,thinkingon a thinking block. Not reinterpreted as "last open call."startedAt= first valid timestamp (set once);lastActivityAt= last valid timestamp (last-write-wins, NOTMath.max— 21 out-of-order cases in real data; matches the original plain assignment).done/error(fromparentResult), NOT stored in the accumulator.emit()reads the live accumulator each call so a lateparentResultflip still re-renders.Wiring (
SubAgentWatcher.ts)entriesByAgent: Map<string, RawEntry[]>→accByAgent: Map<string, SubAgentAccumulator>.readAppendedfolds each parsed line viaaccumulateSubAgentEntrythen lets it die — no truncate, no intern, no array, no 500-cap splice; nothing entry-shaped survives a loop iteration.emit()builds viabuildSubAgentStateFromAccumulator.stop()clearsaccByAgent.What becomes dead code (removed from
SubAgentWatcher.ts)truncateEntryBodies+clampDeep+clampString(the #320/#321 byte-cap machinery, only ever used at the retention site), theinternEntryFields/makeStringPoolusage + theinternfield, and theMAX_ENTRY_FIELD_BYTES/MAX_RETAINED_ENTRIES_PER_AGENT/ENTRY_CLAMP_DEPTHconstants.internEntryFields/makeStringPoolstay ininternEntry.ts— still used byjsonlCoalescer.tsandhistoryLoader.ts(untouched).Conscious behavior delta
Removing the 500-entry cap makes
droppedToolCalls/turnCounttrue running totals rather than relative-to-a-500-tail — higher for very long agents (6/318 transcripts exceed 60 tool calls; max 79). That is more correct: the displayed timeline is still capped toSUBAGENT_TOOL_CALLS_MAX, only the "+N earlier" count grows to the real total.Renderer contract
The
SubAgentState/SubAgentToolCallIPC contract is unchanged.Tests
SubAgentWatcher.test.tsupdated: the 521-tool_use fixture now assertsdroppedToolCalls === 481(true total 521 − 40 visible) instead of 460 (500 retained − 40), with a comment explaining the delta. Passes.npx tsc -p tsconfig.node.jsonshows zero new errors in the changed files (the one pre-existingcodexSubagentState.tsTS18047 is onmainand untouched here).Verification note
Real before/after heap numbers need a fresh
--ownerssnapshot after a rebuild — the dominator chain that pinnedentriesByAgentno longer exists, so the 85% should collapse to the small bounded accumulator footprint.🤖 Generated with Claude Code