fix(compaction): tail sizing reads cumulative step messages — compaction never fires in production (#176)

[OGtwelve]

Closes #176. First of the two P0s from the post-#100 parity verification (#196).

Root cause (verified)

ai@6.0.182 exposes step.response.messages as the cumulative response-message list up to that step, not a per-step delta. Confirmed against a live run's transcript (sum/working-2.jsonl): per-step lengths [2, 4, 6], record 0 a strict prefix of record 1.

buildCompactionStep sized the kept tail by summing those arrays over the last keepLastSteps steps:

const tailCount = steps.slice(-keepLastSteps).reduce((n, s) => n + s.response.messages.length, 0);

For any conversation with ≥ 2 steps in the window, tailCount >= messages.length → splitAt === 0 → older empty → pass-through at every step. So summarize-and-continue compaction (#100's Wave-1 P0, wired in #102) never fired in production; long worker/reviewer/CI-fix conversations still died at the context window.

Fix

• Last-K-steps count = cumulative(last) − cumulative(last − keepLastSteps) (delta over cumulative arrays), so the step-boundary cut is preserved but the count is correct.
• Guard steps.length === 0: the first prepareStep of a run — or of a feat(compat): subagent continuation — resume a completed agent with retained messages #107 priorHandle continuation — has no completed step to cut on, and a continuation's live tail must not be summarized away (the inverse over-compaction edge the issue calls out).
• All existing failure paths stay non-fatal pass-through.

Tests

• The above-threshold test now feeds cumulative fakes [2,4,6,8] (real SDK shape). The pre-fix sum-based math pinned splitAt to 0 and returned undefined — this test is the regression proof that compaction fires.
• New continuation-edge test: steps=[] over threshold → pass-through, summarizer never runs.
• Green: typecheck (both) + test:node (aitm 903) + biome.

Same root cause as #175 (transcript duplication) — fixed separately since it touches different files.

Part of #196.

Summary by CodeRabbit

• Bug Fixes
  • Improved conversation compaction at step boundaries by correctly preserving recent messages and summarizing older history.
  • Fixed cases where compaction could fail to trigger or incorrectly process message history.
  • Added handling for continuation scenarios where no additional compaction is needed.
  • Improved safeguards when summarization encounters failures.

[coderabbitai]

📝 Walkthrough

Walkthrough

Compaction now interprets step response messages as cumulative, calculates the retained tail using cumulative differences, and skips compaction when no completed steps exist. Tests update their fixtures and add coverage for continuation and summarizer failure behavior.

Changes

Compaction boundary correction

Layer / File(s)	Summary
Cumulative compaction cut-point logic packages/aitm/src/compaction/compaction-step.ts	Compaction skips empty-step input and computes the retained tail from cumulative message-count differences.
Cumulative-shape regression coverage packages/aitm/src/compaction/compaction-step.test.ts	Tests use cumulative response-message counts and cover threshold behavior, continuation pass-through, and summarizer failure handling.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related issues

• fix(state): transcript step records duplicate the prior conversation — record per-step deltas #175 — Addresses the same cumulative response.messages semantics in transcript recording.

Possibly related PRs

• developerz-ai/ai-task-master#134 — Wires compaction into subagent loops through the prepareStep path changed here.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main compaction tail-sizing fix and matches the changeset.
Linked Issues check	✅ Passed	The code and tests address the cumulative-step tail sizing bug, continuation guard, and regression coverage required by #176.
Out of Scope Changes check	✅ Passed	The diff appears limited to the compaction fix and its tests, with no unrelated changes visible.
Docstring Coverage	✅ Passed	Docstring coverage is 66.67% which is sufficient. The required threshold is 30.00%.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/compaction-cumulative-steps-176

---

_{Comment @coderabbitai help to get the list of available commands.}

[OGtwelve]

@coderabbitai full review

[coderabbitai]

✅ Action performed

Full review finished.

[coderabbitai]

🧹 Nitpick comments (1)

packages/aitm/src/compaction/compaction-step.ts (1)

70-74: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Guard placement forces a wasted compactor lookup on every continuation step.

The steps.length === 0 check is correct, but it sits after the await init.compactor.shouldCompact(...) call and the estimateTokens(messages) computation — both of whose results are discarded whenever this guard fires (every first-step / priorHandle continuation call). Moving the guard right after the messages.length === 0 check avoids the unnecessary token estimation and compactor round-trip on a path that's guaranteed to pass through anyway.

♻️ Proposed reorder

     if (messages.length === 0) return undefined;
 
+    // No completed steps yet — the first prepareStep of a run, or of a `#107` `priorHandle`
+    // continuation. There is no step boundary to cut, and a continuation's live tail must not be
+    // summarized away (cumulative math below would treat the whole injected history as `older`).
+    // Pass through; compaction resumes once a step has run.
+    if (steps.length === 0) return undefined;
+
     // Size off the LIVE `messages` ...
     const liveInputTokens = estimateTokens(messages);
 
     let decision: Awaited<ReturnType<Compactor['shouldCompact']>>;
     try {
       decision = await init.compactor.shouldCompact(init.modelId, liveInputTokens);
     } catch (err) {
       ...
     }
     if (decision.kind === 'skip') return undefined;
 
-    // No completed steps yet — the first prepareStep of a run, or of a `#107` `priorHandle`
-    // continuation. There is no step boundary to cut, and a continuation's live tail must not be
-    // summarized away (cumulative math below would treat the whole injected history as `older`).
-    // Pass through; compaction resumes once a step has run.
-    if (steps.length === 0) return undefined;
-
     // Cut at a step boundary...

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/aitm/src/compaction/compaction-step.ts` around lines 70 - 74, Move
the existing steps.length === 0 early return in the compaction step flow to
immediately after the messages.length === 0 check, before
estimateTokens(messages) and init.compactor.shouldCompact(...). Preserve the
current undefined pass-through behavior for first-step and priorHandle
continuation calls.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@packages/aitm/src/compaction/compaction-step.ts`:
- Around line 70-74: Move the existing steps.length === 0 early return in the
compaction step flow to immediately after the messages.length === 0 check,
before estimateTokens(messages) and init.compactor.shouldCompact(...). Preserve
the current undefined pass-through behavior for first-step and priorHandle
continuation calls.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 36676f98-d3ed-4b80-b891-869ee6d30e81

📥 Commits

Reviewing files that changed from the base of the PR and between 82921ca and 45ebed4.

📒 Files selected for processing (2)

• packages/aitm/src/compaction/compaction-step.test.ts
• packages/aitm/src/compaction/compaction-step.ts