Skip to content

fix(menu): re-measure merged-menu rows when a tab swap changes card height - #3616

Merged
steipete merged 5 commits into
steipete:mainfrom
zenibako:fix/merged-menu-row-height
Sep 13, 2026
Merged

steipete merged 5 commits into
steipete:mainfrom
zenibako:fix/merged-menu-row-height

Conversation

@zenibako

@zenibako zenibako commented Sep 13, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #3549.

Switching merged-menu tabs could leave a large gap around a short card or clip content and actions. The cached swap preserved an attached AppKit row while replacing its payload with a differently sized card; AppKit retained the row's old measurement.

The in-place swap now requires matching measured card heights. Different-height cards use the existing insert-before-remove path, so AppKit measures the incoming row. Equal-height swaps retain their attached views. Nearby tail, highlight, and metadata reconciliation is simplified; the complete PR is net -2 production lines.

Thanks @zenibako (Chandler Anderson) for isolating the cause, implementing the fix, and supplying the native diagnostic. The diagnostic now requires materialized card geometry, validates the intended tab selection, and compares row height against both the view frame and independently measured intrinsic height. Offscreen virtualized rows remain valid.

Native before/after proof

Same six-provider synthetic fixture on macOS 26.6.2, opened through the production status-item menu with cost sections and a scrollable Overview. Both test bundles were signed with the matching Developer ID. The before control disables only the height check; the final source was restored and rebuilt afterward. No real accounts or provider credentials were used.

Overview → Claude Height check disabled Fix enabled
Table / menu height 735 / 516 pt 516 / 516 pt
Card row / content height 341 / 122 pt 122 / 122 pt
Native assertions Fail on stale geometry All 11 transitions pass
Before After
Before: retained tall row creates gaps and clips actions After: content-sized Claude card and all actions

Validation

Reviewed head: 5af73dc90f6c655927f1a41b8781f6a061cf4f69.

  • Signed native diagnostic: 38.25 seconds, all 11 phases, zero failures. Negative control: 44.28 seconds, fails on the original 735/516 and 341/122 measurements.
  • make test-fast FILTER='StatusMenuTests|MenuRowContainerSizingTests': 182 tests in two suites passed.
  • make check: zero violations in 2,276 files. Independent review and isolated Codex review completed; no accepted actionable findings remain.
  • make test was attempted locally: 47 groups passed before an unrelated CostUsageStoreScaleProofTests 180-second timeout stopped the run. This is not a full local-suite pass.
  • Exact-head CI: passed: both macOS test shards, all three Linux builds, lint, and the aggregate gate.

zenibako and others added 2 commits September 13, 2026 09:04
…eight

AppKit's table-backed menu keeps a reused row's measured height. The merged
menu's cached tab swap exchanges card payloads inside the attached row, so a card
of a different height at the same position (Overview's Codex row becoming the
Claude card, for example) kept the previous height: empty space under shorter
cards and clipped taller ones, until the next rebuild (steipete#3549).

The in-place payload swap now applies only when the row's height is unchanged;
otherwise the incoming item replaces the live one, which AppKit measures. Same-
height swaps keep the flash-free path.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012T91QRonsX591fKr6UuN5R
Opt-in signed native proof that opens the production merged menu with six
synthetic providers of deliberately different card heights, switches tabs while
the menu tracks (Overview also scrolls), records menu, table and row geometry
with item identity, and asserts no row keeps a stale height. Fails on the
reproduced 735pt table / 341pt row without the fix.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012T91QRonsX591fKr6UuN5R
@clawsweeper

clawsweeper Bot commented Sep 13, 2026 •

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Sep 13, 2026
@clawsweeper

clawsweeper Bot commented Sep 13, 2026 •

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed September 13, 2026, 11:36 AM ET / 15:36 UTC (Revision 7).

ClawSweeper review

What this changes

Replace cached menu cards when their measured heights differ, preserve equal-height row reuse, simplify reconciliation, and add regression tests, native diagnostics, and a release note.

Merge readiness

✅ Ready for maintainer review

This remains a useful, well-supported fix: current main and v0.60.1 retain the affected swap behavior. Native before/after evidence supports the repair, the previous diagnostic finding is resolved, and no actionable patch defects remain.

Priority: P2
Reviewed head: 5af73dc90f6c655927f1a41b8781f6a061cf4f69

Review scores

Measure Result What it means
Overall readiness 🦞 diamond lobster (5/6) A small runtime repair has strong native before/after evidence, focused regression coverage, and no remaining actionable findings.
Proof confidence 🦞 diamond lobster (5/6) ✨ media proof bonus Sufficient (screenshot): The signed macOS production-menu diagnostic exercises cached row reconciliation with unequal-height cards; inspected screenshots show corrected spacing and restored actions, while the captured head-specific results report matching row/content geometry across all 11 phases.
Patch quality 🦞 diamond lobster (5/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (screenshot): The signed macOS production-menu diagnostic exercises cached row reconciliation with unequal-height cards; inspected screenshots show corrected spacing and restored actions, while the captured head-specific results report matching row/content geometry across all 11 phases.
Evidence reviewed 9 items Applicable repository guidance: Read the complete root AGENTS.md; no nested AGENTS.md or maintainer-notes directory was present. Applied guidance on small changes, native AppKit validation, isolated fixtures, and provider-data separation. No builds or tests were executed during this read-only review.
Introduced production behavior: The verified base-to-head diff adds a measured-height condition to cached row reuse. Differently sized cards take the existing insert-before-remove path; tail handling and metadata simplifications preserve their prior behavior.
Fix remains necessary on main: Fetched main still swaps compatible card payloads without comparing heights. The caller subsequently refreshes heights only when row width changes, so that existing helper does not replace this repair.
Findings None None.
Security None None.

How this fits together

CodexBar’s merged menu displays cached usage cards when users switch provider tabs. Its reconciliation layer decides whether to reuse attached AppKit rows or insert replacement rows, affecting card sizing and access to menu actions.

flowchart TD
 A[Provider tab selection] --> B[Cached usage cards]
 B --> C[Compare row compatibility and height]
 C -->|Matching heights| D[Reuse attached row]
 C -->|Different heights| E[Insert replacement row]
 D --> F[Content-sized menu and actions]
 E --> F
Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production versus test delta Production −2 lines; tests +432 lines The runtime repair stays small; added coverage includes two regressions and an opt-in native diagnostic.
Native geometry 11 phases pass; stale height mismatch 219 → 0 pt The reported negative control and after-fix measurements exercise the changed AppKit row-reuse path.

Root-cause cluster

Relationship: fixed_by_candidate
Canonical: #3549
Summary: This PR is the focused candidate fix for the reported merged-menu height defect; related account-switching work remains separate.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Technical review

Best possible solution:

Keep content-sized provider tabs by replacing differently sized cached rows while preserving equal-height reuse and the existing menu cache.

Do we have a high-confidence way to reproduce the issue?

Yes: switching between unequal-height cached cards has a concrete native reproduction and negative control in the supplied evidence, supported by current-main source. This reviewer did not execute AppKit.

Is this the best way to solve the issue?

Yes: the guard uses existing measured sizes and replacement behavior, preserving equal-height reuse and avoiding the previously rejected uniform-height padding.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 62a7fa1c409c.

Labels

Label justifications:

  • P2: The fix addresses blank space and clipped menu actions during provider switching, with impact limited to merged-menu layout.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (screenshot): The signed macOS production-menu diagnostic exercises cached row reconciliation with unequal-height cards; inspected screenshots show corrected spacing and restored actions, while the captured head-specific results report matching row/content geometry across all 11 phases.
  • proof: sufficient: Contributor real behavior proof is sufficient. The signed macOS production-menu diagnostic exercises cached row reconciliation with unequal-height cards; inspected screenshots show corrected spacing and restored actions, while the captured head-specific results report matching row/content geometry across all 11 phases.
  • proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence. The signed macOS production-menu diagnostic exercises cached row reconciliation with unequal-height cards; inspected screenshots show corrected spacing and restored actions, while the captured head-specific results report matching row/content geometry across all 11 phases.

Evidence

What I checked:

Likely related people:

  • unknown: The claimed source-line change could not be verified from bounded local history. (role: source history unknown; confidence: low)
  • unknown: The claimed source-line change could not be verified from bounded local history. (role: source history unknown; confidence: low)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (6 earlier review cycles)
  • reviewed 2026-09-13T13:15:06.888Z sha 82c325f :: needs real behavior proof before merge. :: [P3] Use a matching swap script for the two-provider diagnostic
  • reviewed 2026-09-13T13:36:26.454Z sha 0d48ba9 :: needs maintainer review before merge. :: none
  • reviewed 2026-09-13T14:15:20.362Z sha 0d48ba9 :: needs maintainer review before merge. :: none
  • reviewed 2026-09-13T14:45:29.225Z sha f68cbe1 :: needs maintainer review before merge. :: none
  • reviewed 2026-09-13T15:05:20.484Z sha f68cbe1 :: needs maintainer review before merge. :: none
  • reviewed 2026-09-13T15:11:18.494Z sha 5af73dc :: needs maintainer review before merge. :: none

CODEXBAR_MENU_SWAP_SHORT enabled only Codex and Claude while the swap script
still selected Copilot, z.ai and Cursor, so a short run could never satisfy the
phase count. The proof now always enables every provider the script selects.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012T91QRonsX591fKr6UuN5R
@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Sep 13, 2026
Keep the measured-height fix, simplify tail and metadata swaps, and require attached geometry and the intended selection in native proof. Preserve contributor credit in the release note.
@clawsweeper clawsweeper Bot added proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Sep 13, 2026
@steipete
steipete merged commit caad1ca into steipete:main Sep 13, 2026
9 checks passed
@steipete

Copy link
Copy Markdown
Owner

Landed as caad1ca. Thanks @zenibako for the root-cause isolation, fix, and native fixture.

The final head passed 182 focused tests (make test-fast FILTER='StatusMenuTests|MenuRowContainerSizingTests'), make check, and both complete macOS CI shards plus Linux builds in https://github.com/steipete/CodexBar/actions/runs/34764650337. The local full-suite attempt stopped at an unrelated scale-test timeout; that limitation is recorded in the PR body.

The matching Developer ID-signed macOS 26.6.2 diagnostic passed all 11 transitions in 38.25 seconds. Disabling only the height check reproduced the original table/menu mismatch (735/516 points) and card row/content mismatch (341/122 points), failing the diagnostic in 44.28 seconds. The inspected before/after screenshots in the PR contain only synthetic fixture data. Production changes are net -2 lines, and the Unreleased changelog credits the contributor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Merged menu keeps a stale height after switching provider tabs (macOS 26 table-backed menus never shrink)

2 participants