Skip to content
Merged
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
114 changes: 114 additions & 0 deletions devlog/2026-08-18_fix-system-prompt-tokens/DESIGN.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
# DESIGN - Stabilize system prompt token estimate (#255)

- Task ID: `2026-08-18_fix-system-prompt-tokens`
- Home Repo: `opencode-acp`
- Created: 2026-08-18
- Status: Accepted

## 1. Problem Statement

- **What problem are we solving?** After ACP compression removes the true first
assistant message from visible history, nudge and `acp_status` re-derive the
system prompt token estimate from the _first visible assistant_ — a much later
turn whose `input` includes large accumulated history. Both inflate the
estimate, and because each consumer sees a different message view (post-prune
transform array vs. fresh store fetch), the two numbers diverge.
- **Why now?** Issue #255. The `SessionState.systemPromptTokens` field and its
writer (`cacheSystemPromptTokens`) already exist but have no readers — the
missing link is wiring consumers to the cache.

## 2. Goals & Non-Goals

- **Goals**:
- nudge and `acp_status` use the same stable system prompt token estimate
after compression changes visible messages.
- Preserve existing behavior exactly when no cache exists (`undefined`).
- **Non-Goals** (per human review of the investigation phase):
- No persistence of `systemPromptTokens` (survives only in-session).
- No model/provider identity invalidation.
- No `/acp context` changes.
- No refactor of `lib/token-utils.ts`; `estimateSystemPromptTokens()` public
semantics unchanged.

## 3. Current Architecture

**How it works today:**

```
estimateSystemPromptTokens(messages) # token-utils.ts
↑ 现算 ×4(互不共享)
nudge: injectCompressNudges → estimateContextComposition → estimateSystemPromptTokens(messages)
acp_status: collectVisibleMessages → estimateSystemPromptTokens(rawMessages)
/acp stats: buildStatusReport(同 acp_status)
/acp context: analyzeTokens(内联实现)

cacheSystemPromptTokens(state, output.messages) # hooks.ts:233,prune 前,每轮写入
→ state.systemPromptTokens(存在,但无人读取)
```

**Pain points:**

- nudge runs after `prune` (hooks.ts:253 > 247); `filterCompressedRanges`
physically removes compressed messages from the array → first visible
assistant is a late turn → inflated estimate.
- `acp_status` fetches fresh from the store → different view → inconsistent
numbers.

## 4. Proposed Architecture

**Overview (text diagram):**

```
pre-prune transform (hooks.ts:233)
→ cacheSystemPromptTokens(state, output.messages)
→ state.systemPromptTokens ← write-if-undefined(首测后不再覆盖)
↓
nudge estimateContextComposition → state?.systemPromptTokens ?? 现算
acp_status collectVisibleMessages → ctx.state.systemPromptTokens ?? 现算
undefined 时:fallback 到 estimateSystemPromptTokens(messages)(旧行为)
```

**Key components:**

- `lib/ui/utils.ts` — `cacheSystemPromptTokens`: write-if-undefined guard.
- `lib/messages/inject/utils.ts` — `estimateContextComposition`: prefer cache.
- `lib/compress/status.ts` — `collectVisibleMessages`: prefer cache.

**Data flow:** pre-prune cache → `SessionState.systemPromptTokens` → both
consumers → `undefined` fallback to old estimate.

**API / interface changes:** none (fields/functions already exist).

## 5. Design Decisions & Rationale

| Decision | Options Considered | Chosen | Why |
| --------------------------- | ---------------------------------------------------------------- | ------------------ | ------------------------------------------------------------------------------------------------------------------------------------------- |
| Cache source | first measurement wins (write-if-undefined) vs. always overwrite | write-if-undefined | After compression the array is degraded; overwriting would corrupt the stable value. `undefined` stays `undefined` (never cache undefined). |
| Reader fallback | hard-require cache vs. `?? estimateSystemPromptTokens` | `??` fallback | Backward compat: old/restart states lack the field; behavior identical to pre-fix master when `undefined`. |
| Persistence | persist vs. in-session only | in-session only | Per human scope decision; a restart re-measures on first pre-prune transform. Known limitation (see §8). |
| Model identity invalidation | add vs. skip | skip | Per human scope decision; no evidence of mid-session model-switch churn; would expand blast radius. |
| `/acp context` | unify vs. leave | leave | Separate surface; not required to satisfy #255. Would be scope expansion. |

## 6. Impact Analysis

