feat(goal): show what a Goal has spent against the window it is allowed - #11248
Conversation
A Goal's autonomous spend window is enforced but invisible. `tokensUsed` and `tokenBudget` have been on the record since budgets landed, the 30M default stops the Goal and hands it back to the user, and nothing anywhere shows how close it is to that. A `tokensUsed` grep across the TUI and Web Shell returned only test fixtures. The user watched the turn count climb with no way to tell a Goal two percent through its window from one about to stop. The footer pill, the ink status card, its OpenTUI counterpart, and headless TEXT status now report the spend. The pill and the cards abbreviate (`1.2k/30.0m`) through the existing `formatTokenCount`; TEXT spells the figures out, since that output is read in scrollback and piped into scripts, neither of which is helped by `1.2k`. Three display rules, the same at every site. A Goal that has not billed a turn shows nothing: a fresh `0/30.0m` says nothing the status has not already said, and the pill has little room to say it. A Goal with no budget shows what it spent, which is the whole of what is known. A stopped Goal keeps showing its spend, because what a paused or blocked Goal cost is exactly what the user is deciding about. The OpenTUI card computes its own subtitle from its own snapshot type, so both were extended alongside the ink card rather than left to drift apart under the parity gate. `formatGoalState` is exported to be pinned directly: the states worth checking are far cheaper to construct as snapshots than to drive a headless run into. The figure is the Goal meter's own scope -- the model calls a Goal makes in its own turns, excluding subagents and the verifier's checks -- and the user doc says so.
|
|
|
Thanks for the PR! Template looks good ✓ Problem: observed, and I checked it rather than taking the description's word for it. Re-running your grep against the base tree returns exactly the nine files you list, every one a test fixture — Direction: aligned. This is observability for a safety limit that already exists and already stops people's Goals — today the user experiences the ceiling only as "it stopped", with no way to see it coming. There's a comparable signal in the Claude Code CHANGELOG (a spend-limit bar in Size: not applicable — nothing here touches core paths (all Approach: the scope feels right, and pinning the literal rendered string at all four sites is the part I most wanted to see. Three things worth a moment before the code review:
Risk: no elevated risk signals — none of the changed files match the paths this repo's revert history correlates with. The narrow-footer tradeoff you flagged is real and correctly described: the footer's right section is Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：真实存在，而且我自己验证过，没有只采信 PR 描述。在 base 分支上重跑你的 grep，返回的正好是你列出的那 9 个文件，全部是测试夹具—— 方向：对齐。这是为一个已经存在、已经会停下用户 Goal 的安全上限补上可观测性——目前用户只能通过"它停了"来感知这个上限，无法看到它正在逼近。Claude Code 的 CHANGELOG 里有类似信号（ 规模：不适用——没有触及核心路径（全部在 方案：范围合理，四个位置都用测试钉住实际渲染字符串，这正是我最希望看到的部分。进入代码审查前有三点值得考虑：
风险：无升级风险信号——改动文件都没有命中本仓库 revert 历史相关联的路径。你自己提出的窄终端权衡是真实存在且描述准确的：footer 右侧区域是 进入代码审查 🔍 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
Code reviewI traced this end to end rather than reading the diff alone, because the interesting failure mode for a display change is data that never arrives at the surface. The OpenTUI type additions are not decoration. The unlimited path lands where it should. The metering claim in the new doc sentence is accurate. No existing CLI test can move. The new rule only fires on data no current fixture carries: every Two things I'd want an answer on. Neither blocks the merge:
Smaller notes, take them or leave them:
TestingUnattended CI run: I executed nothing from this PR (no build, no test, no checkout). The evidence below is this PR's own CI, read through the API for the reviewed commit, plus the static verification described above. Green and relevant: Lint & Static, TUI parity snapshots (ink vs opentui), OpenTUI no-flicker gate, both Desktop Shell jobs and precheck all completed Not verified, and why: the rendered footer at narrow width. Your captures are at 120x34; the risk you flag yourself is the tight footer, and neither the diff nor the unit suite can show what 80 columns does to it — the right section is Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 (Bot orchestration checks — Classify PR, assign, label, authorize, delay-automatic-review, ack-review-request — are success/skipped and say nothing about the change; Sandboxed verification would settle the one open claim: 中文说明代码审查我没有只读 diff，而是把链路走了一遍——因为展示类改动真正有意思的失败模式，是数据根本没到达展示面。 OpenTUI 那两处类型新增不是摆设。 "无上限"这条路走对了位置。 新文档句子里关于计量范围的说法是准确的。 现有 CLI 测试不会被改动。 新规则只会在当前夹具不具备的数据上触发： 有两点我希望得到答复，但都不阻塞合并：
一些小点，采纳与否都可以：
测试无人值守 CI 运行：我没有执行本 PR 的任何代码（没有构建、没有跑测试、没有 checkout）。下面的证据来自通过 API 读取的、针对被审 commit 的本 PR 自身 CI，加上上文所述的静态验证。 已绿且相关：Lint & Static、TUI parity snapshots (ink vs opentui)、OpenTUI no-flicker gate、两个 Desktop Shell 作业以及 precheck 全部 未验证及其原因：窄终端下的实际 footer 渲染。你的截图是 120x34；你自己标出的风险恰恰是拥挤的 footer，而 diff 和单测都无法说明 80 列时会怎样——右侧区域是 （机器人编排类检查——Classify PR、assign、label、authorize、delay-automatic-review、ack-review-request——为 success/skipped，与改动质量无关；进行中的 沙箱验证可以解决唯一悬着的那个论断： — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
|
Confidence: 4/5 — a clean, well-evidenced display change; what's left is a doc clause and a framing note, not correctness. Before reading the diff I wrote down what I'd do: the same four surfaces, reuse the abbreviation helper the status line already has rather than inventing a formatter, say nothing until a turn has billed, show spend alone when there's no ceiling, and keep the OpenTUI mirror in step so the parity gate doesn't drift. The PR does all of that, and beats my baseline in one place I hadn't thought of — it splits the format by audience, abbreviated in the TUI and spelled out with digit grouping in headless, because that output gets piped into scripts and read in scrollback. I'd have used one formatter everywhere and handed a script consumer What makes me comfortable saying yes is that the load-bearing claims checked out when I tested them instead of accepting them: your base-tree grep reproduced file for file, Everything in the diff is in service of the stated goal. No drive-by refactors, no formatting churn, and the What I could not settle, and am not pretending otherwise: the live footer at narrow width. That's your own flagged risk, the unit suite pins the string but not the layout containing it, and If I pick this up in six months the only way it annoys me is if the three copies of the spend rule drift apart — which is why I named the shared-helper option in Stage 2, and why it isn't a blocker: the ink/OpenTUI pair already duplicates its turns and duration stats the same way, and literal-string tests on both sides catch a divergence loudly. On pattern: you have seven PRs open and #11254 is the Web Shell sibling of this one, so "Web Shell needs the SDK type first and is a separate change" is a real split with the follow-up already filed rather than a way to trim scope. Judged on its own merits, this one earns it. Verdict: approve — deferred until CI lands green on 中文说明Confidence: 4/5 —— 干净、证据充分的展示类改动；剩下的是一条文档从句和一个表述口径问题，不是正确性问题。 读 diff 之前我先写下了自己会怎么做：同样那四个界面、复用 status line 已有的缩写工具而不是另造一个 formatter、在计费之前什么都不显示、没有上限时只显示花费、并让 OpenTUI 那份镜像保持同步以免 parity 门禁漂移。这些本 PR 都做了，而且有一处比我的方案更好，是我没想到的——它按受众区分了格式：TUI 里缩写，headless 里带千分位完整写出，因为那份输出会被管道喂给脚本、也会在终端回滚里读。我大概会到处用同一个 formatter，然后丢给脚本使用者一个 让我能放心说 yes 的原因是：那些关键论断我是自己去验的，而不是照单接受——你的 base 分支 grep 我逐文件复现了； diff 里每一处都服务于既定目标。没有顺手重构，没有格式化噪声，而 我没能确定、也不打算假装确定的：窄终端下真实 footer 的布局。那是你自己标出的风险，单测钉住的是字符串而不是容纳它的布局，而能回答这个问题的是 如果六个月后我再来维护这段代码，唯一会让我皱眉的是那三处花费规则各自漂移——这也是我在 Stage 2 里点名共享 helper 这个选项的原因，而它之所以不是阻塞项：ink/OpenTUI 那对实现本来就用同样方式重复了 turns 和 duration 统计，而且两侧的字符串断言会让任何分叉直接报错。 关于"是否是一批 PR 中的一个"：你有 7 个 PR 处于 open 状态，而 #11254 正是本 PR 的 Web Shell 姊妹篇，所以"Web Shell 需要先改 SDK 类型，是独立改动"是一个真实的拆分、后续 PR 已经提交，而不是为了缩小范围的说辞。就本 PR 自身来看，它站得住。 结论：批准——但推迟到 CI 在 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
…resetting Resuming sets the budget to what the Goal has spent plus another grant, so the pill reads 30.0m/60.0m straight after a resume. The paragraph said the window is what the setting says, which holds only for the first one. Also narrow the no-budget card assertion, which read as "no slash anywhere in the frame" and would have failed on a slash in the objective.
|
Both questions answered, and both turned out to be worth a change. 1. The resumed denominator is cumulative, and it should be. 2. The headless line is not TEXT-only. Correct, and the description was wrong to imply otherwise: Also took the note on the no-budget card assertion. Left the shared On the red checks, none of which are this change:
Happy to have the first two rerun. 中文说明两个问题都答，而且都确实带来了改动。 1. resume 之后的分母是累计的，而且应该是。 2. headless 那行不只是 TEXT。 说得对，描述那样写是错的： 卡片"无预算"用例的那条建议也采纳了。 共享的 关于几个红的检查，没有一个是这次改动引起的：
前两个重跑即可。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
Test Plan (not a blocker): 87.2k/30.0m — no such file or directory; 147.0k/30.0m — no such file or directory; 80.7k/30.0m — no such file or directory.
中文说明
已审查——无阻断问题。 建议见行内评论。
Test Plan（非阻断）：87.2k/30.0m — no such file or directory; 147.0k/30.0m — no such file or directory; 80.7k/30.0m — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.23.0)
|
|
||
| Creating, editing, or resuming a Goal requires a trusted workspace (`/trust`). Headless usage is covered in [Headless Mode](./headless.md#run-a-persistent-goal). | ||
|
|
||
| Once a Goal has billed a turn, the footer pill and every status card show what it has spent against the window it is allowed, as `1.2k/30.0m`. The figure counts the model calls the Goal makes in its own turns; subagents and the verifier's own checks are not included. The window is set by [`model.goalTokenBudget`](../configuration/settings.md); resuming a Goal that has spent its window grants another one on top of what it has already spent, so the figure reads `30.0m/60.0m` rather than starting over. A Goal with no budget shows only what it has spent. A Goal that has not billed a turn yet shows no figures at all. |
There was a problem hiding this comment.
[Suggestion] The sentence this diff adds says the footer pill and every status card show the spend, but two families of Goal status card render no figure.
Every Web Shell Goal surface is one. The daemon payload does carry the numbers, but packages/web-shell/client/daemon/session/mappers.ts:596-691 rebuilds the goal record from an explicit whitelist that omits tokensUsed/tokenBudget, and the hand-copied SDK mirror at packages/sdk-typescript/src/daemon/types.ts:50-62 never declared them — so GoalStatusStrip, GoalsDialog and the web-shell GoalStatusMessage show status, turns and elapsed only. The legacy goal_status card is the other, in ink and OpenTUI alike: projectGoalStateToLegacy (packages/core/src/goals/goal-legacy-projection.ts:56-72) emits only iterations/setAt/durationMs/lastReason, and it is live rather than historical — called from acp-integration/session/emitters/MessageEmitter.ts:57, …/history-replayer.ts:138 and acp-bridge/src/transcript-replay.ts:995. Your own new test leaves the legacy card without spend it cannot know pins that half.
This page treats Web Shell as a Goal surface elsewhere and scopes features away from it explicitly ("nor yet in Web Shell or other ACP-driven sessions"), so an unscoped "every" reads as a claim about coverage rather than an oversight. A user who follows it into the Goals dialog finds 4 turns and an elapsed runtime and no figure, and the paragraph added here is the only place the readout is described. This is about the accuracy of the sentence, not a request to implement Web Shell in this PR — deferring that to a separate change is untouched by the fix.
Scoping the sentence to the surfaces this diff changed is enough, and matches the page's own precedent:
Once a Goal has billed a turn, the footer pill and the CLI Goal status cards
show what it has spent against the window it is allowed, as `1.2k/30.0m`.
Witness:
probe over the real, unmodified web-shell mapper (packages/web-shell), run in a scratch tree:
PROBE_R1_1_INCOMING: {"goalId":"goal-1",...,"turnCount":3,"activeTimeMs":4000,"tokensUsed":4500,"tokenBudget":30000000,...}
PROBE_R1_1_MAPPED: {"goalId":"goal-1",...,"turnCount":3,"activeTimeMs":4000,"createdAt":10,"updatedAt":20}
PROBE_R1_1_KEYS: ["activeTimeMs","createdAt","evidenceCursor","goalId","objective","revision","status","turnCount","updatedAt"]
PROBE_R1_1_hasTokensUsed: false hasTokenBudget: false
If you take the carry-through route instead, mappers.ts:596-691 returns undefined for the whole snapshot unless every required field is present, so the two fields have to go in as optional spreads rather than into the required-field guard — otherwise every pre-tokensUsed daemon snapshot fails the guard and the goal card disappears; and sdk-typescript/src/daemon/types.ts:75-81 states the mirror is hand-duplicated and must match packages/core/src/goals/goal-protocol.ts, where tokensUsed: number is required (:166) and tokenBudget?: number optional (:174).
中文说明
本 diff 新增的那句话说底栏 pill 和每一个状态卡片都会显示花费，但有两类 Goal 状态卡片根本不渲染这个数字。
Web Shell 的所有 Goal 界面是其中一类。daemon 的载荷确实带着这两个数字，但 packages/web-shell/client/daemon/session/mappers.ts:596-691 是按一份显式白名单重建 goal 记录的，白名单里没有 tokensUsed/tokenBudget，而手抄的 SDK 镜像类型 packages/sdk-typescript/src/daemon/types.ts:50-62 也从没声明过它们——所以 GoalStatusStrip、GoalsDialog 和 web-shell 的 GoalStatusMessage 只显示状态、轮数和耗时。另一类是 legacy goal_status 卡片，ink 和 OpenTUI 两边都是：projectGoalStateToLegacy（packages/core/src/goals/goal-legacy-projection.ts:56-72）只发出 iterations/setAt/durationMs/lastReason，而且它是活的、不是历史遗留——调用方有 acp-integration/session/emitters/MessageEmitter.ts:57、…/history-replayer.ts:138 和 acp-bridge/src/transcript-replay.ts:995。你自己新增的用例 leaves the legacy card without spend it cannot know 钉住的正是这一半。
这个文档页在别处是把 Web Shell 当作 Goal 界面来谈的，并且会明确地把功能范围排除出去（"nor yet in Web Shell or other ACP-driven sessions"），所以没有限定范围的 "every" 读起来就是一个关于覆盖面的断言，而不是一时疏忽。用户照着这句话打开 Goals dialog，看到的是 4 turns 和一段运行时长，没有花费数字；而这里新增的段落又是整个读出唯一的说明位置。这一条说的是这句话准不准，不是要求本 PR 去实现 Web Shell——把那件事拆成单独一次改动的决定不受这个修复影响。
把这句话限定到本 diff 真正改过的那些界面就够了，而且这与本页已有的写法一致（建议措辞见上方代码块）。
如果改走"把字段带过去"这条路：mappers.ts:596-691 在缺少任一必填字段时会对整个快照返回 undefined，所以这两个字段必须以可选展开的方式加入，而不能进必填字段的校验里——否则所有早于 tokensUsed 的 daemon 快照都会通不过校验，整张 goal 卡片直接消失；另外 sdk-typescript/src/daemon/types.ts:75-81 写明这份镜像是手抄的、必须与 packages/core/src/goals/goal-protocol.ts 一致，那里 tokensUsed: number 是必填（:166）、tokenBudget?: number 是可选（:174）。
— qwen3.8-max via Qwen Code /review (v0.23.0)
| /** | ||
| * The Goal's spend against the window it is allowed, or nothing. | ||
| * | ||
| * A Goal that has not billed a turn yet shows no figures rather than a |
There was a problem hiding this comment.
[Suggestion] This rule is stated as a turn gate — here, in docs/users/features/goals.md:19 ("Once a Goal has billed a turn"), and in three of the new test titles — but all four implementations gate on recorded spend (tokensUsed > 0 / <= 0), and the two diverge on state that is reachable in production.
A Goal can bill turns and record nothing. llm-chat.ts:5222 declares usageMetadata optional and sets it only from chunk.usageMetadata; chatRecordingService.ts:2121-2124 accumulates only if (data.tokens), and accumulateGoalTurnTokens (:2062-2075) early-returns unless totalTokenCount is finite and greater than zero; meanwhile goal-reducer.ts:232-233 still does turnCount: current.turnCount + 1, tokensUsed: current.tokensUsed + Math.max(0, transition.tokensUsed ?? 0). Render that Goal on all four surfaces and no figure appears anywhere — while the doc promises one "once a Goal has billed a turn". Because this readout is the only user-visible signal that token metering works at all, its absence is indistinguishable from "has not billed a turn yet", and the same Goal never approaches its budget stop: someone debugging "why did this Goal run for six hours" gets no hint that spend was never being counted. The state is documented in-repo too — goal-protocol.ts:160-166 says tokensUsed is "Zero on Goals recovered from a transcript written before the field existed", implemented at goal-reducer.ts:625-626.
The tokensUsed gate itself is right and should stay. What is wrong is the prose describing it, which sends a maintainer to the wrong predicate: reading this comment and the doc, they would conclude the gate is turnCount and either hunt the wrong code or "fix" the pill to gate on turns, reintroducing exactly the 0/30.0m this comment rejects.
Stating the gate as spend in the three places it is claimed closes it — this JSDoc ("A Goal with no recorded spend shows no figures rather than a reassuring 0/30.0m…"), goals.md:19 ("Once a Goal has recorded token spend…"), and the test titles at GoalPill.test.tsx:163, GoalStatusMessage.test.tsx:177, live-session-model.test.ts:687. One clause in goals.md noting that the figure is absent when the model responses carry no token usage would also close it for the operator debugging a Goal that never nears its ceiling.
Witness:
probe rendering/calling all four surfaces on the unmodified PR code,
Goal = { turnCount: 3, tokensUsed: 0, tokenBudget: 30_000_000 } <- HAS billed three turns
PROBE_R1_2_PILL: "◎︎ /goal active (5s)"
PROBE_R1_2_INK_CARD: "◎︎ Goal running · 3 turns · 2s\n Goal: finish the refactor"
PROBE_R1_2_OPENTUI_CARD: {..."subtitle":"3 turns · 2s"...}
PROBE_R1_2_HEADLESS: "Goal active: finish the refactor\nUsage: 3 turns"
The display gate must stay tokensUsed and must not become turnCount while rewording: packages/core/src/goals/goal-protocol.ts:166 documents tokensUsed as "Zero on Goals recovered from a transcript written before the field existed", and goal-reducer.ts:232 increments turnCount in the same reduction that may add zero tokens, so a turn gate would render 0/30.0m for exactly those Goals.
中文说明
这条规则被表述成一个"轮次"闸门——在这里、在 docs/users/features/goals.md:19（"Once a Goal has billed a turn"）、以及三个新增用例的标题里——但四处实现判断的都是已记录的花费（tokensUsed > 0 / <= 0），而这两者在生产中可达的状态上会分叉。
一个 Goal 可以计了轮次却什么都没记下。llm-chat.ts:5222 把 usageMetadata 声明为可选，只在 chunk.usageMetadata 出现时赋值；chatRecordingService.ts:2121-2124 只在 if (data.tokens) 时累加，而 accumulateGoalTurnTokens（:2062-2075）在 totalTokenCount 不是有限的正数时直接提前返回；与此同时 goal-reducer.ts:232-233 仍然执行 turnCount: current.turnCount + 1, tokensUsed: current.tokensUsed + Math.max(0, transition.tokensUsed ?? 0)。把这样一个 Goal 渲染到四个界面上，任何一处都不会出现数字——而文档承诺的是"一旦 Goal 计过一轮"就有数字。因为这个读出是用户唯一能看到的"token 计量在工作"的信号，它的缺失与"还没计过轮次"完全无法区分；同一个 Goal 也永远不会逼近自己的预算停止点：有人在排查"这个 Goal 为什么跑了六小时"时，得不到任何关于花费从未被计入的提示。这个状态在仓库里也是有文档的——goal-protocol.ts:160-166 写明 tokensUsed 在"从早于该字段的 transcript 恢复出来的 Goal 上为零"，实现在 goal-reducer.ts:625-626。
tokensUsed 这个闸门本身是对的，应该保留。错的是描述它的文字，它会把维护者引向错误的判据：读了这段注释和文档，他们会以为闸门是 turnCount，于是要么去查错的代码，要么"修好" pill 让它按轮次判断——那就正好把这段注释想要避免的 0/30.0m 又装了回来。
在声称这条规则的三个地方把闸门改述为"花费"就能收口——本 JSDoc（"A Goal with no recorded spend shows no figures rather than a reassuring 0/30.0m…"）、goals.md:19（"Once a Goal has recorded token spend…"）、以及 GoalPill.test.tsx:163、GoalStatusMessage.test.tsx:177、live-session-model.test.ts:687 三处用例标题。在 goals.md 里再加一个从句，说明模型响应不带 token usage 时这个数字不会出现，也能为那位排查"Goal 永远不接近上限"的使用者把话讲完。
改述时，显示闸门必须继续是 tokensUsed，不能变成 turnCount：packages/core/src/goals/goal-protocol.ts:166 写明 tokensUsed 在"从早于该字段的 transcript 恢复出来的 Goal 上为零"，而 goal-reducer.ts:232 是在同一次 reduce 里递增 turnCount、却可能加进 0 个 token，所以按轮次判断恰恰会对这些 Goal 渲染出 0/30.0m。
— qwen3.8-max via Qwen Code /review (v0.23.0)
| const elapsed = formatElapsed(elapsedActiveTime(goal, Date.now())); | ||
| const suffix = elapsed ? ` (${elapsed})` : ''; | ||
| const spend = formatSpend(goal); | ||
| const detail = [elapsed, spend].filter(Boolean).join(' · '); |
There was a problem hiding this comment.
[Suggestion] The new two-segment detail is tested in three of its four shapes. The one where spend is non-empty while elapsed is '' has no test, because every pill fixture inherits a non-empty elapsed — and a plausible rewrite of this line that breaks exactly that shape survives the whole suite.
GoalPill.test.tsx's snapshot() helper hard-codes activeTimeMs: 2_000 and updatedAt: 7_000 against NOW = 10_000 (:23, :40-44), and none of the four new tests overrides either, so formatElapsed never returns '' in any of them and detail always has its 5s/2s head. The uncovered shape is reachable, not hypothetical: formatElapsed returns '' below 1000 ms (GoalPill.tsx:36-37), and transitionGoal sets activeTimeMs = elapsedActiveTime(goal, now) (goal-reducer.ts:511-521) in the very transition reduceGoalTurnFinished uses to add the turn's spend (:221-241) — so a Goal paused, blocked or usage-limited with under a second of accumulated active time and a billed turn renders spend with no elapsed permanently, and the active form is the same state inside its first second. Rewrite the join as const detail = spend ? [elapsed, spend].join(' · ') : elapsed and that Goal's footer reads ◎ /goal paused ( · 1.2k/30.0m) — a leading separator with nothing before it — with all 15 of the pill's tests still green.
One case covering the shape closes it:
it('shows spend alone when no elapsed second has accrued', () => {
vi.setSystemTime(NOW);
const { lastFrame, unmount } = renderPill({
snapshot: snapshot('paused', 'handoff', {
activeTimeMs: 400,
updatedAt: NOW,
tokensUsed: 1_234,
tokenBudget: 30_000_000,
}),
});
expect(lastFrame()).toContain('(1.2k/30.0m)');
expect(lastFrame()).not.toContain('( ·');
unmount();
});Witness:
mutant applied at GoalPill.tsx:151 -> const detail = spend ? [elapsed, spend].join(' · ') : elapsed;
run against the PR's own suite plus the new case above:
Test Files 1 failed | 1 passed (2) Tests 1 failed | 15 passed (16)
GoalPill.test.tsx: 15/15 GREEN <- the mutant survives the PR's whole suite
new case RED: Expected "(1.2k/30.0m)" Received "! /goal paused ( · 1.2k/30.0m)"
mutant reverted:
new case GREEN: PROBE_R1_6_FRAME: "! /goal paused (1.2k/30.0m)"
formatTokenCount floors non-positive and non-finite input to '0' (packages/cli/src/ui/statusLinePresets.ts:239-241), so the new case has to set tokensUsed above zero explicitly — leaving the helper's tokensUsed: 0 default renders (0/30.0m) and asserts a different thing than intended. The case above is the acceptance criterion: remove the filter(Boolean) join in favour of a spend-guarded one and it must go red, while the four tests this diff adds stay green against that same mutant.
中文说明
新的两段式 detail 有四种形态，测试覆盖了其中三种。spend 非空而 elapsed 为 '' 的那一种没有测试，因为每个 pill 夹具都从 helper 继承了一个非空的 elapsed——而针对这一行的一种很自然的改写，恰恰会在这种形态上出错，却能让整个测试套件全绿。
GoalPill.test.tsx 的 snapshot() helper 把 activeTimeMs: 2_000 和 updatedAt: 7_000 写死在 NOW = 10_000 下（:23、:40-44），四个新增用例没有一个覆盖这两个值，所以 formatElapsed 在其中任何一个里都不会返回 ''，detail 也就总是带着 5s/2s 这个头。没被覆盖的这种形态是可达的，不是假想：formatElapsed 在小于 1000 ms 时返回 ''（GoalPill.tsx:36-37），而 transitionGoal 设置 activeTimeMs = elapsedActiveTime(goal, now)（goal-reducer.ts:511-521）用的正是 reduceGoalTurnFinished 加入本轮花费的那一次 transition（:221-241）——所以一个累计活跃时间不足一秒、又已经计过一轮的 Goal，在 paused / blocked / usage-limited 状态下会永久地只有花费没有耗时；active 状态下则是同一情形在头一秒内的样子。把这个 join 改写成 const detail = spend ? [elapsed, spend].join(' · ') : elapsed，这个 Goal 的底栏就读作 ◎ /goal paused ( · 1.2k/30.0m)——一个前面什么都没有的分隔符——而 pill 的 15 个用例依然全绿。
补一个覆盖该形态的用例即可收口（代码见上方）。
formatTokenCount 会把非正数和非有限值一律归到 '0'（packages/cli/src/ui/statusLinePresets.ts:239-241），所以新用例必须显式把 tokensUsed 设为大于零——沿用 helper 里 tokensUsed: 0 的默认值会渲染出 (0/30.0m)，断言的就是另一件事了。上面这个用例就是验收标准：把 filter(Boolean) 的 join 换成按 spend 判断的写法，它必须变红，而本 diff 新增的那四个用例在同一个变异体下应当仍然全绿。
— qwen3.8-max via Qwen Code /review (v0.23.0)
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Reviewed at head c1ce821. Display-only change with solid parity and negative coverage:
- The spend figure appears on the ink pill, the ink lifecycle card, the OpenTUI card (describeGoalCard), and headless TEXT output (formatGoalState) — all four shapes pinned by tests, including the "no figures before the first billed turn" and "spend-only without a budget" cases.
- Reuses the existing formatTokenCount from statusLinePresets rather than adding a formatter; TEXT output deliberately spells numbers out for terminal scrollback/scripts while TUI abbreviates — both choices are documented in comments and pinned.
- Event-adapter GoalSnapshotLike extension is additive/optional; legacy card props correctly show no spend.
- Round-1 review left 3 Suggestion-level findings (docs wording about "every status card", turn-gate phrasing, one untested two-segment detail shape) — non-blocking, worth a follow-up touch-up.
CI green at head.
qwen-code-dev-bot
left a comment
qwen-code-dev-bot
left a comment
There was a problem hiding this comment.
门禁 Review — head c1ce8211
决定：REQUEST_CHANGES. Nothing here is about behavior — the four surfaces render the right thing and every lane is green. Two sentences this diff introduces state the rule incorrectly, and one new branch is unpinned; all three are minutes of work, and I expect a fast re-review after them.
1. docs/users/features/goals.md:19 — "every status card" is false, and this PR's own test proves it
The added sentence promises that "the footer pill and every status card show what it has spent". The legacy goal_status card shows no figure, in ink and OpenTUI alike: projectGoalStateToLegacy emits only iterations/setAt/durationMs/lastReason (packages/core/src/goals/goal-legacy-projection.ts:56-72, verified at this head) and it is a live path, called from MessageEmitter.ts:57, history-replayer.ts:138 and transcript-replay.ts:995. The new test leaves the legacy card without spend it cannot know (GoalStatusMessage.test.tsx:190) pins exactly that absence. Web Shell is the second family that shows nothing until PR #11254 lands, so scoping to "the CLI's own cards" is not enough either. The page already scopes claims away from Web Shell elsewhere ("nor yet in Web Shell or other ACP-driven sessions"), so an unscoped "every" reads as a coverage claim a user will act on and find wrong. Name the surfaces this diff changed.
2. The gate is recorded spend, but the prose says billed turns
GoalPill.tsx:24-27 says "A Goal that has not billed a turn yet shows no figures", goals.md:19 says "Once a Goal has billed a turn", and four of the new test titles do too (GoalPill.test.tsx:140, :163, GoalStatusMessage.test.tsx:177, live-session-model.test.ts:687) — while all four implementations gate on spend (goal.tokensUsed <= 0 at GoalPill.tsx:30). Those two predicates differ on a state that is reachable in production and documented in-repo: usageMetadata is optional and only set from chunk.usageMetadata, and accumulateGoalTurnTokens returns early unless totalTokenCount is a finite number greater than zero (chatRecordingService.ts:2062-2067, read at this head), while goal-reducer.ts:232-233 still increments turnCount in a reduction that added no tokens. So a Goal can bill turns and show no figure anywhere. The spend gate is right and must stay — the point is that a maintainer following this prose goes to the wrong predicate, and the "fix" they would apply re-adds the 0/30.0m the comment exists to reject. Say "has recorded token spend" in the JSDoc, the doc sentence and the three titles, and add one clause to the doc for the case where responses carry no usage at all.
3. One shape of the new detail has no fixture
const detail = [elapsed, spend].filter(Boolean).join(' · ') (GoalPill.tsx:151) has four input shapes and three are tested. spend non-empty with elapsed === '' has none, because every pill fixture inherits a non-empty elapsed — so a rewrite that drops the filter(Boolean) (or joins unconditionally) still passes the whole suite while emitting " ( · 1.2k/30.0m)". One fixture closes it.
Not blocking, for the record
The behavior itself checked out: the spend gate handles the unbounded case by showing spend only, the zero-spend and missing-field cases are distinct, and the OpenTUI model's string follows ink's rule (tokensUsed > 0, spend-only when tokenBudget is absent), so all four surfaces agree on when a figure appears. CI at this head is complete and green — Test (ubuntu-latest) 40m18s, Lint & Static 21m50s, Integration Tests (no-AK) 26m1s, the TUI parity and OpenTUI no-flicker gates, review-pr — and there is no open Critical on the PR; yiliang114's approval is on this head. Round it back for a vote once the three edits are in.
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
Reviewed at head c1ce8211 with the applicable suites executed locally on linux / node v24.20.0. Approving — the behavior is clean; two doc-accuracy nits ride along, and one open gate item is refuted by execution.
Executed: full monorepo build clean · cli nonInteractiveCli + GoalPill + GoalStatusMessage + live-session-model 220/220 (+1 skip). All four surfaces (ink pill, ink card, OpenTUI card, headless TEXT) agree on the same gate (tokensUsed > 0, spend-only when no budget), reusing formatTokenCount rather than a new formatter; the snapshot-type extensions are optional/additive, so older daemons and legacy cards degrade to no figures.
Cross-check of the open gate review at this head:
- Item 1 (goals.md "every status card"): confirmed —
projectGoalStateToLegacycarries no spend fields and is a live path, and the PR's own test pins the legacy card's absence. The sentence should name the surfaces this diff changed. Minor, docs-only. - Item 2 ("billed a turn" vs recorded spend): confirmed — every implementation gates on
tokensUsed > 0while the prose and four test titles say "billed a turn"; a turn whose provider response lacksusageMetadatabills but shows nothing. Wording fix (or gate comment), minor. - Item 3 (the
[elapsed, spend]join shape unpinned): refuted by execution at this head — droppingfilter(Boolean)fails 3 tests inGoalPill.test.tsx(adds the current active span to persisted active time,shows no figures for a Goal that has not billed a turn,keeps paused elapsed time frozen while wall clock advances). The empty-elapsed-with-spend cell may still lack a dedicated fixture, but the named mutant does not survive, so this item should not hold the vote.
With items 1-2 being minutes of wordsmithing, no behavioral blocker exists at this head. Not covered: a live headless run against a real model (fixtures only), Windows/macOS (linux only).
Reviewed with AI assistance.
Both main (QwenLM#11248) and this branch inserted a new paragraph at the same point in the Goals introduction. The two describe different surfaces and are both kept: main's covers the footer pill and status cards rendering spend against the budget window, this branch's covers the figures and standing guidance the model receives at the start of each autonomous turn. Main's paragraph goes first because it defines "the window it is allowed" and names model.goalTokenBudget, which the second paragraph then relies on.
|
Released in v0.23.1. |
5 participants
What this PR does
Every surface that shows a Goal now also shows what it has spent against the window it is allowed. The footer pill reads
◎ /goal active (28s · 147.0k/30.0m), the status card's subtitle gains a80.7k/30.0m tokensstat, and headless status gains aUsage:line -- in the TEXT output and, because that string is the single final message, inresult.summaryunder--output-format jsonas well.Three display rules, applied identically at every site. A Goal that has not billed a turn shows no figures: a fresh
0/30.0msays nothing the status has not already said, and the pill sits in a footer with little room to say it. A Goal with no budget shows what it has spent, since that is the whole of what is known about it. A stopped Goal keeps showing its spend, because what a paused or blocked Goal cost is exactly what the user is deciding about when they choose whether to resume.The pill and the cards abbreviate through the
formatTokenCountthe status line already uses. The headless line spells the figures out instead, because that output is read in terminal scrollback and piped into scripts, and neither is helped by1.2k.Two things beyond a literal reading of the change. The OpenTUI card computes its own subtitle from its own snapshot type, so both were extended alongside the ink card rather than left to drift apart under the parity gate. And
formatGoalStateis exported so its output can be pinned directly: the states worth checking, a Goal mid-run, one with no budget, one that has billed nothing, are far cheaper to construct as snapshots than to drive a headless run into, and the file already exportsskipHeadlessLoopSentinelfor the same reason.Why it's needed
The spend window is enforced but invisible.
tokensUsedandtokenBudgethave been on the Goal record since budgets landed, the 30,000,000 default stops the Goal and hands it back to the user, and until now nothing showed how close it was. GreppingtokensUsedacross the TUI and the Web Shell returned test fixtures and nothing else.So the user watched the turn count climb with no way to tell a Goal two percent into its window from one about to stop. Codex puts the same pair in its TUI status line as
63.9K/50K; this is the equivalent, at the surfaces this CLI already uses to say what a Goal is doing.Web Shell is deliberately not in this PR. Its strip and dialog read the hand-copied SDK
GoalRecord, which lacks both fields, so it needs a type change first and is a separate change.Reviewer Test Plan
How to verify
The new cases pin the exact rendered string at all four sites, and each site has a case for the with-budget, no-budget, and nothing-billed rules.
formatGoalStateadditionally pins that the stop reason still reads below the usage line.To see it live, set any Goal that runs a few turns and watch the footer.
Evidence (Before & After)
Before. Neither surface shows the pair;
tokensUsedappears nowhere in either package's production code:Every hit is a test fixture. The footer read
◎ /goal active (19s)and the card read1 turn · 21.7s.After — footer pill, live TUI, climbing as the Goal works:
After — status card, live TUI:
After — headless TEXT, resuming a session whose Goal had already run:
After — a Goal that has not billed a turn, which is what
/goal setprints before the first turn runs:No usage line, as intended.
Tested on
Environment (optional)
Linux,
npm run build, interactive TUI under tmux at 120x34 and headless--output-format text, live model.Risk & Scope
formatGoalStatebecomes exported.Linked Issues
Part of #4228.
中文说明
这个 PR 做了什么
现在每一个显示 Goal 的界面都会同时显示它在被允许的额度里已经花掉多少。底栏 pill 读作
◎ /goal active (28s · 147.0k/30.0m)，状态卡片的副标题多出一段80.7k/30.0m tokens，headless 的状态输出多出一行Usage:——TEXT 里有，而且因为那个字符串就是唯一的最终消息，--output-format json的result.summary里同样有。三条显示规则，四个位置完全一致。还没计过费的 Goal 什么数字都不显示：刚创建时的
0/30.0m并没有说出状态本身没说过的东西，而 pill 待在底栏里也没有多余的地方去说它。没有预算的 Goal 只显示已花掉多少，因为那就是已知的全部。已经停下的 Goal 继续显示它的花费，因为一个 paused 或 blocked 的 Goal 花了多少，恰恰就是用户在决定要不要恢复时要看的。pill 和卡片用状态栏已有的
formatTokenCount做缩写。headless 那行则把数字完整写出来，因为那份输出是在终端回滚里读、被管道喂给脚本的，这两种场景都不会因为1.2k而变好。有两处超出字面改动的地方。OpenTUI 的卡片用它自己的快照类型算自己的副标题，所以两边一起改了，而不是留着它们在一致性门禁下分叉。另外
formatGoalState被导出，以便直接钉住它的输出：真正值得检查的那几个状态——跑到一半的 Goal、没有预算的 Goal、还没计过费的 Goal——把它们构造成快照，远比把一次 headless 运行驱动到那个状态便宜；而且这个文件本来就为同样的理由导出了skipHeadlessLoopSentinel。为什么需要
这个额度窗口在执行，但是看不见。
tokensUsed和tokenBudget从预算功能落地起就在 Goal 记录上了，30,000,000 的默认值会停下 Goal 并把控制权交还用户，而在此之前没有任何地方显示离那个上限还有多远。在 TUI 和 Web Shell 里 greptokensUsed，返回的全是测试夹具，别的什么都没有。于是用户只能看着轮数往上爬，无从分辨一个才用掉窗口百分之二的 Goal 和一个马上就要停的 Goal。Codex 在它的 TUI 状态栏里放的就是同一对数字，形如
63.9K/50K；这里做的是等价的事，放在本 CLI 已经用来说明 Goal 在干什么的那些位置上。Web Shell 刻意不在本 PR 里。它的 strip 和 dialog 读的是手抄的 SDK
GoalRecord，那份类型缺这两个字段，所以要先改类型，是单独的一次改动。评审验证方式
如何验证
新增用例在四个位置上分别钉死了渲染出来的确切字符串，每个位置都有"有预算""无预算""还没计费"三条规则各自的用例。
formatGoalState还额外钉住了停机原因仍然排在 usage 行下面。要看实时效果，设一个会跑几轮的 Goal，然后看底栏。
证据（Before & After）
改动前。 两个界面都不显示这对数字；
tokensUsed在这两个包的生产代码里一处都没有：每一处命中都是测试夹具。底栏当时读作
◎ /goal active (19s)，卡片读作1 turn · 21.7s。改动后 —— 底栏 pill，真实 TUI，随着 Goal 干活往上涨：
改动后 —— 状态卡片，真实 TUI：
改动后 —— headless TEXT，续上一个 Goal 已经跑过轮的会话：
改动后 —— 还没计过费的 Goal，也就是
/goal set在第一轮开始前打出来的东西：没有 usage 行，符合预期。
测试平台
环境（可选）
Linux，
npm run build，在 tmux 里以 120x34 跑交互式 TUI，以及--output-format text的 headless，均对真实模型。风险与范围
formatGoalState变成导出。关联 Issue
属于 #4228 的一部分。