Repository navigation
Judge meter follow-ups; meter interview and mission planning as worker spend - #57
solvyxtech wants to merge 3 commits into
Conversation
- The per-turn dollar ceiling prices the turn as worker plus judge whenever either is priced, so an unpriced (local or subscription) worker with a paid judge is still held to a dollar ceiling. - The bench lane median leaves out runs whose judge ran unpriced: their total is a lower bound and would pull the limit down. The run's own alarm still uses the lower bound. - JudgeMeter.since() drops the figure stored at record time before re-pricing, so a cleared price is reflected; the journal keeps the figure as it stood. - The budget-hit message says when the judge's drafting used the budget before the worker's first step. - The judge's and the worker's price lookups run in parallel. - amendReview gets its doc comment back; one import from format.js.
Both ask the worker's own model outside a turn, and their tokens reached no meter. The interview's ask now reports to Engine.workerAskMeter, which adds it to the session meter like a step (cost priced as the worker's) and journals it as worker_ask; the TUI and the desktop pass it. maat mission plan meters its ask at the worker's prices and prints a 'worker (planning)' line. askModel's usage reading is shared as recordUsage for the interview's own HTTP path.
|
Verification on 63aff3c:
One note: an earlier run on this branch had 3 timing failures ( |
Solvyx
left a comment
Solvyx
left a comment
There was a problem hiding this comment.
Grok review: #57 at 63aff3c
Verdict: changes requested (one blocker). Posted as a comment review because GitHub won't take a request-changes review from the PR's own account. Not merging.
What I checked. I read the full diff and ran it locally: tsc -p tsconfig.test.json is clean, and the node suite reports 2458 tests, 2451 passing, 0 failing and 7 todo (the known adversarial expected-failures). The bench pytest run is 89 passed and 5 skipped, and CI is green on all three jobs. I also ran scratch reproducers against this head and against main (308ce47).
Pairing rules. This PR is clean. It adds no subscription backend and no competitor names (I grepped the added lines), and it doesn't touch fine-tune data. The "local or subscription worker" wording in docs/transparency.md means the existing Grok Build and OpenCode plan paths, so it's fine.
Blocking
B1. The lane median drops every run with a plan-paid judge, so the documented OpenCode-judge arm gets pinned to the $0.10 floor. bench/local/run.py:724, confidence high.
median_cost treats judge_calls is not None and judge_cost_usd is None as "unpriced judge, lower bound". But a judge paid by a plan (planFor gives OpenCode for opencode://zen) also has no costUsd in job_end.judge, by design, and judge_columns (line ~684) drops plan. So the bench can't tell "$ unknown" apart from "plan, costs no money".
Scenario: take the arm documented in container/run-in-container.sh:23 (MAAT_JUDGE_MODEL=opencode/big-pickle,MAAT_JUDGE_URL=opencode://zen) with a priced worker. Every row is left out of the median, so the median never forms and the limit stays at max(..., $0.10) for the whole lane. The first ordinary $0.15 run stops the lane. On main, those rows counted at the worker's cost, the median formed, and the limit rose to 5x. I checked it directly: median_cost({'cost_usd': 0.15, **judge_columns({'calls': 3, 'plan': 'OpenCode', ...})}) returns None, and the alarm reads cost $0.1500 (worker $0.1500 + judge $ unknown) > $0.1000 (max(5x lane median n/a, $0.1)).
Why it matters: a plan-paid judge's total is exact (it costs no money), not a lower bound, so the exclusion's own reasoning doesn't apply. Meanwhile it makes a documented bench arm unusable. The detail line also says judge $ unknown for a judge that's actually plan-paid.
Fix: carry the plan through judge_columns (for example judge_plan), exclude only when judge_cost_usd is None and not judge_plan, show judge (OpenCode plan) in the split, and add a lane test with a plan judge.
Non-blocking
N1. Once a priced judge call lands in a turn with an unpriced worker, the per-turn ceiling switches from tokens to dollars, and the worker's tokens stop being bounded by it. src/engine.ts:6752 (with priced at 6761), confidence high on the behaviour, medium on the real-world impact.
When maxTurnUsd > 0, usdThisTurn becomes defined (judge dollars only), priced flips, used/ceiling move to dollars, and maxTurnTokens is never checked again that turn. Reproducer: an unpriced worker sends 100k prompt tokens per step, with maxTurnUsd: 1, maxTurnTokens: 150000, and one priced judge call after step 1. On main it warned at 67%, warned at 133%, and stopped at 200,020 tokens. On this head it ran 7 requests and 700,000 tokens to the step guard, with no ceiling message. In the shipped CLI and TUI this is mostly hidden, because setBudget(n) also sets the session budgetTokens to the same number and overBudget() runs first. But the Engine config takes maxTurnTokens on its own, the warning and raise advice flips units mid-turn, and a stop line would read $0.000 of $1.00 while the worker burns tokens. Suggestion: when the worker is unpriced, enforce both ceilings (dollars for the judge's part, tokens for the whole) and stop at whichever hits first. Also add the reproducer as a test.
N2. The budget-hit hint is keyed off the last job_end, not this turn's drafting. src/engine.ts:6737, confidence low.
this.judgeMeter.mark() > this.judgeReported is true for any judge ask since the last job reported, not just this turn's drafting. A judge ask between jobs (for example a dispute arbiter or a review outside a job) would get described as "drafting this turn's checks". The hint also fires when earlier turns' worker spend used up almost all the budget and the drafting only tipped it over, and "used the budget" overstates that case. Suggestion: take a mark and a token count when drafting starts, and give the hint only when the spend before drafting was under the budget. I couldn't trigger the first case through the plain run() path (a budget-stopped turn still advances judgeReported), so treat this as a hardening note.
N3. maat mission plan ignores the stored worker price. src/cli.tsx:1213, confidence medium.
It either uses hand-set flags or calls fetchPricing. It skips the needsPriceLookup / storedEndpoint() path that priceWorker uses. If a provider publishes no list but the user has a saved price for this model, maat run shows dollars while mission plan prints $ unknown. It also does a network lookup on every plan, even when a stored price exists. Suggestion: resolve the plan's pricing the same way priceWorker does, maybe through a shared helper.
N4. Small notes. Confidence high, impact low.
- Interview asks go into the session meter but not into any job's receipt delta, because they happen before the turn's
beforesnapshot. That's fine, butdocs/transparency.mdcould say so, so a session total that's higher than the sum of its receipts isn't a surprise. - The
worker (planning) ...line usesJudgeMeterandjudgeSpendLinewith a label override. Fine for now, but the names will mislead the next reader. A neutralAskMeter/spendLinealias would help.
Looks good
- The
totalCostUsd()/sumKnownrewrite and the test for an unpriced worker with a priced judge. - Stripping the stored
costUsd/planinsince()and re-deriving it.plancomes fromplanFor(baseUrl), so nothing's lost, and the cleared-price test covers it. recordUsagekeeps the old estimate behaviour, since cache, write and billed figures are only taken when usage was reported.- Running
priceJudgeandpriceWorkerin parallel is safe: they share no state, and onlypriceWorkersaves config. - The doc-comment fix for
amendReviewandamendJudge.
| pull the median (and so the limit) down without anyone seeing why. The alarm on | ||
| that run itself still uses the lower bound (run_cost): under-counting there only | ||
| makes the alarm late, never wrong.""" | ||
| if row.get("judge_calls") is not None and row.get("judge_cost_usd") is None: |
There was a problem hiding this comment.
Blocking (B1), confidence high. A plan-paid judge (opencode://zen) also has judge_calls set and judge_cost_usd None, because job_end.judge carries plan and no costUsd, and judge_columns drops plan. Those runs are exact, not lower bounds, but they get left out here. With the documented OpenCode-judge arm the median never forms, so the limit stays at the $0.10 floor and the first $0.15 run stops the lane. Carry plan through as judge_plan and exclude only when there's no plan.
| const spentThisTurn = this.spentTokens - turnStartTokens; | ||
| const usdThisTurn = | ||
| turnStartCost === undefined ? undefined : (this.totalCostUsd() ?? 0) - turnStartCost; | ||
| const usdThisTurn = sumKnown( |
There was a problem hiding this comment.
Non-blocking (N1). With an unpriced worker, this becomes defined as soon as a priced judge call lands in the turn. priced then flips to true and maxTurnTokens is no longer checked. Reproducer: on main the turn stopped at 200,020 of 150,000 tokens; on this head it ran to the step guard at 700,000 tokens. Mostly hidden in the CLI by the session budget, but consider enforcing both ceilings when the worker is unpriced.
| `) — loop stopped. /budget to raise.` + | ||
| // Hit before the worker took a step, with the judge asked for this | ||
| // turn already (its drafting): say what used it up. | ||
| (step === 0 && this.judgeMeter.mark() > this.judgeReported |
There was a problem hiding this comment.
Non-blocking (N2), confidence low. mark() > judgeReported counts any judge ask since the last job_end, not only this turn's drafting. It also fires when earlier worker spend used nearly all the budget and the drafting only tipped it over. A mark plus a token count taken when drafting starts would make this exact.
| const byHand = args.priceIn !== undefined && args.priceOut !== undefined; | ||
| const [brief, listed] = await Promise.all([ | ||
| buildBrief({ cwd: args.cwd }).catch(() => ({ text: "" })), | ||
| byHand ? Promise.resolve(null) : fetchPricing(args.url, args.model, key).catch(() => null), |
There was a problem hiding this comment.
Non-blocking (N3). This skips the stored price that priceWorker honours (needsPriceLookup / storedEndpoint()). If the provider publishes no list but the user saved a price, mission plan says $ unknown while maat run shows dollars. Worth sharing one resolver.
…ral names - bench: judge_columns carries the judge's plan (judge_plan). A run whose judge was plan-paid counts in the lane median at the worker's cost; only a judge with no price and no plan is left out. The alarm split reads 'judge (OpenCode plan)'. - An unpriced worker with a priced judge is held by both per-turn ceilings, dollars for the judge's part and tokens for the whole turn, and stops at whichever it reaches first. - The drafting hint keys on a mark and token count taken when drafting starts (Engine.draftingStarted), and is given only when the spend before the drafting was under the budget. - mission plan resolves the worker's price through the same helper as run (workerPricing): hand-set, stored for the model, or the provider's list. - Neutral names: SpendMeter, AskSpend, askSpendLine, with JudgeMeter, JudgeSpend and judgeSpendLine kept as aliases. - docs: interview asks are in the session total but in no job's spend.
|
Addressed in 5c80826, item by item: B1 (plan-paid judge in the bench median).
N1 (both ceilings when the worker is unpriced). A priced worker is still held in dollars only. With an unpriced worker and a priced judge, both ceilings apply: dollars for the judge's part, tokens for the whole turn. Whichever is closer to its ceiling gives the warnings and the stop. Your reproducer is now a test (100k tokens per step, N2 (drafting hint). The CLI calls
N3 (mission plan pricing). N4.
Verification
|
2 participants
Follow-ups from the review of #56, plus metering for the two worker-model asks #56 left out.
Commit 1: review follow-ups
/budget $x.totalCostUsd()follows the same rule. Test: an unpriced worker, a priced judge and a $0.01 ceiling.median_cost, plus a unit test and a lane test that fails without the change.since()and a cleared price. I chose to re-price: the figure stored at record time is dropped before re-pricing, so a cleared price now shows as$ unknown. The journal'sjudge_usageentry keeps the figure as it stood when the call was made. The docstring says this, and there's a test.--budget. A test also checks that a budget the worker used up gets no hint.priceEngine→priceJudge+priceWorker).amendReviewhas its doc comment back andamendJudgesays the receipt file keeps its figure.cli.tsxnow has a single import fromformat.js.Commit 2: interview and mission planning as worker spend
interviewTurntakes ameter. The TUI and the desktop passEngine.workerAskMeter, which adds the ask to the session meter like a step, priced as the worker, and journals it asworker_ask. HTTP replies use the provider's usage; ACP replies are estimated and marked.maat mission planmeters its ask at the worker's prices (hand-set, or the provider's list looked up alongside the brief) and prints aworker (planning) ...line.askModel's usage reading is now shared asrecordUsage.test/worker-asks.test.ts.Not changed: cache writes are still priced at the base input rate, as on the worker's side, so both sides undercount them the same way.