Repository navigation
fix(compaction): correct step.response.messages semantics to the installed ai@7 - #379
Conversation
…alled ai@7 The installed SDK is ai@7.0.159, whose `step.response.messages` holds ONLY that step's own generated messages (a per-step delta), not a running total, and whose prepareStep `messages` override DOES persist across steps. Both facts were inverted in the code, and the inversion drove real behavior bugs. - compaction-step.ts: the kept-steps tail and the usage "since" delta were computed from a cumulative reading, so the tail was cut at the wrong boundary and the usage delta was wrong. Both now read the step-local shape. - subagent-session.ts: recordStepDeltas sliced each step's response messages by the previous step's length, silently dropping that step's leading messages from the persisted transcript. It now records each step's own messages. - Comments claiming "ai@6" semantics, or that the override does not persist, are corrected against the installed SDK. Dz-Task-Id: tsk_9f68e592462494c3ac52d242be35e78c
|
Verify compaction message semantics against the installed AI SDK
Work landed on #379 — watching it toward merge. 24m · 184 tool calls · 79 messages Sessions: coding run 🤖 developerz.ai, an automated agent, not a human — hv-dedicated-dev-din (developerz-ai[bot]) · model |
developerz-ai
Bot
added
dz/fleet-pr
✅ Reviewed — nothing survived verification0 actionable comment(s) · 4 refuted by verification · grounded on your code ⏱ 3m 24s wall clock · 🤖 developerz.ai — automated review, running on your model and your box. |
![developerz-ai[bot]](https://avatars.githubusercontent.com/in/3783704?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review summary — 9 file(s), 0 finding(s).
🤖 developerz.ai review — automated, running on your model and your box. What is this? · Reviewed by router/gpt-6.1-sol on box sebastian-3 — 500k in / 6.1k out over 10 calls; cost unknown: no price on record for router/gpt-6.1-sol.
What
The installed SDK is ai@7.0.159, whose
step.response.messagesholds ONLYthat step's own generated messages (a per-step delta), not a running
total, and whose prepareStep
messagesoverride DOES persist acrosssteps. Both facts were inverted in the code, and the inversion drove
real behavior bugs.
were computed from a cumulative reading, so the tail was cut at the
wrong boundary and the usage delta was wrong. Both now read the
step-local shape.
messages by the previous step's length, silently dropping that step's
leading messages from the persisted transcript. It now records each
step's own messages.
persist, are corrected against the installed SDK.
Acceptance
Changes
packages/aitm/src/compaction/compaction-step.ts(edit) — Fix the step.response.messages semantics: replace the cumulativeAt() helper (reading step.response.messages.length as a running total) with step-local math — the kept-steps tail = sum of the last K steps' own response.messages.length, and the usage 'since' delta = the last step's own response.messages.length. Correct the stale comments at ~:101, ~:112 and ~:156 that claim ai@6 cumulative semantics and that the prepareStep messages override does not persist across steps.packages/aitm/src/compaction/compaction-step.test.ts(edit) — Replace the cumulative-shaped step() fixture (one step carrying N messages, growing N) with step-LOCAL fixtures mirroring the verified ai@7 shape, and update the affected expectations (tail length, compactedOlder length, sinceTokens, the same-length-cache and grown-history scenarios) to the step-local arithmetic. Add regression coverage asserting the kept-steps tail is the sum of the last K steps' own response.messages and that the usage delta equals the last step's own message count.packages/aitm/src/compaction/compactor.ts(edit) — Correct the effectiveInputTokens doc comment (~:113-120) that justifies the estimate floor by claiming 'the installed ai does not persist a prepareStep messages override across steps' — verified false against the installed ai@7.0.159 (dist/index.js:5375 + :5729 persist the override). Keep the floor itself (a safe guard), describe it accurately, drop the unsupported version claim.packages/aitm/src/loop/run-loop-adapter.test.ts(edit) — Update the recordStepDeltas tests at ~:1262-1305 that fabricate cumulative response.messages ([m0,m1], [m0..m3], [m0..m5]) and the comment at ~:1275 asserting 'ai@6 hands the callback the CUMULATIVE response list each step'; fabricate step-local arrays and assert every step's messages are recorded in full.packages/aitm/src/loop/subagent-session.ts(edit) — Fix recordStepDeltas (~:39-51): it slices each step's response.messages by the previous step's length, but ai@7 hands onStepFinish a step-local list (onStepFinish receives a StepResult — index.d.ts:4448 — whose response.messages is that step's own output). Record each step's own messages in full instead of slicing by a running offset, and rewrite the ~:34-38 comment that asserts the cumulative shape.packages/aitm/src/loop/subagent-session.test.ts(edit) — Update the recordStepDeltas tests at ~:71-91 that fabricate cumulative response.messages and assert sliced deltas; fabricate step-local arrays and assert each step's messages are recorded in full, so a regression that slices by a running offset fails.packages/aitm/src/subagents/factory.ts(edit) — [docs] factory.ts:81-82 asserts "the transcript recorder slices cumulative response messages (fix(state): transcript step records duplicate the prior conversation — record per-step deltas #175), which interleaved parallel editor conversations would corrupt" — FALSE after this diff: subagent-session.ts:47-50 now records each step's own messages with no slicing. Rewrite to the verified ai@7 fact: step.response.messages is STEP-LOCAL (ai/dist/index.js:5701), so the recorder no longer slices; editors still need per-editor handlers because each editor is its own agent/generate with its own step stream, not because of cumulative slicing. Keep the rest of the paragraph intact.packages/aitm/src/observability/step-progress.ts(edit) — [docs] step-progress.ts:8-11 asserts the transcript recorder does "delta slicing (issue fix(state): transcript step records duplicate the prior conversation — record per-step deltas #175)" and contrasts it with this handler — FALSE after this diff: subagent-session.ts:47-50 records each step's own step-local messages and no longer slices by a running offset. Rewrite to the verified ai@7 fact: step.response.messages is STEP-LOCAL (ai/dist/index.js:5701), so this handler reads only per-step fields and never the cumulative aggregate, while the recorder now writes each step's own messages in full.packages/aitm/src/state/transcript-store.ts(edit) — [docs] transcript-store.ts:116-119 states "Post-fix (per-step-delta) records don't overlap and append whole — the same rule handles both" — written for the old running-offset slicing this diff removed. Rewrite to the verified ai@7 fact: records written now carry each step's OWN response.messages (step-local, ai/dist/index.js:5701), so consecutive records are disjoint and the isLeadingRun de-overlap at :140 fires only on pre-fix(state): transcript step records duplicate the prior conversation — record per-step deltas #175 cumulative files (which re-include the whole conversation); post-fix(state): transcript step records duplicate the prior conversation — record per-step deltas #175 files append whole. Note it exists purely for backwards compatibility with already-writ [trimmed]Verification
./bin/checkpassed (exit 0) in 4m 46s.Notes
Premises of the request the team disproved:
ai@6exposesstep.response.messagesas the CUMULATIVE response list up to that step, not a per-step delta'. FALSE for the installed SDK: it is a per-step delta. dist/index.js:5701 assignsmessages: cloneModelMessages(stepResponseMessages)from that step's own call (:5690); the SDK's own cumulative aggregate is `steps.flatMap((step) => step.respons [trimmed]messagesoverride across steps'. FALSE: dist/index.js:5375 sendsprepareStepResult?.messages ?? stepInputMessages, and dist/index.js:5729 setsmessagesForNextStep = [...stepMessages, ...stepResponseMessages]wherestepMessagesIS the overridden array.cumulativeAt(last) - cumulativeAt(last - keepLastSteps)from step-local lengths; the correct value is the SUM of the last K steps' own response.messages.length.sinceLastCall = cumulativeAt(last) - cumulativeAt(last - 1)is not the last step's own message count.event.response.messages.slice(recorded)withrecorded= previous step's length. Since onStepFinish receives a StepResult whose response.messages is step-local (index.d.ts:4448 -> StepResult; index.d.ts:196), every step after the first loses its leading messages from the persisted transcript — live data loss, not a stale comment. Th [trimmed]🤖 Opened by developerz.ai for task
tsk_9f68e592462494c3ac52d242be35e78con developerz-ai/ai-task-master.Raised automatically by the
raw_task_triagelane.