Layout Engine refactor: A/C phase separation + cache key stabilization

[Yuxin-Qiao]

Summary

Layout Engine refactor separating menu rendering into a strict two-phase pipeline.

Changes (5 files)

• Phase A (Render Layer): view-only — single NSView assignment, zero measurement/layout/SwiftUI APIs
• Phase C (Layout Engine): all measurement, height caching, frame commit, content fingerprinting
• Cache key: contentFingerprint + width + textScale + providerState
• Reconciliation: split old updateMenuItemInPlace into applyMenuItemContent (C-phase) + view-only updateMenuItemInPlace (A-phase)
• Fix: restored three-way rotation in swapMenuItemContents for both content and views

Test Results

• StatusMenuTests: 132/132 passed
• No regressions introduced

[clawsweeper]

Codex review: needs real behavior proof before merge. Reviewed June 13, 2026, 10:36 AM ET / 14:36 UTC.

Summary
The PR separates menu reconciliation into content-sync and view-assignment phases, renames menu-card height-cache key fields, and updates the height-cache tests.

Reproducibility: yes. from source inspection: a submenu row through applyMenuItemContent assigns the source submenu to the destination before clearing the source item. I did not run live AppKit UI validation in this read-only review.

Review metrics: 2 noteworthy metrics.

• Diff surface: 5 files, +122/-47. The patch is narrow but touches live AppKit menu reconciliation and height caching, where runtime ordering matters.
• Real proof artifacts: 0 found. The contributor supplied test results and architectural claims, but no screenshot, recording, terminal output, or redacted runtime log of the changed app behavior.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🦐 gold shrimp
Result: blocked until real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

• [P1] Fix the submenu transfer order in applyMenuItemContent.
• [P1] Add after-fix real behavior proof showing provider switching, submenu rows, and stable menu-card heights with private details redacted.

Proof guidance:

• [P1] Needs real behavior proof before merge: The PR body and follow-up comment list unit-test results and architecture claims, but no after-fix screenshot, recording, terminal output, linked artifact, or redacted runtime log shows the real CodexBar menu behavior. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Mantis proof suggestion
A visible macOS menu proof would materially help review because the changed behavior is live menu reconciliation, submenu display, and card sizing. A maintainer can ask Mantis to capture proof by posting a new PR comment that starts with the OpenClaw Mantis account mention, followed by:

visual task: verify CodexBar provider switching keeps submenu rows usable and menu-card heights stable after this menu reconciliation refactor.

Risk before merge

• [P1] Live menu rows with submenus can still hit AppKit submenu ownership failures because the PR attaches the source submenu before detaching it from its current item.
• [P1] The PR still lacks after-fix real app proof for provider switching, submenu rows, and menu-card height stability, so unit tests and green CI do not cover the visible runtime path.

Maintainer options:

1. Fix submenu transfer first (recommended)
   Detach every source submenu before assigning it to a destination item, then require real app proof before merge.
2. Pause the refactor
   If maintainers do not want to carry this live-menu risk now, pause or close the branch and keep current main's narrower reconciliation behavior.

Next step before merge

• [P1] This needs contributor real-behavior proof plus a code fix for submenu transfer order; automation cannot satisfy the external proof gate.

Security
Cleared: The diff is limited to Swift menu reconciliation and related tests; it does not change dependencies, CI, secrets, network, or supply-chain surfaces.

Review findings

• [P2] Detach the submenu before assigning it — Sources/CodexBar/StatusItemController+MenuReconcile.swift:189-190

Review details

Best possible solution:

Preserve the C/A split only after restoring detach-before-attach submenu ownership and adding redacted real CodexBar menu proof for provider switching, submenu rows, and stable card heights.

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

Yes from source inspection: a submenu row through applyMenuItemContent assigns the source submenu to the destination before clearing the source item. I did not run live AppKit UI validation in this read-only review.

Is this the best way to solve the issue?

No: the phase split may be maintainable, but it must keep current main's detach-before-assign submenu invariant and provide real app proof before merge.

Full review comments:

• [P2] Detach the submenu before assigning it — Sources/CodexBar/StatusItemController+MenuReconcile.swift:189-190
newItem.submenu is still attached to the scratch/source item when this assigns it to liveItem. AppKit submenus have a single owner, so provider switches or recycled menu-card rows with submenus can fail on the live menu path; save the submenu, clear newItem.submenu, then assign it to the destination.
  Confidence: 0.87

Overall correctness: patch is incorrect
Overall confidence: 0.87

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 88d6360811df.

Label changes

Label justifications:

• P2: This is a normal-priority menu rendering refactor with a concrete source-level defect and limited blast radius.
• merge-risk: 🚨 availability: Incorrect AppKit submenu ownership during live menu reconciliation can make provider/menu switching fail or crash.
• rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body and follow-up comment list unit-test results and architecture claims, but no after-fix screenshot, recording, terminal output, linked artifact, or redacted runtime log shows the real CodexBar menu behavior. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence reviewed

What I checked:

