Repository navigation
feat(canary): report Reborn inference cost - #5931
serrrfirat wants to merge 6 commits into
Conversation
🔎 IronLoop Review StatusHead: Current reviewers:
Reviewer summaries
Recent activity
Available commands
Run metadataAdmission: webhook accepted the request and IronLoop persisted reviewer state before this projection. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds model inference usage pricing and tracing, aggregates priced and unpriced calls into live QA results, and exposes per-case and cross-lane token and cost summaries in Slack notifications. ChangesInference usage telemetry
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ModelGateway
participant SemanticJudge
participant LiveQaRunner
participant ResultsJson
participant SlackNotifier
ModelGateway->>LiveQaRunner: emit model usage events
SemanticJudge->>LiveQaRunner: return inference_usage
LiveQaRunner->>ResultsJson: persist case and aggregate usage
ResultsJson->>SlackNotifier: provide inference_usage
SlackNotifier->>SlackNotifier: render lane and case summaries
Possibly related issues
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
github-actions
Bot
added
risk: low
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements telemetry for tracking LLM provider usage, token counts, and estimated USD costs across Reborn and semantic-judge inferences, updating the Slack notification and test reporting systems to aggregate and display these metrics. A critical issue was identified in run_live_qa.py where summing Decimal values without an explicit Decimal(0) start value will raise a TypeError at runtime.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| "input_tokens": sum(_non_negative_int(item.get("input_tokens")) for item in case_summaries), | ||
| "output_tokens": sum(_non_negative_int(item.get("output_tokens")) for item in case_summaries), | ||
| "estimated_usd": str( | ||
| sum((_decimal(item.get("estimated_usd")) or Decimal(0)) for item in case_summaries) |
There was a problem hiding this comment.
In Python, summing Decimal objects using the built-in sum() function without an explicit Decimal start value will raise a TypeError: unsupported operand type(s) for +: 'int' and 'decimal.Decimal'. This is because sum() defaults to a start value of 0 (an integer), and Python does not allow implicit addition of int and Decimal in this context.
To fix this, provide Decimal(0) as the second argument to sum() to initialize the accumulator correctly.
| sum((_decimal(item.get("estimated_usd")) or Decimal(0)) for item in case_summaries) | |
| sum((_decimal(item.get("estimated_usd")) or Decimal(0) for item in case_summaries), Decimal(0)) |
![ironloopai[bot]](https://avatars.githubusercontent.com/in/4116490?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ IronLoop Review: reviewer
Review at a glance
| Verdict | Blocking | Notes | Inline | Head |
|---|---|---|---|---|
| ✅ Approved | 0 | 2 | 2 | 7384b476bb69 |
Head: 7384b476bb69046637e432587206e8631336513b
Next: No reviewer action needed.
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
No blocking runtime or security issues found. I found two non-blocking telemetry accounting gaps in the new inference-cost reporting path.
Findings
Blocking: 0 / Notes: 2
Non-blocking notes (2)
1. 💬 [LOW] Usage telemetry is enabled on an env var Reborn does not read
Location: scripts/reborn_webui_v2_live_qa/run_live_qa.py:739-745
ironclaw-reborn serve initializes tracing from IRONCLAW_REBORN_LOG / IRONCLAW_REBORN_OPERATOR_LOG, not RUST_LOG. Appending ironclaw_runner::model_usage=info to RUST_LOG here does not force the new REBORN_INFERENCE_USAGE events when the Reborn log filter is tightened, so an environment with IRONCLAW_REBORN_LOG=warn will under-report product inference usage. Set or append the target on the Reborn log env var for the child process, and update the test to cover that variable.
2. 💬 [LOW] Failed semantic-judge verdicts are omitted from usage totals
Location: scripts/reborn_webui_v2_live_qa/run_live_qa.py:4546-4549
_case_inference_usage() only searches result.details for semantic-judge usage. When _wait_for_assistant_reply() calls the judge and the judge returns a non-passing verdict, that payload is only embedded in the assertion text; _live_chat_case() then builds failure details from observed, which never receives the judge payload. Those judge calls are therefore missing from per-case and top-level inference_usage on failed canary cases. Persist the judge payload or its inference_usage into the failure details before raising, or carry it via a typed exception.
Developer follow-up
After fixing this feedback:
- Push the fix to this PR branch.
- Re-run this reviewer with
@ironloopai review --agent reviewerif you only changed this reviewer's findings. - Re-run all reviewers with
@ironloopai reviewwhen the fix may affect multiple areas. - Use
@ironloopai statusto check queued/running/completed/failed/superseded state while reviewers run.
| "RUST_LOG", | ||
| "ironclaw=warn,ironclaw_reborn=warn,ironclaw_reborn_webui_ingress=info", | ||
| ) | ||
| if "ironclaw_runner::model_usage" not in rust_log: |
There was a problem hiding this comment.
This appends the usage target to RUST_LOG, but ironclaw-reborn serve reads IRONCLAW_REBORN_LOG for its stderr tracing filter. If that Reborn log filter is set to warn, the new usage events will still be dropped and product inference usage will report as zero.
|
|
||
| def _case_inference_usage(output_dir: Path, result: ProbeResult) -> dict[str, object]: | ||
| events = _product_inference_usage(output_dir) | ||
| events.extend(_semantic_judge_usage(result.details)) |
There was a problem hiding this comment.
This only finds semantic-judge usage that made it into result.details. If the judge runs and returns a failing verdict, _wait_for_assistant_reply() raises with the judge payload only in the assertion string, so the judge inference is not counted for failed cases.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/live-canary/notify_slack.py`:
- Around line 254-259: Update _non_negative_decimal to reject non-finite Decimal
values before comparing or returning: after parsing, check parsed.is_finite()
and return Decimal(0) for NaN or Infinity; retain the existing handling for
parse errors and negative values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 41e41b9f-d3f5-4da5-96f9-494e19ef49c9
📒 Files selected for processing (7)
crates/ironclaw_runner/src/model_gateway.rsscripts/live-canary/README.mdscripts/live-canary/notify_slack.pyscripts/live-canary/test_notify_slack.pyscripts/reborn_webui_v2_live_qa/run_live_qa.pyscripts/reborn_webui_v2_live_qa/semantic_judge.pyscripts/reborn_webui_v2_live_qa/test_run_live_qa.py
Coverage ratchetReborn integration-tier coverageLine coverage (Reborn crates): 85.2% — 290574 / 341066 lines Per-crate breakdown (63 crates, lowest-covered first)
This table itself is informational and never gates the PR on its own — not the percentage, not the per-crate holes, not the 0-coverage callout. A separate coverage ratchet (dry-run until enforce=true; see tests/integration/coverage-floor.toml) can fail the build on specific configured floors. Exemptions (3 entry/entries excluded from the accounting above)
|
7384b47 to
59bd4b3
Compare
|
🚅 Deployed to the ironclaw-pr-5931 environment in ironclaw-ci-preview
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/ironclaw_runner/src/model_gateway.rs`:
- Line 51: Change the model usage telemetry helpers trace_model_usage and
trace_unpriced_model_call from info! to debug! while preserving the
REBORN_INFERENCE_USAGE target and all call sites. Update run_live_qa.py’s
_log_filter_with_model_usage filter directive from =info to =debug so the
harness matches the new logging level.
- Around line 1382-1389: Update the `trace_model_usage` call for the
`provider_complete` event to pass `response.cache_read_input_tokens` and
`response.cache_creation_input_tokens` instead of hardcoded zeros, matching the
tool-call usage logging path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c644d507-ccf3-410b-8ae5-8e0e14ffe189
📒 Files selected for processing (7)
crates/ironclaw_runner/src/model_gateway.rsscripts/live-canary/README.mdscripts/live-canary/notify_slack.pyscripts/live-canary/test_notify_slack.pyscripts/reborn_webui_v2_live_qa/run_live_qa.pyscripts/reborn_webui_v2_live_qa/semantic_judge.pyscripts/reborn_webui_v2_live_qa/test_run_live_qa.py
59bd4b3 to
118b9fe
Compare
github-actions
Bot
added
scope: ci
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/reborn_webui_v2_live_qa/semantic_judge.py`:
- Around line 194-216: Update the token extraction logic in the response-usage
parsing function: require both prompt_tokens and completion_tokens to be present
integer values greater than or equal to zero, and return None when either is
missing or invalid. Avoid defaulting absent provider counts to zero so
downstream aggregation treats incomplete usage as unpriced; retain valid
cached_tokens handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 491bbd37-a8e3-4a60-885d-08bc3851bab6
📒 Files selected for processing (8)
.github/workflows/live-canary.ymlcrates/ironclaw_runner/src/model_gateway.rsscripts/live-canary/README.mdscripts/live-canary/notify_slack.pyscripts/live-canary/test_notify_slack.pyscripts/reborn_webui_v2_live_qa/run_live_qa.pyscripts/reborn_webui_v2_live_qa/semantic_judge.pyscripts/reborn_webui_v2_live_qa/test_run_live_qa.py
118b9fe to
3bb49c6
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/ironclaw_runner/src/model_gateway.rs`:
- Around line 73-98: Update trace_model_usage and its callers to accept the
effective request model name and input/output token rates captured at request
start, rather than reading provider.active_model_name() or
provider.cost_per_token() after completion. In NearAiChatProvider::complete,
resolve req.take_model_override() and the corresponding pricing before
performing the request, then pass these captured values through the
usage-tracing path so concurrent switches cannot alter attribution.
In `@scripts/live-canary/test_notify_slack.py`:
- Around line 390-400: Extend the test around the existing QA and cross-lane
Slack assertions to cover a payload with unpriced_call_count > 0. Assert that
the QA-group text includes the required unpriced-call marker and that the
cross-lane context text includes its corresponding unpriced marker, preserving
the existing priced-call assertions.
In `@scripts/reborn_webui_v2_live_qa/run_live_qa.py`:
- Around line 4500-4511: Update _record_assistant_reply_wait_result() and
routine_confirmation_follow_up handling so every semantic-judge usage event is
preserved instead of overwriting observed["semantic_judge"]; store payloads as a
list or aggregate calls immediately, then update _semantic_judge_usage() to
traverse and count all preserved payloads, including follow-up calls, tokens,
and cost.
- Around line 1317-1332: The failure handling around `_result` and the
corresponding assertion path must not persist the raw `semantic_judge` payload.
Replace the full `semantic_judge` dict with only safe usage fields and redacted
verdict metadata, excluding `response_excerpt` and any prompt/response content,
and ensure assertion text uses the sanitized representation rather than
embedding the original payload.
- Around line 739-750: Use the merged environment mapping when deriving log
filters: in the code that assigns rust_log and reborn_log, replace both
os.environ.get calls with env.get calls so extra_env overrides for RUST_LOG and
IRONCLAW_REBORN_LOG are honored by the child process.
- Around line 4434-4439: Update _decimal to reject non-finite Decimal values
before the nonnegative comparison: after parsing inside the try block, check
parsed.is_finite() and return None when false, then perform the existing >= 0
validation.
In `@scripts/reborn_webui_v2_live_qa/test_run_live_qa.py`:
- Around line 1257-1298: The test’s helper functions, nested exception context,
and dynamic attribute access trigger Ruff warnings. In
test_wait_for_assistant_reply_attaches_failed_semantic_judge_to_error, annotate
fake_judge and fake_sleep parameters and return types, flatten the nested
assertRaisesRegex context using the repository’s preferred unittest style or a
targeted exemption, and replace getattr on raised.exception with direct
semantic_judge access (or an approved static alternative).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: da7736ac-50fb-40c7-9537-3f4e20c54d31
📒 Files selected for processing (9)
.github/workflows/live-canary.ymlcrates/ironclaw_llm/src/nearai_chat.rscrates/ironclaw_runner/src/model_gateway.rsscripts/live-canary/README.mdscripts/live-canary/notify_slack.pyscripts/live-canary/test_notify_slack.pyscripts/reborn_webui_v2_live_qa/run_live_qa.pyscripts/reborn_webui_v2_live_qa/semantic_judge.pyscripts/reborn_webui_v2_live_qa/test_run_live_qa.py
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/ironclaw_runner/src/model_gateway.rs`:
- Around line 74-100: Remove the duplicate is_explicit_free_model predicate from
model_gateway.rs and reuse a shared helper from the owning ironclaw_llm crate,
exposing it if necessary. Update fallback_usage_rates_for_model and related call
sites to use that helper, preserving the existing :free, openrouter/free, and
free handling in one centralized implementation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 57358bc8-64f7-4488-a93d-29ec10c3f4b8
📒 Files selected for processing (6)
crates/ironclaw_runner/src/model_gateway.rsscripts/live-canary/notify_slack.pyscripts/live-canary/test_notify_slack.pyscripts/reborn_webui_v2_live_qa/run_live_qa.pyscripts/reborn_webui_v2_live_qa/semantic_judge.pyscripts/reborn_webui_v2_live_qa/test_run_live_qa.py
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/reborn_webui_v2_live_qa/test_run_live_qa.py (1)
1420-1435: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAssert that
extra_envoverrides inherited log filters.These
assertInchecks pass even ifserver_envaccidentally preserves the process-levelRUST_LOGorIRONCLAW_REBORN_LOGvalues instead of applyingextra_envprecedence. Seed sentinel inherited filters and assert they are absent, or compare the complete expected values while retaining the telemetry directive.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/reborn_webui_v2_live_qa/test_run_live_qa.py` around lines 1420 - 1435, Strengthen test_server_env_honors_extra_env_log_filters by seeding sentinel inherited RUST_LOG and IRONCLAW_REBORN_LOG values before calling server_env, then assert those sentinels are absent and the resulting values exactly reflect extra_env plus the required ironclaw_runner::model_usage=debug directive. This verifies extra_env precedence rather than only checking substring presence.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/ironclaw_llm/src/costs.rs`:
- Around line 77-79: Normalize model IDs to lowercase in model_cost() before the
pricing match, or make the DeepSeek-V4-Flash match case-insensitive, so
mixed-case identifiers resolve to the configured DeepSeek rates rather than the
zero-cost fallback used by cache_read_input_rate_for_model().
In `@crates/ironclaw_runner/src/model_gateway.rs`:
- Around line 103-114: Move cache-read and base pricing ownership into a shared
ironclaw_llm::costs API that returns input, output, and optional cache-read
rates for a model, then update cache_read_input_rate_for_model and
locked_usage_rates_for_model to use it instead of hardcoded model checks and
direct model_cost calls. Reuse the existing nearai_chat.rs
remote_model_fallback_cost behavior so free-model handling remains consistent,
and remove the duplicated deepseek-v4-flash pricing logic from model_gateway.rs.
In `@scripts/reborn_webui_v2_live_qa/run_live_qa.py`:
- Around line 4600-4610: Use an explicit None check when assigning
cache_read_input_rate in the cost calculation: select rates[2] whenever it is
not None, including Decimal("0"), and fall back to rates[0] only when it is
unset. Keep the behavior consistent with the existing rates[2] is not None
check.
---
Outside diff comments:
In `@scripts/reborn_webui_v2_live_qa/test_run_live_qa.py`:
- Around line 1420-1435: Strengthen test_server_env_honors_extra_env_log_filters
by seeding sentinel inherited RUST_LOG and IRONCLAW_REBORN_LOG values before
calling server_env, then assert those sentinels are absent and the resulting
values exactly reflect extra_env plus the required
ironclaw_runner::model_usage=debug directive. This verifies extra_env precedence
rather than only checking substring presence.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3bb56e33-78c9-4a72-b157-1e62d010de1c
📒 Files selected for processing (5)
crates/ironclaw_llm/src/costs.rscrates/ironclaw_llm/src/nearai_chat.rscrates/ironclaw_runner/src/model_gateway.rsscripts/reborn_webui_v2_live_qa/run_live_qa.pyscripts/reborn_webui_v2_live_qa/test_run_live_qa.py
0a1b328 to
f8280ad
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/ironclaw_llm/src/costs.rs`:
- Around line 170-181: The test
test_deepseek_v4_flash_uses_remote_pricing_before_local_heuristic only checks
two casing variants; add a mixed-case assertion using
model_cost("deepseek-ai/Deepseek-v4-Flash") and verify it does not return
Some((ZERO, ZERO)), preserving the regression coverage.
In `@crates/ironclaw_llm/src/nearai_chat.rs`:
- Around line 1207-1218: Centralize the remote model fallback pricing logic in a
single public function in ironclaw_llm::costs, including the shared
explicit-free-model check and zero-cost guard. Replace nearai_chat.rs functions
remote_model_fallback_cost and is_explicit_free_model, the inline logic in
costs.rs, and model_gateway.rs functions fallback_usage_rates_for_model and
is_explicit_free_model with calls to this shared function, removing duplicate
implementations.
In `@scripts/reborn_webui_v2_live_qa/run_live_qa.py`:
- Around line 6981-7007: Malformed priced-usage records are being discarded
instead of counted as unpriced. In the event-building logic, replace the
continue triggered when input_rate, output_rate, or estimated_usd is missing or
invalid with creation of the same unpriced event shape used by the
usage_available=="false" branch, preserving operation, model, token counts, and
unpriced accounting before appending it.
- Around line 1214-1250: The _safe_semantic_judge_payload function still
persists free-text judge reasoning that may contain response content. Remove the
reason field from the sanitized payload, or replace it with a bounded,
predefined classification value; do not persist the model-generated text while
retaining the existing structured fields and usage sanitization.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: fb3939ab-f779-40fe-9beb-69b622b7b06d
📒 Files selected for processing (11)
.github/workflows/live-canary.ymlcrates/ironclaw_llm/src/costs.rscrates/ironclaw_llm/src/nearai_chat.rscrates/ironclaw_runner/Cargo.tomlcrates/ironclaw_runner/src/model_gateway.rsscripts/live-canary/README.mdscripts/live-canary/notify_slack.pyscripts/live-canary/test_notify_slack.pyscripts/reborn_webui_v2_live_qa/run_live_qa.pyscripts/reborn_webui_v2_live_qa/semantic_judge.pyscripts/reborn_webui_v2_live_qa/test_run_live_qa.py
This branch was successfully deployed
1 participant
Summary
Change Type
Linked Issue
Security Impact
Blast Radius
Rollback Plan
Reborn Trust-Boundary Checklist
Notes
Verification
Known unrelated check