Fix Codex weekly window detection - #268
Conversation
|
Thanks for the PR, I will review it ASAP. |
Finesssee
left a comment
There was a problem hiding this comment.
Request changes (narrow, one structural blocker)
Blocking: rust/src/providers/codex/api.rs:587-596 (no_session_primary()) is a behavioral duplicate of rust/src/providers/claude/web_api.rs:652-661 (synthetic_no_session_primary()). Both construct the identical informational 5-hour "No active session" RateWindow: 0.0%, window_minutes: Some(300), the same "No active 5h session" description, is_informational = true.
With this PR there are now two independent copies of one cross-provider invariant ("the shared 5h-session lane is absent"). The next time the wording, minutes, or placeholder shape changes for one provider, the other silently diverges — and every downstream surface (tray label, Settings usage bar, CLI line, hooks, forecasts) keys off this exact contract.
Remedy: add a canonical constructor in rust/src/core/rate_window.rs, e.g. RateWindow::no_active_session(), using the existing SESSION_WINDOW_MINUTES constant, and call it from both providers. Codex's version is the better one to promote — it already uses SESSION_WINDOW_MINUTES where Claude's uses the literal 300, so promoting it fixes both.
Everything else in the normalization design looks strong: the fix reuses the canonical is_informational contract that guard/hooks/forecast/tray consumers already honor, deletes the duplicated typed-response parsing path, and hardens the integer boundaries (u32::try_from, json_i64). Verified locally in a read-only worktree at 2cbb67d: cargo test -p codexbar codex → 79 passed, commands::bridge → 5 passed. This is intentionally a narrow request — land the shared constructor and this is an approve.
Codex no_session_primary() and Claude synthetic_no_session_primary() were duplicate provider-local copies of the same informational 5h session placeholder. Promote the shared shape to a canonical RateWindow::no_active_session() built on RateWindow::informational() plus SESSION_WINDOW_MINUTES, and call it from both providers.
Finesssee
left a comment
There was a problem hiding this comment.
Approve
The blocking duplication is resolved: RateWindow::no_active_session() in rust/src/core/rate_window.rs is now the single canonical constructor (built on RateWindow::informational("No active 5h session") plus SESSION_WINDOW_MINUTES), and both provider-local copies — Codex no_session_primary() and Claude synthetic_no_session_primary() — are deleted with all call sites migrated. Field-for-field behavior is identical (0.0%, 5h window, same description, informational), confirmed by the unchanged provider tests.
Validation basis (accurate, exact head 5540635): this repository has no GitHub required checks configured on main (no branch protection, no required status checks), so approval rests on local validation run against this exact head:
vitest run src/surfaces/settings/providers/sections/UsageSection.test.tsx(pnpm 10.18.1, vitest 3.2.4): 2/2 passed — includes the new informational-unavailable-lane test- Full frontend suite
pnpm run test: 37 files / 228 tests passed cargo testrate_window: 6 passed (new core test covers informational=true, minutes=300, description, 0%, no reset)cargo testcodex: 79 passed · claude: 77 passed ·commands::bridge: 5 passedcargo fmtclean;cargo clippy --all-targets -- -D warningsclean on both therustandsrc-tauricrates
Fixes #267
Summary
Root cause
Codex weekly-only payloads were promoted into the primary window, while array payloads assumed the first window was always the 5-hour session. That mislabeled weekly usage and left the weekly and pace lanes incorrect.
Validation
cargo fmt --allgit diff --checkcargo test --manifest-path rust/Cargo.toml codexcargo test --manifest-path apps/desktop-tauri/src-tauri/Cargo.toml commands::bridgecargo clippy --manifest-path rust/Cargo.toml --all-targets -- -D warningscargo clippy --manifest-path apps/desktop-tauri/src-tauri/Cargo.toml --all-targets -- -D warningsapps/desktop-tauri/node_modulesis not installed.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.