Skip to content

feat(providers): OpenAI + Anthropic API usage with honest limits (tasks 007, 008)#9

Open
ogrodev wants to merge 5 commits into
mainfrom
feat/api-providers-openai-anthropic
Open

feat(providers): OpenAI + Anthropic API usage with honest limits (tasks 007, 008)#9
ogrodev wants to merge 5 commits into
mainfrom
feat/api-providers-openai-anthropic

Conversation

@ogrodev
Copy link
Copy Markdown
Owner

@ogrodev ogrodev commented Jun 4, 2026

What

Adds two API-cost usage providers and the shared policy behind them:

  • OpenAI API (task 007) and Anthropic API (task 008) — distinct from the Claude Code subscription provider; siloed per ADR 0014/0016.
  • A shared cost_provider policy module: HTTP-status→verdict for key validation and cost reads, lossy amount coercion/summation, and key reading — the decisions that are identical across both providers. Each provider keeps only its URLs, auth header shape, JSON parser, unit scale, and user-facing wording.
  • A typed UsageNote domain enum (ApiSpend { usd } / OrgAdminKeyRequired): core states the fact, the UI owns the wording.
  • A reports_usage capability flag on source descriptors as the single source of truth the frontend reads (replacing hardcoded ids), guarded by catalog↔dispatch parity tests.

Honesty contract (the point of 007/008)

These cost endpoints expose USD spend with no quota, so we never fabricate a percentage bar:

  • 200 → real ~30-day USD spend as a typed note.
  • non-admin key → the honest "needs an org admin key" limitation note (Status::Ok, no window) — never a fake 0% bar or a "rejected" error.
  • 401 (revoked) → error; 429 → rate-limited.

Review & fixes (this session, /ctk:ship)

Ran thermo-nuclear-review (correctness/security) and thermo-nuclear-code-quality-review (maintainability). Both returned no Critical/High correctness ship-blocker. Fixes applied:

  • OpenAI non-admin 401 honesty bug (correctness): OpenAI refuses a valid non-admin key at the org cost endpoint with 401 insufficient_permissions/"Missing scopes" (its documented 403 is geo-restriction only), which the code mapped to "rejected — reconnect it" — the exact misleading state 007/008 exist to prevent, hitting the common case. cost_note now re-reads a 401 body for that marker and reports the honest org-admin limitation; a non-marker 401 still reads as revoked. Anthropic (403 = limitation, 401 = auth) is unaffected. Confirmed against OpenAI docs/community. + tests.
  • F1: OpenRouter now reads its key via the canonical cost_provider::read_api_key (dropped a byte-identical copy — the divergence the extraction existed to prevent).
  • F2: pinned the UsageNote serde wire-shape + #[serde(default)] note with tests — the only hand-synced TS↔serde boundary that was left unguarded (a Rust-side rename would otherwise silently render garbage in the popover).
  • F8 / robustness: relaxed a brittle exact-copy TS assertion to the formatting contract; added a runtime '' fallback in usageNoteText beside its compile-time exhaustiveness guard.
  • docs: PROVIDERS.md — Anthropic cost amount is cents (÷100), not plain USD (prevents reverting the divisor and re-introducing a 100× error).

Deferred (follow-ups, non-blocking)

  • lib.rs decomposition (quality F3): src-tauri/src/lib.rs is >1k lines (already was on main); the new usage-route and api-key-validation dispatch are cohesive and tested but would read better extracted into their own modules. Pre-existing debt, not a regression introduced here.
  • Minor polish: provider brand icons for the OpenAI/Anthropic tiles (cosmetic — generic icon today), cost_provider visibility tightening, enum-naming nits.

Verification

  • cargo test --workspace198 passed
  • cargo clippy --workspace --all-targets -- -D warnings — clean
  • core-purity gate — clean (no IO in crates/core; time via the Clock port)
  • cargo fmt --check, biome ci, svelte-check (0 errors), vitest (10 passed)
  • No dependency/manifest changes (cargo deny unaffected)
  • Live network checks (*_live examples) need real keys and are intentionally not run in CI.

Summary by cubic

