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. |
Codex session fixtures in the cost scanner tests stamped their events at now - 1h but filed the session under today's local date folder. In the first hour after local midnight (00:00-01:00Z on the UTC CircleCI runner) that hour fell on yesterday, so the tests read the wrong day bucket, and the fork fixtures landed in two different date folders, which flipped the folder scan order. CircleCI builds 977, 978 and 979 failed this way: codex_source_recovery_keeps_appended_duplicate_unpriced_after_cache_reload, paginated_continuation_raises_inherited_baseline_from_total_last and paginated_history_base_equal_parent_keeps_true_fork_subtraction. A shared helper now returns now - 1h, or the start of the local day when that hour reaches back into yesterday, and the fixtures take both the event time and the date folder from it. A unit test pins the helper at fixed times around midnight in UTC and UTC+7. Production code is unchanged.
|
CI failure diagnosed and fixed at the new head f328c66 (was 5d99354): Diagnosis. The failing pr-check (2026-09-30T10:02Z, 10m9s, CircleCI) could not be reproduced in a full local CI-slice run of the recorded head 5d99354 — every step green with the CI-pinned toolchain (rust 1.98.0 from scripts/circleci-pinned-rust.txt): fmt ✓, clippy --workspace (both crates) 0 errors ✓, cargo test codexbar 2196/0 fail/1 ign ✓ (openai focused 75/75), vitest 402/0 ✓, lint 0 errors ✓, check:anti-slop ✓, test:anti-slop ✓, build ✓, interaction-guard tests ✓, circleci-pr.tests.ps1 ✓. The only local failure is the documented #684 bootstrap env baseline, which passes on clean CI hosts. The recorded head's tests are hermetic (fixed clocks), and the run's start time rules out the 00:00-01:00 UTC window… but the flaky cost_scanner fixtures (CI builds 977/978/979: Fix. Cherry-picked 400f805 ("Keep Codex cost fixtures on the local day") onto this branch as f328c66 — test-fixture-only change (rust/src/cost_scanner/tests.rs + tests/paginated.rs), no production code, merge-tree verified conflict-free against the recorded head. This is the same flake fix already delivered for main; it does not alter the PR's behavior. Validation at f328c66 (with the pinned 1.98.0 toolchain):
CI re-check. Hosted pr-check at f328c66: pass (13m54s, workflow 33fc20fe). Fast-forward push 5d99354..f328c66; ls-remote verified. Stack note: #701 (chart UI) is stacked on this branch and already carries its own validation; the cherry-pick is behind #701's head (813458d contains 5d99354), so #701 should merge the new head in — its CI will then include the flake fix too. |
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).