Fix tracked Codex card refresh - #3189
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
![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: b277023d51
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
clawsweeper
Bot
added
merge-risk: 🚨 other
|
Codex review: needs real behavior proof before merge. Reviewed August 25, 2026, 9:29 AM ET / 13:29 UTC. ClawSweeper reviewWhat this changesThis PR updates tracked Codex usage cards in place during refreshes, preserving the existing menu-row geometry while publishing new quota, reset-credit, and pace values. Merge readiness⛔ Blocked until real behavior proof from a real setup is added - 5 items remain Keep this PR open: it addresses the linked stale-card bug, but two geometry regressions can hide newly refreshed content or truncate a longer reset label. Priority: P2 Review scores
Verification
Live VerificationCommand: Result: FAIL (failed) — execution before step 1 Assertions:
How this fits togetherCodexBar resolves provider usage snapshots into SwiftUI menu cards. While a macOS menu is tracked, the refresh monitor can publish a compatible live model into existing rows instead of rebuilding the menu. flowchart LR
A[Codex usage snapshot] --> B[Refresh monitor]
B --> C{Tracked layout compatible?}
C -->|Yes| D[Live card model]
C -->|No| E[Rebuild menu rows]
D --> F[SwiftUI usage card]
E --> F
Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Preserve stable menu height while ensuring every refreshed value is visible: rebuild for newly introduced metadata or reserve its row, and size reset-label geometry for the refreshed text or a safe width bound. Do we have a high-confidence way to reproduce the issue? Yes—source inspection gives a high-confidence path: refresh from absent to present metric metadata, or from a short to longer reset label, while the card remains tracked. Is this the best way to solve the issue? No—the in-place strategy is appropriate, but the proposed frozen geometry does not preserve visibility for all compatible refreshed values. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against f10b6056554e. 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
|
|
Addressed both review findings in 1d01763 and resolved the inline threads. Local validation: 101 focused refresh/layout tests, 38 architecture gatekeeper tests, and make check all pass. @codex review |
|
🦞👀 Re-review progress:
|
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Co-authored-by: Trim <womayeyonggugele@gmail.com>
steipete
left a comment
steipete
left a comment
There was a problem hiding this comment.
Reviewed reset-card updates, metadata layout stability, hidden-account and workspace isolation, focused regressions, and architecture guardrails.
|
ClawSweeper status: review started. I am starting a fresh review of this pull request: Fix tracked Codex card refresh This is item 1/1 in the current shard. Shard 0/1. This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking. Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted. |
2 participants
Summary
Tests
Runtime proof
Not included: reproducing the reported transition requires live provider reset timing. The PR includes deterministic projection, refresh-monitor, and SwiftUI geometry regressions without accessing real account credentials.
Closes #3168