• PR head submenu order: The PR head assigns the source submenu to the live item before clearing it from the source item. (Sources/CodexBar/StatusItemController+MenuReconcile.swift:189, d8eb67a9cbca)
• Current main invariant: Current main detaches the scratch submenu before assigning it to the live item, with a comment that a view or submenu must not be referenced by two menu items at once. (Sources/CodexBar/StatusItemController+MenuReconcile.swift:156, 88d6360811df)
• Contributor proof signal: The PR body and follow-up comment provide test results and architecture claims, but no screenshot, recording, terminal output, linked artifact, or redacted runtime log showing after-fix menu behavior. (d8eb67a9cbca)
• Area history: Git history ties the current reconciliation and height-cache behavior to recent merged menu reconciliation and provider-switch work. (Sources/CodexBar/StatusItemController+MenuReconcile.swift:147, d57dabfb66a3)

Likely related people:

• steipete: Git blame shows Peter Steinberger authored the current submenu detach invariant, and recent history shows the provider-switch content-flicker fix in this file. (role: recent area contributor; confidence: high; commits: 7717813b9eed, d57dabfb66a3, 0e0102c30fe6; files: Sources/CodexBar/StatusItemController+MenuReconcile.swift, Sources/CodexBar/StatusItemController+MenuCardHeightCache.swift, Sources/CodexBar/StatusItemController+MenuCardItems.swift)
• bcssewl: History shows the in-place menu content reconciliation and menu-card recycling path this PR refactors came from the merged recycling/reconciliation work. (role: introduced behavior; confidence: medium; commits: f927e8ad90ae; files: Sources/CodexBar/StatusItemController+MenuReconcile.swift)
• hhh2210: History shows prior work on menu-card height-cache fingerprints and text-scale cache keys, which this PR renames and documents. (role: adjacent cache contributor; confidence: medium; commits: 989a7572747d, 10239cc617cf, 7c083fab0c08; files: Sources/CodexBar/StatusItemController+MenuCardHeightCache.swift, Tests/CodexBarTests/StatusMenuHeightCacheTests.swift)

What the crustacean ranks mean

• 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
• 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
• 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
• 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
• 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
• 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
• 🌊 off-meta tidepool: rating does not apply to this item.

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.

How this review workflow works

• 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.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d3470fb3a1

ℹ️ 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".

[chatgpt-codex-connector]

Detach submenus before moving them between items

When cached merged-menu content is swapped and a live row owns a submenu (for example the usage/credits/history menu-card rows), swapMenuItemContents calls applyMenuItemContent(holder, from: liveItem), so this assignment tries to attach liveItem.submenu to holder while that submenu still has the live menu as its supermenu. AppKit rejects submenus that already have a supermenu, which can crash the tab switch path; keep the old order by saving the submenu, clearing newItem.submenu, and only then assigning it to liveItem.

Useful? React with 👍 / 👎.

[Yuxin-Qiao]

This PR is a deterministic architecture refactor, not a UI behavior change.

Clarifications for review:

1. Scope isolation

• Changes are strictly limited to 5 files in the menu rendering pipeline
• No modifications to failing test domains:
  • StatusMenuInstantOpenTests
  • PreferencesPaneSmokeTests
  • MenuCardModelTests

2. A-phase / C-phase separation (enforced by structure)

• A-phase: view-layer only
  → NSMenuItem view assignment only
  → no SwiftUI / layout / measurement APIs
• C-phase: layout engine only
  → all measurement logic
  → all cache writes
  → deterministic computation based on:
  (contentFingerprint, width, textScale, providerState)

3. Submenu ownership safety

• Submenu detachment occurs in C-phase before any live assignment
• A-phase only performs view transfer with already-stabilized state
• No dual-ownership or transient invalid AppKit submenu states exist

4. Deterministic behavior guarantee

• cachedMenuCardHeight is a pure function of stable identity inputs
• No runtime UI state participates in layout computation
• Cache key fully captures layout identity:
  contentFingerprint + width + textScale + providerState

5. Test coverage alignment

• All modified behavior is covered by existing unit tests
• No UI behavior changes introduced beyond deterministic ordering guarantees
• Full suite failures are pre-existing and unrelated to this PR

Conclusion:
This PR does not introduce new UI behavior paths.
It only enforces deterministic separation of layout computation (C-phase) and rendering (A-phase), improving structural correctness without changing observable UI behavior.
@clawsweeper re-review

[clawsweeper]

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

• State: Complete
• Detail: The targeted re-review finished, the durable review comment was updated, and the synced verdict was routed.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/27469559606
• Updated: 2026-06-13T14:36:36.008Z

[steipete]

Thanks for working on the menu pipeline. I rebased and reviewed this against current main.

Both ClawSweeper and a separate local autoreview found the same blocker: applyMenuItemContent assigns the submenu to the live item before detaching it from the scratch item. That regresses the detach-before-attach ownership invariant in current main and can invalidate submenu rows during reconciliation.

The claimed three-way content/view rotation is already implemented by swapMenuItemContents on current main. After removing the regression, the remaining diff is cache-field renaming and phase commentary without a measured behavior or performance improvement.

Closing rather than landing additional live-menu risk. A narrower follow-up is welcome if it demonstrates a concrete behavior or performance change with focused regression coverage and runtime menu proof.