Skip to content

Commit d8eb67a

Browse files
Yuxin QiaoYuxin Qiao
authored andcommitted
Layout Engine refactor: A/C phase separation + cache key stabilization
1 parent 71c124e commit d8eb67a

5 files changed

Lines changed: 122 additions & 47 deletions

Sources/CodexBar/StatusItemController+Menu.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1085,7 +1085,7 @@ extension StatusItemController {
10851085
return true
10861086
}
10871087

1088-
func resolvedMenuProvider(enabledProviders: [UsageProvider]? = nil) -> UsageProvider? {
1088+
private func resolvedMenuProvider(enabledProviders: [UsageProvider]? = nil) -> UsageProvider? {
10891089
let enabled = enabledProviders ?? self.store.enabledProvidersForDisplay()
10901090
if enabled.isEmpty { return .codex }
10911091
if let selected = self.selectedMenuProvider, enabled.contains(selected) {

Sources/CodexBar/StatusItemController+MenuCardHeightCache.swift

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,22 @@
11
import AppKit
22

3+
// === LAYOUT ENGINE CONTRACT (PHASE C) ===
4+
// Height measurement is a deterministic function of:
5+
// 1. contentFingerprint — hash of the data shaping the card (model fields, account state)
6+
// 2. width — the resolved menu rendering width (×100, integer-quantized)
7+
// 3. textScale — system text-size token at measurement time
8+
// 4. providerState — provider/identity scope (provider.rawValue, account.id, etc.)
9+
// `id` identifies a specific card slot so two cards with identical content get
10+
// independent entries. Same key → same height, no recomputation. Key change →
11+
// forced re-measurement in the next C-phase.
12+
313
extension StatusItemController {
414
struct MenuCardHeightCacheKey: Hashable {
5-
let id: String
6-
let scope: String
7-
let width: Int
8-
let textScale: Int
9-
let fingerprint: String
15+
let id: String // card slot identifier
16+
let providerState: String // provider / account / scope
17+
let width: Int // menu width × 100
18+
let textScale: Int // system text-size token
19+
let contentFingerprint: String // hash of content payload
1020
}
1121

1222
/// Measured card height also depends on the resolved font sizes, which the menu cards
@@ -19,19 +29,23 @@ extension StatusItemController {
1929
Int((NSFont.preferredFont(forTextStyle: .body).pointSize * 100).rounded())
2030
}
2131

32+
/// === PHASE C: LAYOUT ENGINE — DETERMINISTIC HEIGHT LOOKUP ===
33+
/// Same (id, providerState, width, textScale, contentFingerprint) → identical height.
34+
/// The cache is the source of truth for C-phase measurement output. A-phase
35+
/// render-layer callers MUST NOT invoke this — height is finalized at C-phase exit.
2236
func cachedMenuCardHeight(
2337
for id: String,
24-
scope: String,
38+
providerState: String,
2539
width: CGFloat,
26-
fingerprint: String? = nil,
40+
contentFingerprint: String? = nil,
2741
measure: () -> CGFloat) -> CGFloat
2842
{
2943
let key = MenuCardHeightCacheKey(
3044
id: id,
31-
scope: scope,
45+
providerState: providerState,
3246
width: Int((width * 100).rounded()),
3347
textScale: Self.menuCardHeightTextScaleToken(),
34-
fingerprint: fingerprint ?? "version:\(self.menuContentVersion)")
48+
contentFingerprint: contentFingerprint ?? "version:\(self.menuContentVersion)")
3549
if let cached = self.menuCardHeightCache[key] {
3650
return cached
3751
}
@@ -46,7 +60,7 @@ extension StatusItemController {
4660
func pruneVersionScopedMenuCardHeightCache() {
4761
let currentVersionFingerprint = "version:\(self.menuContentVersion)"
4862
for key in self.menuCardHeightCache.keys
49-
where key.fingerprint.hasPrefix("version:") && key.fingerprint != currentVersionFingerprint
63+
where key.contentFingerprint.hasPrefix("version:") && key.contentFingerprint != currentVersionFingerprint
5064
{
5165
self.menuCardHeightCache.removeValue(forKey: key)
5266
}

Sources/CodexBar/StatusItemController+MenuCardItems.swift

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,19 @@
11
import AppKit
22
import SwiftUI
33

4+
// === PHASE C: LAYOUT ENGINE — MEASUREMENT PIPELINE ===
5+
// All measurement, height caching, and frame commit happens here. A-phase render
6+
// layer must not call any of the functions in this file.
7+
48
extension StatusItemController {
59
func refreshMenuCardHeights(in menu: NSMenu) {
610
let width = self.renderedMenuWidth(for: menu)
711
for item in menu.items {
812
guard let view = item.view, view is any MenuCardMeasuring else { continue }
913
guard abs(view.frame.width - width) > 0.5 else { continue }
1014
let id = item.representedObject as? String ?? "menuCard"
11-
let scope = self.menuProvider(for: menu)?.rawValue ?? id
12-
let height = self.cachedMenuCardHeight(for: id, scope: scope, width: width) {
15+
let providerState = self.menuProvider(for: menu)?.rawValue ?? id
16+
let height = self.cachedMenuCardHeight(for: id, providerState: providerState, width: width) {
1317
self.menuCardHeight(for: view, width: width)
1418
}
1519
view.frame = NSRect(
@@ -72,9 +76,9 @@ extension StatusItemController {
7276
}
7377
let height = self.cachedMenuCardHeight(
7478
for: id,
75-
scope: heightCacheScope ?? id,
79+
providerState: heightCacheScope ?? id,
7680
width: width,
77-
fingerprint: heightCacheFingerprint)
81+
contentFingerprint: heightCacheFingerprint)
7882
{
7983
self.menuCardHeight(for: hosting, width: width)
8084
}

Sources/CodexBar/StatusItemController+MenuReconcile.swift

Lines changed: 73 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,31 @@
11
import AppKit
22

3+
// ============================================================================
4+
// LAYOUT ENGINE ARCHITECTURE
5+
// ============================================================================
6+
// CodexBar menu rendering follows a strict two-phase deterministic pipeline:
7+
//
8+
// PHASE C — LAYOUT ENGINE
9+
// • All SwiftUI measurement, intrinsic sizing, fittingSize evaluation
10+
// • All content fingerprinting, height caching, LayoutGraph construction
11+
// • All menu-item property setup (title, action, target, submenu, …)
12+
// • View allocation: NSHostingView(rootView:) — exactly once per content
13+
// • Frame commit: hosting.frame = precomputedSize
14+
// • Output: frozen NSMenuItem snapshot (view + properties)
15+
// • MUST NOT touch NSMenu — output is the LayoutGraph
16+
//
17+
// PHASE A — RENDER LAYER
18+
// • ONLY responsibility: assign NSView to NSMenuItem
19+
// • NO measurement, NO SwiftUI interaction, NO invalidateIntrinsicContentSize
20+
// • NO fittingSize / intrinsicContentSize access
21+
// • NO layout computation of any kind
22+
// • NSMenu is a passive renderer; A-phase hands it precomputed views
23+
//
24+
// The boundary is enforced structurally: A-phase functions are 1-line assignments.
25+
// Any future code that wants to do "more" in A-phase must be justified against
26+
// this contract.
27+
// ============================================================================
28+
329
/// Pre-harvest snapshot of one live content row, captured before card views are detached
430
/// into the recycle pool so reconciliation can still compare row shapes afterwards.
531
struct MenuRowShape {
@@ -60,12 +86,20 @@ extension StatusItemController {
6086
}
6187

6288
for offset in 0..<prefix {
63-
self.updateMenuItemInPlace(menu.items[fromIndex + offset], from: newItems[offset])
89+
let live = menu.items[fromIndex + offset]
90+
let scratch = newItems[offset]
91+
// Phase C: content sync (properties only, no view).
92+
self.applyMenuItemContent(live, from: scratch)
93+
// Phase A: view handoff (precomputed view, no measurement).
94+
self.updateMenuItemInPlace(live, from: scratch)
6495
}
6596
for offset in 0..<suffix {
66-
self.updateMenuItemInPlace(
67-
menu.items[menu.items.count - 1 - offset],
68-
from: newItems[newItems.count - 1 - offset])
97+
let live = menu.items[menu.items.count - 1 - offset]
98+
let scratch = newItems[newItems.count - 1 - offset]
99+
// Phase C: content sync.
100+
self.applyMenuItemContent(live, from: scratch)
101+
// Phase A: view handoff.
102+
self.updateMenuItemInPlace(live, from: scratch)
69103
}
70104

71105
let liveMiddleCount = shapes.count - prefix - suffix
@@ -106,6 +140,7 @@ extension StatusItemController {
106140
}
107141
displacedItems.append(newItem)
108142
} else {
143+
// Phase A: structural replacement when shape doesn't match.
109144
menu.insertItem(newItem, at: index)
110145
menu.removeItem(liveItem)
111146
displacedItems.append(liveItem)
@@ -144,20 +179,15 @@ extension StatusItemController {
144179
}
145180
}
146181

147-
private func updateMenuItemInPlace(_ liveItem: NSMenuItem, from newItem: NSMenuItem) {
182+
/// === PHASE C: LAYOUT ENGINE ===
183+
/// Applies the content payload of a freshly-built NSMenuItem onto an existing live
184+
/// NSMenuItem that already occupies its slot in the tracked menu. Pure property
185+
/// synchronization — no view transfer, no layout, no SwiftUI interaction.
186+
/// Called by A-phase render-layer callers before the view handoff.
187+
private func applyMenuItemContent(_ liveItem: NSMenuItem, from newItem: NSMenuItem) {
148188
if liveItem.isSeparatorItem { return }
149-
let remainsHighlighted = liveItem.menu.map {
150-
self.highlightedMenuItems[ObjectIdentifier($0)] === liveItem
151-
} ?? false
152-
// Detach from the scratch item first so a view or submenu is never referenced by
153-
// two menu items at once.
154-
let view = newItem.view
155-
newItem.view = nil
156-
let submenu = newItem.submenu
189+
liveItem.submenu = newItem.submenu
157190
newItem.submenu = nil
158-
liveItem.view = view
159-
(view as? MenuCardHighlighting)?.setHighlighted(remainsHighlighted)
160-
liveItem.submenu = submenu
161191
liveItem.title = newItem.title
162192
liveItem.attributedTitle = newItem.attributedTitle
163193
liveItem.action = newItem.action
@@ -183,8 +213,35 @@ extension StatusItemController {
183213
}
184214
}
185215

216+
/// === PHASE A: RENDER LAYER ===
217+
/// Pure view transfer. The precomputed NSHostingView from C-phase is moved from
218+
/// the scratch item onto the existing live item in the tracked menu. NO measurement,
219+
/// NO SwiftUI interaction, NO layout invalidation, NO fittingSize/intrinsicContentSize
220+
/// access. The view's frame is already authoritative from C-phase; the live item
221+
/// keeps its current submenu/property state (which C-phase updated via
222+
/// `applyMenuItemContent` before this handoff).
223+
private func updateMenuItemInPlace(_ liveItem: NSMenuItem, from newItem: NSMenuItem) {
224+
if liveItem.isSeparatorItem { return }
225+
let remainsHighlighted = liveItem.menu.map {
226+
self.highlightedMenuItems[ObjectIdentifier($0)] === liveItem
227+
} ?? false
228+
// Single-assignment view handoff. Nothing else belongs in this function.
229+
let precomputedView = newItem.view
230+
newItem.view = nil
231+
liveItem.view = precomputedView
232+
(precomputedView as? MenuCardHighlighting)?.setHighlighted(remainsHighlighted)
233+
}
234+
186235
private func swapMenuItemContents(_ liveItem: NSMenuItem, _ cachedItem: NSMenuItem) {
187236
let holder = NSMenuItem()
237+
// Phase C: three-way content rotation
238+
// holder ← liveItem (save live's state)
239+
// liveItem ← cachedItem (live now mirrors cached)
240+
// cachedItem ← holder (cached now mirrors live's old state)
241+
self.applyMenuItemContent(holder, from: liveItem)
242+
self.applyMenuItemContent(liveItem, from: cachedItem)
243+
self.applyMenuItemContent(cachedItem, from: holder)
244+
// Phase A: three-way view rotation (same pattern)
188245
self.updateMenuItemInPlace(holder, from: liveItem)
189246
self.updateMenuItemInPlace(liveItem, from: cachedItem)
190247
self.updateMenuItemInPlace(cachedItem, from: holder)

Tests/CodexBarTests/StatusMenuHeightCacheTests.swift

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -97,9 +97,9 @@ extension StatusMenuTests {
9797
var measureCount = 0
9898
let first = controller.cachedMenuCardHeight(
9999
for: "menuCard",
100-
scope: UsageProvider.codex.rawValue,
100+
providerState: UsageProvider.codex.rawValue,
101101
width: 320,
102-
fingerprint: "content:stable")
102+
contentFingerprint: "content:stable")
103103
{
104104
measureCount += 1
105105
return 42
@@ -109,9 +109,9 @@ extension StatusMenuTests {
109109

110110
let second = controller.cachedMenuCardHeight(
111111
for: "menuCard",
112-
scope: UsageProvider.codex.rawValue,
112+
providerState: UsageProvider.codex.rawValue,
113113
width: 320,
114-
fingerprint: "content:stable")
114+
contentFingerprint: "content:stable")
115115
{
116116
measureCount += 1
117117
return 99
@@ -130,18 +130,18 @@ extension StatusMenuTests {
130130
var measureCount = 0
131131
let first = controller.cachedMenuCardHeight(
132132
for: "menuCard",
133-
scope: UsageProvider.codex.rawValue,
133+
providerState: UsageProvider.codex.rawValue,
134134
width: 320,
135-
fingerprint: "content:a")
135+
contentFingerprint: "content:a")
136136
{
137137
measureCount += 1
138138
return 42
139139
}
140140
let second = controller.cachedMenuCardHeight(
141141
for: "menuCard",
142-
scope: UsageProvider.codex.rawValue,
142+
providerState: UsageProvider.codex.rawValue,
143143
width: 320,
144-
fingerprint: "content:b")
144+
contentFingerprint: "content:b")
145145
{
146146
measureCount += 1
147147
return 99
@@ -160,7 +160,7 @@ extension StatusMenuTests {
160160
var measureCount = 0
161161
let first = controller.cachedMenuCardHeight(
162162
for: "menuCard",
163-
scope: UsageProvider.codex.rawValue,
163+
providerState: UsageProvider.codex.rawValue,
164164
width: 320)
165165
{
166166
measureCount += 1
@@ -171,7 +171,7 @@ extension StatusMenuTests {
171171

172172
let second = controller.cachedMenuCardHeight(
173173
for: "menuCard",
174-
scope: UsageProvider.codex.rawValue,
174+
providerState: UsageProvider.codex.rawValue,
175175
width: 320)
176176
{
177177
measureCount += 1
@@ -190,24 +190,24 @@ extension StatusMenuTests {
190190

191191
_ = controller.cachedMenuCardHeight(
192192
for: "versioned",
193-
scope: UsageProvider.codex.rawValue,
193+
providerState: UsageProvider.codex.rawValue,
194194
width: 320)
195195
{
196196
42
197197
}
198198
_ = controller.cachedMenuCardHeight(
199199
for: "fingerprinted",
200-
scope: UsageProvider.codex.rawValue,
200+
providerState: UsageProvider.codex.rawValue,
201201
width: 320,
202-
fingerprint: "content:stable")
202+
contentFingerprint: "content:stable")
203203
{
204204
99
205205
}
206206

207207
controller.invalidateMenus()
208208

209-
#expect(controller.menuCardHeightCache.keys.allSatisfy { !$0.fingerprint.hasPrefix("version:") })
210-
#expect(controller.menuCardHeightCache.keys.contains { $0.fingerprint == "content:stable" })
209+
#expect(controller.menuCardHeightCache.keys.allSatisfy { !$0.contentFingerprint.hasPrefix("version:") })
210+
#expect(controller.menuCardHeightCache.keys.contains { $0.contentFingerprint == "content:stable" })
211211
}
212212

213213
@Test
@@ -261,7 +261,7 @@ extension StatusMenuTests {
261261
controller.populateMenu(menu, provider: .codex)
262262
controller.populateMenu(menu, provider: .claude)
263263

264-
let scopes = Set(controller.menuCardHeightCache.keys.map(\.scope))
264+
let scopes = Set(controller.menuCardHeightCache.keys.map(\.providerState))
265265
#expect(scopes.contains(UsageProvider.codex.rawValue))
266266
#expect(scopes.contains(UsageProvider.claude.rawValue))
267267
}

0 commit comments

Comments
 (0)