- **Backward compatibility:** full — optional field, `undefined` fallback keeps
old behavior byte-for-byte.
- **Performance:** cache read is O(1); saves the existing per-consumer estimate
work in the common (cached) path.
- **Security:** none.
- **Dependencies:** none.

## 7. Migration Plan

- **Steps:** not applicable — no persisted state changes, no config changes.
- **Feature flags:** none.

## 8. Open Questions / Known Limitations

- Plugin restart: cache is re-measured on the first pre-prune transform. If
OpenCode compaction has already dropped the true first assistant from the
store, the re-measured value is degraded — out of scope for #255 (would need
persistence).
- AGENTS.md / project instruction changes mid-session do not actively invalidate
the cache — out of scope for #255 (would need model-identity or file-watch
invalidation).
91 changes: 91 additions & 0 deletions devlog/2026-08-18_fix-system-prompt-tokens/REQ.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
# REQ - Stabilize system prompt token estimate (#255)

- Task ID: `2026-08-18_fix-system-prompt-tokens`
- Home Repo: `opencode-acp`
- Created: 2026-08-18
- Status: Done
- Priority: P1
- Owner: beatrice
- References: Issue #255

## 1. Background & Problem Statement

- **Context**: `estimateSystemPromptTokens()` (`lib/token-utils.ts`) derives the
system prompt token estimate from the _first visible assistant message_ with
token data (`tokens.input + cache.read + cache.write` minus first user text).
After ACP compression, `prune()` (`lib/messages/prune.ts:52-90`,
`filterCompressedRanges`) physically removes compressed messages from the
transform array. The first visible assistant then belongs to a much later
turn, whose `input` includes a large accumulated history — inflating the
system prompt estimate to roughly the size of the whole context.
- **Current behavior (symptom)**: nudge (`estimateContextComposition` in
`lib/messages/inject/utils.ts:565`) and `acp_status`
(`lib/compress/status.ts:175`) each re-derive the system estimate from their
own message views (post-prune transform array vs. fresh store fetch), so the
two numbers diverge and both can be wildly overestimated.
- **Expected behavior**: once a reliable positive `state.systemPromptTokens` has
been measured (before compression removed the true first assistant), nudge and
`acp_status` both use that stable cached value; later compression/compaction
of visible messages must not overwrite or invalidate it.
- **Impact**: wrong system prompt numbers mislead both the model (nudge
breakdown, compressibleTokens = total − systemTokens − ...) and the user
(`acp_status` breakdown).

## 2. Reproduction (if applicable)

- **Environment**:
- Node: 22+
- OS/Arch: linux-x64
- **Minimal reproduction steps**:
1. Session with first assistant input ≈ `system + first user` (system ≈ 10_000).
2. ACP compression removes the first assistant from visible messages.
3. First _visible_ assistant input ≈ `system + large history` (≈ 200_000).
4. nudge `estimateContextComposition` reports system ≈ 200_000 while the
cached `state.systemPromptTokens` is 10_000.
- **Relevant configuration**: default ACP config.

## 3. Constraints & Non-Goals

