Skip to content

fix(acp): trigger automatic compaction from provider context usage - #64

Merged
Blankeos merged 2 commits into
Blankeos:mainfrom
yan-ad:fix/acp-auto-compaction
Nov 11, 2026
Merged

Blankeos merged 2 commits into
Blankeos:mainfrom
yan-ad:fix/acp-auto-compaction

Conversation

@yan-ad

@yan-ad yan-ad commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fix ACP automatic compaction using the context usage reported by the provider, rather than only the persisted transcript estimate. Sessions could show usage beyond the configured threshold while never automatically compacting; /compact still worked because it bypasses that threshold gate.

  • Count input, cached input, output, and context added after each provider usage observation; never sum repeated tool-step input billing into a context size.
  • Include the composed system prompt and pending user message before checking the threshold. Record the previous prompt estimate on usage parts so subsequent agent/prompt changes can replace it without double-counting.
  • Ignore pre-compaction usage and retained-tail billing as context measurements. Preserve the last valid provider input measurement when newer usage omits input tokens.
  • Notify ACP clients when automatic compaction succeeds and send refreshed context usage/cumulative cost; keep compaction summary billing in subsequent session totals without double-counting completed turns.
  • Filter archived history before inserting the fresh system prompt so it survives conversion after compaction.
  • Add regression coverage and a local mock-provider stdio test exercising automatic compaction and a second turn. Correct the existing command-dispatch test's offline model fixture so it does not require Ollama.

ACP versus TUI context and credit analysis

ACP does not have a separate larger model context limit or credit allowance. Both paths use the model's configured/discovered limits, the shared compaction threshold (context_window - reserved), and the same provider/tool loop.

Before this fix:

  • ACP usage notifications used provider observations, but its compaction gate used only transcript tokens, excluding request overhead. Large provider prompts could cross the threshold without triggering automatic compaction.
  • TUI includes MCP tool-prefix estimates in its threshold and usage display; ACP's gate did not include those estimates or its composed system prompt.
  • Auto-compaction changed persisted context silently, leaving ACP clients displaying pre-compaction usage. Summary billing also disappeared from the next prompt's cumulative cost base.

Delayed compaction can increase billed input because each model/tool request resends a larger context; cached input still occupies the context window and can carry provider-dependent charges. Cumulative billed usage is not the same thing as current context size. This change does not claim measured cost savings or change provider pricing.

Remaining differences outside this focused fix: TUI scopes prompt tool guidance to the active agent, whereas ACP composes it from the full registry. Both paths still check automatic compaction at turn boundaries, not midway through a running tool loop. Auto-compaction still requires known model limits and a sufficiently large compactable head, and compaction failures remain logged rather than making the prompt fail.

Validation

  • cargo test acp:: --bin crabcode: 75 passed.
  • cargo test session::compaction:: --bin crabcode: 21 passed.
  • cargo test --test acp_stdio: 8 passed, including the local mock-provider regression (also run repeatedly on its own).
  • cargo fmt --all -- --check and git diff --check: passed.
  • cargo test --bin crabcode -- --test-threads=1: 1,850 passed, 3 ignored, 1 unrelated existing failure: ui::components::chat::tests::test_edit_tool_renders_codex_style_diff_summary expects a basename from a hardcoded /Users/carlo/Desktop/Projects/crabcode path on another checkout. The parallel full-unit run additionally exposes existing catalog/Ollama shared-state test failures.

Built the required remote-client assets using the existing remote-client-build recipe's Bun commands before Rust validation; no dependency or generated-asset changes are included.

Follow-up: compaction mode tooltip

A configured visible compaction agent without a description now advertises Compaction agent. Summarizes conversation context for continued work. in both legacy availableModes and modern mode configOptions. Explicit descriptions remain unchanged. This does not add a built-in selectable compaction agent, expose hidden/subagents, or change how automatic compaction runs.

  • Added coverage for blank/whitespace descriptions, custom text, existing build descriptions, and hidden/subagent filtering.
  • Verified serialized descriptions over ACP stdio.
  • Updated validation: 76 ACP tests and 8 stdio tests pass; formatting and whitespace checks pass.

@yan-ad

yan-ad commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

Added the configured compaction-mode tooltip in 4a4f25a, with custom descriptions preserved and both legacy modes and modern config options covered. Refactoring is separate in #67 and based on this PR. Both changes are synced to yan-ad/crabcode:yan-flavor at 62a9a4f; flavor validation passes (61 ACP, 21 compaction, 4 stdio tests, with five repeated parallel stdio runs). Existing flavor lifecycle behavior and concurrent install/upgrade commits are preserved.

@Blankeos

Blankeos commented Nov 7, 2026