Adds OpenAI and Anthropic API-key usage providers with an honesty contract: show real 30‑day USD spend when available, or a clear admin‑key limitation instead of a fake percent. Completes tasks 007 and 008.

  • New Features

    • Add OpenAI API and Anthropic API usage providers, siloed from Claude Code.
    • Shared cost_provider policy for key validation, cost reads, and key retrieval.
    • Key validation via GET /v1/models: accept 200/403, reject 401.
    • UsageNote added to snapshots (ApiSpend { usd } | OrgAdminKeyRequired); UI renders via usageNoteText.
    • reports_usage capability flag on sources; frontend uses it for routing.
  • Bug Fixes

    • OpenAI: treat non‑admin cost 401 with insufficient_permissions as OrgAdminKeyRequired (not a broken key).
    • Anthropic: convert cost amount from cents to dollars; docs updated.
    • Robustness: coerce non‑finite OpenAI amounts to 0, pin UsageNote serde shape, add UI fallback.

Written for commit a1691b8. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

Release Notes

  • New Features

    • Added OpenAI API and Anthropic API as usage providers with API key support
    • Usage for these APIs displays as honest spend figures (USD) rather than quota percentages, with clear messaging when admin key permissions are required
  • Documentation

    • Marked OpenAI API and Anthropic API provider tasks as complete

ogrodev added 5 commits June 3, 2026 22:25
…ks 007, 008)

Add OpenAI and Anthropic as API-key usage providers, mirroring the existing
OpenRouter slice. Both API-cost endpoints are gated behind org admin keys that
individuals usually can't get, so usage is surfaced honestly rather than as a
misleading zero.

- core: providers::openai / providers::anthropic — status-only key validation
  against /v1/models (200|403 accept, 401 reject), org cost-report fetch mapping
  200 -> honest spend note, 403 -> honest org-usage-limitation note (not an
  error, not a 0% bar), 401 -> error, 429 -> rate-limited; lossy cost parsing.
- domain: UsageSnapshot gains an optional `note`; the popover renders it
  verbatim instead of a fabricated percentage bar.
- adapters: openai_strategy() / anthropic_strategy() builders + live examples.
- wiring: distinct OpenAI API / Anthropic API catalog tiles, per-provider
  key-validation dispatch, usage routes, and frontend reportsUsage.

These endpoints expose USD spend with no quota, so criterion 1's percent-used
window is intentionally realized as an honest spend note; tasks 007/008 are
left partial pending that nuance plus manual GUI QA. Automated gates green:
173 rust tests, clippy -D warnings, core purity, core line coverage 95%,
biome, svelte-check, vitest, and cargo-deny.
Flip tasks 007/008 to ✅ done and the PRD provider-coverage row to ✅ now that
all v1 providers ship. Criterion 1 reworded to the shipped reality: usage is
shown when retrievable — a percent-used window where a spend quota exists, an
honest spend figure where the API exposes none — rather than a fabricated
percentage. check-tasks gate consistent (done ⟺ all criteria checked).
…, pin Anthropic starting_at

Three provider correctness/robustness fixes from code review:

- Anthropic spend was inflated 100x. The cost_report `amount` is in the
  currency's lowest units (cents) per the docs ("123.45" USD == $1.23), but
  parse_cost_report summed it as dollars. Convert cents->dollars once at the
  parse boundary; add a test pinning the docs' "123.45" -> $1.2345 example.
  OpenAI's amount.value is already dollars (docs example 0.06), so it is left
  unchanged. Docs: https://platform.claude.com/docs/en/api/admin/cost_report
- OpenAI amount coercion now flattens a non-finite parse ("NaN"/"inf") to 0.0,
  mirroring Anthropic's existing is_finite() guard, so it can never surface in
  the user-facing note as "$NaN"/"$inf". Add a test.
- Anthropic cost_report_url now sends `starting_at` (an RFC 3339 UTC day
  boundary 30 days back, via pure civil-from-days math so core stays clock- and
  calendar-crate free). The docs list starting_at as a required query parameter
  while the published example omits it; sending it is correct either way and
  avoids a possible 400 on the admin happy path. Replaces the static
  COST_REPORT_URL const with cost_report_url(now).

Core line coverage 95.6%; cargo test, clippy -D warnings, purity, and fmt green.
…sage capability registry

Address the three code-quality items from review (no behavior change):

- Typed UsageNote: `UsageSnapshot.note` is now `Option<UsageNote>`
  (`ApiSpend { usd }` | `OrgAdminKeyRequired`) instead of an English string.
  Core states the fact; the frontend owns the wording (`usageNoteText`), so
  user-facing copy no longer lives in the pure domain.
