From f3b13e6b121e924868b6cabf1c45c3509efa5ac9 Mon Sep 17 00:00:00 2001 From: ework-agent Date: Thu, 3 Sep 2026 18:09:17 +0800 Subject: [PATCH 1/3] fix: align nudge recommendation counter with pipeline min-size check (#359) buildCompressibleRanges sized non-text parts with JSON.stringify(whole part)/4, systematically overstating tool-heavy ranges (~10-40%) vs the pipeline's countMessageCharacters. Sub-floor ranges passed the recommendation floor but were rejected by the min-size check (Range too small), inviting guaranteed- failed retry loops (#37 ses_7fb5cbc8; #355 incident v1.14.26: 2760 chars vs min 3000). Both branches now use Math.round(countMessageCharacters(msg) / 4); per-part loops retained for classification only. Residual divergence is per-message rounding (<=0.5 tokens/msg), asserted as a band in the regression suite. Tests: tests/recommend-exec-counter-alignment.test.ts (7 new; verified to fail 5/7 against pre-fix code per AGENTS.md 5.7.3). Full suite 1069/1069. --- .../REQ.md | 72 ++++ .../WORKLOG.md | 72 ++++ lib/messages/inject/utils.ts | 16 +- .../recommend-exec-counter-alignment.test.ts | 366 ++++++++++++++++++ 4 files changed, 516 insertions(+), 10 deletions(-) create mode 100644 devlog/2026-09-03_recommend-exec-counter-alignment/REQ.md create mode 100644 devlog/2026-09-03_recommend-exec-counter-alignment/WORKLOG.md create mode 100644 tests/recommend-exec-counter-alignment.test.ts diff --git a/devlog/2026-09-03_recommend-exec-counter-alignment/REQ.md b/devlog/2026-09-03_recommend-exec-counter-alignment/REQ.md new file mode 100644 index 00000000..899455a6 --- /dev/null +++ b/devlog/2026-09-03_recommend-exec-counter-alignment/REQ.md @@ -0,0 +1,72 @@ +# REQ: Align nudge recommendation-side and compress execution-side character counting (Issue #359) + +**Source**: https://github.com/ranxianglei/opencode-acp/issues/359 (found during analysis of #355; same family as historical incident #37) + +## Problem + +Ranges listed in the nudge recommendation can still be rejected by the compress +pipeline's min-size check because the two sides count characters differently: + +| Side | Location | text part | tool part | +| -------------- | -------------------------------------------------------------------------------------------------- | ----------------- | -------------------------------------------------------------------------------------------------------------------------- | +| Recommendation | `buildCompressibleRanges` (`lib/messages/inject/utils.ts`) | `text.length / 4` | `JSON.stringify(whole part).length / 4` — includes `type/tool/callID/state.status/metadata` field overhead + JSON escaping | +| Execution | `countMessageCharacters` (`lib/token-utils.ts:224-237`), summed in `lib/compress/range.ts:180-201` | `text.length` | `extractToolContent` = input + output/error content length (raw string used as-is) | + +Measured (v1.14.26, #355 author session): 4-message range dominated by two tool +parts passed the nudge-side 750-token floor but the pipeline reported +`Range too small (2760 chars, min 3000)`. The nudge is the primary guidance +surface for most agents (they never call `acp_status`), so the recommendation +itself was untrustworthy. Same family as #37 (ses_7fb5cbc8: displayed 10.8K +compressible → pipeline resolved 3066 chars → rejected → model retried ×10). + +## Root cause (verified in code) + +- Execution side: after soft filters (`filterProtectedToolMessages`, + `filterLastUserMessage`, `filterProtectedRecentMessages`), `range.ts` sums + `countMessageCharacters(rawMessage)` per surviving message and throws when the + sum < `compress.minCompressRange`. +- Recommendation side: `buildCompressibleRanges` accumulates + `Math.round(JSON.stringify(part).length / 4)` per non-text/non-reasoning part. + For tool parts this overstates content by the part-wrapper JSON field names + + metadata + escaping overhead (every newline in an error stack doubles under + `JSON.stringify`; quotes inside stringified-JSON string outputs get escaped). + Systematic ~10–40%+ overestimate for tool-heavy messages; pure-text messages + agree on both sides. +- Floor: `resolveEffectiveFloor(config)` = `minCompressRange / 4` tokens, applied + to `effectiveTokens` in `filterRecommendedRanges`. Because `effectiveTokens` + inherits the inflated per-part counter, sub-floor tool-heavy ranges pass the + recommendation gate and fail the execution gate. + +Not a duplicate of #325: #325 fixed the _soft-filter_ dimension (raw → +effectiveTokens + config-derived floor) but kept the divergent per-part counter. + +## Goal / Acceptance criteria + +1. `buildCompressibleRanges` computes per-message tokens with + `countMessageCharacters(msg) / 4` in BOTH branches (compressible + protected), + making the recommendation gate ≡ the execution-side acceptance predicate + (modulo per-message rounding ≤ 0.5 tokens/message — negligible against the + default 5000-char / 1250-token floor). +2. Per-part loops retained ONLY for classification (`isTool`, `toolPct`, + `hasMeaningfulPart`, protected tool-name collection) — no behavioral change to + grouping, soft-filter mirroring, or zone sizing. +3. Regression tests pin the two-side delta with fixtures: + - pure-text message + - normal-completed tool (string output) + - error-state tool with multi-line stack trace + - deeply nested JSON object output +4. §5.7.3: new regression tests verified to FAIL against pre-fix code (surgical + revert → red → restore → green). +5. `npm run typecheck`, `npm run test`, `npm run format:check` all green. + +## Non-goals + +- Display-only counters using the same pattern (`estimateContextComposition` + breakdown, `acp_status` largest-ranges, notification stats) — cosmetic, do not + affect any acceptance predicate; separate follow-up if wanted. +- Zone-sizing counters (`computeProtectedRefs` ↔ `filterProtectedRecentMessages`) + — BOTH sides intentionally use the identical counter there, so zone boundaries + already match; untouched. +- Adding a `CompressibleRange.chars` field (acp-kernel style) — per-message ÷4 + rounding drift is bounded (≤ 0.5 tokens/msg) and negligible; keeps the public + range shape/API stable. diff --git a/devlog/2026-09-03_recommend-exec-counter-alignment/WORKLOG.md b/devlog/2026-09-03_recommend-exec-counter-alignment/WORKLOG.md new file mode 100644 index 00000000..8b4e38dd --- /dev/null +++ b/devlog/2026-09-03_recommend-exec-counter-alignment/WORKLOG.md @@ -0,0 +1,72 @@ +# WORKLOG: Align nudge recommendation-side and compress execution-side character counting (Issue #359) + +**Branch**: `2026-09-03_recommend-exec-counter-alignment` +**Issue**: https://github.com/ranxianglei/opencode-acp/issues/359 (source: #355 analysis; same family as #37 incident ses_7fb5cbc8) +**Date**: 2026-09-03 + +## Changes + +| File | Change | +|------|--------| +| `lib/messages/inject/utils.ts` | `buildCompressibleRanges` now sizes every message with `Math.round(countMessageCharacters(msg) / 4)` in BOTH branches (compressible ~line 811, protected ~line 785). Per-part loops retained only for classification (`isTool`, `toolPct`, `hasMeaningfulPart`) and protected tool-name collection. Merged the two `../../token-utils` imports into one (added `countMessageCharacters`). One-line invariant comment at each fixed site. | +| `tests/recommend-exec-counter-alignment.test.ts` | NEW — 7 regression tests (see below). | +| `devlog/2026-09-03_recommend-exec-counter-alignment/REQ.md` | Ticket written BEFORE implementation. | + +No changes to `filterRecommendedRanges`, `resolveEffectiveFloor`, `CompressibleRange` shape, +grouping logic, soft-filter mirroring, zone sizing, or any display-only counter +(`estimateContextComposition`, `computeProtectedRefs`, notification stats) — see REQ non-goals. + +## Tests + +New file `tests/recommend-exec-counter-alignment.test.ts` (7 tests): + +1. pure-text message: rec-side tokens === exec-side `countMessageCharacters ÷ 4` (exact) +2. pure-text: new counter identical to pre-fix estimator (guards against over-correction) +3. completed tool (object input + multiline string output): rec == exec, legacy estimator provably overstated +4. error-state tool (multi-line stack trace): rec == exec, legacy overstated +5. deeply nested JSON object output (8 levels × 5 items): rec == exec, legacy overstated +6. incident shape (#355 v1.14.26, min 3000): 4-message tool-heavy span with exec total + 2866 chars < 3000 but pre-fix inflated estimate 780 ≥ floor 750 → post-fix DROPPED by + `filterRecommendedRanges`; counterfactual synthetic range with the legacy estimate is KEPT + (pins both sides of the regression) +7. protected branch: protected-range `tokens` also use the shared counter + +### Verification (§5.7.3 — tests must fail against buggy code) + +Surgical revert (`git stash push lib/messages/inject/utils.ts` → run → `git stash pop`): + +- **Pre-fix code: 5/7 FAIL** (all tool-shape tests + incident + protected branch); the 2 + pure-text tests PASS by design (old counter agreed for text) — proves the suite targets + exactly this bug with no false positives. +- **Post-fix: 7/7 PASS.** + +Full gate results (post-fix): + +| Gate | Result | +|------|--------| +| `npm run typecheck` | ✅ clean | +| `npm run test` (full suite) | ✅ **1069/1069** (was 1062; +7 new) | +| `npx prettier --check` on changed files | ✅ test file + REQ/WORKLOG clean | + +Formatting note: `lib/messages/inject/utils.ts` carries 70 lines of PRE-EXISTING prettier +drift (part of 421 repo-wide unformatted files on master). Verified via normalized diff +(prettier under `.prettierrc` on HEAD vs worktree) that my hunks introduce ZERO new drift — +only the intended logical changes remain after normalization. Left un-reformatted to keep +the PR diff minimal; repo-wide format cleanup is out of scope. + +## Fixture-tuning notes (lesson learned) + +The incident fixture must satisfy two constraints simultaneously: exec total < 3000 AND +pre-fix inflated estimate ≥ 750 tokens. For this message shape the inflation gap +(legacy − exec) is nearly constant (~256 chars — wrapper fields + escaping density), so the +feasible window is exec ∈ [~2744, 3000). Final constants: 16 stack frames, 20 body +paragraphs, 105-char summary → exec = 2866 (margin 134), legacy = 780 (margin 30). Both +constraints are asserted dynamically in-test, so any future fixture edit that breaks them +fails loudly instead of silently weakening the regression pin. + +Residual divergence after the fix: per-message rounding keeps multi-message ranges within +±0.5 tokens/message (≤ 2 chars/msg) of the pipeline's whole-range char sum — asserted as a +rounding band in test 6, orders of magnitude below the pre-fix 10–40% systematic bias. +A strict `CompressibleRange.chars` field (acp-kernel style) was considered and deferred +(REQ non-goals) to keep the public range shape stable; revisit only if sub-band precision +ever matters. diff --git a/lib/messages/inject/utils.ts b/lib/messages/inject/utils.ts index b5a7e07f..bc35398d 100644 --- a/lib/messages/inject/utils.ts +++ b/lib/messages/inject/utils.ts @@ -14,7 +14,7 @@ import { type MessagePriority, listPriorityRefsBeforeIndex, } from "../priority" -import { estimateSystemPromptTokens } from "../../token-utils" +import { countMessageCharacters, estimateSystemPromptTokens, getCurrentTokenUsage } from "../../token-utils" import { appendToTextPart, appendToLastTextPart, @@ -22,7 +22,6 @@ import { hasContent, } from "../utils" import { getLastUserMessage, isIgnoredUserMessage, isSyntheticMessage } from "../query" -import { getCurrentTokenUsage } from "../../token-utils" import { getActiveSummaryTokenUsage } from "../../state/utils" export interface LastUserModelContext { @@ -782,13 +781,11 @@ export function buildCompressibleRanges( (protectedTools.length > 0 || protectedFilePatterns.length > 0) && messageContainsProtectedTool(msg, protectedTools, protectedFilePatterns) ) { - let tokens = 0 + // Issue #359: must match the pipeline min-size check counter; JSON.stringify(part) overstates tool parts + const tokens = Math.round(countMessageCharacters(msg) / 4) const tools = new Set() for (const part of msg.parts || []) { - if (part.type === "text" && typeof (part as any).text === "string") { - tokens += Math.round(((part as any).text as string).length / 4) - } else if (part.type !== "text" && part.type !== "reasoning") { - tokens += Math.round(JSON.stringify(part).length / 4) + if (part.type !== "text" && part.type !== "reasoning") { const toolName = (part as any)?.tool const callID = (part as any)?.callID if (toolName && callID) { @@ -810,15 +807,14 @@ export function buildCompressibleRanges( continue } - let tokens = 0 + // Issue #359: must match the pipeline min-size check counter; JSON.stringify(part) overstates tool parts + const tokens = Math.round(countMessageCharacters(msg) / 4) let isTool = false let hasMeaningfulPart = false for (const part of msg.parts || []) { if (part.type === "text" && typeof (part as any).text === "string") { - tokens += Math.round(((part as any).text as string).length / 4) if ((part as any).text.trim().length > 0) hasMeaningfulPart = true } else if (part.type !== "text" && part.type !== "reasoning") { - tokens += Math.round(JSON.stringify(part).length / 4) isTool = true hasMeaningfulPart = true } diff --git a/tests/recommend-exec-counter-alignment.test.ts b/tests/recommend-exec-counter-alignment.test.ts new file mode 100644 index 00000000..8fce70bf --- /dev/null +++ b/tests/recommend-exec-counter-alignment.test.ts @@ -0,0 +1,366 @@ +import assert from "node:assert/strict" +import test from "node:test" +import { createSessionState } from "../lib/state" +import type { WithParts } from "../lib/state" +import { assignMessageRefs } from "../lib/message-ids" +import { countMessageCharacters } from "../lib/token-utils" +import { + buildCompressibleRanges, + filterRecommendedRanges, + resolveEffectiveFloor, + type CompressibleRange, +} from "../lib/messages/inject/utils" + +/** + * Issue #359 regression: the nudge/acp_status recommendation side and the + * compress pipeline execution side must size ranges with the SAME counter. + * + * Pre-fix, buildCompressibleRanges sized non-text parts with + * JSON.stringify(whole part).length / 4 — which includes the part wrapper + * fields (type/tool/callID/state.status/...) plus JSON escaping of the + * content (every \n and " in tool output/error costs an extra char). That + * systematically overstated tool-heavy ranges (~10–40%), so sub-floor + * ranges passed the recommendation floor (minCompressRange ÷ 4) and were + * then rejected by the pipeline's min-size check ("Range too small"), + * inviting guaranteed-failed retry loops (#37 ses_7fb5cbc8, #355 incident + * v1.14.26: 2760 exec chars rejected against min 3000 while the nudge had + * recommended the range). + * + * These tests pin the shared-counter contract with the four fixture shapes + * named in the issue: pure text / completed tool / error-state with stack / + * deeply nested JSON output — plus the incident-shape gate-equivalence case. + * Single-message fixtures assert exact equality; multi-message ranges allow + * the per-message rounding band (±0.5 tokens/message), which is the only + * residual divergence from the pipeline's whole-range char sum. + */ + +const SID = "ses-recommend-exec-align-359" + +function makeMsg( + id: string, + role: "user" | "assistant", + text: string, + toolParts: any[] = [], +): WithParts { + const parts: any[] = [] + if (text) parts.push({ type: "text", text }) + for (const tp of toolParts) parts.push(tp) + return { + info: { id, role, sessionID: SID, agent: "a", time: { created: 1 } } as any, + parts, + } as WithParts +} + +function completedToolPart(callID: string, tool: string, input?: any, output?: any): any { + return { type: "tool", callID, tool, state: { status: "completed", input, output } } +} + +function erroredToolPart(callID: string, tool: string, input: any, error: string): any { + return { type: "tool", callID, tool, state: { status: "error", input, error } } +} + +/** Replicates the PRE-FIX estimator (JSON.stringify of the whole part) so + * tests can prove a fixture would have been mis-sized before the fix. */ +function legacyPartTokens(part: any): number { + if (part.type === "text" && typeof part.text === "string") { + return Math.round(part.text.length / 4) + } + if (part.type !== "text" && part.type !== "reasoning") { + return Math.round(JSON.stringify(part).length / 4) + } + return 0 +} + +function legacyMessageTokens(msg: WithParts): number { + return (msg.parts || []).reduce((sum, p) => sum + legacyPartTokens(p), 0) +} + +function buildSession(messages: WithParts[]) { + const state = createSessionState() + assignMessageRefs(state, messages) + return state +} + +// --------------------------------------------------------------------------- +// Fixture shape 1: pure text +// --------------------------------------------------------------------------- + +test("#359 pure-text message: rec-side tokens equal exec-side countMessageCharacters ÷ 4", () => { + const text = "The compression pipeline resolves boundary refs to message indices. ".repeat(60) + const userMsg = makeMsg("m1", "user", "please summarize the exploration above") + const textMsg = makeMsg("m2", "assistant", text) + const state = buildSession([userMsg, textMsg]) + + const { compressible } = buildCompressibleRanges([userMsg, textMsg], state) + assert.equal(compressible.length, 1, "single compressible range") + const range = compressible[0] + + const execChars = countMessageCharacters(textMsg) + assert.equal(execChars, text.length, "exec counter counts full text length") + assert.equal(range.effectiveTokens, Math.round(text.length / 4), "rec-side uses same counter") +}) + +test("#359 pure-text: new counter identical to pre-fix estimator (no behavior change for text)", () => { + const text = "plain words, no tool parts involved here at all. ".repeat(50) + const textMsg = makeMsg("m1", "assistant", text) + const state = buildSession([makeMsg("u1", "user", "hi"), textMsg]) + + const { compressible } = buildCompressibleRanges([makeMsg("u1", "user", "hi"), textMsg], state) + assert.equal( + compressible[0].effectiveTokens, + legacyMessageTokens(textMsg), + "text-only sizing unchanged", + ) +}) + +// --------------------------------------------------------------------------- +// Fixture shape 2: completed tool with string output +// --------------------------------------------------------------------------- + +test("#359 completed tool (object input + multiline string output): rec == exec counter", () => { + const output = Array.from({ length: 300 }, (_, i) => `line ${i}: npm test output data`).join( + "\n", + ) + const toolMsg = makeMsg("m2", "assistant", "", [ + completedToolPart("t1", "bash", { command: "npm test" }, output), + ]) + const userMsg = makeMsg("m1", "user", "run the tests") + const state = buildSession([userMsg, toolMsg]) + + const { compressible } = buildCompressibleRanges([userMsg, toolMsg], state) + const execChars = countMessageCharacters(toolMsg) + const expected = JSON.stringify({ command: "npm test" }).length + output.length + assert.equal(execChars, expected, "exec counter = stringified input + raw output length") + assert.equal( + compressible[0].effectiveTokens, + Math.round(execChars / 4), + "rec-side uses same counter", + ) + + // Sanity: the pre-fix estimator OVERSTATED this part (escaping + wrapper). + assert.ok( + legacyMessageTokens(toolMsg) > compressible[0].effectiveTokens, + "fixture exhibits the #359 overstatement", + ) +}) + +// --------------------------------------------------------------------------- +// Fixture shape 3: error-state tool with multi-line stack trace +// --------------------------------------------------------------------------- + +test("#359 error-state tool (stack trace): rec == exec counter", () => { + const stack = + "Error: fetch failed\n" + + Array.from( + { length: 40 }, + (_, i) => ` at async Step.${i} (file:///app/src/probe.js:${100 + i}:${3})`, + ).join("\n") + + "\nCaused by: ConnectTimeoutError: connect ETIMEDOUT 10.0.0.1:443" + const toolMsg = makeMsg("m2", "assistant", "", [ + erroredToolPart( + "t1", + "webfetch", + { url: "https://api.github.com/repos/x/y/issues" }, + stack, + ), + ]) + const userMsg = makeMsg("m1", "user", "fetch the issue") + const state = buildSession([userMsg, toolMsg]) + + const { compressible } = buildCompressibleRanges([userMsg, toolMsg], state) + const execChars = countMessageCharacters(toolMsg) + const expected = + JSON.stringify({ url: "https://api.github.com/repos/x/y/issues" }).length + stack.length + assert.equal(execChars, expected, "exec counter = stringified input + raw error body") + assert.equal( + compressible[0].effectiveTokens, + Math.round(execChars / 4), + "rec-side uses same counter", + ) + assert.ok( + legacyMessageTokens(toolMsg) > compressible[0].effectiveTokens, + "fixture exhibits the #359 overstatement", + ) +}) + +// --------------------------------------------------------------------------- +// Fixture shape 4: deeply nested JSON object output +// --------------------------------------------------------------------------- + +test("#359 deeply nested JSON object output: rec == exec counter", () => { + let deep: any = "leaf" + for (let level = 0; level < 8; level++) { + deep = { + level, + items: Array.from({ length: 5 }, (_, j) => ({ + id: j, + note: `note-${level}-${j}`, + tags: ["a", "b"], + })), + next: deep, + } + } + const toolMsg = makeMsg("m2", "assistant", "", [ + completedToolPart( + "t1", + "gh", + { command: "gh issue view 355 --json title,body,labels" }, + deep, + ), + ]) + const userMsg = makeMsg("m1", "user", "view the issue") + const state = buildSession([userMsg, toolMsg]) + + const { compressible } = buildCompressibleRanges([userMsg, toolMsg], state) + const execChars = countMessageCharacters(toolMsg) + const expected = + JSON.stringify({ command: "gh issue view 355 --json title,body,labels" }).length + + JSON.stringify(deep).length + assert.equal( + execChars, + expected, + "exec counter = stringified input + stringified object output", + ) + assert.equal( + compressible[0].effectiveTokens, + Math.round(execChars / 4), + "rec-side uses same counter", + ) + assert.ok( + legacyMessageTokens(toolMsg) > compressible[0].effectiveTokens, + "fixture exhibits the #359 overstatement", + ) +}) + +// --------------------------------------------------------------------------- +// Incident shape: tool-heavy range below the exec min-size threshold but +// above the floor under the pre-fix inflated estimator. +// Mirrors the #355 report (v1.14.26, min 3000): webfetch failure + gh JSON +// output + short summary, exec total 2760 chars → "Range too small". +// --------------------------------------------------------------------------- + +const INCIDENT_MIN_COMPRESS_RANGE = 3000 + +function buildIncidentMessages(): WithParts[] { + const stack = + "Error: fetch failed\n" + + Array.from( + { length: 16 }, + (_, i) => ` at async Retry.${i} (file:///app/src/net.js:${50 + i}:${11})`, + ).join("\n") + const issueJson = JSON.stringify( + { + number: 355, + title: "long-session gaps leave orphaned context between compression boundaries", + body: Array.from( + { length: 20 }, + (_, i) => + `paragraph ${i}: observed residue near boundary m${String(i).padStart(5, "0")} in the exported transcript.`, + ).join("\n"), + labels: [{ name: "bug" }, { name: "context-pruning" }], + }, + null, + 2, + ) + return [ + makeMsg("m1", "user", "fetch the issue and summarize what you find"), + makeMsg("m2", "assistant", "", [ + erroredToolPart("c1", "webfetch", { url: "https://example.invalid/issue/355" }, stack), + ]), + makeMsg("m3", "assistant", "", [ + completedToolPart("c2", "gh", { command: "gh issue view 355 --json" }, issueJson), + ]), + makeMsg( + "m4", + "assistant", + "Summary: the issue reports residual context stranded between compression boundaries during long sessions.", + ), + ] +} + +test("#359 incident shape: sub-floor tool-heavy range is NOT recommended (was recommended pre-fix)", () => { + const messages = buildIncidentMessages() + const state = buildSession(messages) + const { compressible } = buildCompressibleRanges(messages, state) + assert.equal(compressible.length, 1, "single range covering the whole span") + const range = compressible[0] + + // Execution-side truth: the pipeline sums countMessageCharacters over the + // plan's surviving messages (last user message soft-filtered out). + const execChars = messages.slice(1).reduce((sum, m) => sum + countMessageCharacters(m), 0) + assert.ok( + execChars < INCIDENT_MIN_COMPRESS_RANGE, + `fixture must sit below the exec threshold (got ${execChars} chars, min ${INCIDENT_MIN_COMPRESS_RANGE})`, + ) + + // Pre-fix estimator on the same messages WOULD have crossed the floor — + // proving this fixture reproduces the incident's divergence. + const legacyEffective = messages.slice(1).reduce((sum, m) => sum + legacyMessageTokens(m), 0) + const floor = resolveEffectiveFloor({ + compress: { minCompressRange: INCIDENT_MIN_COMPRESS_RANGE }, + }) + assert.equal(floor, INCIDENT_MIN_COMPRESS_RANGE / 4, "floor derives from minCompressRange ÷ 4") + assert.ok( + legacyEffective >= floor, + `pre-fix estimator (${legacyEffective} eff tokens) must have passed the floor (${floor})`, + ) + + // Post-fix: rec-side sizing equals exec sizing, so the gate agrees with + // the pipeline and the range is dropped instead of recommended. Per-message + // rounding keeps the deviation within ±0.5 tokens/message (≤ 2 chars/msg) — + // orders of magnitude below the pre-fix systematic 10–40% overstatement. + const survivingCount = messages.length - 1 + assert.ok( + Math.abs(range.effectiveTokens - execChars / 4) <= survivingCount / 2, + `rec-side ${range.effectiveTokens} eff tokens within per-message rounding band of exec ${execChars} chars`, + ) + assert.ok(range.effectiveTokens < floor, "post-fix effective tokens below floor") + const recommended = filterRecommendedRanges(compressible, [], { + minEffectiveTokens: resolveEffectiveFloor({ + compress: { minCompressRange: INCIDENT_MIN_COMPRESS_RANGE }, + }), + }) + assert.equal( + recommended.length, + 0, + "sub-floor tool-heavy range dropped — no guaranteed-failed compress call", + ) + + // Counterfactual: had the pre-fix inflated estimate reached the filter, + // the same range would have been recommended. + const legacyRange: CompressibleRange = { ...range, effectiveTokens: legacyEffective } + const legacyRecommended = filterRecommendedRanges([legacyRange], [], { + minEffectiveTokens: floor, + }) + assert.equal( + legacyRecommended.length, + 1, + "pre-fix estimate would have been recommended (regression pinned)", + ) +}) + +// --------------------------------------------------------------------------- +// Protected branch: protected-tool message sizing uses the same counter +// --------------------------------------------------------------------------- + +test("#359 protected branch: protected-range tokens equal exec-side countMessageCharacters ÷ 4", () => { + const skillOutput = "skill guidance: always verify before finishing. ".repeat(60) + const skillMsg = makeMsg("m3", "assistant", "", [ + completedToolPart("c1", "skill", { name: "verify" }, skillOutput), + ]) + const messages = [ + makeMsg("m1", "user", "load the skill"), + makeMsg("m2", "assistant", "on it"), + skillMsg, + ] + const state = buildSession(messages) + + const { protected: protectedRanges } = buildCompressibleRanges(messages, state, ["skill"]) + assert.equal(protectedRanges.length, 1, "protected range tracked") + const execChars = countMessageCharacters(skillMsg) + assert.equal( + protectedRanges[0].tokens, + Math.round(execChars / 4), + "protected branch uses the same shared counter", + ) +}) From 191d53f7eb80a2e118f36b83f2ea0ab9c013bfa8 Mon Sep 17 00:00:00 2001 From: ework-agent Date: Thu, 3 Sep 2026 18:24:33 +0800 Subject: [PATCH 2/3] test: address PR #360 review nits (#359) - wrap merged token-utils import to satisfy printWidth (no new prettier violations) - add compacted-tool-output fixture pinning the placeholder path (review M3) - document incident-fixture triple constraint + safe edit window (review M2) - reword exec-side comment to its actual scope (review N2) --- lib/messages/inject/utils.ts | 6 +- .../recommend-exec-counter-alignment.test.ts | 56 ++++++++++++++++++- 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/lib/messages/inject/utils.ts b/lib/messages/inject/utils.ts index bc35398d..59455905 100644 --- a/lib/messages/inject/utils.ts +++ b/lib/messages/inject/utils.ts @@ -14,7 +14,11 @@ import { type MessagePriority, listPriorityRefsBeforeIndex, } from "../priority" -import { countMessageCharacters, estimateSystemPromptTokens, getCurrentTokenUsage } from "../../token-utils" +import { + countMessageCharacters, + estimateSystemPromptTokens, + getCurrentTokenUsage, +} from "../../token-utils" import { appendToTextPart, appendToLastTextPart, diff --git a/tests/recommend-exec-counter-alignment.test.ts b/tests/recommend-exec-counter-alignment.test.ts index 8fce70bf..7057a6d1 100644 --- a/tests/recommend-exec-counter-alignment.test.ts +++ b/tests/recommend-exec-counter-alignment.test.ts @@ -3,7 +3,7 @@ import test from "node:test" import { createSessionState } from "../lib/state" import type { WithParts } from "../lib/state" import { assignMessageRefs } from "../lib/message-ids" -import { countMessageCharacters } from "../lib/token-utils" +import { COMPACTED_TOOL_OUTPUT_PLACEHOLDER, countMessageCharacters } from "../lib/token-utils" import { buildCompressibleRanges, filterRecommendedRanges, @@ -232,6 +232,49 @@ test("#359 deeply nested JSON object output: rec == exec counter", () => { ) }) +// --------------------------------------------------------------------------- +// Fixture shape 5: compacted tool output (placeholder path) +// --------------------------------------------------------------------------- + +test("#359 compacted tool output: exec counts placeholder, rec matches", () => { + const bigOutput = "x".repeat(5000) + const toolMsg = makeMsg("m2", "assistant", "", [ + { + type: "tool", + callID: "t1", + tool: "read", + state: { + status: "completed", + input: { filePath: "/tmp/big.log" }, + output: bigOutput, + time: { compacted: true }, + }, + }, + ]) + const userMsg = makeMsg("m1", "user", "read the log") + const state = buildSession([userMsg, toolMsg]) + + const { compressible } = buildCompressibleRanges([userMsg, toolMsg], state) + const execChars = countMessageCharacters(toolMsg) + const expected = + JSON.stringify({ filePath: "/tmp/big.log" }).length + + COMPACTED_TOOL_OUTPUT_PLACEHOLDER.length + assert.equal( + execChars, + expected, + "exec counter = stringified input + compacted placeholder (not full output)", + ) + assert.equal( + compressible[0].effectiveTokens, + Math.round(execChars / 4), + "rec-side uses same counter", + ) + assert.ok( + legacyMessageTokens(toolMsg) > compressible[0].effectiveTokens, + "pre-fix estimator counted the full pre-compaction output", + ) +}) + // --------------------------------------------------------------------------- // Incident shape: tool-heavy range below the exec min-size threshold but // above the floor under the pre-fix inflated estimator. @@ -241,6 +284,12 @@ test("#359 deeply nested JSON object output: rec == exec counter", () => { const INCIDENT_MIN_COMPRESS_RANGE = 3000 +// Triple constraint on this fixture (asserted dynamically inside the test): +// execChars < 3000 AND legacyEffective >= floor(750) AND post-fix +// effectiveTokens < 750. Current margins: exec 2866 (-134), legacy 780 (+30), +// post-fix 716 (-34). Legacy inflation is ~constant (~256 chars) for this +// shape, so keep future text edits within exec ∈ [~2744, 3000); the dynamic +// asserts fail loudly if an edit breaks any side. function buildIncidentMessages(): WithParts[] { const stack = "Error: fetch failed\n" + @@ -285,8 +334,9 @@ test("#359 incident shape: sub-floor tool-heavy range is NOT recommended (was re assert.equal(compressible.length, 1, "single range covering the whole span") const range = compressible[0] - // Execution-side truth: the pipeline sums countMessageCharacters over the - // plan's surviving messages (last user message soft-filtered out). + // Min-size-check counter over last-user-filtered survivors (the other soft + // filters — protected tools / recent zone — are out of scope for this + // shared-counter pin). const execChars = messages.slice(1).reduce((sum, m) => sum + countMessageCharacters(m), 0) assert.ok( execChars < INCIDENT_MIN_COMPRESS_RANGE, From 16db2a07442f46ca03912afa304c2653f04c62a7 Mon Sep 17 00:00:00 2001 From: ework-agent Date: Thu, 3 Sep 2026 18:36:29 +0800 Subject: [PATCH 3/3] docs: update WORKLOG test counts for compacted-output test (review nit) --- .../WORKLOG.md | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/devlog/2026-09-03_recommend-exec-counter-alignment/WORKLOG.md b/devlog/2026-09-03_recommend-exec-counter-alignment/WORKLOG.md index 8b4e38dd..6837090b 100644 --- a/devlog/2026-09-03_recommend-exec-counter-alignment/WORKLOG.md +++ b/devlog/2026-09-03_recommend-exec-counter-alignment/WORKLOG.md @@ -18,34 +18,36 @@ grouping logic, soft-filter mirroring, zone sizing, or any display-only counter ## Tests -New file `tests/recommend-exec-counter-alignment.test.ts` (7 tests): +New file `tests/recommend-exec-counter-alignment.test.ts` (8 tests): 1. pure-text message: rec-side tokens === exec-side `countMessageCharacters ÷ 4` (exact) 2. pure-text: new counter identical to pre-fix estimator (guards against over-correction) 3. completed tool (object input + multiline string output): rec == exec, legacy estimator provably overstated 4. error-state tool (multi-line stack trace): rec == exec, legacy overstated 5. deeply nested JSON object output (8 levels × 5 items): rec == exec, legacy overstated -6. incident shape (#355 v1.14.26, min 3000): 4-message tool-heavy span with exec total +6. compacted tool output (`state.time.compacted`): exec counts the 33-char placeholder, not + the full pre-compaction output; rec matches; pre-fix estimator provably counted the full output +7. incident shape (#355 v1.14.26, min 3000): 4-message tool-heavy span with exec total 2866 chars < 3000 but pre-fix inflated estimate 780 ≥ floor 750 → post-fix DROPPED by `filterRecommendedRanges`; counterfactual synthetic range with the legacy estimate is KEPT (pins both sides of the regression) -7. protected branch: protected-range `tokens` also use the shared counter +8. protected branch: protected-range `tokens` also use the shared counter ### Verification (§5.7.3 — tests must fail against buggy code) Surgical revert (`git stash push lib/messages/inject/utils.ts` → run → `git stash pop`): -- **Pre-fix code: 5/7 FAIL** (all tool-shape tests + incident + protected branch); the 2 - pure-text tests PASS by design (old counter agreed for text) — proves the suite targets - exactly this bug with no false positives. -- **Post-fix: 7/7 PASS.** +- **Pre-fix code: 6/8 FAIL** (all tool-shape tests incl. compacted-placeholder + incident + + protected branch); the 2 pure-text tests PASS by design (old counter agreed for text) — + proves the suite targets exactly this bug with no false positives. +- **Post-fix: 8/8 PASS.** Full gate results (post-fix): | Gate | Result | |------|--------| | `npm run typecheck` | ✅ clean | -| `npm run test` (full suite) | ✅ **1069/1069** (was 1062; +7 new) | +| `npm run test` (full suite) | ✅ **1070/1070** (was 1062; +8 new) | | `npx prettier --check` on changed files | ✅ test file + REQ/WORKLOG clean | Formatting note: `lib/messages/inject/utils.ts` carries 70 lines of PRE-EXISTING prettier