Conversation
Preserve the latest account-scoped pool observation per hour, including replenishment and delayed captures, without inventing reset windows or five-hour pace forecasts. Keep recognized structured summaries on their existing peak-history path. Retain version-1 persistence and account adoption while consolidating duplicate history decoding, ordering, and empty-chart construction. Add regression coverage and synthetic before/after chart proof. Refs #3619 Co-authored-by: hhh2210 <hzy2210@gmail.com>
|
🦞👀 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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: da627ae534
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| $0.id.hasPrefix("antigravity-quota-summary-") && $0.usageKnown | ||
| && !$0.window.isSyntheticPlaceholder && $0.window.usedPercent.isFinite | ||
| && [self.sessionWindowMinutes, self.weeklyWindowMinutes].contains($0.window.windowMinutes ?? 0) |
There was a problem hiding this comment.
Preserve history for partial structured summaries
When a valid response contains only one recognized 5-hour or weekly quota-summary bucket, this predicate suppresses all cadence-less observation samples. However, the structured path in recordPlanUtilizationHistorySample calls sessionEquivalentWindows, which returns no samples unless it finds a complete Gemini session/weekly pair, and the already-computed detectorSamples are not used as a fallback. Such partial summaries therefore record nothing and can make existing observation history disappear from the chart; only select structured mode when the structured recorder can actually emit samples, or retain observations for the unmatched lanes.
Useful? React with 👍 / 👎.
|
Codex review: needs changes before merge. Reviewed September 22, 2026, 1:55 AM ET / 05:55 UTC. ClawSweeper reviewWhat this changesRetains account-scoped Antigravity quota observations without a known reset cadence, preserves them through persistence and adoption, and removes unsupported five-hour pace forecasts. Merge readiness⛔ Needs changes before merge - 2 items remain The quota-history fix remains useful, but the reported partial-summary defect is still present: a valid refresh can hide the newly retained observations without recording replacement history. Priority: P2 Review scores
Verification
How this fits togetherAntigravity quota responses feed CodexBar’s account-scoped usage history. The history recorder stores samples, and the menu chart selects either structured quota windows or cadence-less observations. flowchart TD
A[Antigravity quota response] --> B[Parsed usage snapshot]
B --> C[Choose history format]
C --> D[Structured quota samples]
C --> E[Hourly observations]
D --> F[Account history and disk storage]
E --> F
F --> G[Menu history chart]
B --> G
Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Technical reviewBest possible solution: Use a consistent recordability decision for history capture and chart selection so incomplete summaries preserve usable observations while complete Gemini pairs retain structured peak semantics. Do we have a high-confidence way to reproduce the issue? Yes, from source: record a cadence-less pool snapshot, then supply a valid Gemini-session-only summary; observation selection becomes false while structured recording yields no samples. This review did not execute tests. Is this the best way to solve the issue? Mostly: separate observation series and explicit-duration pace rules fit the existing architecture, but format selection must agree with what the structured recorder can emit. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against b99a91694d78. LabelsLabel changes:
Label justifications:
EvidenceAcceptance criteria:
What I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
|
Preserve the latest account-scoped pool observation per hour, including replenishment and delayed captures, without inventing reset windows or five-hour pace forecasts. Keep recognized structured summaries on their existing peak-history path. Retain version-1 persistence and account adoption while consolidating duplicate history decoding, ordering, and empty-chart construction. Add regression coverage and synthetic before/after chart proof. Refs #3619 Co-authored-by: hhh2210 <hzy2210@gmail.com> (cherry picked from commit da627ae)
Preserve the latest account-scoped pool observation per hour, including replenishment and delayed captures, without inventing reset windows or five-hour pace forecasts. Keep recognized structured summaries on their existing peak-history path. Retain version-1 persistence and account adoption while consolidating duplicate history decoding, ordering, and empty-chart construction. Add regression coverage and synthetic before/after chart proof. Refs #3619 Co-authored-by: hhh2210 <hzy2210@gmail.com> (cherry picked from commit da627ae)
Antigravity fallback pool responses can report usable balances without a reset duration. They currently disappear from quota history, while pace forecasts assume a five-hour window. This adopts the quota-history portion of #3619: account-scoped hourly observations retain the latest capture, including replenishment and delayed responses; persistence and history adoption preserve them. Unknown-cadence summaries use observations, and unavailable snapshots select the freshest stored format.
Recognized structured summaries keep their existing peak history and chart selection, including mixed responses. Local pricing remains on the shared catalog implementation landed in #3757. Duplicate history decoding, ordering, and empty-chart construction are consolidated; the on-disk schema remains version 1.
Thanks @hhh2210! Contributor credit is retained in the commit and changelog.
Validation
With
source Scripts/test_environment.sh(isolated session files; Keychain access suppressed):Before the fix: 2 tests in 1 suite failed with 3 issues: the invented pace forecast and missing observations with/without reset metadata.
After the fix: 420 tests in 22 suites passed. Both regressions passed; structured summary peaks passed with and without reset metadata. The version-1 upgrade, account adoption, persistence, chart selection, and architecture gatekeeper passed.
make checkpassed: 0 violations in 2,551 Swift files, plus formatting, generated-file, locale, and repository checks.git diff --checkandScripts/check_repository_size.shpassed. Isolated Codex review is clean through P2.AntigravityHistoryNativeProofTestspassed with its render environment variable enabled on both versions (1 test each). See the committed render instructions. The full sharded suite and live account probes were not run, per the lane contract.Production budget:
git diff --shortstat origin/main -- Sources WidgetExtension→ 5 files changed, 135 insertions(+), 153 deletions(-) (net −18). Tests: 6 files changed, 474 insertions(+), 16 deletions(-) (net +458).Synthetic chart proof
Both captures use the production SwiftUI chart and the same five synthetic observations, recorded through
UsageStore. They are offscreen renders; no app launch, account, credential, provider request, or menu interaction was used.Refs #3619
Refs #3757