feat(agent-runtime): describe a failed compaction's range without a model - #688
zhangqingkun976 wants to merge 1 commit into
Conversation
|
The
So the red gate will clear once the login-shell probe type error is fixed — For this PR itself the relevant surfaces are green: |
4620603 to
dad2224
Compare
dad2224 to
71a52fe
Compare
|
The first CI run on this branch failed one test, and it was mine to fix rather
That expression is now Verified locally after the change: I also checked which other desktop tests read this source: only that file The branch is rebuilt on the current |
|
CI status on the current head (
I can't re-run the job myself (the API answers |
|
#688 / #700 / #705 are all included in #721 These three are the same work split small. I have assembled them (plus the Nothing in them changed; #721 is strictly a superset. Review whichever shape you |
…r-model context settings # Context and compaction, as one set This assembles the context/compaction work into a single reviewable change: the model can read its own history back, compaction stops being a one-way door, and the two thresholds that decide *when* it happens are configurable per model. It subsumes my three open PRs (vastsa#688, vastsa#700, vastsa#705) — those are the same code, split differently; review whichever shape you prefer. ## 1. The model can read its own history (`recall`, `recall_project`) **Host.** `recall_transcript` searches a session's complete transcript and ranks candidates by match strength (how many query words, how often), and `read_message_text` pages one message back in bounded character windows — which is how a tool result (not in the word index) is read. `search_project_messages` + `read_project_messages` do the same across one project's sessions, and both take a project id the host resolves from a path; a session bound to another project reads as *not found*, so session existence never leaks across projects. `SleepRecord` + `append_sleep` add a `sleep` line kind to the transcript: the layout scan ignores it and it never counts as a message. Five RPC methods carry this: `session.recall`, `session.readMessage`, `search.query`, `session.readProject`, `session.appendSleep`. **Model side.** `packages/agent-runtime/src/recall-tools.ts` holds both tools and takes a two-method host adapter (so both are unit-testable without a session, a provider or a process). Both are registered in the **core** tool set: a capability the model has to go looking for is one it will not reach for while it is trying to recover what a summary left out. The query rule is the host's, not a preference, so the description states it (split into words, every word must appear, unspaced Chinese matches literally) and tells the model to write the query in the words of whoever wrote the message. An empty answer says what to try instead; a bare "no matches" teaches nothing. ## 2. Compaction stops being a one-way door - **The projected summary carries a recall pointer**: pre-boundary messages are not deleted, and `recall` reads them back verbatim. The pointer is a promise, so it is only appended because the tool exists. - **A session ledger** is built mechanically when a checkpoint is installed (files read/modified, commands, message and tool-call counts, plus the goal and the unresolved items) and stored on the checkpoint's opaque `details`. The projection renders it as a bounded block after the summary, so the next window keeps an account of what the boundary covered — text that is one `recall` call away. A checkpoint written before the ledger existed projects exactly the pointer and nothing else. - **The wording now matches the mechanism.** The second budget reminder (Codex's `AutoCompactFallbackPrompt`: "unsummarized detail will not be available afterwards") is deleted rather than rephrased — that sentence is false here. The rollover text says where the messages went and how to read them again instead of telling the model the transcript is available "to the user". ## 3. Two thresholds, per model | Setting | Default | What it does | | --- | --- | --- | | `dynamicContext.thresholdPercent` | window-aware (`100 − max(100k, 15%×window)/window`, clamped 40–95) | At this share of the hard limit, old tool results are shortened to a head plus a recovery pointer. Off means nothing is narrowed. | | `earlyCompaction` `{thresholdPercent, delaySeconds, silent}` | 75 / 120 s / silent | After a run settles and the session stays idle that long, the pass compacts **in the background**. Any new prompt stands it down before it spends its summary request. | | `sleepTime` `{enabled, maxRunsPerHour}` | off / 2 | On the idle arm, write a deterministic digest of where the work stands to the transcript (no model call), so a summary request that never returns still leaves the next window with the goal and the open items. | The **coupling** is the point: the narrowing gate is what the outgoing request, the hard-boundary check and the early pass all read, so what narrowing saves is visible to compaction instead of compaction firing while real room remained. The early pass is the one place that spends a summary request off the critical path, and it is silent **only when it succeeded** — a degraded pass warns exactly like the inline path, and the hard boundary always warns. `compaction_end` reports `idle` and `silent`; the transcript row, the context inspector and `recall` record the checkpoint either way, and the renderer skips only the routine toast. ## Verification (Windows 11) cargo test -p host-core --bin pi-desktop-host-core 555 passed, 4 failed cargo fmt / clippy clean (1 pre-existing warning) pnpm --filter @pi-desktop/shared typecheck|build exit 0 pnpm --filter @pi-desktop/agent-runtime typecheck exit 0 pnpm --filter @pi-desktop/agent-runtime test 45/46 files pnpm --filter @pi-desktop/desktop typecheck exit 0 - The 4 Rust failures are pre-existing Windows path-separator assertions in `mcp_servers`, `user_skills`, `scheduled_rpc` and `scheduled_tools`; two of those files are untouched by this change (hash-verified). - The 1 agent-runtime failure is the pre-existing `src/native-pi-session.test.ts` case ("never deletes a foreign publication…"), which fails identically without this change. - New tests: 6 Rust (recall matching/ranking, non-ASCII folding, character paging, sleep append + layout, project search scoping, project read isolation), 10 for the recall tools (both tools, paging, the empty-answer advice, the output cap, project scoping, a host failure surfacing as text), 2 for the summary projection (pointer + ledger; a pre-ledger checkpoint projects the pointer alone). - Four existing tests were updated to the new contract, each strengthened rather than relaxed: the core tool list now includes both recall tools; the budget reminder is asserted to have **one** tier (and the sharper second one to be absent); the projected summary is asserted to carry the recall pointer; the dormant second-tier flag is gone. ## What is deliberately not here - **The settings pane UI.** The three settings are typed, clamped, persisted with the model binding and carried on the launch payload, but the pane's controls and their translations are not in this change — so today they are set through the binding rather than the UI. That is the next piece. - **Estimate calibration (vastsa#683).** The reviewer asked for a one-way (never lower) correction and a ratio-based unanchored term before it lands; that rework is in progress and will come as its own PR. - **The semantic/embeddings recall channel.** Measured and frozen in my build: it needs a model asset and a host channel for a gain the lexical channel already gets on the queries that matter here.
71a52fe to
e3bdd90
Compare
|
Rebased onto This also re-runs the Rust job, which failed on the previous run only on Local before pushing: This PR is also part of #721, if you would rather review the feature as a whole. |
73dd622 to
bb5ecfe
Compare
|
Rebased onto The only conflicts were append-vs-append — the E2E scenario catalog and the decisions log, where main and this branch each added an entry — and both sides are kept. The identifier bands are still free: upstream's newest ADR is 0299 and its newest decisions-log entry is D607 (the plugin crash report, which is this repository's own fix for issue #747 built on my #756). |
bb5ecfe to
182b289
Compare
|
Rebased onto Local gates on the rebased head:
The description now carries the base/head line and the E2E status in full. |
182b289 to
cdaab56
Compare
|
Rebased onto It applied cleanly, including the Local gates on the rebased head: One thing to know when reading the JS job: |
…odel Rebased onto main (79cd7ae); the rebase applied cleanly, no conflicts. When summary generation fails, the checkpoint is now built from the range it covers — a deterministic description plus a retained tail — instead of a notice that leaves the next window without anything to restore. Both degraded layers share one helper, so an empty tail can never be persisted for a completed turn. C:\Users\10470\.pi-desktop\scratch\cb9e2d41-8a55-4a18-899d-92ca2791ab51\commit-688-r2.txt
cdaab56 to
09c26ba
Compare
|
Correction to my previous comment on this PR. I wrote that pristine What actually happened: my local After rebuilding
The two real baseline failures are:
So This head Apologies for the noise — and for stating a baseline I had not verified. The CI panel on this head is the authoritative record, and it is green. |
This branch was previously deployed
1 participant
When the summary request fails, the session drops to a carried-forward summary
plus a recovery notice: the range's real history is gone and the next window is
told nothing about what the work was. The failure this was written for is the
evidence — 750k tokens of work replaced by a notice and a list of file names,
with no statement of the goal, of what went wrong, or of what was left to do.
This adds one rung to that ladder, ahead of the notice. It is model-free: no
provider request, no clock, no filesystem, so it cannot fail on a provider and it
costs nothing to try. Everything it does is a pure function of the messages in
the range, which is what lets the whole degradation path be tested without a
provider stub.
New module
packages/agent-runtime/src/compaction-trajectory.ts. It buildsa summary from the range mechanically:
on purpose: a paraphrase is what lets a next window redo work whose wording it
cannot tell apart from a different request.
tool calls failed.
landed for the same path afterwards resolves its failure and drops it, so a
test run that failed and was then fixed is not listed; a failure that cannot be
attributed to a path is always kept, because an unexplained failure is exactly
what a next window needs to know about. Two attribution rules keep that from
hiding a real failure: a call id is matched to the nearest preceding call
with that id (OpenAI-compatible local servers emit
1,2, … per response, soids repeat inside a range), and a repair counts only when its own result came
back without an error.
TODOlines from the lastassistant message. Only markers that cannot appear by accident are read; a
looser rule (any line under a "next steps" heading) would turn prose into a
task list the model never committed to.
The section headings and the minimum length are the ones the existing summary
check enforces, so a trajectory checkpoint is structurally a real summary: the
next summarization request can carry it forward and every reader sees the same
shape regardless of which layer produced it.
Wiring in
recoverCompactionFailure: the deterministic record is built andpersisted first, and the retained-tail notice stays exactly where it was as the
last resort — when the record does not fit the safe budget either, the notice
still runs, so a summary failure never ends without a checkpoint. The carried
summary is placed ahead of
COMPACTION_FALLBACK_MARKER, the same layout thenotice uses, so a chained compaction recovers the real carried history and drops
this range's description instead of cementing it (#224). Its
detailscarrystrategy: "trajectory",failureCode,retainedTailModeand a smalltrajectoryblock (counts, open items, whether a goal was found) for thesession inspector;
fallbackis deliberately left unset, so the last resortkeeps its own identity and existing readers are unaffected. The retained-tail
selection both layers use is now one helper (
retainedTailForDegradedCheckpoint)rather than two copies.
Verification (Windows 11, re-run on the rebased head
09c26ba1, with@pi-desktop/sharedrebuilt first — see the correction comment below about astale local artifact):
The one failing test is the pre-existing
src/native-pi-session.test.ts(
native fork children > never deletes a foreign publication ...), which failsidentically on pristine
main. The two skipped E2E stepsare gated on
PI_DESKTOP_TEST_API_KEY(E2E-008-live-model,E2E-009-stream).Three existing tests changed because the ladder changed, and their expectations
were tightened rather than relaxed: the summary-failure test now asserts the
record was persisted once, that its
details.strategyistrajectory, that thegoal text and the
## Goalsection are in the summary and that the marker isstill there; the two carried-summary tests assert the same while keeping their
original "the carried summary survives" checks. A fourth test was added for the
last resort: with the record forced oversized, the notice is written and it does
not carry the record's sections. Mutation-checked: forcing the record's persist
to report
oversizedturns the recovery tests red and restoring it turns themgreen.
shrinkSummarizeRange(retrying an oversized summary over a smaller range) isdeliberately not part of this PR.
fitSummaryInputToBudgetalready handlesthat case by reducing the prompt while keeping every message in scope (ADR 0282),
and dropping the oldest part of the range is the trade that decision made
against. If you would rather have the range shrink as a second rung after the
reduced prompt, it is a separate, smaller change and I can open it on its own.
Base and head
base 206085c07· head09c26ba1· 5 files, +900 / −32. Rebased onto currentmain(82 commitsfurther on); it applied cleanly except for one same-point insertion in
ModelSelectionPanes.tsxon the branches that touch it, where both sides werekept. The published tree is verified to be the same object the gates ran on: