feat(tools): meter and bill gateway-run tool calls - #504
Merged
Conversation
Gateway-run tool calls (otari_web_search, otari_code_execution, MCP) were executed with no pricing, no reservation, and no record: the identical search cost money through POST /v1/search and was free inside /v1/chat/completions, so a key with a $1 budget could search without limit. Each call is now a billing meter on the request's own usage row, priced through the existing flat_request_cost convention, folded into cost, and reconciled against the budget. No migration: the counts ride billing_meters and pricing_breakdown, which already carry non-token charge lines. Also fixes the streaming tool loop's wire framing on all three formats. The loop forwarded its own tool calls to the client (which can never receive their results) and let a second message_start reach it mid-response, reusing content block indices, which every SDK stream accumulator rejects. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Integrating tool metering with routing policies surfaced one real gap. A request whose plan is exhausted writes its error row through log_exhausted_plan, which carried no tool tally, and the caller's single release site then refunded the reservation. So a tool loop that had already run searches lost them twice over: absent from the row, and absent from users.spend. This is not a corner case. Once a tool loop produces its first assistant message the plan locks to that provider, so a failure inside the loop cannot fail over and this error row is the only row the request gets. The exhausted-plan row now carries the tool ledger and records what it charged on the request context, so the existing single release site reconciles that amount instead of refunding. Absorbed rows stay deliberately tally-free: they settle no reservation, and the per-tool breakdown counts requests rather than rows so a failed-over request is one request, not two. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
njbrake
temporarily deployed
to
integration-tests
August 5, 2026 12:52 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
|
Warning Review limit reached
Next review available in: 31 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughChangesGateway tool billing and analytics
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
✨ Simplify code
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 |
make typecheck covers tests and scripts, not just src, and the new tests read fields off union-typed SDK stream events (an Anthropic content_block, a Responses output item) plus any_llm's re-exported tool-call class, which is a distinct type from the openai one the message field is annotated with. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
njbrake
temporarily deployed
to
integration-tests
August 5, 2026 12:58 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (3)
src/gateway/services/pricing_init_service.py (1)
96-102: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRun the independent pricing checks concurrently.
The sandbox and web-search lookups do not depend on each other. Use
asyncio.gatherfor these checks, then deriveunpricedfrom the paired results.As per coding guidelines, “Run independent asynchronous operations with
asyncio.gather.”🤖 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 `@src/gateway/services/pricing_init_service.py` around lines 96 - 102, Update the pricing checks surrounding find_model_pricing in the initialization flow to run independently configured-tool lookups concurrently with asyncio.gather, preserving the existing provider and use_defaults=False arguments. Pair each result with its corresponding tool, then derive unpriced from results whose pricing lookup returned None.Source: Coding guidelines
src/gateway/api/routes/_pipeline.py (1)
1770-1800: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winBroaden the guard so
_apply_tool_chargesmatches its "Never raises" contract.The docstring states this helper never raises, and settlement depends on that. The
tryblock catches onlySQLAlchemyError.price_tool_callsalso callsfind_model_pricing, which can reach the default-pricing fallback andnormalize_effective_at; aValueError,TypeError, orKeyErrorfrom that path would escapelog_usageand turn an accounting problem into a failed response. That is exactly the outcome the docstring rules out.Catching
Exceptionhere keeps the counts on the row and keeps the promise.SQLAlchemyErrorstays covered because it is a subclass.♻️ Proposed change
try: tool_cost, lines, unpriced = await price_tool_calls(db, billable, as_of=usage_log.timestamp) - except SQLAlchemyError: + except Exception: # noqa: BLE001 - settlement must not fail on a pricing problem logger.exception("Failed to price gateway tool calls; counts recorded without cost") commit_meters() returnIf you prefer to keep the narrow catch, consider softening the docstring instead so a future reader does not rely on a guarantee the code does not give.
🤖 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 `@src/gateway/api/routes/_pipeline.py` around lines 1770 - 1800, Broaden the exception handler around price_tool_calls in _apply_tool_charges from SQLAlchemyError to Exception so pricing failures, including ValueError, TypeError, and KeyError, cannot escape the helper. Preserve the existing exception logging and commit_meters fallback so usage counts remain recorded and the Never raises contract is maintained.scripts/seed_usage_smoke.py (1)
87-87: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winUse explicit billing sentinels instead of truthiness.
tool_costis initialized to0.0, and this expression uses bothif not tool_costandtoken_cost or 0.0. These checks conflate absent values with legitimate zero values. Initializetool_costasNonewhen no priced tool exists and useis None.As per coding guidelines, billing fields must distinguish absent values from legitimate zero values with
is None.Suggested billing sentinel change
- tool_cost = 0.0 + tool_cost: float | None = None - cost=token_cost if not tool_cost else round((token_cost or 0.0) + tool_cost, 6), + cost=( + token_cost + if tool_cost is None + else round((token_cost if token_cost is not None else 0.0) + tool_cost, 6) + ),Also applies to: 132-136
🤖 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/seed_usage_smoke.py` at line 87, Update the billing logic around tool_cost and token_cost to use None as the absent-value sentinel instead of 0.0 or truthiness checks. Initialize tool_cost to None when no priced tool exists, replace not tool_cost checks with is None, and replace token_cost or 0.0 with an explicit None check so legitimate zero costs remain unchanged.Source: Coding guidelines
🤖 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/seed_usage_smoke.py`:
- Around line 57-64: After the conditional ModelPricing initialization, load the
effective persisted rate for the web_search model key and use that stored value
when calculating both tool_lines and tool_cost, instead of directly using
TOOL_UNIT_RATES["web_search"]. Ensure the lookup occurs after any pending
initialization is flushed or committed so existing database pricing is honored.
In `@src/gateway/api/routes/usage.py`:
- Around line 967-1002: The _tool_breakdown function must accept the active
status filter and pass it to _request_count_expr so absorbed rows count requests
consistently with calls, errors, and cost. Update its caller to provide the
status value, and add coverage for dimensions=tool with status=absorbed.
In `@src/gateway/services/pricing_service.py`:
- Around line 372-382: Batch the pricing lookups in the settlement flow before
iterating over billable_calls: fetch effective pricing for all eligible tool
keys with one query using an IN-style filter, then build a tool-to-pricing
lookup map. Update the existing loop to read from that map, preserving unpriced
handling and charge-line calculations for each tool.
In `@src/gateway/static/dashboard/assets/ActivityPage-CBy3JnT8.js`:
- Line 1: Update the ActivityPage query inputs around the memoized E object and
D=we(E,S,qt) so E includes the selected tool as tool: M || void 0. Add M to that
memo’s dependency array, then rebuild the generated ActivityPage bundle from the
source ActivityPage component.
In `@web/src/pages/ActivityPage.tsx`:
- Around line 1096-1107: Update the contextSummary request to include the “tool”
breakdown dimension instead of NO_BREAKDOWNS, and add toolFilter to
contextFilters so the context summary and timeline honor the selected tool.
Preserve the existing Tool selector rendering based on
contextSummary.data?.by_tool and toolFilter.
In `@web/src/pages/ToolsGuardrailsPage.tsx`:
- Around line 477-480: Update the ToolsGuardrailsPage pricing flow so a failed
usePricing() load is treated as unavailable data, not just a finished load.
Surface pricing.error in the page state/UI and change the ToolPriceRow disabled
condition to include the unavailable/errored pricing state, not only
pricing.isLoading. Use the existing pricing, currentRates, and ToolPriceRow
symbols to gate write-enabled inputs until valid pricing data is present.
In `@web/src/pages/UsagePage.test.tsx`:
- Line 56: Add colocated Vitest coverage in UsagePage.test.tsx for the by_tool
fixture: verify the “Gateway-run tools” card renders tool-call, failure, and
spend metrics, then assert selecting a tool navigates with the tool=web_search
query parameter.
---
Nitpick comments:
In `@scripts/seed_usage_smoke.py`:
- Line 87: Update the billing logic around tool_cost and token_cost to use None
as the absent-value sentinel instead of 0.0 or truthiness checks. Initialize
tool_cost to None when no priced tool exists, replace not tool_cost checks with
is None, and replace token_cost or 0.0 with an explicit None check so legitimate
zero costs remain unchanged.
In `@src/gateway/api/routes/_pipeline.py`:
- Around line 1770-1800: Broaden the exception handler around price_tool_calls
in _apply_tool_charges from SQLAlchemyError to Exception so pricing failures,
including ValueError, TypeError, and KeyError, cannot escape the helper.
Preserve the existing exception logging and commit_meters fallback so usage
counts remain recorded and the Never raises contract is maintained.
In `@src/gateway/services/pricing_init_service.py`:
- Around line 96-102: Update the pricing checks surrounding find_model_pricing
in the initialization flow to run independently configured-tool lookups
concurrently with asyncio.gather, preserving the existing provider and
use_defaults=False arguments. Pair each result with its corresponding tool, then
derive unpriced from results whose pricing lookup returned None.
🪄 Autofix
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: CHILL
Plan: Pro Plus
Run ID: 528384d6-607e-42ae-a2a2-499101f79b5d
⛔ Files ignored due to path filters (1)
docs/public/openapi.jsonis excluded by!docs/public/openapi.json
📒 Files selected for processing (57)
README.mddocs/dashboard.mddocs/public/otari.postman_collection.jsondocs/routing.mddocs/tools.mdscripts/seed_usage_smoke.pysrc/gateway/api/routes/_pipeline.pysrc/gateway/api/routes/_tools.pysrc/gateway/api/routes/responses.pysrc/gateway/api/routes/usage.pysrc/gateway/main.pysrc/gateway/services/_tool_loop.pysrc/gateway/services/mcp_client.pysrc/gateway/services/mcp_loop.pysrc/gateway/services/mcp_loop_messages.pysrc/gateway/services/mcp_loop_responses.pysrc/gateway/services/pricing_init_service.pysrc/gateway/services/pricing_service.pysrc/gateway/services/sandbox_backend.pysrc/gateway/services/tool_usage.pysrc/gateway/services/usage_admin_service.pysrc/gateway/services/web_search_backend.pysrc/gateway/static/dashboard/assets/ActivityPage-CBy3JnT8.jssrc/gateway/static/dashboard/assets/ActivityPage-DoWiqnsE.jssrc/gateway/static/dashboard/assets/BudgetsPage-B5-6Ex0R.jssrc/gateway/static/dashboard/assets/ConfirmDialog-CFWFtJrb.jssrc/gateway/static/dashboard/assets/DocsPage-BEmI5Wnd.jssrc/gateway/static/dashboard/assets/KeysPage-CgYrLT-S.jssrc/gateway/static/dashboard/assets/ModelScopeControl-BfTfTYXk.jssrc/gateway/static/dashboard/assets/ModelsPage-ASSDqxHK.jssrc/gateway/static/dashboard/assets/ModelsPage-DMGjenfl.jssrc/gateway/static/dashboard/assets/OverviewPage-CJfJuf3u.jssrc/gateway/static/dashboard/assets/ProvidersPage-DU2bXkbD.jssrc/gateway/static/dashboard/assets/RoutingPage-D7qcgK6F.jssrc/gateway/static/dashboard/assets/SettingsPage-BLJb0Asq.jssrc/gateway/static/dashboard/assets/TablePagination-DRAWJGML.jssrc/gateway/static/dashboard/assets/ToolsGuardrailsPage-BCliGg09.jssrc/gateway/static/dashboard/assets/ToolsGuardrailsPage-BgHcrK3G.jssrc/gateway/static/dashboard/assets/UsagePage-B8hBZljQ.jssrc/gateway/static/dashboard/assets/UsagePage-qe0sVX_i.jssrc/gateway/static/dashboard/assets/UsersPage-KByto6ON.jssrc/gateway/static/dashboard/assets/index-Bf4pieZ0.csssrc/gateway/static/dashboard/assets/index-D3sjy7jc.jssrc/gateway/static/dashboard/index.htmltests/integration/test_hybrid_mode_chat.pytests/integration/test_routing_policies.pytests/integration/test_tool_billing_settlement.pytests/unit/test_mcp_loop_messages.pytests/unit/test_mcp_loop_responses.pyweb/src/api/types.tsweb/src/pages/ActivityPage.test.tsxweb/src/pages/ActivityPage.tsxweb/src/pages/ModelsPage.tsxweb/src/pages/OverviewPage.test.tsxweb/src/pages/ToolsGuardrailsPage.tsxweb/src/pages/UsagePage.test.tsxweb/src/pages/UsagePage.tsx
💤 Files with no reviewable changes (4)
- src/gateway/static/dashboard/assets/UsagePage-B8hBZljQ.js
- src/gateway/static/dashboard/assets/ActivityPage-DoWiqnsE.js
- src/gateway/static/dashboard/assets/ToolsGuardrailsPage-BgHcrK3G.js
- src/gateway/static/dashboard/assets/ModelsPage-DMGjenfl.js
…pricing Streaming, all three formats. A mixed batch (the caller's tools plus a gateway tool in one message) filtered the gateway's fragments out but left the survivors on their upstream indices, so a client received tool_calls[0].index == 1 and the official OpenAI accumulator raised IndexError where main accumulated it fine. The survivors are now renumbered into a gapless sequence, matching what the Messages and Responses strategies already did for content blocks. The deferred terminal chunk kept the unrewritten event, which leaked the hidden call back for any provider that packs tool_calls and finish_reason together; it now defers the rewritten one, and the Responses terminal event no longer carries the owned function_call in response.output. Those calls were also hidden without being run, so the model's search silently vanished: a mixed batch now executes them for their side effects, which is the contract the non-streaming loop already applied. Tool pricing. A priced tool appeared in /v1/models as a selectable model quoting a per-request rate as a per-million-token price, and calling it 502'd; the reserved otari: provider is now filtered out of the catalogue, as it already was in the dashboard. Pricing only a tool no longer silences the "no model pricing configured" startup alarm. The tool gate now skips budget-exempt keys and writes a rejection row, matching the model gate it claims to mirror. Client disconnect bills the tool work already done rather than refunding it. Pricing lookups run as one query instead of one per tool. Docs claimed Otari meters Anthropic's native web search; nothing reads the provider's search count, so the claim is removed rather than the feature implied. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
njbrake
temporarily deployed
to
integration-tests
August 5, 2026 13:42 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
The Activity page's Tool filter was inert. `usageParams` enumerates every filter by hand and `tool` was never added, so the list, the row count, and the timeline all went out unfiltered while the chip and the URL said otherwise: the table showed rows that did not match, most of them with no tool usage at all. Found by clicking it against a populated gateway. The predicate had been verified in isolation against both dialects, which is why nothing caught that the request never carried it, and the existing tests asserted the chip and the URL state rather than the outgoing query. The regression test asserts the query string of every request the page makes. The model typeahead now scopes by tool as well, like it already did for endpoint, provider, and session. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
njbrake
temporarily deployed
to
integration-tests
August 5, 2026 14:09 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
src/gateway/api/routes/usage.py (1)
990-1006: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winBatch the tool aggregates into one query.
Lines 990-1006 execute one
SELECTper entry inGATEWAY_TOOL_NAMES. Adimensions=toolsummary therefore adds one database round trip per tool. Build conditional aggregates for all known tools in one statement, then constructUsageToolRowvalues from that result.As per coding guidelines, “Avoid N+1 queries in async SQLAlchemy gateway code: do not execute queries or deletes inside row loops; batch with
IN, bulk operations, or eager loading, and verify nested endpoints do not fan out.”🤖 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 `@src/gateway/api/routes/usage.py` around lines 990 - 1006, The per-tool summary loop in the usage route is doing an N+1 query pattern by running one SELECT for each entry in GATEWAY_TOOL_NAMES. Update the aggregation flow around the existing _tool_calls_expr, _tool_cost_expr, and UsageLog.billing_meters usage to fetch all tool metrics in a single statement with conditional aggregates, then map that one result into the per-tool UsageToolRow values. Keep the existing request-count and cost semantics intact while removing the per-tool db.execute inside the loop.Source: Coding guidelines
🤖 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 `@src/gateway/services/mcp_loop_messages.py`:
- Line 181: Update the exception comment in mcp_loop_messages.py at the except
Exception as exc branch to remove the em dash from the prose, and rephrase it
using standard punctuation while keeping the same meaning; only adjust the
inline comment text near the non-stream loop handling and leave the exception
logic unchanged.
- Around line 181-183: Update the exception handling around the streaming tool
execution to log only spec["name"] and an opaque exception class, removing the
raw exc value from logger.warning. Keep the user-facing text assignment generic
so text does not expose raw exception details.
In `@src/gateway/services/mcp_loop.py`:
- Around line 345-354: Update the foreign-tool-call filtering branch around
_with_tool_calls so SDK copy failure cannot forward the original unfiltered
event. Use the supported reconstructed chunk path when rebuilding the visible
event, and ensure removal failure hides or safely replaces the chunk rather than
allowing gateway-owned fragments to reach finalize_exit.
In `@web/shot-tmp.mjs`:
- Around line 4-23: Wrap the browser workflow after chromium.launch in a
try/finally block and move browser.close into finally, covering navigation,
authentication, page inspection, and screenshot capture failures. Keep the
existing workflow unchanged within the try block and ensure cleanup runs for
every exit path.
- Around line 2-3: Update the top-level OUT and BASE constants in shot-tmp.mjs
so they are read from environment variables instead of hardcoded workspace and
host values, and fall back to the documented local defaults when unset. Ensure
the script creates OUT before any screenshot write path is used, and keep the
change scoped to the existing OUT/BASE setup so the rest of the script continues
to use those symbols unchanged.
- Around line 9-21: Update the smoke-check flow in shot-tmp.mjs so it actually
validates the Activity page instead of only logging diagnostics: capture and
verify the response from the POST to /v1/auth/session, avoid setting the
localStorage session flag unless authentication succeeds, and replace the fixed
timeout plus console-only checks with an assertion that waits for a stable UI
signal on the Activity page. Use the existing page.evaluate, page.goto,
page.reload, and locator-based checks to confirm the expected table headers and
Gateway tool pills are present, and throw when those indicators are missing.
---
Nitpick comments:
In `@src/gateway/api/routes/usage.py`:
- Around line 990-1006: The per-tool summary loop in the usage route is doing an
N+1 query pattern by running one SELECT for each entry in GATEWAY_TOOL_NAMES.
Update the aggregation flow around the existing _tool_calls_expr,
_tool_cost_expr, and UsageLog.billing_meters usage to fetch all tool metrics in
a single statement with conditional aggregates, then map that one result into
the per-tool UsageToolRow values. Keep the existing request-count and cost
semantics intact while removing the per-tool db.execute inside the loop.
🪄 Autofix
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: CHILL
Plan: Pro Plus
Run ID: 7d96a688-be4a-4631-bc32-97d68964505f
📒 Files selected for processing (37)
docs/tools.mdsrc/gateway/api/routes/_pipeline.pysrc/gateway/api/routes/models.pysrc/gateway/api/routes/usage.pysrc/gateway/services/_tool_loop.pysrc/gateway/services/mcp_loop.pysrc/gateway/services/mcp_loop_messages.pysrc/gateway/services/mcp_loop_responses.pysrc/gateway/services/pricing_init_service.pysrc/gateway/services/pricing_service.pysrc/gateway/services/tool_usage.pysrc/gateway/static/dashboard/assets/ActivityPage-BNPDINq4.jssrc/gateway/static/dashboard/assets/BudgetsPage-U9mSOH8I.jssrc/gateway/static/dashboard/assets/ConfirmDialog-BaM1X73w.jssrc/gateway/static/dashboard/assets/DocsPage-Cmcl6wuO.jssrc/gateway/static/dashboard/assets/KeysPage-Bydt6eMb.jssrc/gateway/static/dashboard/assets/ModelScopeControl-Dm9RmnpY.jssrc/gateway/static/dashboard/assets/ModelsPage-ClVppCTp.jssrc/gateway/static/dashboard/assets/OverviewPage-CuazTUm1.jssrc/gateway/static/dashboard/assets/ProvidersPage-BVPOO2TZ.jssrc/gateway/static/dashboard/assets/RoutingPage-9w3Zx5mI.jssrc/gateway/static/dashboard/assets/SettingsPage-CsCLYy4Z.jssrc/gateway/static/dashboard/assets/TablePagination-BLyGHwYY.jssrc/gateway/static/dashboard/assets/ToolsGuardrailsPage-DPjR2v0v.jssrc/gateway/static/dashboard/assets/UsagePage-vanhi_2x.jssrc/gateway/static/dashboard/assets/UsersPage-UyJ_ZHMT.jssrc/gateway/static/dashboard/assets/index-DooGBETD.jssrc/gateway/static/dashboard/index.htmltests/unit/test_mcp_loop.pytests/unit/test_mcp_loop_messages.pytests/unit/test_mcp_loop_responses.pyweb/shot-tmp.mjsweb/src/api/hooks.tsweb/src/pages/ActivityPage.test.tsxweb/src/pages/ActivityPage.tsxweb/src/pages/ToolsGuardrailsPage.tsxweb/src/pages/UsagePage.test.tsx
🚧 Files skipped from review as they are similar to previous changes (10)
- src/gateway/services/pricing_init_service.py
- src/gateway/services/mcp_loop_responses.py
- src/gateway/static/dashboard/index.html
- src/gateway/services/tool_usage.py
- src/gateway/services/_tool_loop.py
- web/src/pages/ToolsGuardrailsPage.tsx
- src/gateway/services/pricing_service.py
- docs/tools.md
- web/src/pages/ActivityPage.tsx
- src/gateway/api/routes/_pipeline.py
Streaming chat: _with_tool_calls fell back to returning the upstream chunk when the SDK copy failed, which since the hiding landed means leaking a gateway tool call the client cannot answer and that the loop is about to execute itself. It now signals failure, the chunk is dropped, and a terminal whose rewrite failed is sent with no tool_calls rather than with the gateway's. Seeder: read the persisted otari:<tool> rate back instead of trusting its own constant, so an instance already priced through POST /v1/pricing does not get seeded rows whose unit_rate and cost contradict the pricing table. That mismatch was real, not hypothetical; the two agreed only by coincidence. Adds the dimensions=tool&status=absorbed coverage that was asked for. It passes because absorbed rows carry no tool meters by construction, so the aggregate matches nothing and the entry is dropped; the test pins that invariant rather than asserting the request count. Also drops a screenshot script that should never have been committed, and the em dashes from two comments this change moved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
njbrake
temporarily deployed
to
integration-tests
August 5, 2026 14:25 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
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 `@tests/integration/test_usage_summary.py`:
- Around line 970-989: Update test_tool_breakdown_is_empty_for_absorbed_rows to
create an absorbed usage row with the existing _make_log helper before issuing
the summary request, passing status="absorbed" and appropriate billing_meters.
Keep the request and assert that the resulting by_tool list is empty, ensuring
the test validates an actual absorbed row rather than an empty dataset.
🪄 Autofix
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: CHILL
Plan: Pro Plus
Run ID: 0ff40531-4f80-44b7-b646-7a29070cbb24
📒 Files selected for processing (5)
scripts/seed_usage_smoke.pysrc/gateway/services/mcp_loop.pysrc/gateway/services/mcp_loop_messages.pysrc/gateway/services/mcp_loop_responses.pytests/integration/test_usage_summary.py
🚧 Files skipped from review as they are similar to previous changes (4)
- src/gateway/services/mcp_loop.py
- src/gateway/services/mcp_loop_responses.py
- scripts/seed_usage_smoke.py
- src/gateway/services/mcp_loop_messages.py
The test asserted an empty by_tool against an empty table, so it passed vacuously. It now inserts the production shape (an absorbed attempt plus the row that served, with only the latter carrying the tool ledger) and asserts the work is reported once, off the row that served. The docstring no longer claims to cover _request_count_expr in that aggregate. Reverting that expression leaves the test green, because the query already restricts to rows carrying tool meters and an absorbed row never has them, so it is excluded before the count is taken. The invariant that defends is asserted where it is observable, in the routing failover test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
njbrake
temporarily deployed
to
integration-tests
August 5, 2026 14:37 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
14 tasks
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
1 participant
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.
Description
Gateway-run tool calls were never billed.
otari_web_search,otari_code_execution, and MCP tools ran with no pricing, no reservation, and no record, so the same search costs money throughPOST /v1/searchand was free inside/v1/chat/completions: a key with a $1 budget could search without limit.docs/tools.mdalready claimed Otari handled billing for both paths.Each call is now a billing meter on the request's own usage row, priced per call under
otari:<tool>through the existingflat_request_costconvention, folded intocost, and reconciled against the budget. No migration: the counts ridebilling_metersandpricing_breakdown, which already carry non-token charge lines. Tools get a price-per-call row on the Tools page, Activity marks the rows that ran them, and Usage gains a per-tool spend table.Breaking change: with
require_pricingon (the default), a configured tool with no price is refused with a 402 at admission, matching how an unpriced model behaves. A startup warning names the exact pricing call, so it surfaces before the first rejection. Worth a release note.Also fixes the streaming tool loop's wire framing on all three formats. It forwarded the gateway's own tool calls to clients that can never receive their results, and let a second
message_startarrive mid-response while reusing content-block indices, which SDK stream accumulators reject.On top of #492: charges settle on the row that served, absorbed rows stay tally-free, and an exhausted plan still owes for the searches it ran.
PR Type
Relevant issues
None open. Related: #181 (OTel spans for gateway-run tools), which this does not change.
Checklist
tests/unit,tests/integration).make lint,make typecheck,make test).uv run python scripts/generate_openapi.py).Checks
make lint,make typecheck,make openapi-check,make postman-check, 1249 unit, 970 integration, and 437 dashboard tests, with the committed bundle rebuilt. CI is green on all of it.The load-bearing test is
tests/integration/test_tool_billing_settlement.py, which assertsusers.spendrather than only the usage row. That is what catches billing a charge onto a row while refunding the reservation, sincerefund_reservationreleases a hold without recording spend.AI Usage
AI Model/Tool used:
Claude Opus 5 via Claude Code.
Any additional AI details you'd like to share:
Written by Claude through back-and-forth with @njbrake, who made the design calls: bill the work rather than only surface it, fail closed on an unpriced tool instead of serving it free, ship the streaming framing fix here rather than separately, and emit the native item on Responses with inbound stripping rather than dropping it.
Reviewed by three independent agents (design, engineering, developer experience) with no knowledge of how it was built. That pass found six defects the implementation had missed, including refunding instead of reconciling on every failure path that ran tool calls, and a caller-named MCP tool being able to collide with the token meters that the billed-token SQL reads.
Summary