Skip to content

fix(agent-runtime): retry and right-size the compaction summary before falling back - #563

Merged
vastsa merged 1 commit into
mainfrom
fix/compaction-summary-retry
Sep 17, 2026
Merged

vastsa merged 1 commit into
mainfrom
fix/compaction-summary-retry

Conversation

@vastsa

@vastsa vastsa commented Sep 17, 2026

Copy link
Copy Markdown
Owner

Fixes #543. Supersedes #554 (direction kept, mechanisms replaced — see ADR 0282 "Alternatives").

Root cause

On real long sessions most automatic compactions ended as the ~112-token retained-tail recovery notice, and the transcript row labelled each one summary ≈112 tokens. Three things in packages/agent-runtime made the ADR 0049 fallback the common outcome:

  1. No retry on the summary request. compact() was called with retry: undefined, so pi-ai returned the first failed response as-is. The main turn retries transient failures through the streamFn wrapper (D186), but pi-agent-core's summary goes through Models.completeSimple and never reaches it. One dropped stream or 503 discarded the whole summary.
  2. The preflight guard measured the wrong thing. compactionSummaryWouldExceedBudget summed estimateTokens over the raw messages, while pi-agent-core serializes the conversation to text and caps every tool result at 2 000 characters. A tool-heavy session looked several times larger than the prompt that would be sent, and the guard skipped the model for summaries that would have fit. The reporter's tokensBefore values (200k–885k on a 200k window) are exactly this shape.
  3. The UI could not tell a fallback from a summary. ContextCompactionMark only distinguished fresh_window; a retained-tail checkpoint rendered as a successful summary of N tokens.

Change

  • generateCompaction passes pi-ai a bounded RetryPolicy (3 retries, 2s/4s/8s, abort-aware). pi-ai's classifier decides what is transient; quota/auth/malformed requests still return at once.
  • The budget guard sizes the prompt pi actually serializes (convertToLlm + serializeConversation). When that still exceeds the window, exactly one reduced input is tried (tool results cut to a 500-char prefix with a visible marker, thinking dropped, no message removed) before the fallback runs. The checkpoint's messagesToSummarize / retainedTail are the originals; only the request payload is reduced.
  • ContextCompactionMark gains an additive fallback?: "retained_tail" derived from persisted details.fallback. CompactionRow renders it as "summary generation failed · recent context retained" in all 8 locales. Historical fallback rows get the honest label too, since the mark is derived on session open.

No record schema, protocol version, or host-core change. fresh_window untouched. Manual /compact inherits retry + sizing and keeps fail-fast semantics.

Why not #554 as-is

  • Its outer loop retried every recoverable failure, including deterministic ones ("no new context", 401, quota), sleeping 2s+4s each time.
  • Its shrink step dropped the oldest messages while the checkpoint's throughMessageId still covered them, so the summary silently claimed a range it had not seen.
  • Its test file re-declared the constants and a "mirror" of the shrink function; it never imported the runtime.
  • Conflicts with current main; no docs/ADR.

Docs

ADR 0282 (amends ADR 0049 decision 1) · D445 · 02-agent-runtime.md · 01-ipc-protocol.md mark shape · E2E-084 · zh mirrors.

Validation

  • pnpm --filter @pi-desktop/shared test — 748 passed
  • pnpm --filter @pi-desktop/agent-runtime test — 589 passed (8 new runtime tests: retry-then-succeed, deterministic no-retry, retry budget exhausted, abort during backoff, guard sizes serialized prompt, reduce-once, fallback only when reduced still overflows, reduced prompt reaches the model with the checkpoint covering every message; 9 unit tests on the new module)
  • pnpm --filter @pi-desktop/i18n test — 25 passed (key parity across 8 locales)
  • apps/desktop context-compaction + assistant-turns contract tests — 29 passed
  • pnpm build:js, desktop + agent-runtime typecheck, pnpm lint, pnpm docs:check, check-architecture, check:agent-policy — all green
Task candidate: fix/compaction-summary-retry @ 3295c1dbc (rebased on origin/main 0aef7911f)
Base main: 0aef7911f
E2E suites: test:e2e (smoke) 22/22 passed, 2 skipped (E2E-008/009 need PI_DESKTOP_TEST_API_KEY);
            test:e2e:transcript probe ok, 0 failures; test:e2e:boot 3/3 passed
Result: PASS
Environment: macOS 24.6.0, Node 22, pnpm 11.18.0, host-core debug build from this candidate

NOT RUN: live-provider E2E (E2E-008/009) — no test API key in this environment; the retry path is covered by scripted completeSimple responses through the real pi-ai retryAssistantCall. Residual risk: a sustained provider outage now delays the fallback by up to ~14s (bounded, Stop-cancellable).

…e falling back

Issue #543: on real long sessions most automatic compactions ended as the
~112-token retained-tail recovery notice, and the transcript row labelled
each one "summary ≈112 tokens". Three things made the ADR 0049 fallback
the common outcome instead of the exception:

- compact() was called without a RetryPolicy, so pi-ai returned the first
  failed response as-is. The main turn retries transient failures through
  the streamFn wrapper, but the summary goes through Models.completeSimple
  and never reaches it; one dropped stream or 503 discarded the summary.
- The preflight guard summed estimateTokens over the raw messages, while
  pi-agent-core serializes the conversation to text and caps every tool
  result at 2 000 characters. Tool-heavy sessions looked several times
  larger than the prompt that would be sent and skipped the model for
  summaries that would have fit.
- ContextCompactionMark could not tell a fallback from a summary, so the
  row rendered the recovery notice as a successful summary of N tokens.

The summary request now carries a bounded pi-ai RetryPolicy (3 retries,
2s/4s/8s, abort-aware; pi-ai's classifier decides what is transient, so
quota and auth failures still return at once). The guard sizes the prompt
pi actually serializes, and when that still exceeds the window it tries
exactly one reduced input (tool results cut to a 500-char prefix, thinking
dropped, no message removed) before the fallback runs. The mark gains an
additive `fallback?: "retained_tail"` derived from persisted details, and
the row labels such checkpoints as a failed summary in all locales.

Supersedes PR #554, which retried every recoverable failure including
deterministic ones and shrank the input by dropping the oldest messages
while the checkpoint still claimed to cover them. Records ADR 0282 /
D445 and updates E2E-084, the runtime spec, and the IPC mark shape.
Copilot AI lite review requested due to automatic review settings September 17, 2026 19:00

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

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.

[Bug] 摘要失败后立刻丢弃旧上下文,不会重试压缩直到成功 / Failed compaction drops history instead of retrying summarization

2 participants