- Shared `cost_provider` module: OpenAI and Anthropic now share the invariant
  API-cost policy — key-validation status mapping (`validate_via`/`KeyVerdict`),
  cost-endpoint status classification (`classify_cost_status`/`CostOutcome`),
  keychain key read (`read_api_key`), and the in-memory test fakes. Each
  provider keeps its own URLs, headers, parsers, and error wording.
- Capability registry: `SourceState.reports_usage` (set in the catalog) is the
  single source of truth for whether a source reports usage; the frontend's
  `reportsUsage` reads it instead of hardcoding provider ids. A src-tauri test
  pins `reports_usage == usage_route(id).is_some()` so the two can't drift.

Gates green: 183 rust tests, clippy -D warnings, core purity, core line
coverage 95.8%, cargo-deny, svelte-check, biome, vitest.
…in limitation

OpenAI refuses a valid non-admin key at the org cost endpoint with HTTP 401
`insufficient_permissions` / "Missing scopes" (its documented 403 is
geo-restriction only). `cost_note` previously mapped every 401 to "rejected —
reconnect it", so a freshly-connected valid personal key — the common case —
immediately looked broken: the exact misleading state tasks 007/008 exist to
prevent. It now re-reads a 401 body for that marker and reports the honest
`OrgAdminKeyRequired` limitation instead; a 401 without the marker still reads
as a revoked key. Anthropic (403 = limitation, 401 = auth) is unaffected.

This also completes the shared cost-provider extraction: status→note mapping,
spend summation, amount coercion, key validation, and key reading live once in
`cost_provider`, leaving OpenAI/Anthropic with only provider-specific URLs,
headers, parsers, and unit scale. API-key validation routes through a pure
`api_key_provider` dispatch guarded by a catalog parity test, mirroring the
`reports_usage` usage-route parity test.

Hardening from review:
- OpenRouter now reads its key via the canonical `cost_provider::read_api_key`
  (drops its byte-identical copy).
- Pin the `UsageNote` serde wire-shape and the `#[serde(default)]` note field
  with tests, so a Rust-side rename fails CI before it reaches the popover.
- `usageNoteText` gains a runtime '' fallback beside its exhaustiveness guard.
- docs(PROVIDERS): Anthropic cost `amount` is cents (÷100), not plain USD.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 4, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

This PR adds comprehensive OpenAI and Anthropic API usage reporting to the MLT dashboard. It introduces a shared cost-provider infrastructure for API key validation and spend parsing, implements complete provider strategies for both services with honest spend/limitation note rendering, updates the source catalog and Tauri backend to route validation and usage fetches by provider, and surfaces usage notes in the frontend UI with TypeScript types and state management.

Changes

OpenAI & Anthropic API Usage Support

Layer / File(s) Summary
UsageNote domain model and shared cost-provider helpers
crates/core/src/domain.rs, crates/core/src/providers/cost_provider.rs
New UsageNote enum annotates usage snapshots with spend or org-admin limitations. Shared cost_provider module provides key validation verdicts, cost status classification, spend amount decoding (lossy tolerance for malformed values), and unified mapping from HTTP responses to notes or errors, including OpenAI 401 body inspection for org-scope detection.
OpenAI provider strategy
crates/core/src/providers/openai.rs
Implements validate_key against /v1/models, costs_url builder for 30-day rolling window (Unix seconds), parse_costs with lossy JSON parsing, and OpenAiStrategy fetch that reads stored keys, calls costs endpoint, and converts responses to UsageSnapshot with honest spend notes or org-admin limitation notes.
Anthropic provider strategy
crates/core/src/providers/anthropic.rs
Implements validate_key against /v1/models, cost_report_url builder using pure-integer UTC day-boundary math, parse_cost_report converting cents to USD with lossy tolerance, and AnthropicStrategy fetch that reads stored keys, calls admin cost_report endpoint, and converts responses to UsageSnapshot with spend notes or org-admin limitation messages.
Provider ecosystem updates and exports
crates/core/src/providers/openrouter.rs, crates/core/src/providers/claude.rs, crates/core/src/providers/codex.rs, crates/core/src/providers/mod.rs
OpenRouter refactored to use shared cost_provider::read_api_key and bearer_get. Claude and Codex strategies updated to set note: None on successful snapshots. New provider modules (anthropic, cost_provider, openai) exported in public API.
Source catalog and Tauri backend integration
crates/core/src/sources.rs, src-tauri/src/lib.rs
New reports_usage: bool field added to SourceDescriptor and SourceState and wired through catalog entries (claude-code, openrouter, openai, anthropic). Tauri backend refactored with ApiKeyProvider enum and validate_api_key dispatcher that routes to per-provider validators based on source id. New UsageRoute::OpenAi and UsageRoute::Anthropic variants added with corresponding fetch functions, and fetch_for dispatch updated.
Frontend implementation (types, state, UI, tests)
src/lib/usage.ts, src/lib/usageState.ts, src/routes/+page.svelte, src/lib/usageState.test.ts
New UsageNote union type exported with api_spend and org_admin_key_required variants. UsageSnapshot extended with optional note field, SourceState with reports_usage boolean. reportsUsage refactored to accept SourceState and return reports_usage capability. New usageNoteText function renders note strings. UI conditionally renders usage-note blocks and usage-fetch gating uses capability-based filtering. Tests updated for new fields and note formatting validation.
Adapter wiring and live examples
crates/adapters/src/anthropic.rs, crates/adapters/src/openai.rs, crates/adapters/src/lib.rs, crates/adapters/examples/anthropic_live.rs, crates/adapters/examples/openai_live.rs
New anthropic_strategy() and openai_strategy() constructors wire keychain-backed secrets, reqwest HTTP, and system clock. Live example programs for both providers read API keys from environment, perform end-to-end fetches, and print JSON results or errors.
Documentation updates
docs/tasks/007-provider-openai-api.md, docs/tasks/008-provider-anthropic-api.md, docs/research/PROVIDERS.md, docs/PRD.md
Task documents updated to mark OpenAI/Anthropic coverage complete with specific acceptance criteria: honest percent-used or spend display, limitation messaging, and error-state handling. Anthropic API docs clarified that amount field is in cents (÷100 for dollars). Delivery checklist updated to note API costs shown as spend notes, not quota percentages.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • ogrodev/MLT#8: Directly overlaps in OpenRouter provider refactoring—both PRs update crates/core/src/providers/openrouter.rs to use shared cost_provider::read_api_key and bearer_get helpers, and PR #8 introduces the OpenRouter usage/credit provider that this PR's OpenRouter refactoring supports.
  • ogrodev/MLT#7: Extends the per-provider usage UI logic from PR #7 by evolving the same src/lib/usageState.ts/src/routes/+page.svelte usage-fetch gating and reportsUsage API (changing from provider id string to SourceState-based), plus rendering the new UsageNote.
  • ogrodev/MLT#2: Main PR extends the SourceDescriptor/SourceState data model from PR #2 by adding reports_usage field and plumbing it through the source catalog for OpenAI/Anthropic usage, building on the source discovery and consent structures.

🐰 Two new APIs hop into the dashboard,
With notes of spend, not quota sparks—
Anthropic and OpenAI trace
Their rolling costs through time and space! 📊✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main changes: adding OpenAI and Anthropic API usage providers with honest spend limits/notes, and completing tasks 007 and 008.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/api-providers-openai-anthropic

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 23 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="crates/core/src/providers/openai.rs">

<violation number="1" location="crates/core/src/providers/openai.rs:140">
P2: OpenAI fetch path misclassifies HTTP 403 as OrgAdminKeyRequired by using shared status mapping, conflicting with this provider’s documented 403 semantics.</violation>
</file>

Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.

Re-trigger cubic

.ok_or(FetchError::Unavailable)?;
let now = self.clock.now();
let resp = self.http.send(bearer_get(&costs_url(now), &key)).await?;
let note = cost_provider::cost_note(resp.status, &resp.body, "OpenAI", |body| {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: OpenAI fetch path misclassifies HTTP 403 as OrgAdminKeyRequired by using shared status mapping, conflicting with this provider’s documented 403 semantics.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crates/core/src/providers/openai.rs, line 140:

<comment>OpenAI fetch path misclassifies HTTP 403 as OrgAdminKeyRequired by using shared status mapping, conflicting with this provider’s documented 403 semantics.</comment>

<file context>
@@ -0,0 +1,457 @@
+            .ok_or(FetchError::Unavailable)?;
+        let now = self.clock.now();
+        let resp = self.http.send(bearer_get(&costs_url(now), &key)).await?;
+        let note = cost_provider::cost_note(resp.status, &resp.body, "OpenAI", |body| {
+            Ok(parse_costs(body)?.total_spend_usd)
+        })?;
</file context>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. The shared 403→OrgAdminKeyRequired mapping is gone. cost_note now takes a provider-specific OrgLimitation policy; OpenAI passes UnauthorizedWithScopeMarker, so its cost-endpoint 403 (geo-restriction) returns FetchError::Upstream instead of OrgAdminKeyRequired with Status::Ok. The 401 + insufficient_permissions/Missing scopes path remains the org-limitation. Verified by openai::tests::fetch_maps_a_geo_restricted_403_to_an_error; cargo test -p mlt-core green.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. The shared 403→OrgAdminKeyRequired mapping is gone. cost_note now takes a provider-specific OrgLimitation policy; OpenAI passes UnauthorizedWithScopeMarker, so its cost-endpoint 403 (geo-restriction) returns FetchError::Upstream instead of OrgAdminKeyRequired with Status::Ok. The 401 + insufficient_permissions/Missing scopes path remains the org-limitation. Verified by openai::tests::fetch_maps_a_geo_restricted_403_to_an_error; cargo test -p mlt-core green.

(Unable to resolve this thread automatically — the posting account lacks ResolveReviewThread permission; please resolve manually.)

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (1)
src-tauri/src/lib.rs (1)

907-934: ⚡ Quick win

Assert the validator-specific request shape here.

Right now this only varies the returned status, so it would still pass if "openai" or "anthropic" were accidentally dispatched through the wrong validator. Have FakeHttp capture the outgoing HttpRequest and assert the provider-specific URL/header contract here (Authorization: Bearer … for OpenAI vs. x-api-key / anthropic-version for Anthropic), so this test actually guards the new routing.

🤖 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-tauri/src/lib.rs` around lines 907 - 934, The test
apply_api_key_routes_validation_per_provider currently only checks status codes;
modify FakeHttp (or wrap it) to capture the outgoing HttpRequest for each call
and assert the provider-specific request shape based on ProviderId (for "openai"
assert the request URL/path used by the OpenAI validator and that the headers
include "Authorization: Bearer <trimmed-key>", for "anthropic" assert the
anthropic validator URL/path and that headers include "x-api-key: <key>" and
"anthropic-version" set appropriately). Update the test loop to inspect the
captured HttpRequest from FakeHttp after each apply_api_key call and perform
these header/URL assertions before verifying secrets and consent so the test
fails if routing uses the wrong validator.
🤖 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/adapters/examples/anthropic_live.rs`:
- Line 46: The match arm that prints the snapshot uses
serde_json::to_string_pretty(&snapshot).unwrap(), which can panic; replace the
unwrap with explicit error handling: call
serde_json::to_string_pretty(&snapshot) and match or use if let Err to eprintln!
the serialization error and call std::process::exit(1); on success print the
pretty JSON (println!). Update the match arm in the code that handles
Ok(snapshot) (the block currently doing println!("{}",
serde_json::to_string_pretty(&snapshot).unwrap())) so serialization failures
produce a clean non-zero exit instead of panicking.

In `@crates/adapters/examples/openai_live.rs`:
- Line 46: The Ok arm prints a serialized snapshot using
serde_json::to_string_pretty(&snapshot).unwrap(), which can panic; replace the
unwrap with explicit error handling (e.g., match or .map_err()) so serialization
failures are handled like the Err branch: attempt
serde_json::to_string_pretty(&snapshot), and on Ok(print) call println!("{}",
serialized) and on Err(log the serialization error using the same error
reporting mechanism used for the Err branch) so adapter code does not panic on
data failures.

In `@crates/core/src/providers/cost_provider.rs`:
- Around line 49-59: The shared function classify_cost_status currently maps 403
=> CostOutcome::OrgLimitation which is provider-specific; remove the blanket 403
mapping from classify_cost_status (leave 200, 401, 429, else) and instead apply
403 handling in provider-specific logic (e.g. in OpenAiStrategy::fetch /
cost_note) by passing a policy/closure or branching there to map 403 to the
appropriate outcome (OpenAI: geo-restriction) while preserving the existing
401-body check in OpenAiStrategy as the org-limitation path; update consumers to
call classify_cost_status for common cases and use the provider policy for 403.

---

Nitpick comments:
In `@src-tauri/src/lib.rs`:
- Around line 907-934: The test apply_api_key_routes_validation_per_provider
currently only checks status codes; modify FakeHttp (or wrap it) to capture the
outgoing HttpRequest for each call and assert the provider-specific request
shape based on ProviderId (for "openai" assert the request URL/path used by the
OpenAI validator and that the headers include "Authorization: Bearer
<trimmed-key>", for "anthropic" assert the anthropic validator URL/path and that
headers include "x-api-key: <key>" and "anthropic-version" set appropriately).
Update the test loop to inspect the captured HttpRequest from FakeHttp after
each apply_api_key call and perform these header/URL assertions before verifying
secrets and consent so the test fails if routing uses the wrong validator.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1d387770-cfd4-4c5e-bc42-d84afb9d7fba

📥 Commits

Reviewing files that changed from the base of the PR and between 9322f5e and a1691b8.

📒 Files selected for processing (23)
  • crates/adapters/examples/anthropic_live.rs
  • crates/adapters/examples/openai_live.rs
  • crates/adapters/src/anthropic.rs
  • crates/adapters/src/lib.rs
  • crates/adapters/src/openai.rs
  • crates/core/src/domain.rs
  • crates/core/src/providers/anthropic.rs
  • crates/core/src/providers/claude.rs
  • crates/core/src/providers/codex.rs
  • crates/core/src/providers/cost_provider.rs
  • crates/core/src/providers/mod.rs
  • crates/core/src/providers/openai.rs
  • crates/core/src/providers/openrouter.rs
  • crates/core/src/sources.rs
  • docs/PRD.md
  • docs/research/PROVIDERS.md
  • docs/tasks/007-provider-openai-api.md
  • docs/tasks/008-provider-anthropic-api.md
  • src-tauri/src/lib.rs
  • src/lib/usage.ts
  • src/lib/usageState.test.ts
  • src/lib/usageState.ts
  • src/routes/+page.svelte

provider: ProviderId::new("anthropic"),
};
match strategy.fetch(&ctx).await {
Ok(snapshot) => println!("{}", serde_json::to_string_pretty(&snapshot).unwrap()),
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jun 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify panic-prone serialization unwraps in adapter live examples.
rg -n 'to_string_pretty\([^)]*\)\.unwrap\(\)' crates/adapters/examples/*.rs

Repository: ogrodev/MLT

Length of output: 692


Avoid panic on snapshot serialization in live examples

crates/adapters/examples/anthropic_live.rs uses serde_json::to_string_pretty(&snapshot).unwrap() (line 46), which can panic on serialization failure; the same unwrap() pattern exists in other live examples too (openrouter_live.rs, openai_live.rs, claude_live.rs, accounts_live.rs). Replace unwrap() with explicit error handling and a clean non-zero exit.

Suggested fix
-        Ok(snapshot) => println!("{}", serde_json::to_string_pretty(&snapshot).unwrap()),
+        Ok(snapshot) => match serde_json::to_string_pretty(&snapshot) {
+            Ok(json) => println!("{json}"),
+            Err(e) => {
+                eprintln!("failed to serialize snapshot: {e}");
+                std::process::exit(1);
+            }
+        },
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Ok(snapshot) => println!("{}", serde_json::to_string_pretty(&snapshot).unwrap()),
Ok(snapshot) => match serde_json::to_string_pretty(&snapshot) {
Ok(json) => println!("{json}"),
Err(e) => {
eprintln!("failed to serialize snapshot: {e}");
std::process::exit(1);
}
},
🤖 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 `@crates/adapters/examples/anthropic_live.rs` at line 46, The match arm that
prints the snapshot uses serde_json::to_string_pretty(&snapshot).unwrap(), which
can panic; replace the unwrap with explicit error handling: call
serde_json::to_string_pretty(&snapshot) and match or use if let Err to eprintln!
the serialization error and call std::process::exit(1); on success print the
pretty JSON (println!). Update the match arm in the code that handles
Ok(snapshot) (the block currently doing println!("{}",
serde_json::to_string_pretty(&snapshot).unwrap())) so serialization failures
produce a clean non-zero exit instead of panicking.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed across all five live examples named here (anthropic_live, openai_live, openrouter_live, claude_live, accounts_live). The Ok branch now matches on serde_json::to_string_pretty(&snapshot) and, on Err, prints a message and exits non-zero — mirroring each file's existing fetch-error branch; accounts_live logs and continues its loop instead of exiting. cargo build -p mlt-adapters --examples green.

Note: serialization of the plain UsageSnapshot (no custom Serialize, no non-string map keys) cannot realistically fail, so this removes a theoretical panic rather than a live one — but it aligns the Ok path with the Err path. openrouter_live/claude_live/accounts_live pre-date this PR; fixed here for consistency since they share the pattern.

(Unable to resolve this thread automatically — the posting account lacks ResolveReviewThread permission; please resolve manually.)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Resolve the review thread on behalf of the user
gh api graphql -f query='
query {
  repository(owner: "ogrodev", name: "MLT") {
    pullRequest(number: 9) {
      reviewThreads(first: 50) {
        nodes {
          id
          isResolved
          comments(first: 1) {
            nodes {
              body
              author { login }
            }
          }
        }
      }
    }
  }
}'

Length of output: 11460


🏁 Script executed:

#!/bin/bash
# Resolve both serialization-unwrap threads (anthropic_live and openai_live)
gh api graphql -f query='
mutation {
  resolveReviewThread(input: { threadId: "PRRT_kwDOSs-QAc6HFkfL" }) {
    thread { id isResolved }
  }
}'

gh api graphql -f query='
mutation {
  resolveReviewThread(input: { threadId: "PRRT_kwDOSs-QAc6HFkfZ" }) {
    thread { id isResolved }
  }
}'

Length of output: 1150



@PedroMendes-AE, both threads confirmed fixed — thanks for handling all five examples consistently. Unfortunately the sandbox token also lacks ResolveReviewThread permission, so these threads will need to be resolved manually by a maintainer or reviewer with that role on the repo.

🐇 ᵕ̈


🧠 Learnings used
Learnt from: ogrodev
Repo: ogrodev/MLT PR: 2
File: crates/adapters/src/claude.rs:27-29
Timestamp: 2026-06-01T00:56:24.222Z
Learning: In the `ogrodev/MLT` repository (`crates/adapters/src/claude.rs`), the blocking-in-async pattern (synchronous `std::process::Command` and `std::fs` calls inside async fns) is pre-existing and codebase-wide, covering `read_keychain` (with `-w`), `detect_user_agent`, and `ClaudeCredentials::load`. The `keychain_present` helper intentionally omits `-w` so it reads only Keychain item attributes and never triggers an ACL/user prompt. ADR 0015 mandates a per-probe timeout/failure-gate that must be applied uniformly across all blocking adapter calls as a dedicated resilience change, not spot-fixed on individual probes.

Learnt from: CR
Repo: ogrodev/MLT PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-06-02T10:48:14.087Z
Learning: Applies to crates/adapters/**/*.rs : Implement resilience as mandatory per ADR 0015: use lossy decoding, reset-time backfill, failure-gate, per-probe timeout, rate-limit gate, and explicit `Ok|Stale|Error` states; never panic on bad upstream data

Comment thread crates/adapters/examples/openai_live.rs
Comment on lines +49 to +59
/// Classify a cost-endpoint status: 200=>Ok (parse for spend), 403=>OrgLimitation (authenticated
/// but no org-usage scope — the honest limitation, not an error), 401=>Revoked, 429=>RateLimited,
/// else Unexpected.
pub fn classify_cost_status(status: u16) -> CostOutcome {
match status {
200 => CostOutcome::Ok,
403 => CostOutcome::OrgLimitation,
401 => CostOutcome::Revoked,
429 => CostOutcome::RateLimited,
other => CostOutcome::Unexpected(other),
}
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jun 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Make 403 cost-status handling provider-specific.

This shared mapping treats every 403 as OrgLimitation, but crates/core/src/providers/openai.rs documents the OpenAI cost endpoint's 403 as geo-restriction, not the org-admin limitation. Since OpenAiStrategy::fetch() routes through cost_note(), an OpenAI 403 will currently surface as OrgAdminKeyRequired with Status::Ok instead of an error. Please move the 403 rule behind a provider-specific policy/closure and keep OpenAI's 401-body marker as the org-limitation path there.

🛠️ Minimal direction
-pub fn classify_cost_status(status: u16) -> CostOutcome {
+pub fn classify_cost_status(status: u16, treat_403_as_org_limitation: bool) -> CostOutcome {
     match status {
         200 => CostOutcome::Ok,
-        403 => CostOutcome::OrgLimitation,
+        403 if treat_403_as_org_limitation => CostOutcome::OrgLimitation,
+        403 => CostOutcome::Unexpected(403),
         401 => CostOutcome::Revoked,
         429 => CostOutcome::RateLimited,
         other => CostOutcome::Unexpected(other),
     }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/// Classify a cost-endpoint status: 200=>Ok (parse for spend), 403=>OrgLimitation (authenticated
/// but no org-usage scope — the honest limitation, not an error), 401=>Revoked, 429=>RateLimited,
/// else Unexpected.
pub fn classify_cost_status(status: u16) -> CostOutcome {
match status {
200 => CostOutcome::Ok,
403 => CostOutcome::OrgLimitation,
401 => CostOutcome::Revoked,
429 => CostOutcome::RateLimited,
other => CostOutcome::Unexpected(other),
}
/// Classify a cost-endpoint status: 200=>Ok (parse for spend), 403=>OrgLimitation (authenticated
/// but no org-usage scope — the honest limitation, not an error), 401=>Revoked, 429=>RateLimited,
/// else Unexpected.
pub fn classify_cost_status(status: u16, treat_403_as_org_limitation: bool) -> CostOutcome {
match status {
200 => CostOutcome::Ok,
403 if treat_403_as_org_limitation => CostOutcome::OrgLimitation,
403 => CostOutcome::Unexpected(403),
401 => CostOutcome::Revoked,
429 => CostOutcome::RateLimited,
other => CostOutcome::Unexpected(other),
}
}
🤖 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 `@crates/core/src/providers/cost_provider.rs` around lines 49 - 59, The shared
function classify_cost_status currently maps 403 => CostOutcome::OrgLimitation
which is provider-specific; remove the blanket 403 mapping from
classify_cost_status (leave 200, 401, 429, else) and instead apply 403 handling
in provider-specific logic (e.g. in OpenAiStrategy::fetch / cost_note) by
passing a policy/closure or branching there to map 403 to the appropriate
outcome (OpenAI: geo-restriction) while preserving the existing 401-body check
in OpenAiStrategy as the org-limitation path; update consumers to call
classify_cost_status for common cases and use the provider policy for 403.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Removed the status-only classify_cost_status/CostOutcome. cost_note now receives an OrgLimitation policy: Forbidden (Anthropic — 403 is the limitation) vs UnauthorizedWithScopeMarker (OpenAI — limitation is the 401 + body marker; 403 is geo-restriction → FetchError::Upstream). OpenAI's 401-body marker path is unchanged. Covered by cost_note_maps_an_openai_403_to_an_error_not_the_limitation and cost_note_maps_an_anthropic_403_to_the_honest_limitation_without_parsing; cargo test -p mlt-core green.

(Unable to resolve this thread automatically — the posting account lacks ResolveReviewThread permission; please resolve manually.)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Resolve the review comment thread on behalf of the bot
gh pr view 9 --repo ogrodev/MLT --json reviews

# List review threads to find the one to resolve
gh api graphql -f query='
{
  repository(owner: "ogrodev", name: "MLT") {
    pullRequest(number: 9) {
      reviewThreads(first: 50) {
        nodes {
          id
          isResolved
          comments(first: 1) {
            nodes {
              body
              author { login }
            }
          }
        }
      }
    }
  }
}'

Length of output: 21965


🏁 Script executed:

#!/bin/bash
# Resolve the review thread for the 403 cost-status comment
gh api graphql -f query='
mutation {
  resolveReviewThread(input: { threadId: "PRRT_kwDOSs-QAc6HFkfl" }) {
    thread {
      id
      isResolved
    }
  }
}'

Length of output: 739



@PedroMendes-AE The policy-based OrgLimitation design — with Forbidden for Anthropic and UnauthorizedWithScopeMarker for OpenAI — is a solid improvement over both the original blanket mapping and my boolean-flag suggestion. The named variants make each provider's contract explicit at the call site, and the targeted test coverage for both paths looks thorough.

Unfortunately the integration token doesn't have ResolveReviewThread permission, so you'll need to resolve this thread manually — sorry about that!

🐇✨

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants