Repository navigation
feat(spend): show native Codex turn performance - #4304
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
|
Codex review: blocked before merge. Reviewed October 8, 2026, 2:58 AM ET / 06:58 UTC (Revision 9). ClawSweeper reviewWhat this changesAdds completed-turn performance measurements and expandable statistics to native Codex sessions in Usage & Spend. Example: Select one day in Usage & Spend
Review scores
ProductKind: Feature · Worth it: Yes Merge readiness⛔ Blocked before merge - 2 items remain This PR remains a useful, distinct feature with accepted owner direction and sufficient historical native-app proof. No concrete patch defect was identified. Priority: P3 Before merge
FindingsNone. Agent review detailsHow this fits togetherCodexBar reads local Codex session logs into a cached usage ledger and displays estimated spending in Settings. Completed-turn timing joins those records to provide performance observations alongside billing totals. flowchart TD
A[Local Codex session logs] --> B[Usage and completion parser]
B --> C[Cached usage ledger]
C --> D[Validate completed turns]
D --> E[Filter by completion day]
E --> F[Performance statistics]
C --> G[Session billing totals]
F --> H[Usage and Spend session rows]
G --> H
Technical reviewBest possible solution: Expose qualified native turn observations through the existing ledger while preserving cost ranks, billing dates, saved prices, and unavailable-data semantics. Do we have a high-confidence way to reproduce the issue? Not applicable as a bug reproduction; historical installed-window checks demonstrate the requested measurements, disclosure, and day selection. Is this the best way to solve the issue? Yes: extending the existing local ledger preserves authoritative accounting and avoids a competing parser or data source. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 4614415bbed7. Merge-risk optionsMaintainer options:
Provenance checked
TestingProof path: shipped entry point. Added test files: 16. SecurityNone. EvidenceWhat I checked:
Likely related people:
Review metrics
LabelsLabel changes: No label changes. Label justifications:
Rating scale6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior. WorkflowClawSweeper edits this one comment on every review. Comment HistoryReview history (8 earlier review cycles)
|
616d2c0 to
ce92708
Compare
Integrate the existing contributor branch with current main without rewriting its history. Keep native turn timing separate from billing and saved pricing, retain the cost-ranked header, and exclude malformed supplied start times. Parser revision 12 preserves predecessor stores for bounded timing backfill. Replace historical proof diaries and opt-in private/benchmark harnesses with a shared synthetic turn fixture and concise metric documentation. Keep behavioral coverage and headless production-row rendering, and add the Unreleased changelog credit. Co-authored-by: Yuxin Qiao <104957188+Yuxin-Qiao@users.noreply.github.com>
# Conflicts: # CHANGELOG.md # Sources/CodexBar/Resources/ar.lproj/Localizable.strings # Sources/CodexBar/Resources/ca.lproj/Localizable.strings # Sources/CodexBar/Resources/de.lproj/Localizable.strings # Sources/CodexBar/Resources/en.lproj/Localizable.strings # Sources/CodexBar/Resources/es.lproj/Localizable.strings # Sources/CodexBar/Resources/fa.lproj/Localizable.strings # Sources/CodexBar/Resources/fr.lproj/Localizable.strings # Sources/CodexBar/Resources/gl.lproj/Localizable.strings # Sources/CodexBar/Resources/id.lproj/Localizable.strings # Sources/CodexBar/Resources/it.lproj/Localizable.strings # Sources/CodexBar/Resources/ja.lproj/Localizable.strings # Sources/CodexBar/Resources/ko.lproj/Localizable.strings # Sources/CodexBar/Resources/nl.lproj/Localizable.strings # Sources/CodexBar/Resources/pl.lproj/Localizable.strings # Sources/CodexBar/Resources/pt-BR.lproj/Localizable.strings # Sources/CodexBar/Resources/ru.lproj/Localizable.strings # Sources/CodexBar/Resources/sv.lproj/Localizable.strings # Sources/CodexBar/Resources/th.lproj/Localizable.strings # Sources/CodexBar/Resources/tr.lproj/Localizable.strings # Sources/CodexBar/Resources/uk.lproj/Localizable.strings # Sources/CodexBar/Resources/vi.lproj/Localizable.strings # Sources/CodexBar/Resources/zh-Hans.lproj/Localizable.strings # Sources/CodexBar/Resources/zh-Hant.lproj/Localizable.strings
|
Thanks @Yuxin-Qiao! Merged in a88f081 with your commits preserved: native Codex session rows in Usage & Spend can now show whole-turn output throughput, median first-token latency and the number of valid timed turns, with billing semantics and cost ranks unchanged (verified against the 0.73.0 ledger work after rebasing and regenerating the parser fingerprint) and a reproduced timing-validation gap closed in review. Ships in the next release. |
Native Codex sessions in Usage & Spend now show weighted whole-turn output, median model-first-token latency, and median completed-turn duration beneath the existing cost-ranked header. Performance details start collapsed and expose sample coverage, percentiles, cache fraction, and model/effort groups. Sessions without valid samples show no performance line.
Timing is joined to owned, deduplicated requests in the existing ledger and filtered by completion day. Failed/incomplete turns, malformed supplied start times, invalid counters, and mismatched turn totals are excluded. Missing first-token observations stay unavailable. Billing keeps its request dates, saved prices, and existing range totals; whole-turn throughput includes reasoning, tools, and waits and is not streaming generation speed.
Parser revision 12 (
0d8f9504f8e63d0f) adopts compatible predecessor stores and uses bounded backfill. The regression coverage and synthetic renderer remain; historical proof diaries and private-input/benchmark harnesses have been replaced by a shared fixture and a concise metric contract.Thanks @Yuxin-Qiao! The original contributor history is preserved.
Verification
The Swift wrapper sources
Scripts/test_environment.sh, disables real Keychain access, and forwardstest/buildtoswiftwith--build-system native --jobs 4 -Xswiftc -gnone./tmp/pr-4304-swift test --filter 'CostUsage(Turn|RequestLedger|Store|CoverageCompatibility)|SpendSessionPerformance'reported 239 tests in 19 suites and five failures, all from the new malformed-start parameter cases./tmp/pr-4304-swift test --filter 'CostUsageTurnPerformanceTests|CostUsageTurnPerformanceDetailsTests|CostUsageTurnCompletionDayTests|CostUsageRequestLedgerMigrationTests|CostUsageStoreTests|CostUsageCoverageCompatibilityTests|SpendSessionPerformanceTests|SpendDashboardSessionRowTests|ProviderArchitectureGatekeeperTests|LocalizationLanguageCatalogTests'passed 205 tests in 10 suites. All five malformed-start cases pass while retaining their 220 billed tokens; missing/null start times remain supported.make check: passed, zero violations in 2,861 Swift files. The initial check caught a system/pinned formatter mismatch; the final code uses the repository-pinned formatter.env -u CODEXBAR_ALLOW_TEST_KEYCHAIN_ACCESS CODEXBAR_TEST_SUITE_TIMEOUT=900 ./Scripts/test.sh --swift-command /tmp/pr-4304-swift --direct-workers 4: passed all 1,602 selections in 145 groups in direct mode: 144 groups passed first attempt, one recovered on a fresh group retry, zero timeouts (2,062.1 s). The initial run with the default 180-second harness limit exited 124; larger cost suites exceeded that limit, and an Antigravity fixture assertion recovered on isolation. The successful rerun changed only the harness timeout, not selections or assertions.fbec7506af51130390afd9b19896a8ee9d4d393cwas cancelled; a fresh required-check run is still needed before merge.Synthetic UI
Before the rank restoration (original proposed row):
After retaining cost ranks; the third example has no timing samples:
Narrow dark layout: