Skip to content

fix: stabilize system prompt token estimate (#255) - #322

Merged
ranxianglei merged 4 commits into
ranxianglei:masterfrom
Beatrice0377:2026-08-18_fix-system-prompt-tokens
Aug 18, 2026
Merged

ranxianglei merged 4 commits into
ranxianglei:masterfrom
Beatrice0377:2026-08-18_fix-system-prompt-tokens

Conversation

@Beatrice0377

Copy link
Copy Markdown
Contributor

Summary

ACP already populated state.systemPromptTokens before pruning, but nudge and
acp_status ignored that cache — each re-estimated the system prompt tokens
from a different message view. After compression, the first visible assistant
can belong to a much later turn whose input includes large accumulated history,
inflating the estimate and causing the two numbers to diverge.

Root Cause

  • The stable pre-prune cache (state.systemPromptTokens) existed but was never
    consumed.
  • nudge re-estimated from the post-prune transform array, where compressed
    messages are removed and the first visible assistant is a late turn.
  • acp_status re-estimated from its own fetched message view.
  • After compression those views can select different late assistant messages,
    producing inflated and divergent estimates.

Fix

Three minimal production changes:

  1. cacheSystemPromptTokens preserves the first stable positive estimate —
    later transforms no longer overwrite it with a degraded post-compression
    estimate (write-if-undefined guard).
  2. estimateContextComposition prefers state.systemPromptTokens when it is a
    positive value; falls back to the existing
    estimateSystemPromptTokens(messages) unchanged when undefined.
  3. acp_status (collectVisibleMessages) prefers the same cached value; falls
    back to the existing estimate when undefined.

Scope

  • No persistence change.
  • No model/provider identity invalidation.
  • No /acp context change.
  • No public API change.
  • No version bump.

Tests

  • Pure regression tests: degraded post-compression visible history must not
    inflate the system estimate (cached 10K survives first-visible-assistant
    input of ~200K), and undefined cache falls back to the original estimate.
  • acp_status overview regression asserting the cached value appears in the
    breakdown.
  • §5.7 multi-turn growth-cycle test (preserveRecentMessages: 20): baseline →
    growth → nudge → compress → new baseline → growth → nudge again, asserting the
    stable system estimate survives compression.
  • Docker E2E: autonomous nudge → compress → growth → re-nudge with a verifier
    that ≥2 nudge observations report the same system prompt estimate.
  • Bug-targeting tests verified to FAIL with the fix reverted and PASS with it
    applied.
  • Verified: npm run typecheck, npm run build, npm run test (1010 pass),
    Docker E2E (12/12 scenarios), check-pr.sh.
  • Live OpenCode smoke: plugin loads, ACP state initializes, acp_status
    executes without ACP errors. This verifies the runtime path only — the precise
    estimateSystemPromptTokens inconsistent between nudge and acp_status after compression #255 regression is covered by the automated tests; the live environment was
    not used to reconstruct a 200K post-compression session.

Known limitations

  • The cache is in-session only; a restart after host compaction may re-estimate
    from degraded history.
  • Mid-session instruction/AGENTS.md changes do not invalidate the cache.

Both are intentionally out of scope for this fix.

Fixes #255

@Beatrice0377

Copy link
Copy Markdown
Contributor Author

Dual-agent review

Independent review of the current PR diff is complete.

  • Reviewer A: APPROVE — no blocking or non-blocking code findings.
  • Reviewer B: APPROVE — no blocking or non-blocking code findings.
  • Source changes, regression tests, §5.7 multi-turn coverage, and Docker E2E changes were reviewed independently.
  • No additional code changes are requested from the reviews.

CI note

All code-validation checks are passing:

  • CI / build ✅
  • CI / test (Node 22) ✅
  • CI / test (Node 24) ✅
  • CI / e2e ✅
  • PR Checks / pr-validation ✅

The only failing required check is PR Build Artifact / build-artifact.

Its checkout, npm ci, and npm run build steps succeed; it fails specifically at npm publish with ENEEDAUTH because NODE_AUTH_TOKEN is empty for this fork PR. I have not modified the workflow, package version, or npm authentication to work around this.

Leaving the artifact check for maintainer-side handling.

@ranxianglei
ranxianglei merged commit d24b175 into ranxianglei:master Aug 18, 2026
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

estimateSystemPromptTokens inconsistent between nudge and acp_status after compression

2 participants