fix(antigravity): preserve history around UUID-less bookkeeping steps - #3462
Merged
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 88f731dd0e
ℹ️ 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".
clawsweeper
Bot
added
P2
|
Codex review: needs maintainer review before merge. Reviewed September 7, 2026, 4:00 AM ET / 08:00 UTC. ClawSweeper reviewWhat this changesPreserve Antigravity token history when bookkeeping steps lack UUIDs while retaining timestamp ambiguity checks, with focused regression tests and documentation. Merge readiness✅ Ready for maintainer review The fix remains necessary and has no remaining actionable findings. The earlier ambiguity defect is repaired, the integrated result preserves the reviewed implementation, and the contributor’s live comparison supports the recovery behavior. Priority: P2 Review scores
Verification
How this fits togetherCodexBar reads Antigravity session databases to calculate local token history. Its timestamp recovery and completeness checks determine whether the CLI and app can display that history. flowchart TD
A[Local session databases] --> B[Read generation usage]
B --> C[Recover missing timestamps]
D[Step metadata and bot identities] --> C
C --> E{Complete and consistent?}
E -->|Yes| F[Display token history]
E -->|No| G[Withhold uncertain history]
Before mergeNone. Agent review detailsSecurityNone. Review metrics
Technical reviewBest possible solution: Retain bounded timestamp recovery that tolerates unrelated bookkeeping while withholding history whenever identity or date evidence remains ambiguous. Do we have a high-confidence way to reproduce the issue? Yes: current main explicitly invalidates the scan when a timestamp recovery database includes a UUID-less step; the supplied live comparison corroborates that mechanism. This review inspected source without executing tests. Is this the best way to solve the issue? Yes: the patch repairs the existing reader in place, preserves budget and malformed-row checks, and prevents unidentified rows from creating falsely unique timestamp evidence. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 46b8840b2205. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
History |
Unify step occurrence records and preserve ambiguous positional slots without rebuilding a second lookup. Co-authored-by: Peter Urda <peter.urda@gmail.com>
steipete
changed the title
clawsweeper
Bot
added
rating: 🐚 platinum hermit
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Antigravity bookkeeping rows without a step UUID caused the entire local history provider to be withheld as incomplete. Permit those rows after charging their row and byte budgets, while keeping malformed data and exhausted budgets as hard failures. A lone identified timestamp cannot stand for every occurrence of a reused UUID when unidentified rows are present.
Rows without UUIDs can still carry bot identity. Record that evidence before skipping UUID positioning: a duplicate bot ID with unknown UUID permanently invalidates exact and positional matches, regardless of scan order, matching timestamps, or absent timestamps. This addresses the review finding on the original revision. Keep ambiguous positional slots intact so later timestamps never move into them.
Share the identical step-occurrence structs, remove an unused counts projection, and apply positional ambiguity during timestamp selection. The final PR reduces production code by four lines. Token counts, pricing, credentials, discovery roots, and stored data remain unchanged. Changelog and documentation are updated; thanks @urda.
Maintainer validation: six synthetic duplicate-bot combinations failed 18 assertions on the original PR and pass after repair. All 406 focused Antigravity tests across 34 suites pass, including harmless bookkeeping, reused UUIDs, embedded timestamps, malformed rows, and budgets.
make checkpasses and independent review is clean. The fullmake testsuite passed all 1,028 selections across 86 groups without retries. The main integration preserves the tested Antigravity source, tests, and documentation byte-for-byte; integrationmake checkalso passes. Exact-head CI must pass before merge.Contributor live evidence, collected on the original proposal: @urda compared the shipped 0.56.6 CLI and a rebuilt CLI against the same 211 stable local session databases from agy 1.1.27, using read-only scratch copies with WAL sidecars. Complete coverage increased from 208 to 211; the three recovered sessions contained 80,067, 48,183, and 5,299,395 tokens. Removing UUID-less bookkeeping rows from copies also made the shipped CLI return complete coverage, isolating the original cause. One previously passing session's output, excluding updatedAt, matched byte-for-byte. Three actively written sessions were excluded because both binaries reported them incomplete until checkpointing. This is contributor-reported live evidence; the maintainer's final ambiguity repair uses synthetic native SQLite fixtures.
This builds on #3266, #3396, and #3403. It is distinct from #3412's cost-estimation changes. Earlier proposal #3451 remains related; no unrelated issue is claimed resolved.