Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 72 additions & 0 deletions devlog/2026-09-03_recommend-exec-counter-alignment/REQ.md
Original file line number Diff line number Diff line change
@@ -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.
74 changes: 74 additions & 0 deletions devlog/2026-09-03_recommend-exec-counter-alignment/WORKLOG.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
# 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` (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. 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)
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: 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) | ✅ **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
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.
20 changes: 10 additions & 10 deletions lib/messages/inject/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,18 @@ import {
type MessagePriority,
listPriorityRefsBeforeIndex,
} from "../priority"
import { estimateSystemPromptTokens } from "../../token-utils"
import {
countMessageCharacters,
estimateSystemPromptTokens,
getCurrentTokenUsage,
} from "../../token-utils"
import {
appendToTextPart,
appendToLastTextPart,
createSyntheticTextPart,
hasContent,
} from "../utils"
import { getLastUserMessage, isIgnoredUserMessage, isSyntheticMessage } from "../query"
import { getCurrentTokenUsage } from "../../token-utils"
import { getActiveSummaryTokenUsage } from "../../state/utils"

export interface LastUserModelContext {
Expand Down Expand Up @@ -782,13 +785,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<string>()
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) {
Expand All @@ -810,15 +811,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
}
Expand Down
Loading
Loading