fix(analytics): usage_source must reflect unmeasured prompt tokens, not just output source - #1071
fix(analytics): usage_source must reflect unmeasured prompt tokens, not just output source#1071seonghobae wants to merge 6 commits into
Conversation
spend_analytics()["by_model"][].usage_source was classified purely from each model's output-token source (reported/tokenizer/mixed/unavailable across that model's steps this run), while completely ignoring whether prompt tokens were ever available for that model. A model with exactly one step, exact-tokenizer output, and zero reported prompt tokens was labeled pure "tokenizer" — dishonestly implying full count-based knowledge of that model's usage when the prompt side was entirely unmeasured. Add per-model prompt-token tracking (`bucket["prompt_reported_steps"]`) alongside the existing per-model output-source counters, and require full per-model prompt availability before classifying a row as "reported" or "tokenizer". A model whose output is fully known but whose prompt tokens are partially or entirely unmeasured now correctly falls through to "mixed" — the same honest-composite label already used for genuinely mixed output sources. The four-way usage_source vocabulary (reported/tokenizer/mixed/unavailable) is unchanged, matching the documented contract in README.md. The existing report-wide `prompt_available` flag (used for `totals.prompt_tokens`/`measurement_status`) is untouched: it already forces "unavailable" whenever any step in the run lacks a valid reported prompt count, so this fix only tightens per-model row classification, not the report-level status. Validated: tests/test_spend_analytics.py now passes (previously failed with `'tokenizer' == 'mixed'`); full suite `PYTHONPATH=. python -m pytest tests -q --ignore=tests/test_psychometric_routing.py` now shows 3383 passed, 5 failed, 2 skipped — the 5 failures are pre-existing, unrelated `test_nim_benchmark.py` date-evidence-expiry failures (confirmed identical via `git stash` against unmodified origin/main), not caused by or related to this change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KPmJErfkcHer4UVEgrQxUX
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthrough모델별 프롬프트 토큰 보고 단계 수를 집계합니다. 출력 토큰이 완전히 알려져도 프롬프트 토큰이 일부 또는 전부 누락되면 Changes사용량 소스 정확성
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to Spend analytics now marks models with incomplete prompt-token measurements as mixed while preserving reported classification for complete usage, improving accuracy without an identified remaining merge risk. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Five tests in tests/test_nim_benchmark.py exercise unrelated run_mode="live" behavior (missing credential, transport wiring, a malformed-catalog contract failure, CLI exit codes) that incidentally passes through _require_current_actual_cost_evidence(). That check fails closed once nim_benchmark.ACTUAL_COST_EVIDENCE["valid_until_date"] — a human-reviewed date for NVIDIA's published hosted-endpoint terms — is in the past. Today (2026-09-05) is now one day past the literal "2026-09-04" recorded there, so all five started failing for a reason unrelated to what each actually asserts: a classic ticking-time-bomb hard-coded date, not a code regression. The production literal is deliberate and stays as-is: it is a real record of when someone last verified NVIDIA's terms, and live runs are meant to fail closed until that review happens again. tests/test_nim_benchmark_release_acceptance.py already covers that review-cadence invariant directly via explicit `today` injection into _require_current_actual_cost_evidence, and is unaffected. Added an autouse fixture in test_nim_benchmark.py that monkeypatches ACTUAL_COST_EVIDENCE's reviewed_at_date/valid_until_date to bracket real "now" for the duration of this file's tests only, so they stay green regardless of wall-clock date while still exercising the real evidence-gated code path. Verified: `python -m pytest tests/test_nim_benchmark.py -v` and `python tests/test_nim_benchmark.py` both 98 passed (previously 5 failed); tests/test_nim_benchmark_release_acceptance.py unaffected (28 passed); interrogate scoped to contextual_orchestrator/ stays 100%; git diff --check clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KPmJErfkcHer4UVEgrQxUX
The prior fix used an autouse fixture across all 98 tests in tests/test_nim_benchmark.py, silently giving every test synthetic current cost evidence and deriving dates from date.today() — too broad for a security/commercial fail-closed gate, since unrelated tests (present and future) would stop observing the literal production evidence dict. Make the fixture opt-in (current_actual_cost_evidence, no autouse) and request it explicitly only from the five tests that need to get past the evidence gate to reach their own live-path assertion. Production ACTUAL_COST_EVIDENCE is unchanged. Add a regression test proving a test that does not request the fixture still observes the literal production evidence and fails closed at the gate (verified RED against the reverted autouse design, GREEN against this fix). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KPmJErfkcHer4UVEgrQxUX
…ce tests test_live_run_rejects_unreviewed_pricing_before_egress and test_live_run_rejects_incomplete_or_expired_pricing_before_egress now request the current_actual_cost_evidence fixture so they exercise validate_live_pricing_scenario's own fail-closed branches (lines 1708/1718) instead of incidentally matching _require_current_actual_cost_evidence's "expired" error once ACTUAL_COST_EVIDENCE's literal valid_until_date lapsed. Restores 100% branch coverage on nim_benchmark.py's pricing-scenario gate. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KPmJErfkcHer4UVEgrQxUX
|
Ported Generated by Claude Code |
test_live_run_without_evidence_fixture_still_fails_closed_on_expired_evidence asserted that a test not requesting the current_actual_cost_evidence fixture observes real-world ACTUAL_COST_EVIDENCE as expired -- true only because today happened to be past the literal valid_until_date at authoring time. Once #1073 refreshed that production evidence (reviewed_at_date=2026-09-05, valid_until_date=2026-10-05), this PR's own required "Tests and package quality" check failed for real: the evidence gate no longer raised, so the test fell through to a genuine network probe against NVIDIA's live API and got HTTP 403 -- the exact same class of ticking-time-bomb bug this PR is otherwise fixing, just introduced by this PR's own new regression test. Fix: the test now deliberately monkeypatches ACTUAL_COST_EVIDENCE to a fixed past window (2020) itself, proving the same opt-in-fixture-vs-file-wide mechanism without depending on real wall-clock time relative to whatever the production valid_until_date currently is. Verified locally against both the unmodified branch and a merge of this branch with current origin/main (post-#1073 evidence). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KPmJErfkcHer4UVEgrQxUX
|
Root cause: that test (added by the ported Verified locally: Generated by Claude Code |
|
Adjudication evidence (host 1 session, 2026-09-06 KST; full report with commands in #1080). Nothing here closes, flips, or retargets anything — the decision is the opener's. This head is byte-identical to #1070 (same 385-line three-dot patch, 6 files +193/−10; |
|
Closing as duplicate of
Per the merge-order note in the adjudication comment: Generated by Claude Code Generated by Claude Code |
2 participants
Bug / realistic RED
Protected
main@2e414d15ba58f28597751b625a8a2f00fc9fadcfdeterministically misclassified a model row as puretokenizerwhen output tokens were exactly counted but prompt-token usage was unavailable.tests/test_spend_analytics.py::test_exact_output_without_prompt_usage_is_explicitly_unavailableexposedAssertionError: assert 'tokenizer' == 'mixed'on the protected source.The root cause was row classification based only on per-model output-token source. The existing report-wide prompt availability flag cannot safely classify individual model rows in a multi-model run.
Causal repair
The production change adds
prompt_reported_stepsto each model bucket for both ordinary trace steps and judge accounting. Purereportedortokenizernow requires complete prompt evidence for that same model; otherwise an output-known row ismixed. The four-value public vocabulary remainsreported | tokenizer | mixed | unavailable; report-level prompt totals and measurement-status rules are not widened.README and CHANGELOG are aligned with that contract.
Cross-model regression
Exact descendant
2bfb603af517e40381d0914e30221eefb62a006aaddstest_usage_source_is_scoped_per_model_when_prompt_evidence_differs. One report contains:known-model: provider-reported prompt and completion tokens → row must remainreported;missing-model: provider-reported completion tokens with prompt usage absent → row must bemixed;This pins the reason for per-model tracking rather than relying only on the original single-model regression.
Exact identity
main@2e414d15ba58f28597751b625a8a2f00fc9fadcfef126ae009a5874eadfd1534fc453d7e8af4e279(recovered viagit push origin <sha>:refs/heads/...after a local worktree-cleanup mishap deleted the branch ref before push — the commit object itself was never lost)CHANGELOG.md,README.md,contextual_orchestrator/orchestrator.py,tests/test_spend_analytics.pyThis same fix is also cherry-picked into
contextual-orchestrator#1070(commitse4cda9cc/bcecfa16there), since every currently-open PR merging againstmainhits this same pre-existing failure until this lands — it no-ops once this PR merges.Evidence boundary
Keep Draft until one unchanged exact head obtains all applicable test/docstring/security/SAST/CodeQL evidence terminal-success, live review requirements are satisfied, and zero valid findings remain. The separate NVIDIA hosted-cost evidence-expiry defect (tracked in
#1070) is not masked or converted into test-relative production evidence. No self-approval, bypass, force-push, destructive rebase, no-op retrigger, or predecessor-evidence transfer.Generated by Claude Code