Repository navigation
feat(steps): report what the planning pass costs - #74
Merged
Merged
Conversation
The planning pass asks the run's own model for a plan before step 1 and every `n` steps after it — but that call happens **outside** the loop. Adaptive MCP's observation path is about tool executions, so nothing saw it: no telemetry, no `/cost`, no `costSummary()`, no decoding report. With `--plan-interval 1` on an 8-step run, half the model calls were invisible, so the reported cost could understate the run by roughly half — on exactly the flag a budget-conscious user turns on. The host half is a report; the library half is a decision (#73): - `PlanningPassOptions.onCall` fires for **every** planning call — including one that failed and one whose answer was unusable, because both were paid for. It reports the step, the latency, the backend's `usage` (when it has one) and whether the call worked. - `PlanUsage` accumulates those into the run summary: a run with `--plan-interval` now ends `stop=completed steps=4 plans=2 plan_tokens=1234 plan_ms=4100`. A run without the flag prints exactly what it printed before (`""` when nothing was planned), so nothing else changes. - `plan_tokens` is **omitted** rather than printed as `0` when no backend reports usage: `plan_tokens=0` would read as "planning is free", which is not what happened. - The REPL gets the same number where a REPL user can see it: `/steps` ends with a `planning: plans=…` footer next to the history it describes. The issue's third acceptance point is respected: no new protocol concept. A planning call is a *host* model call, and the place to account for it is the host. Verified end to end: the `--plan-interval 1` e2e run now asserts `plans=2` in its summary. Unit tests cover the hook (success, failure, unusable answer, not-due), the totals, and the omission of `plan_tokens`. `pnpm build`, `pnpm typecheck`, `pnpm lint` and `pnpm test:coverage` are green (328 passed / 5 skipped; coverage 90.3 / 84.1 / 89.3 / 90.3). Pre-existing prettier drift in `src/steps.ts` and `src/cli.e2e.test.ts` is left alone (see #49); the lines added here follow prettier's output.
kemalelmizan
added a commit
that referenced
this pull request
Oct 17, 2026
… it knows fix(repl): forward the run's policy instead of re-listing the options it knows The REPL built its `runAgent` call field by field, and the list was incomplete: `steer` and `stopGuard` were never passed. So `--plan-interval` planned nothing interactively and `--mode tasks` could end a turn with its list unfinished — both worked in a one-shot run, and both did nothing in the REPL, silently. That is the same failure shape as a mode whose tools are wired but whose runner is not (#69): the capability exists, the host drops it on the way to the loop. The fix is structural rather than a third field on the list. `ReplOptions` now extends the loop's `RunAgentOptions` (minus `messages`/`onEvent`, which the REPL owns), and the turn passes its options through wholesale: const result = await runAgent({ ...options, messages: next, onEvent: render }); so a new run seam is forwarded by default and omitting one has to be written down. The `ReplOptions` doc comment says why, because the next person to add an option to the loop should not have to rediscover this. Two unit tests pin the seams that were missing — a `steer` whose text must reach the model, and a `stopGuard` that must earn a second model call — and both fail against the old hand-picked list. One e2e test drives the real REPL over stdin (answer-driven, because `readline.question` drops anything typed while no prompt is pending) and asserts a `--plan-interval 1` turn plans, records the plans, and reports the cost in `/steps`. That last assertion also needed the `/steps` footer to read `planning: plans=…` rather than `planning:plans=…`; the footer could not appear before this fix, so the spacing had no chance to be seen (it landed in #74). Docs: `docs/architecture.md` states the forwarding rule where the opt-in seams are described, `README.md` says run policy applies interactively, and `AGENTS.md` gains the REPL paragraph next to the loop seams. Verified: `pnpm build`, `pnpm typecheck`, `pnpm lint`, `pnpm test:coverage` all green (coverage 90.3 / 84.1 / 89.3 / 90.3). `src/repl.ts`, `src/repl.test.ts` and the touched docs are prettier-clean; the pre-existing drift in `src/cli.e2e.test.ts` is left alone (#49).
kemalelmizan
added a commit
that referenced
this pull request
Oct 17, 2026
feat(decision): account for the decision layer's own tokens #74 made the planning pass's model calls visible in the run summary. The System One decision layer spends the same way and was still invisible: with `--system1-backend openai` every step makes a real, billed completion through `/chat/completions`, and nothing in the run reported what it cost. Tool telemetry never sees it either — a decision is not a tool execution, and the decision layer is deliberately not on the `ChatModel` seam. - `SystemOneAdvisor` gains `onCall`: every backend call, reported with the step, latency, the backend's tokens (when it reports any) and whether it answered. It fires for a call that **failed** and for one whose verdict was **discarded** (below the confidence floor, so the loop never applies it), because both were paid for — the same rule `PlanningPassOptions.onCall` follows. - Tokens are omitted rather than reported as `0` when a backend reports no usage: both transports default the counts to `0` when a response has no `usage`, so `decision_tokens=0` would claim the decision was free. - `AgentRuntime` accumulates what it reports and exposes `decisionCallSummary()`, which the CLI appends to both run-summary sites: [agent] stop=completed steps=4 plans=2 plan_tokens=1234 plans_ms=4100 decisions=2 decision_tokens=52 decision_ms=180 `""` when the decision layer never ran, so a run without System One prints exactly what it printed before. - `/decision` (the REPL's view of the decision layer, which has no run summary) now ends with the same running total, from the verdict outwards: the tier that answered, the rationale, then what the calls cost this run. The remote tier's tokens are priced by its provider and invisible to this repo, so the count is reported rather than converted to money — the agent does not know the decision model's price. Noted in the docs. Verified: `pnpm build`, `pnpm typecheck`, `pnpm lint`, `pnpm test:coverage` all green (335 passed / 5 skipped; coverage 90.4 / 84.0 / 89.4 / 90.4). Tests cover the report on success, on failure and on a discarded verdict, the omission of tokens a backend does not report, the accumulation, and that `/decision` shows it. Pre-existing prettier drift in `src/decision/advisor.ts`, `src/decision/advisor.test.ts` and `src/runtime.ts` is left alone (#49); the lines added here match prettier's output.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The gap
PlanningPass.steer(from #40) asks the run's own model for a plan before step 1 and everynsteps after it. Those calls happen outside the loop, and Adaptive MCP's observation path is about tool executions — so nothing accounted for them: not telemetry, not/cost, notcostSummary(), not the decoding report. With--plan-interval 1on an 8-step run, half of the model calls were invisible, so the reported cost and latency could understate a run by roughly half.The fix (host half)
A hook for every planning call.
PlanningPassOptions.onCallfires for each one — including a call that failed and one whose answer was unusable, because both were paid for. It reports{ step, durationMs, usage?, ok }.PlanUsageaccumulates those into the run summary:A run without
--plan-intervalprints exactly what it printed before (summary()returns""when nothing was planned), so no existing summary string changes.plan_tokensis omitted rather than printed as0when no backend reports usage, becauseplan_tokens=0reads as "planning is free", which is not what happened.The REPL gets the same number where a REPL user can see it:
/stepsends with aplanning: plans=…footer, next to the history it describes.What is deliberately not here
The library-side question — whether Adaptive MCP should own a shape for a host's non-tool model call (an insight like "planning is X% of this workflow's spend" would be useful to
routing/evaluation, but aToolRecordis not the right home) — is a design decision, not a mechanical fix. The issue's third acceptance point says so explicitly: no new protocol concept, and if the library should own one, that is its own issue.Verified
--plan-interval 1e2e run assertsplans=2in its run summary.plans=2 plan_tokens=15 plan_ms=50across a successful and a failed call); and that tokens are omitted when no backend reports them.pnpm build,pnpm typecheck,pnpm lint,pnpm test:coverage— green: 328 passed / 5 skipped, coverage 90.3 / 84.1 / 89.3 / 90.3.Pre-existing prettier drift in
src/steps.tsandsrc/cli.e2e.test.tsis left untouched (#49); the lines added here match prettier's output.Closes #73.