Copy link
Copy Markdown
Owner

fix/acp-auto-compaction → main
Model: claude-sonnet-5-5, effort unknown

Confidence: 4/5

The fix is correct as far as I can verify and the new end-to-end test really exercises it. I hold back one point for two reasons. The token accounting is heuristic (it only counts a provider observation once, using max() over parts). It is also stacked with #67, which rewrites the same file.

Checks run (worktree of the PR head 4a4f25a, separate target dir):

  • cargo fmt --check: clean.
  • cargo build: OK.
  • cargo test --bin crabcode -- --test-threads=1 (whole unit suite): 1851 passed, 1 failed, 3 ignored. The failure is ui::components::chat::tests::test_edit_tool_renders_codex_style_diff_summary. It hardcodes /Users/carlo/Desktop/Projects/crabcode, is in code this PR does not touch, and is the same failure the author reports. It is unrelated.
  • cargo test --test acp_stdio: 8 passed. This includes the new automatically_compacts_provider_usage_over_stdio: a real crabcode acp child process talks to a local mock OpenAI-compatible server. The first turn reports 85k prompt tokens. The test then asserts that a "Context compacted" notice and a usage_update below 80k arrive, that exactly one summary request is made, that the second turn does not recompact, and that the second request carries the fresh system prompt and the summary but not the archived prompt.
  • clippy on lines this PR adds: only too many arguments (9/7) on receive_prompt_stream (the PR adds one parameter to a function that was already over the limit). Nothing else new.
  • No UI or TUI output changes. The only user-visible differences are an ACP notification and a mode description string. Both are covered over stdio by the tests above, so screenshots do not apply.

Confidence bumps:

  • Run against a real provider and a real ACP client (Zed or similar) to confirm the usage numbers line up with the provider's own accounting (needs provider credentials, which I did not use) → 4.5/5.

Summary

ACP auto-compaction used only the persisted-transcript estimate, so sessions whose real provider-reported context was above the threshold never auto-compacted. Only /compact worked. This PR does the following:

  • It computes context size from the latest provider usage (input + cache + output, plus content added after that observation). It ignores usage from before a compaction marker.
  • It adds the freshly composed system prompt and the pending user message to the check. It records the previous prompt estimate on each usage part (acp_prompt_tokens) so a changed prompt replaces the old estimate rather than double-counting.
  • The check now runs after the system prompt is composed, rather than before.
  • It sends a "Context compacted" notice and a refreshed usage_update (with cost) after auto-compaction.
  • It filters archived history before inserting the system prompt, so the prompt survives compaction.
  • Session cost now includes summary billing.
  • It also adds an unrelated but tiny follow-up: a default description for a configured compaction mode, in both availableModes and configOptions.

Scope is limited to src/acp/service.rs, one visibility change in src/session/compaction.rs, and tests. Roughly 600 of the 956 added lines are tests.

Map

flowchart LR
  P[prompt in service.rs] --> SP[compose system prompt]
  SP --> M[maybe_auto_compact_session]
  M --> C[acp_context_tokens]
  C --> K[session::compaction totals and markers]
  M --> N[notify_compaction: text + usage_update]
  P --> R[acp_request_messages: filter archived history]
  P --> S[receive_prompt_stream: stamps acp_prompt_tokens on usage parts]
  T[tests/acp_stdio.rs mock provider] -.-> P
Loading

Start at acp_context_tokens in src/acp/service.rs.

Regressions

  • None found. The existing auto-compaction path is only made more accurate. Failures are still logged and swallowed, as before.
  • Behavior change: compaction is now evaluated after system-prompt composition. If the prompt is cancelled earlier, nothing compacts. That is fine.
  • Cost: acp_session_cost falls back to usage_cost() when message.cost is None. Previously only message.cost was summed, so cumulative cost shown to ACP clients may rise for sessions whose messages had usage parts but no message-level cost. This is probably the intent (the PR says summary billing was being dropped), but the maintainer should know the displayed number can change.
  • Heuristic: acp_context_tokens takes the max across usage parts of the last assistant message that has provider input. A provider that reports cumulative rather than per-request input would overcount. The unit tests cover the OpenAI-style semantics only.

Migrations

None. A new optional acp_prompt_tokens key is added to usage-part JSON. Older sessions without it work, and the key is ignored by older builds.

Before merging

@Blankeos
Blankeos merged commit 21df873 into Blankeos:main Nov 11, 2026
6 checks passed
Blankeos added a commit to yan-ad/crabcode that referenced this pull request Nov 11, 2026
Resolve tests/acp_stdio.rs against Blankeos#64's mock provider harness and pass the new prompt-token argument in the provider failure tests.
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.

2 participants