- **Constraints**:
- Backward compatibility: `state.systemPromptTokens` may be `undefined`;
all readers must fall back to the existing
`estimateSystemPromptTokens(messages)` behavior unchanged.
- Performance requirements: no new per-turn computation; reuse the cache.
- Resource limits: none.
- **Non-Goals** (explicitly out of scope, per human review of prior
investigation):
- No persistence changes (`PersistedSessionState` / save/load stay untouched).
- No model/provider identity invalidation.
- No `/acp context` changes (separate surface; not required to satisfy #255).
- No `lib/token-utils.ts` refactor; `estimateSystemPromptTokens()` public
semantics unchanged.
- No `package.json` version change.

## 4. Acceptance Criteria (must be testable)

- **Correctness**:
- [x] `cacheSystemPromptTokens()`: once `state.systemPromptTokens` holds a
positive value, subsequent calls with a degraded messages array (first
visible assistant input ≈ 200_000) must NOT overwrite it (stays 10_000).
- [x] `estimateContextComposition()`: with `state.systemPromptTokens = 10_000`
and a degraded messages array, `composition.systemTokens === 10_000`
(not ≈ 200_000).
- [x] `acp_status` uses the same cached value (if test structure allows).
- **Performance / Stability**: no new per-turn work; cache read is O(1).
- **Regression**:
- [x] New/modified test cases added to test suite and passing
(`tests/inject-utils-pure.test.ts` + cache overwrite regression).

## 5. Proposed Approach (optional)

- **Affected modules & entry files**:
- `lib/ui/utils.ts` — `cacheSystemPromptTokens`: write-if-undefined guard.
- `lib/messages/inject/utils.ts` — `estimateContextComposition`: prefer
`state?.systemPromptTokens` over fresh estimate.
- `lib/compress/status.ts` — `collectVisibleMessages`: prefer
`ctx.state.systemPromptTokens` over fresh estimate.
- `tests/inject-utils-pure.test.ts` — regression tests.
- **Risks**: low — readers only prefer the cache when it is a positive number;
`undefined` preserves current behavior exactly.
- **Rollback strategy**: revert the three file changes; tests would then fail
(proving they target the fix).
81 changes: 81 additions & 0 deletions devlog/2026-08-18_fix-system-prompt-tokens/WORKLOG.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
# WORKLOG - Stabilize system prompt token estimate (#255)

## Changes

1. `lib/ui/utils.ts` — `cacheSystemPromptTokens`: added write-if-undefined
guard. Once `state.systemPromptTokens` holds a stable positive value, later
transforms never overwrite it — after compression/compaction the first
visible assistant's input includes large history, which would inflate the
estimate. `undefined` stays `undefined` when no reliable assistant token data
is present.

2. `lib/messages/inject/utils.ts` — `estimateContextComposition`: `systemTokens`
now prefers `state?.systemPromptTokens` (when positive); falls back to
`estimateSystemPromptTokens(messages)` when undefined — old behavior
preserved.

3. `lib/compress/status.ts` — `collectVisibleMessages`: `systemTokens` now
prefers `ctx.state.systemPromptTokens` (when positive); falls back to
`estimateSystemPromptTokens(rawMessages)` when undefined.

4. Tests (unit):
- `tests/inject-utils-pure.test.ts` — 5 new tests: cached-value preference,
undefined-cache fallback, cache-overwrite protection, initial write,
keep-undefined-when-no-data.
- `tests/acp-status.test.ts` — 1 new test: overview breakdown uses cached
value instead of re-estimating from degraded visible messages.
- `tests/inject.test.ts` — 1 new §5.7-compliant multi-turn growth-cycle test
(4 turns: baseline → growth → nudge → compress → new baseline → growth →
nudge) with `preserveRecentMessages = 20` (production default),
side-effect assertions (`shouldInjectThisTurn` + `lastPerMessageNudgeTokens`
after every turn), and #255 assertions (cached system value survives
degraded post-compression arrays).

5. E2E infrastructure (`scripts/e2e/`, test-only — not production):
- `fake-llm-server.ts` — observation now records `nudgeSystemTokens` parsed
from the nudge breakdown text (`Breakdown: N system`).
- `verify.ts` — new `nudgeSystemTokensStable` assertion: all nudge
observations must report the same system estimate (requires ≥2 nudge
observations).
- `scenarios/10-autonomous-nudge-refire.json` — added
`nudgeSystemTokensStable: true` (extended existing scenario; scenario 10
already exercises multi-turn nudge→compress→growth→re-nudge with ≥2 nudge
observations).

## Test Results (pre-fix verification)

The bug-targeting tests were confirmed FAILING on unmodified production code
before the fix:

- `estimateContextComposition: prefers cached...` — FAIL (199996 !== 10000)
- `cacheSystemPromptTokens: does not overwrite...` — FAIL (199996 !== 10000)
- `acp_status: overview prefers cached...` — FAIL
- §5.7 multi-turn cycle test — FAIL under reverted production fix
- E2E scenario 10 with `nudgeSystemTokensStable` — FAIL under reverted fix
(observed system values `[18200, 18700]` diverged after compression)

All confirmed via `git show 09bf2b8 -- lib/ | git apply -R` + test run + `git
apply` restore. No temporary reverts committed.

## Test Results (post-fix)

- typecheck: pass (`tsc --noEmit`, exit 0)
- build: pass (tsup success)
- test: 1009 + 1 new = 1010 pass / 0 fail (full suite, `npm run test`)
- Docker E2E: all 12 scenarios pass (`SKIP_BUILD=1 ./scripts/e2e/run-e2e.sh`),
including scenario 10 with the new `nudgeSystemTokensStable` assertion
- Live OpenCode smoke test: plugin loads, ACP state file written, no
ACP-related errors (the only observed errors were opencode→model-gateway TLS
certificate failures, unrelated to ACP)

## Scope Notes

Per human review of the investigation phase, this PR is intentionally minimal:

- No persistence changes (`PersistedSessionState` / save / load untouched).
- No model/provider identity invalidation.
- No `/acp context` changes.
- `estimateSystemPromptTokens()` public semantics unchanged.
- `package.json` version unchanged.
- Production changes are 3 small source edits; test/E2E additions are scoped to
verification of the fix.
9 changes: 8 additions & 1 deletion lib/compress/status.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,14 @@ function collectVisibleMessages(
}
})

return { messages: result, summaryTokens, systemTokens: estimateSystemPromptTokens(rawMessages) }
return {
messages: result,
summaryTokens,
systemTokens:
ctx.state.systemPromptTokens !== undefined && ctx.state.systemPromptTokens > 0
? ctx.state.systemPromptTokens
: estimateSystemPromptTokens(rawMessages),
}
}

function renderOverview(
Expand Down
5 changes: 4 additions & 1 deletion lib/messages/inject/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,10 @@ export function estimateContextComposition(
.map(([tool, tokens]) => ({ tool, tokens }))
.sort((a, b) => b.tokens - a.tokens)

const systemTokens = estimateSystemPromptTokens(messages)
const systemTokens =
state?.systemPromptTokens !== undefined && state.systemPromptTokens > 0
? state.systemPromptTokens
: estimateSystemPromptTokens(messages)

return {
toolTokens,
Expand Down
7 changes: 7 additions & 0 deletions lib/ui/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,13 @@ export function formatProgressBar(
}

export function cacheSystemPromptTokens(state: SessionState, messages: WithParts[]): void {
// [FIX #255] Never overwrite a stable positive cache - after compression
// the first visible assistant's input includes large history, inflating
// the estimate.
if (state.systemPromptTokens !== undefined && state.systemPromptTokens > 0) {
return
}

let firstInputTokens = 0
for (const msg of messages) {
if (msg.info.role !== "assistant") {
Expand Down
22 changes: 20 additions & 2 deletions scripts/e2e/fake-llm-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ export interface RequestObservation {
/** Count of `compress` tool_use calls visible to the LLM in this request */
compressCallCount: number
nudgeDetected: boolean
nudgeSystemTokens?: number
isChild: boolean
isAuxiliary: boolean
}
Expand All @@ -100,10 +101,11 @@ function recordObservation(
messageCount: number,
compressCallCount: number,
nudgeDetected: boolean,
nudgeSystemTokens: number | undefined,
isChild: boolean,
isAuxiliary: boolean,
): void {
observations.requests.push({ turn, inputTokens, messageCount, compressCallCount, nudgeDetected, isChild, isAuxiliary })
observations.requests.push({ turn, inputTokens, messageCount, compressCallCount, nudgeDetected, nudgeSystemTokens, isChild, isAuxiliary })
try {
writeFileSync(OBSERVATIONS_FILE, JSON.stringify(observations, null, 2))
} catch {
Expand Down Expand Up @@ -199,7 +201,7 @@ async function handleChatCompletion(req: Request): Promise<Response> {

const isAuxiliary = tools.length === 0

recordObservation(readTurnCounter(), inputTokens, messages.length, compressCallCount, nudgeDetected, isChild, isAuxiliary)
recordObservation(readTurnCounter(), inputTokens, messages.length, compressCallCount, nudgeDetected, extractNudgeSystemTokens(messages), isChild, isAuxiliary)

log(
` body: stream=${isStream} msgs=${messages.length} ` +
Expand Down Expand Up @@ -379,6 +381,22 @@ function detectNudge(messages: any[]): boolean {
return false
}

function extractNudgeSystemTokens(messages: any[]): number | undefined {
const breakdownRe = /Breakdown: ([0-9.]+)K? system/
for (const msg of messages) {
if (msg?.role === "user") {
const text = extractMessageText(msg)
const match = text.match(breakdownRe)
if (match) {
const value = parseFloat(match[1])
const kilo = text.includes(`${match[1]}K system`)
return kilo ? Math.round(value * 1000) : Math.round(value)
}
}
}
return undefined
}

function handleNudgeCompressStep(
model: string,
messages: any[],
Expand Down
3 changes: 2 additions & 1 deletion scripts/e2e/scenarios/10-autonomous-nudge-refire.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
"maxBlockCount": 5,
"nudgeBaselineSet": true,
"maxCompressCallsVisible": 2,
"maxNudgeCount": 10
"maxNudgeCount": 10,
"nudgeSystemTokensStable": true
}
}
Loading
Loading