feat(agent-runtime): tier old tool results when the outgoing view is under pressure - #705
zhangqingkun976 wants to merge 1 commit into
Conversation
35e304f to
1caab75
Compare
|
#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.
1caab75 to
a11b2d2
Compare
|
Rebased onto New head Local before pushing: |
abac678 to
5d519fb
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). |
5d519fb to
a7ff76d
Compare
|
Rebased onto One real conflict, and it is worth a look. convertToLlm: (messages) =>
alignRetainedReasoningIdentity(
convertToLlm(this.narrowToolResultsUnderPressure(this.dropDuplicateToolCalls(messages))),Both passes are identity functions when they have nothing to do, so the composition is inert on an ordinary request — the same promise the PR made before the rebase. Local gates on the rebased head:
The description now carries the base/head line, the conflict resolution and the E2E status in full. |
a7ff76d to
94a3e16
Compare
|
Rebased onto It applied cleanly, including the Local gates on the rebased head: One thing to know when reading the JS job: |
…under pressure Rebased onto main (79cd7ae). One conflict, from vastsa#733 extracting the budget formula: `ContextBudget` and the `retainedUserMessageBudget` doc comment now live in `context-budget.ts`, so they stay out of `runtime.ts`, and `narrowToolResultsUnderPressure` sits beside the import. Standalone shape: the gate is the fixed `TOOL_RESULT_TIER_PRESSURE` share of the hard limit. The per-model configurable threshold is in vastsa#721, which contains this change. C:\Users\10470\.pi-desktop\scratch\cb9e2d41-8a55-4a18-899d-92ca2791ab51\commit-705-r2.txt
94a3e16 to
a90bcb1
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
Tool results are most of the context mass, and a single request can carry many
individually-legal ones: the per-result budgets in
docs/spec/03-runtime/16-tool-result-limits.md§2 bound each result as it isproduced (Read 128 KB, shell 96 KB, spill files), but nothing bounds the
aggregate a request carries, so a few old results stay in the window at full
size until the whole thing trips the compaction threshold and pays for a lossy
summary.
This adds the missing layer: under pressure, an outgoing view keeps the head of
an old tool result and names the way to read the rest. The stored messages are
never touched — the projection is rebuilt per request — so a later request with
room renders the full text again.
A recovery path or nothing. A result is narrowed only when this module can
name a real place the rest of it lives, and the pointer names that place
exactly:
Readresult points at the next line of that file(
Continue with Read path="…" offset=N).Nis exact rather thanapproximate: host-core renders read windows as
N: linewith the file's ownline numbers, so the last row kept is where the continuation starts;
(
Full output saved to …, spec §3a) and says toRead/Grepit;GreporGlobresult (re-running it isnot guaranteed to return the same lines, so the pointer would not be exact), a
shell result that was never spilled, a
Readwhose call named no path. Apointer the reader cannot act on is worse than a long result.
Four precision knobs, each with a documented default:
keepMarkers([keep],matched as a case-insensitive plain substring — result text is user content and
its metacharacters must not be interpreted),
excludeTools(subagent reports andtool-discovery results are the least replaceable evidence),
workingSetPaths(the newest file-touching calls in the same batch stay whole: a file the session
just re-opened is still in play), and
clearAtLeastChars(8 000 — rewriting theview breaks the prompt cache for everything after it, so a small saving is not
worth taking).
Wiring, two places:
convertToLlm— the outgoing view is built there, so that is where the passruns. Below the gate (0.6 × hard limit) it returns the same array, so an
ordinary request is byte-identical to before.
automaticCompactionNeeded— the decision reads the same narrowed view, sothe room the pass recovers is visible to it instead of compaction firing while
real room remained.
Verification (Windows 11; re-run on the rebased head
a90bcb18, with@pi-desktop/sharedrebuilt first):Mutation-checked, both directions: removing the pressure gate turns
leaves the view untouched below the pressure gatered; removing therecovery-path refusal turns
leaves a result whole when no recovery path can be namedred; restoring each turns them green again.Two honest characterizations:
touched path is inside the working set, which is the point of that rule but
also means this pass is inert in the single-file case. It targets the long
session whose window is mostly old tool output.
and it does not need to during context compaction". That sentence stays true as
written — compaction-time retention is untouched — but the request before
compaction is no longer the same shape, so it may be worth a clause there. I
left the spec alone to keep this change reviewable; happy to add it, in both
languages, if you want it.
Base and head
base 206085c07· heada90bcb18· 4 files, +1048 / −2. 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: