Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
|
Reviewed by Codex gpt-6-luna (xhigh); verified and validated by Claude Thermo-nuclear review of #699. Part (a) is complete against the approved design note (Admin API history, typed bridge, balance-fallback behavior, backend and bridge tests). Chart UI and locale keys belong to part (b).
|
|
Fixes for both findings landed in commit "Address thermo review". No items left open. Commands run: cargo +1.98.0 fmt --all; cargo +1.98.0 clippy --all-targets -D warnings on rust and apps/desktop-tauri/src-tauri (clean); cargo +1.98.0 test --lib filtered to openai/open_ai/usage_snapshot (88 passed). No frontend change, so vitest not run. |
Summary
Part (a) of the OpenAI per-day usage chart (upstream 0.66.0): the OpenAI Admin usage fetch now also returns a per-UTC-day history (cost, requests, input/cached/output/total tokens, line items, models). The history travels as
ProviderFetchResult.open_ai_api_usage, reaches the frontend as theopenAiApiUsagebridge field, and is typed intypes/bridge.ts. No UI in this PR; the chart is part (b).rust/src/core/usage_snapshot.rs:OpenAiApiUsageHistory,OpenAiApiDailyUsage,OpenAiApiLineItemCost,OpenAiApiModelUsage;ProviderFetchResult::with_open_ai_api_usage.rust/src/providers/openaiapi/history.rs(new): per-day bucketing keyed bystart_time.result_from_admin_usagenow derives its totals, top models and top line items from these days, so there is one parsing path.apps/desktop-tauri/src-tauri/src/commands/bridge/openai_usage.rs(new): camelCase DTOs (openAiApiUsage, epoch seconds) andFromconversions;ProviderUsageSnapshot.open_ai_api_usage(omitted when absent).apps/desktop-tauri/src/types/bridge.ts:OpenAiApiUsageSnapshotand friends,ProviderUsageSnapshot.openAiApiUsage.billing-api) has no history, as designed.Approved design
Design note:
design-openai-per-day-chart.md(approved; open questions resolved by the note's recommendations). Summary:card.openAIAPIUsage = {historyDays, projectID|null, daily[]}beside the cost summary. Each UTC-day bucket holdsstartTime/endTime,costUSD,requests,inputTokens(input + input_audio),cachedInputTokens(separate, a subset of input),outputTokens(output + output_audio),totalTokens,lineItems[](desc by cost, then name) andmodels[](desc by tokens, then name).openaiapi, Admin API path only. Reuses the two Admin calls already made (/v1/organization/costsgrouped byline_item,/v1/organization/usage/completionsgrouped bymodel,bucket_width=1d); no new endpoint, credential or dependency.OpenAiApiUsageHistoryinusage_snapshot.rs, attached toProviderFetchResult; bridge DTOopenAiApiUsage(camelCase, epoch seconds) mirrored intypes/bridge.ts. Upstream bounds apply (366 days, 10000 line-item + model entries).MenuCardand a Settings > ProvidersChartsSectiontab, reusingcomponents/charts/BarChart, Cost | Tokens toggle, one bar per UTC day, keyboard-navigable day selection, detail panel per day, empty state viaDetailChartEmpty, 13 newOpenAIChart*locale keys across the 8 locales.OPENAI_HISTORY_DAYSsetting exists (the payload carrieshistoryDays); USD only, no currency conversion; project id masking under hide-personal-info is a UI concern for part (b); theopenaiapiid stays out ofPROVIDER_CHART_DATA_IDS.Upstream reference
v0.66.0.Sources/CodexBarCore/Resources/Plugins/openai.js(dailymap,bucket(),card.openAIAPIUsage).Sources/CodexBarCore/Providers/OpenAI/OpenAIAPIProviderDescriptor.swift(mapPluginCardbounds and validation).Sources/CodexBarCore/Providers/OpenAI/OpenAIAPIUsageSnapshot.swift(bucket shape, sort orders).Tests/CodexBarTests/OpenAIAPIUsageFetcherTests.swift(parses admin costs and completions usage into daily summaries; its wire pages are the fixture for the new tests).Ported / Deferred
Ported: per-day bucketing with upstream semantics (buckets keyed by
start_time, firstend_timewins, audio tokens folded into input/output, cached kept separate, days afternowdropped, newesthistoryDayskept, upstream sort orders, blank names fall back toAPI/Responses and Chat Completions, negative or beyond-2^53 counts are parse failures like upstreaminteger()).One deliberate deviation: upstream fails the whole fetch when a card bound is broken (a bucket whose end is not after its start, or more than 10000 line-item + model rows). Here the history is dropped with a
tracing::warn!and the spend summary still shows, because the chart is auxiliary. With the fixed 30-day window the 366-day bound cannot be reached.Deferred (per the design): all UI (part b), locale keys,
OPENAI_HISTORY_DAYS, currency conversion, proof-harness variant for seeding an OpenAI API snapshot (CODEXBAR_SEED_USAGE_JSONcurrently seeds only Codex). The design note's risk about payload size (up to 366 days throughprovider_cacheand events) is not an issue at 30 days; revisit if the window becomes configurable.Behavior change to existing summary: totals, top models and top line items now come from the per-day buckets, so a bucket that starts after
nowno longer counts (upstreamfilter(start <= now)), and equal-cost line items tie-break by name.Validation
Toolchain
cargo +1.98.0, E-core wrappers, slot-5 target dir.cargo fmt --all: clean.cargo clippy --workspace --all-targets -- -D warnings: pass (both manifests).cargo test -p codexbar openaiapi: 40 passed, 0 failed (11 new tests: 10 inhistory_tests.rs, 1 wire-page end-to-end intests.rs; plus a no-history assertion on the balance-fallback test).cargo test -p codexbar(full, shared core touched): 2194 passed, 0 failed, 1 ignored.cargo test -p codexbar-desktop-tauri(full): 465 passed, 1 failed. The failure iscommands::tests::bootstrap_payload_exposes_every_provider_variant(catalog has 79 entries, 78 active providers); this PR does not touch the provider catalog, and the test reads this machine's settings, so it looks environment-dependent. Not confirmed against the base branch.openai_usage_tests.rs(4) pass: camelCase epoch-second payload, omitted when absent, round trip, empty history.pnpm exec tsc --noEmit: clean.pnpm run lint: only pre-existing warnings.vitest run src/components/MenuCard.test.tsx: 31 passed.File sizes: no file crosses 1000 lines.
mod.rs783 -> 748,usage_snapshot.rs819 -> 881;bridge.rs,commands/tests.rsandtypes/bridge.tswere already over 1000 (bridge.rs and tests.rs only gain the one-lineopen_ai_api_usage: Nonefield in literals;bridge.tsgains ~45 lines of type declarations per the design).Affected areas
rust/src/core,rust/src/providers/openaiapi)commands/bridge)types/bridge.ts)UI proof
Not applicable: no rendered surface changes in this PR. CUA proof belongs to part (b).