From 586cfb6d6b18acb14c32f3799cdacd3840b15aa2 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Mon, 3 Aug 2026 13:12:46 +0300 Subject: [PATCH 1/7] feat(tray): render the 24h activity histogram inline at the top of the menu MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The chart used to hide behind an "Activity (24h)" submenu — the overview was the only glance row that required a second navigation step. It now renders inline, directly under the summary line, so the day's shape is on screen the moment the tray icon is clicked. - GlanceSection builds the row eagerly but caches the chart item by its shaped bars (HistogramBar is Equatable), so the debounced closed-menu rebuilds do not re-render a SwiftUI chart nobody is looking at. - The row's KIND (loading placeholder / failure row / 132pt chart) is structural in updateInPlace — a kind flip resizes the menu, so it defers to menuDidClose like every other structural change. Within a kind, bars swap the hosted view in place and a failure refreshes its tooltip. - The submenu's "Not updating" marker is not ported: the inline row sits directly under the summary line, whose "not updating" segment already says it. - HistogramSubmenuDelegate is deleted; its loading/failed/chart decision (ActivityHistogram.state) and the chart factory seam are unchanged. --- .../macos/MCPProxy/MCPProxy/MCPProxyApp.swift | 5 - .../Menu/Glance/ActivityHistogramView.swift | 126 +------- .../MCPProxy/Menu/Glance/GlanceSection.swift | 161 ++++++---- .../ActivityHistogramTests.swift | 286 ++++++++---------- .../MCPProxyTests/GlanceSectionTests.swift | 43 +-- 5 files changed, 268 insertions(+), 353 deletions(-) diff --git a/native/macos/MCPProxy/MCPProxy/MCPProxyApp.swift b/native/macos/MCPProxy/MCPProxy/MCPProxyApp.swift index 9bbaeca2..f242ad0e 100644 --- a/native/macos/MCPProxy/MCPProxy/MCPProxyApp.swift +++ b/native/macos/MCPProxy/MCPProxy/MCPProxyApp.swift @@ -331,11 +331,6 @@ final class AppController: NSObject, NSApplicationDelegate, NSWindowDelegate, NS // a full rebuild (removeAllItems) on a menu already on screen and // re-arm/disarm the guard under the parent — exactly the // restructuring-while-open the design forbids. - // - // The glance histogram submenu does build its single row lazily on open, - // but it does NOT reach this method: GlanceSection installs its own - // HistogramSubmenuDelegate on that submenu, precisely so opening it - // cannot rebuild the tray menu underneath the cursor. guard menu === menuHost?.menu else { return } // Spec 048: dropped the per-click `client.servers()` fetch. appState diff --git a/native/macos/MCPProxy/MCPProxy/Menu/Glance/ActivityHistogramView.swift b/native/macos/MCPProxy/MCPProxy/Menu/Glance/ActivityHistogramView.swift index 00b7028f..2ea50d59 100644 --- a/native/macos/MCPProxy/MCPProxy/Menu/Glance/ActivityHistogramView.swift +++ b/native/macos/MCPProxy/MCPProxy/Menu/Glance/ActivityHistogramView.swift @@ -1,12 +1,12 @@ // ActivityHistogramView.swift // MCPProxy // -// The 24-hour calls-per-hour bar chart shown in the tray glance's -// "Activity (24h)" submenu, plus the pure bucket-shaping and accessibility -// helpers it renders from. +// The 24-hour calls-per-hour bar chart rendered inline at the top of the tray +// glance, plus the pure bucket-shaping and accessibility helpers it renders +// from. // -// The chart renders from `AppState.usageTimeline` only — opening the submenu -// performs no network request (spec 048 invariant). +// The chart renders from `AppState.usageTimeline` only — building it performs +// no network request (spec 048 invariant). import SwiftUI import Charts @@ -33,9 +33,9 @@ struct HistogramBar: Identifiable, Equatable { var total: Int { succeeded + errors } } -// MARK: - What the submenu shows +// MARK: - What the histogram row shows -/// What the histogram submenu renders right now. +/// What the histogram row renders right now. /// /// `loading` and `failed` are deliberately distinct: both leave the timeline /// nil, and telling the user "Loading…" forever after a failed fetch is the @@ -123,7 +123,7 @@ enum ActivityHistogram { + "Busiest hour \(formatter.string(from: peak.hourStart)) with \(peak.total) calls." } - /// Decide what the submenu shows. A timeline that has loaded wins over a + /// Decide what the histogram row shows. A timeline that has loaded wins over a /// recorded failure: showing real (if slightly stale) data beats showing an /// error row. static func state(timeline: [UsageBucket]?, errorMessage: String?, now: Date) -> HistogramState { @@ -215,7 +215,7 @@ extension ActivityHistogram { /// `testRealChartItemIsSizedAndLabelled` keeps the two in step. static let chartItemSize = NSSize(width: 288, height: 132) - /// The submenu's single custom item: an `NSHostingView` wrapping the chart. + /// The glance's single custom item: an `NSHostingView` wrapping the chart. /// /// Custom menu-item views receive mouse events but not keyboard events, so /// the item is disabled (nothing to activate) and carries the whole series @@ -234,111 +234,3 @@ extension ActivityHistogram { return item } } - -// MARK: - Submenu delegate - -/// Builds the single row of `GlanceSection`'s "Activity (24h)" submenu when -/// that submenu opens. -/// -/// This is NOT a second submenu — `GlanceSection` owns the item and the menu, -/// and this object only fills it in on demand. It is a separate `NSObject` -/// purely because `NSMenuDelegate` requires `NSObjectProtocol`, which -/// `GlanceSection` (a plain `@MainActor final class`) does not conform to. -/// -/// Building on open — rather than inside `items(for:)` — keeps the chart off -/// the menu's hot path: `rebuildMenu()` runs on every debounced -/// `objectWillChange`, menu open or closed, so building eagerly would construct -/// an `NSHostingView` and render a SwiftUI Chart on every state change, -/// including for a menu nobody has opened. Reading `AppState` at open time also -/// means a timeline that arrives while the menu sits closed is shown on the -/// next open, with no rebuild of the parent menu. -/// -/// It reads `AppState` and nothing else: opening the submenu performs no -/// network request (spec 048 invariant). -/// -/// `NSMenu.delegate` is a WEAK reference, so `GlanceSection` must retain this. -final class HistogramSubmenuDelegate: NSObject, NSMenuDelegate { - - private let appState: AppState - private let chartItemFactory: ([HistogramBar]) -> NSMenuItem - - /// - Parameter chartItemFactory: injected so submenu-structure tests are - /// independent of how the chart itself renders. It defaults to the real - /// chart: the seam this replaced was optional, nothing in production ever - /// set it, and the tray consequently shipped a text row instead of a - /// chart. A default that already works cannot fail that way. - init(appState: AppState, - chartItemFactory: @escaping ([HistogramBar]) -> NSMenuItem = ActivityHistogram.chartMenuItem) { - self.appState = appState - self.chartItemFactory = chartItemFactory - super.init() - } - - // MARK: NSMenuDelegate - - func menuNeedsUpdate(_ menu: NSMenu) { - menu.removeAllItems() - for item in currentItems() { menu.addItem(item) } - } - - // MARK: Rows - - /// The rows the submenu shows for the current `AppState`: the data row, - /// preceded by a stale marker when the feeds have stopped arriving. - /// - /// `ActivityHistogram.state()` charts a loaded timeline in preference to a - /// recorded failure, which is right for a blip and wrong for a core that is - /// never coming back — the failure is then recorded every 30 seconds and - /// rendered never. The marker is what makes it visible without taking the - /// real (if stale) data off the screen. - /// - /// It is suppressed when the data row is itself the failure row, which - /// already says the same thing. - func currentItems() -> [NSMenuItem] { - let item = currentItem() - guard appState.glanceStale, item.title != Self.unavailableTitle else { return [item] } - - let marker = Self.mutedItem("Not updating") - marker.toolTip = appState.glanceError - return [marker, item] - } - - /// Title of the row shown when the usage fetch failed with nothing loaded. - private static let unavailableTitle = "Usage unavailable" - - /// The single row the submenu shows for the current `AppState`. - /// - /// The clock is read here, at open time, rather than injected: every - /// assertion about this row is structural (which row, how many), and the - /// axis contents `now` decides are covered exhaustively by the pure - /// `ActivityHistogram.bars` tests. - func currentItem() -> NSMenuItem { - switch ActivityHistogram.state( - timeline: appState.usageTimeline, - errorMessage: appState.usageError, - now: Date() - ) { - case .loading: - return Self.mutedItem("Loading…") - case .failed(let message): - let item = Self.mutedItem(Self.unavailableTitle) - item.toolTip = message - return item - case .loaded(let bars): - return chartItemFactory(bars) - } - } - - /// A disabled, secondary-coloured text row. Setting `attributedTitle` - /// leaves `title` intact, so the plain string stays available to tests and - /// to accessibility. - static func mutedItem(_ title: String) -> NSMenuItem { - let item = NSMenuItem(title: title, action: nil, keyEquivalent: "") - item.isEnabled = false - item.attributedTitle = NSAttributedString(string: title, attributes: [ - .font: NSFont.menuFont(ofSize: 0), - .foregroundColor: NSColor.secondaryLabelColor - ]) - return item - } -} diff --git a/native/macos/MCPProxy/MCPProxy/Menu/Glance/GlanceSection.swift b/native/macos/MCPProxy/MCPProxy/Menu/Glance/GlanceSection.swift index 1342a0a9..d94981d3 100644 --- a/native/macos/MCPProxy/MCPProxy/Menu/Glance/GlanceSection.swift +++ b/native/macos/MCPProxy/MCPProxy/Menu/Glance/GlanceSection.swift @@ -2,14 +2,16 @@ // MCPProxy // // Builds the "glance" block at the top of the tray menu: a one-line summary, -// the most recent qualifying tool calls, the active MCP clients, and the -// 24h histogram submenu. +// the inline 24h histogram, the most recent qualifying tool calls, and the +// active MCP clients. // // Every text row is a plain NSMenuItem. Custom (view-backed) menu items receive // mouse events but NOT keyboard events, so building the rows as hosted SwiftUI // would silently cost keyboard navigation and VoiceOver. Only the histogram — -// which genuinely needs drawing — is view-backed, and it lives alone inside its -// own submenu. +// which genuinely needs drawing — is view-backed; it renders inline, directly +// under the summary line, so the day's shape is visible the moment the menu +// opens (it used to hide in an "Activity (24h)" submenu, which made the +// overview the only row that required a second navigation step). // // This component never builds a Web UI URL. It is handed only AppState, whose // webUIBaseURL is scheme/host/port by design, while the API key lives on the @@ -122,16 +124,18 @@ final class GlanceSection { private var hasBuilt = false private var builtVisible = false - /// Held only so ownership of the submenu is explicit; `updateInPlace` - /// deliberately never touches it (re-creating it would disturb an open - /// submenu), so nothing reads this back. - private var histogramItem: NSMenuItem? + /// What kind of row the histogram block was last built with. A text row and + /// a 132 pt chart have different heights, so a kind change is structural; + /// content changes within a kind are in-place rewrites. + private enum HistogramRowKind: Equatable { case loading, failed, chart } + private var builtHistogramKind: HistogramRowKind? - /// The submenu's delegate, which fills the submenu in when it opens. - /// `NSMenu.delegate` is a WEAK reference, so without this the delegate - /// would deallocate the moment `items(for:)` returned and the submenu would - /// silently open empty forever. - private var histogramDelegate: HistogramSubmenuDelegate? + /// The histogram row currently installed, and — when it is the chart — the + /// shaped bars it renders. The pair is the cache that keeps `items(for:)` + /// from re-rendering a SwiftUI chart on every debounced rebuild of a menu + /// nobody has opened: same bars, same item, no work. + private var histogramRow: NSMenuItem? + private var builtHistogramBars: [HistogramBar]? init(target: AnyObject?, action: Selector) { self.clickTarget = target @@ -155,7 +159,6 @@ final class GlanceSection { activityRows = [] clientRows = [] clientOverflowItem = nil - histogramItem = nil hasBuilt = true builtVisible = isVisible(for: state) guard builtVisible else { return [] } @@ -168,13 +171,10 @@ final class GlanceSection { // The day before the minute (FR-021). The histogram answers "what has // been happening?" in one glyph, so it belongs beside the summary line - // it illustrates — it used to sit at the very bottom, below every row it - // summarises, which made the user read the detail to reach the overview. - // It stays above the separator for that reason: summary and shape are - // one block, the rows below are another. - let histogram = makeHistogramItem(for: state) - histogramItem = histogram - items.append(histogram) + // it illustrates, above the separator: summary and shape are one block, + // the rows below are another. It renders inline — no submenu — so the + // shape is on screen the moment the menu opens. + items.append(histogramRowItem(for: state, now: now)) items.append(.separator()) items.append(disabledItem(titled: "Recent")) @@ -234,17 +234,22 @@ final class GlanceSection { /// text would leave a row whose click still opened the previous record's /// session. See `apply(_:to:now:)` for how "different record" is decided. /// - /// The histogram submenu is deliberately not touched, and does not need to - /// be: its single row is built by `HistogramSubmenuDelegate` when it opens, - /// reading `AppState` at that moment. Whether the timeline has loaded - /// therefore changes nothing about the structure built here, which is why - /// this no longer reports structural when it flips. + /// The histogram renders inline, so its KIND is structural again: a text + /// placeholder and a 132 pt chart have different heights, and swapping one + /// for the other resizes an open menu exactly as an extra row does. Within + /// a kind it is an ordinary in-place rewrite — a same-size view swap when + /// the bars moved, a tooltip refresh on the failure row. @discardableResult func updateInPlace(for state: AppState, now: Date = Date()) -> Bool { guard hasBuilt else { return false } guard isVisible(for: state) == builtVisible else { return false } guard builtVisible else { return true } + let histogramState = ActivityHistogram.state(timeline: state.usageTimeline, + errorMessage: state.usageError, + now: now) + guard Self.kind(of: histogramState) == builtHistogramKind else { return false } + let runs = GlanceSelection.activityRows(from: state.glanceActivity) let presence = Self.clientList(for: state, now: now) let clients = presence.rows @@ -283,6 +288,7 @@ final class GlanceSection { let title = Self.overflowTitle(presence.hidden) if clientOverflowItem.title != title { clientOverflowItem.title = title } } + refreshHistogramRow(with: histogramState) return true } @@ -644,33 +650,84 @@ final class GlanceSection { // MARK: Histogram - /// The "Activity (24h)" item and its (initially empty) submenu. - /// - /// The submenu's single row is built by its delegate when it opens, not - /// here: `items(for:)` runs on every `rebuildMenu()` — which itself runs on - /// every debounced `objectWillChange`, menu open or closed — and building - /// eagerly would render a SwiftUI Chart on every state change, including - /// for a menu nobody has opened. + /// The inline histogram row: the chart when the timeline has loaded, a + /// muted placeholder while it is loading or after the fetch failed. /// - /// The submenu carries its OWN delegate rather than the tray menu's. That - /// is what keeps opening it off `AppController.menuWillOpen`, which - /// rebuilds the whole menu; a submenu opening under the cursor must not - /// restructure the menu it hangs from. - private func makeHistogramItem(for state: AppState) -> NSMenuItem { - let item = NSMenuItem(title: "Activity (24h)", action: nil, keyEquivalent: "") - let submenu = NSMenu(title: "Activity (24h)") - // Nothing in here is actionable, and AppKit's automatic enabling runs - // its own validation at display time. Turning it off makes the row's - // disabled state ours — and makes what the tests assert the same thing - // the user sees. - submenu.autoenablesItems = false - - let delegate = HistogramSubmenuDelegate(appState: state, - chartItemFactory: histogramChartItemFactory) - histogramDelegate = delegate - submenu.delegate = delegate - - item.submenu = submenu + /// The chart reads `AppState` and nothing else — building this row + /// performs no network request (spec 048 invariant). It is view-backed and + /// eagerly built — but `items(for:)` runs on every `rebuildMenu()`, which + /// itself runs on every debounced `objectWillChange`, menu open or closed. The + /// bars cache is what keeps that affordable: the chart item is rebuilt only + /// when the shaped 24-hour axis actually changed (`HistogramBar` is + /// Equatable precisely so that is cheap to decide), and every other rebuild + /// hands back the item it already has. `rebuildMenu` reuses one `NSMenu` + /// via `removeAllItems()`, so re-adding the cached item is safe. + private func histogramRowItem(for state: AppState, now: Date) -> NSMenuItem { + let histogramState = ActivityHistogram.state(timeline: state.usageTimeline, + errorMessage: state.usageError, + now: now) + switch histogramState { + case .loading: + builtHistogramKind = .loading + builtHistogramBars = nil + let item = Self.mutedItem("Activity (24h) — loading…") + histogramRow = item + return item + case .failed(let message): + builtHistogramKind = .failed + builtHistogramBars = nil + let item = Self.mutedItem("Activity (24h) unavailable") + item.toolTip = message + histogramRow = item + return item + case .loaded(let bars): + builtHistogramKind = .chart + if let cached = histogramRow, builtHistogramBars == bars { + return cached + } + let item = histogramChartItemFactory(bars) + histogramRow = item + builtHistogramBars = bars + return item + } + } + + private static func kind(of state: HistogramState) -> HistogramRowKind { + switch state { + case .loading: return .loading + case .failed: return .failed + case .loaded: return .chart + } + } + + /// Refresh the histogram row's content within its built kind: a same-size + /// view swap when the bars moved, a tooltip refresh on the failure row. + /// Never a resize — `updateInPlace` already reported a kind change as + /// structural, and the chart's frame is a fixed `chartItemSize`. + private func refreshHistogramRow(with histogramState: HistogramState) { + guard let item = histogramRow else { return } + switch histogramState { + case .loading: + break + case .failed(let message): + if item.toolTip != message { item.toolTip = message } + case .loaded(let bars): + guard builtHistogramBars != bars else { return } + item.view = histogramChartItemFactory(bars).view + builtHistogramBars = bars + } + } + + /// A disabled, secondary-coloured text row. Setting `attributedTitle` + /// leaves `title` intact, so the plain string stays available to tests and + /// to accessibility. + static func mutedItem(_ title: String) -> NSMenuItem { + let item = NSMenuItem(title: title, action: nil, keyEquivalent: "") + item.isEnabled = false + item.attributedTitle = NSAttributedString(string: title, attributes: [ + .font: NSFont.menuFont(ofSize: 0), + .foregroundColor: NSColor.secondaryLabelColor + ]) return item } diff --git a/native/macos/MCPProxy/MCPProxyTests/ActivityHistogramTests.swift b/native/macos/MCPProxy/MCPProxyTests/ActivityHistogramTests.swift index 21a41c7f..d5a2c50f 100644 --- a/native/macos/MCPProxy/MCPProxyTests/ActivityHistogramTests.swift +++ b/native/macos/MCPProxy/MCPProxyTests/ActivityHistogramTests.swift @@ -213,12 +213,18 @@ final class AppStateUsageErrorTests: XCTestCase { } -/// The histogram submenu belongs to `GlanceSection` — there is exactly one, and -/// it builds its single row when it opens rather than on every `rebuildMenu()`. -/// These tests drive it through the delegate the section installs, which is the -/// same path AppKit uses. +/// The inline histogram row belongs to `GlanceSection`: it renders directly +/// under the summary line — no submenu — so the day's shape is on screen the +/// moment the menu opens. These tests assert the row's three kinds (loading, +/// failed, chart), the cache that keeps eager builds affordable, and the +/// structural rules `updateInPlace` enforces for it. +/// +/// The old "Not updating" marker tests were dropped, not ported: the submenu +/// needed its own stale marker because the summary line was not visible from +/// inside it, while the inline row sits directly under the summary — whose +/// "not updating" segment (`GlanceSectionTests` header tests) already says it. @MainActor -final class GlanceHistogramSubmenuTests: XCTestCase { +final class GlanceInlineHistogramTests: XCTestCase { private final class ClickStub: NSObject { @objc func openGlanceRow(_ sender: NSMenuItem) {} @@ -226,16 +232,6 @@ final class GlanceHistogramSubmenuTests: XCTestCase { private static let clickStub = ClickStub() - /// Every section built during a test, kept alive for its whole duration. - /// The section is the only strong reference to the submenu delegate — the - /// menu's own is weak — so letting one die mid-test empties the submenu. - private var sections: [GlanceSection] = [] - - override func tearDown() { - sections = [] - super.tearDown() - } - /// A connected core — the block is hidden otherwise. private func connectedState() -> AppState { let state = AppState() @@ -243,7 +239,7 @@ final class GlanceHistogramSubmenuTests: XCTestCase { return state } - /// A section whose chart row is stubbed, so these tests assert on submenu + /// A section whose chart row is stubbed, so these tests assert on block /// structure alone, independent of how the chart itself renders. private func makeSection() -> GlanceSection { let section = makeBareSection() @@ -255,195 +251,183 @@ final class GlanceHistogramSubmenuTests: XCTestCase { /// A section with nothing injected, so the production defaults apply. private func makeBareSection() -> GlanceSection { - let section = GlanceSection(target: Self.clickStub, - action: #selector(ClickStub.openGlanceRow(_:))) - sections.append(section) - return section + GlanceSection(target: Self.clickStub, + action: #selector(ClickStub.openGlanceRow(_:))) } - /// The "Activity (24h)" item, wherever it sits in the block. - private func histogramItem(_ section: GlanceSection, _ state: AppState) -> NSMenuItem { + /// The histogram row: always the summary line's neighbour. + private func histogramRow(_ section: GlanceSection, _ state: AppState) -> NSMenuItem { let items = section.items(for: state, now: Fixture.now) - guard let item = items.first(where: { $0.title == "Activity (24h)" }) else { - XCTFail("no Activity (24h) item in the block") + guard items.count > 1 else { + XCTFail("the block is hidden, so there is no histogram row") return NSMenuItem() } - return item - } - - /// Fire the delegate the way AppKit does, through the menu's own reference, - /// so a delegate that was never installed fails the test. - private func open(_ menu: NSMenu) { - guard let delegate = menu.delegate else { - return XCTFail("the submenu has no delegate, so opening it would build nothing") - } - delegate.menuNeedsUpdate?(menu) - } - - /// Nothing is built until the submenu opens. `rebuildMenu()` runs on every - /// debounced state change, menu open or closed, so building the chart there - /// would render a SwiftUI Chart nobody is looking at. - func testSubmenuIsEmptyUntilItOpens() { - let item = histogramItem(makeSection(), connectedState()) - - XCTAssertEqual(item.submenu?.numberOfItems, 0) - } - - /// `NSMenu.delegate` is a WEAK reference: if the section does not retain the - /// delegate it deallocates the moment `items(for:)` returns, and the submenu - /// silently opens empty forever. Nothing but a test catches that. - func testTheDelegateOutlivesTheBuildCall() { - let section = makeSection() - let item = histogramItem(section, connectedState()) - - XCTAssertNotNil(item.submenu?.delegate, - "the section must retain the submenu delegate") + return items[1] } - /// The submenu delegate must be its own object, not the section's owner: - /// `AppController.menuWillOpen` rebuilds the whole tray menu, and having it - /// fire for a submenu opening under the cursor is exactly the - /// restructuring-while-open the design forbids. - func testTheSubmenuHasItsOwnDelegateNotTheTrayMenusOwner() { - let section = makeSection() - let item = histogramItem(section, connectedState()) - - let delegate = item.submenu?.delegate - XCTAssertNotNil(delegate) - XCTAssertFalse(delegate === Self.clickStub) - XCTAssertFalse(delegate === section as AnyObject) - } + // MARK: - The three kinds func testLoadingRowWhileTheTimelineIsNil() { - let menu = histogramItem(makeSection(), connectedState()).submenu! - - open(menu) + let row = histogramRow(makeSection(), connectedState()) - XCTAssertEqual(menu.numberOfItems, 1) - XCTAssertEqual(menu.items[0].title, "Loading…") - XCTAssertFalse(menu.items[0].isEnabled) - let attributes = menu.items[0].attributedTitle!.attributes(at: 0, effectiveRange: nil) + XCTAssertEqual(row.title, "Activity (24h) — loading…") + XCTAssertFalse(row.isEnabled) + XCTAssertNil(row.submenu, "the histogram renders inline, never behind a submenu") + let attributes = row.attributedTitle!.attributes(at: 0, effectiveRange: nil) XCTAssertEqual(attributes[.foregroundColor] as? NSColor, NSColor.secondaryLabelColor) } func testErrorRowWhenTheFetchFailedBeforeAnyTimelineArrived() { let state = connectedState() state.recordUsageFailure("connection refused") - let menu = histogramItem(makeSection(), state).submenu! - open(menu) + let row = histogramRow(makeSection(), state) - XCTAssertEqual(menu.numberOfItems, 1) - XCTAssertEqual(menu.items[0].title, "Usage unavailable") - XCTAssertEqual(menu.items[0].toolTip, "connection refused") - XCTAssertFalse(menu.items[0].isEnabled) - let attributes = menu.items[0].attributedTitle!.attributes(at: 0, effectiveRange: nil) + XCTAssertEqual(row.title, "Activity (24h) unavailable") + XCTAssertEqual(row.toolTip, "connection refused") + XCTAssertFalse(row.isEnabled) + let attributes = row.attributedTitle!.attributes(at: 0, effectiveRange: nil) XCTAssertEqual(attributes[.foregroundColor] as? NSColor, NSColor.secondaryLabelColor) } - /// The case that made the block confidently wrong: a timeline is loaded, so - /// `ActivityHistogram.state()` charts it and the recorded failure is never - /// rendered — every 30 seconds, forever. Real data still wins the chart row, - /// but a failure that keeps happening now gets a row of its own above it. - func testAPersistentFailureIsShownEvenWithALoadedTimeline() { + /// The headline behaviour: with a timeline loaded, the chart itself is the + /// second row of the menu — visible on open, no navigation step. + func testChartRendersInlineWithNoSubmenu() { let state = connectedState() state.usageTimeline = [Fixture.bucket(start: Fixture.currentHour, calls: 3, errors: 1)] - for _ in 0.. chart resizes the menu, so it defers to close") } - /// Opening the submenu must not restructure the menu it hangs from. The - /// whole point of the lazy build is that it touches the submenu and nothing - /// else — a parent that grew, shrank or re-created its rows while the user - /// had it open is the irritation `MenuRebuildGuard` exists to prevent. - func testOpeningTheSubmenuDoesNotRestructureTheParentMenu() { + func testLosingTheTimelineIsStructuralToo() { let section = makeSection() let state = connectedState() + state.usageTimeline = [] + _ = section.items(for: state, now: Fixture.now) + + state.usageTimeline = nil + + XCTAssertFalse(section.updateInPlace(for: state, now: Fixture.now)) + } + + /// Within the chart kind, new bars are an ordinary in-place rewrite: the + /// item keeps its place and its frame, only its hosted view is swapped. + func testBarsChangingUnderAnOpenMenuSwapTheViewInPlace() { + let section = makeBareSection() + section.histogramChartItemFactory = { bars in + let item = NSMenuItem(title: "CHART:\(bars.count)", action: nil, keyEquivalent: "") + let view = NSView(frame: NSRect(x: 0, y: 0, width: 288, height: 132)) + view.setAccessibilityLabel("total \(bars.reduce(0) { $0 + $1.total })") + item.view = view + return item + } + let state = connectedState() state.usageTimeline = [Fixture.bucket(start: Fixture.currentHour, calls: 3, errors: 1)] + let row = histogramRow(section, state) + XCTAssertEqual(row.view?.accessibilityLabel(), "total 3") - let parent = NSMenu() - for item in section.items(for: state, now: Fixture.now) { parent.addItem(item) } - let countBefore = parent.numberOfItems - let itemsBefore = parent.items + state.usageTimeline = [Fixture.bucket(start: Fixture.currentHour, calls: 7, errors: 0)] - open(parent.items.first { $0.title == "Activity (24h)" }!.submenu!) + XCTAssertTrue(section.updateInPlace(for: state, now: Fixture.now)) + XCTAssertEqual(row.view?.accessibilityLabel(), "total 7", + "the same row now hosts the fresh chart") + } - XCTAssertEqual(parent.numberOfItems, countBefore) - XCTAssertTrue(zip(parent.items, itemsBefore).allSatisfy { $0 === $1 }, - "opening the submenu must not replace any row of the parent") - XCTAssertTrue(section.updateInPlace(for: state, now: Fixture.now), - "and must not make the block look structurally different afterwards") + func testFailureTooltipRefreshesInPlace() { + let section = makeSection() + let state = connectedState() + state.recordUsageFailure("connection refused") + let row = histogramRow(section, state) + + state.recordUsageFailure("socket closed") + + XCTAssertTrue(section.updateInPlace(for: state, now: Fixture.now)) + XCTAssertEqual(row.toolTip, "socket closed") } + // MARK: - The real chart + /// The real chart item, not the stub: `chartItemSize` is otherwise an /// unverified constant, and a mismatch with the view's own size shows up as /// a band of dead space under the chart. @@ -460,20 +444,18 @@ final class GlanceHistogramSubmenuTests: XCTestCase { XCTAssertFalse(item.isEnabled) } - /// With no factory injected the submenu must still show a REAL chart. The + /// With no factory injected the block must still show a REAL chart. The /// seam this replaced was optional and nothing in production ever set it, /// so the shipped tray showed a text fallback and never a chart; a default /// that already works cannot fail that way. func testTheDefaultFactoryProducesTheRealChart() { let state = connectedState() state.usageTimeline = [Fixture.bucket(start: Fixture.currentHour, calls: 3, errors: 1)] - let menu = histogramItem(makeBareSection(), state).submenu! - open(menu) + let row = histogramRow(makeBareSection(), state) - XCTAssertEqual(menu.numberOfItems, 1) - XCTAssertEqual(menu.items[0].view?.frame.size, ActivityHistogram.chartItemSize) - XCTAssertNotNil(menu.items[0].view?.accessibilityLabel()) + XCTAssertEqual(row.view?.frame.size, ActivityHistogram.chartItemSize) + XCTAssertNotNil(row.view?.accessibilityLabel()) } } diff --git a/native/macos/MCPProxy/MCPProxyTests/GlanceSectionTests.swift b/native/macos/MCPProxy/MCPProxyTests/GlanceSectionTests.swift index efe2e542..8396e3a4 100644 --- a/native/macos/MCPProxy/MCPProxyTests/GlanceSectionTests.swift +++ b/native/macos/MCPProxy/MCPProxyTests/GlanceSectionTests.swift @@ -80,7 +80,7 @@ final class GlanceSectionTests: XCTestCase { } XCTAssertEqual(Array(titles.prefix(7)), [ "12 calls this hour · 1 active", - "Activity (24h)", + "Activity (24h) — loading…", "—", "Recent", "github:create_issue — 30s", @@ -512,31 +512,22 @@ final class GlanceSectionTests: XCTestCase { XCTAssertEqual(items[9].title, "Codex — 1 call · seen 3h") } - /// The submenu's row is built by its delegate when it opens, so these two - /// tests fire `menuNeedsUpdate` where they previously read the row straight - /// out of `items(for:)`. What they assert is unchanged. - private func open(_ menu: NSMenu?) { - guard let menu, let delegate = menu.delegate else { - return XCTFail("the histogram submenu has no delegate, so opening it builds nothing") - } - delegate.menuNeedsUpdate?(menu) - } - - func testHistogramSubmenuShowsLoadingUntilUsageArrives() { + /// The histogram renders inline: until the usage feed arrives its row is a + /// muted placeholder, read straight out of `items(for:)` — there is no + /// submenu to open any more. + func testHistogramShowsLoadingUntilUsageArrives() { let section = Self.makeSection() let histogram = section.items(for: Self.busyState(), now: Self.now)[1] - XCTAssertEqual(histogram.title, "Activity (24h)") - open(histogram.submenu) - XCTAssertEqual(histogram.submenu?.item(at: 0)?.title, "Loading…") + XCTAssertEqual(histogram.title, "Activity (24h) — loading…") + XCTAssertNil(histogram.submenu) } - func testHistogramSubmenuUsesInjectedViewWhenAvailable() { + func testHistogramUsesInjectedViewWhenAvailable() { let state = Self.busyState() state.usageTimeline = [UsageBucket(start: Self.now, calls: 12, errors: 1, totalRespBytes: 0)] let section = Self.makeSection() - // The seam now takes the shaped 24-hour axis and returns the whole item, - // rather than taking raw buckets and returning a view — so the count - // below is the axis width, not the timeline length. + // The seam takes the shaped 24-hour axis and returns the whole item — + // so the count below is the axis width, not the timeline length. section.histogramChartItemFactory = { bars in let item = NSMenuItem(title: "", action: nil, keyEquivalent: "") let view = NSView(frame: NSRect(x: 0, y: 0, width: 240, height: 90)) @@ -544,11 +535,9 @@ final class GlanceSectionTests: XCTestCase { item.view = view return item } - let submenu = section.items(for: state, now: Self.now)[1].submenu - open(submenu) - let chart = submenu?.item(at: 0) - XCTAssertNotNil(chart?.view) - XCTAssertEqual(chart?.view?.accessibilityLabel(), "24 bars") + let chart = section.items(for: state, now: Self.now)[1] + XCTAssertNotNil(chart.view) + XCTAssertEqual(chart.view?.accessibilityLabel(), "24 bars") } // `testHistogramSubmenuFallsBackToTextWithoutABuilder` was REMOVED here, not @@ -570,7 +559,7 @@ final class GlanceSectionTests: XCTestCase { let titles = items.map { $0.isSeparatorItem ? "—" : $0.title } XCTAssertEqual(titles, [ "12 calls this hour · 1 active", - "Activity (24h)", + "Activity (24h) — loading…", "—", "Recent", "github:create_issue — 30s", @@ -591,8 +580,8 @@ final class GlanceSectionTests: XCTestCase { let items = section.items(for: Self.busyState(), now: Self.now) XCTAssertEqual(items[0].title, "12 calls this hour · 1 active") - XCTAssertEqual(items[1].title, "Activity (24h)") - XCTAssertNotNil(items[1].submenu, "it is still the histogram submenu, only moved") + XCTAssertEqual(items[1].title, "Activity (24h) — loading…") + XCTAssertNil(items[1].submenu, "the histogram renders inline, not behind a submenu") XCTAssertTrue(items[2].isSeparatorItem) XCTAssertEqual(items[3].title, "Recent") } From c3cb0edbf14852adb27696485e835b3ad90a034f Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Mon, 3 Aug 2026 13:28:13 +0300 Subject: [PATCH 2/7] fix(tray): never freeze the glance for a histogram kind flip (review round 1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The adversarial panel + opencode cross-model review caught a real regression: making the histogram kind structural meant a timeline that loads while the menu is open — the common open-just-after-launch case — returned false from updateInPlace on every tick, freezing the summary, row identities and client presence for the whole menu session. Main has a test pinning exactly this (testTheTimelineArrivingWhileTheMenuIsOpen- KeepsRowsUpdating), and it failed on the branch. - The histogram never vetoes the in-place update. Its two text kinds share one-line geometry and rewrite each other in place (a failing fetch replaces "loading…" instead of leaving it up); a text-to-chart flip keeps the built row on screen — the height cannot change under the cursor — and the next rebuild installs the chart, since menuWillOpen runs one before every display. - The chart cache is also keyed by the current time zone: the bars are UTC-keyed and survive a zone change, but the VoiceOver "busiest hour" label does not. - Stale "submenu" wording in the factory seam doc corrected. --- .../MCPProxy/Menu/Glance/GlanceSection.swift | 89 +++++++++++++------ .../ActivityHistogramTests.swift | 47 ++++++++-- 2 files changed, 100 insertions(+), 36 deletions(-) diff --git a/native/macos/MCPProxy/MCPProxy/Menu/Glance/GlanceSection.swift b/native/macos/MCPProxy/MCPProxy/Menu/Glance/GlanceSection.swift index d94981d3..993108a7 100644 --- a/native/macos/MCPProxy/MCPProxy/Menu/Glance/GlanceSection.swift +++ b/native/macos/MCPProxy/MCPProxy/Menu/Glance/GlanceSection.swift @@ -51,10 +51,9 @@ final class GlanceSection { private weak var clickTarget: AnyObject? private let clickAction: Selector - /// Builds the histogram submenu's single custom item from a shaped 24-hour - /// axis. Injected so submenu-structure tests are independent of how the - /// chart renders; it defaults to the real chart, so no wiring step can - /// forget to set it. + /// Builds the inline chart item from a shaped 24-hour axis. Injected so + /// block-structure tests are independent of how the chart renders; it + /// defaults to the real chart, so no wiring step can forget to set it. var histogramChartItemFactory: ([HistogramBar]) -> NSMenuItem = ActivityHistogram.chartMenuItem // MARK: Configuration @@ -136,6 +135,10 @@ final class GlanceSection { /// nobody has opened: same bars, same item, no work. private var histogramRow: NSMenuItem? private var builtHistogramBars: [HistogramBar]? + /// Time zone the cached chart's VoiceOver summary was formatted in. The + /// bars are UTC-keyed and survive a zone change, but "Busiest hour 14:00" + /// does not — a zone change invalidates the cache even with equal bars. + private var builtHistogramTimeZoneID: String? init(target: AnyObject?, action: Selector) { self.clickTarget = target @@ -234,22 +237,19 @@ final class GlanceSection { /// text would leave a row whose click still opened the previous record's /// session. See `apply(_:to:now:)` for how "different record" is decided. /// - /// The histogram renders inline, so its KIND is structural again: a text - /// placeholder and a 132 pt chart have different heights, and swapping one - /// for the other resizes an open menu exactly as an extra row does. Within - /// a kind it is an ordinary in-place rewrite — a same-size view swap when - /// the bars moved, a tooltip refresh on the failure row. + /// The histogram never freezes the block: the timeline loading while the + /// menu is open must not cost the user live rows for the whole menu + /// session (see `testTheTimelineArrivingWhileTheMenuIsOpenKeepsRowsUpdating`). + /// Its two TEXT kinds share one-line geometry and rewrite each other in + /// place; a text ↔ chart flip DOES change the row's height, so that row + /// alone stays as built until the next rebuild — `menuWillOpen` runs one + /// before every display — while everything else keeps updating. @discardableResult func updateInPlace(for state: AppState, now: Date = Date()) -> Bool { guard hasBuilt else { return false } guard isVisible(for: state) == builtVisible else { return false } guard builtVisible else { return true } - let histogramState = ActivityHistogram.state(timeline: state.usageTimeline, - errorMessage: state.usageError, - now: now) - guard Self.kind(of: histogramState) == builtHistogramKind else { return false } - let runs = GlanceSelection.activityRows(from: state.glanceActivity) let presence = Self.clientList(for: state, now: now) let clients = presence.rows @@ -288,7 +288,9 @@ final class GlanceSection { let title = Self.overflowTitle(presence.hidden) if clientOverflowItem.title != title { clientOverflowItem.title = title } } - refreshHistogramRow(with: histogramState) + refreshHistogramRow(with: ActivityHistogram.state(timeline: state.usageTimeline, + errorMessage: state.usageError, + now: now)) return true } @@ -682,12 +684,14 @@ final class GlanceSection { return item case .loaded(let bars): builtHistogramKind = .chart - if let cached = histogramRow, builtHistogramBars == bars { + if let cached = histogramRow, builtHistogramBars == bars, + builtHistogramTimeZoneID == TimeZone.current.identifier { return cached } let item = histogramChartItemFactory(bars) histogramRow = item builtHistogramBars = bars + builtHistogramTimeZoneID = TimeZone.current.identifier return item } } @@ -700,21 +704,42 @@ final class GlanceSection { } } - /// Refresh the histogram row's content within its built kind: a same-size - /// view swap when the bars moved, a tooltip refresh on the failure row. - /// Never a resize — `updateInPlace` already reported a kind change as - /// structural, and the chart's frame is a fixed `chartItemSize`. + /// Refresh the histogram row without restructuring the menu — never a + /// resize. Within the chart kind, new bars swap the hosted view (the frame + /// is a fixed `chartItemSize`). The two TEXT kinds share one-line + /// geometry, so loading ↔ failed rewrites the row in place — a fetch that + /// fails while the menu is open must not leave "loading…" on screen + /// telling a quiet lie. A text ↔ chart flip is the one transition that + /// would change the row's height; that row alone stays as built, and the + /// next rebuild installs the right one. private func refreshHistogramRow(with histogramState: HistogramState) { - guard let item = histogramRow else { return } + guard let item = histogramRow, let builtKind = builtHistogramKind else { return } + switch (builtKind, Self.kind(of: histogramState)) { + case (.chart, .chart): + guard case .loaded(let bars) = histogramState, builtHistogramBars != bars else { return } + item.view = histogramChartItemFactory(bars).view + builtHistogramBars = bars + builtHistogramTimeZoneID = TimeZone.current.identifier + case (.loading, .loading), (.failed, .failed), (.loading, .failed), (.failed, .loading): + applyMutedHistogramText(for: histogramState, to: item) + case (.chart, _), (_, .chart): + break + } + } + + /// Rewrite a text-kind histogram row to describe `histogramState`. + private func applyMutedHistogramText(for histogramState: HistogramState, to item: NSMenuItem) { switch histogramState { case .loading: - break + Self.setMutedTitle("Activity (24h) — loading…", on: item) + item.toolTip = nil + builtHistogramKind = .loading case .failed(let message): + Self.setMutedTitle("Activity (24h) unavailable", on: item) if item.toolTip != message { item.toolTip = message } - case .loaded(let bars): - guard builtHistogramBars != bars else { return } - item.view = histogramChartItemFactory(bars).view - builtHistogramBars = bars + builtHistogramKind = .failed + case .loaded: + break } } @@ -722,13 +747,21 @@ final class GlanceSection { /// leaves `title` intact, so the plain string stays available to tests and /// to accessibility. static func mutedItem(_ title: String) -> NSMenuItem { - let item = NSMenuItem(title: title, action: nil, keyEquivalent: "") + // Created with an empty title so `setMutedTitle`'s no-change guard + // cannot skip the attributed styling on first install. + let item = NSMenuItem(title: "", action: nil, keyEquivalent: "") item.isEnabled = false + setMutedTitle(title, on: item) + return item + } + + private static func setMutedTitle(_ title: String, on item: NSMenuItem) { + guard item.title != title else { return } + item.title = title item.attributedTitle = NSAttributedString(string: title, attributes: [ .font: NSFont.menuFont(ofSize: 0), .foregroundColor: NSColor.secondaryLabelColor ]) - return item } // MARK: Header diff --git a/native/macos/MCPProxy/MCPProxyTests/ActivityHistogramTests.swift b/native/macos/MCPProxy/MCPProxyTests/ActivityHistogramTests.swift index d5a2c50f..b78b381a 100644 --- a/native/macos/MCPProxy/MCPProxyTests/ActivityHistogramTests.swift +++ b/native/macos/MCPProxy/MCPProxyTests/ActivityHistogramTests.swift @@ -368,27 +368,58 @@ final class GlanceInlineHistogramTests: XCTestCase { // MARK: - In-place rules /// A text placeholder and a 132 pt chart have different heights, so the - /// row's KIND changing is structural: it must wait for the menu to close. - func testAKindFlipIsStructural() { + /// flip cannot happen under the cursor — but it must not freeze the block + /// either (the timeline loads seconds after launch, exactly when the menu + /// is likely open). The update succeeds, the placeholder stays, and the + /// next rebuild — `menuWillOpen` runs one before every display — installs + /// the chart. + func testATextToChartFlipKeepsThePlaceholderButNotForever() { let section = makeSection() let state = connectedState() - _ = section.items(for: state, now: Fixture.now) + let items = section.items(for: state, now: Fixture.now) + XCTAssertEqual(items[1].title, "Activity (24h) — loading…") state.usageTimeline = [] - XCTAssertFalse(section.updateInPlace(for: state, now: Fixture.now), - "loading -> chart resizes the menu, so it defers to close") + XCTAssertTrue(section.updateInPlace(for: state, now: Fixture.now), + "the rest of the block keeps updating in place") + XCTAssertEqual(items[1].title, "Activity (24h) — loading…", + "the row's height cannot change under the cursor") + XCTAssertEqual(histogramRow(section, state).title, "CHART:24", + "the next rebuild installs the chart") } - func testLosingTheTimelineIsStructuralToo() { + func testLosingTheTimelineKeepsTheChartUntilTheNextRebuild() { let section = makeSection() let state = connectedState() state.usageTimeline = [] - _ = section.items(for: state, now: Fixture.now) + let row = histogramRow(section, state) + XCTAssertEqual(row.title, "CHART:24") state.usageTimeline = nil - XCTAssertFalse(section.updateInPlace(for: state, now: Fixture.now)) + XCTAssertTrue(section.updateInPlace(for: state, now: Fixture.now)) + XCTAssertEqual(row.title, "CHART:24", + "real (if stale) data stays on screen; the next rebuild decides") + } + + /// The two text kinds share one-line geometry, so a fetch that fails while + /// the menu is open replaces "loading…" in place — leaving it up would be + /// the quiet lie `HistogramState` exists to prevent. + func testALoadingRowBecomesTheFailureRowInPlace() { + let section = makeSection() + let state = connectedState() + let row = histogramRow(section, state) + XCTAssertEqual(row.title, "Activity (24h) — loading…") + + state.recordUsageFailure("connection refused") + + XCTAssertTrue(section.updateInPlace(for: state, now: Fixture.now)) + XCTAssertEqual(row.title, "Activity (24h) unavailable") + XCTAssertEqual(row.toolTip, "connection refused") + let attributes = row.attributedTitle!.attributes(at: 0, effectiveRange: nil) + XCTAssertEqual(attributes[.foregroundColor] as? NSColor, NSColor.secondaryLabelColor, + "the in-place rewrite keeps the muted styling") } /// Within the chart kind, new bars are an ordinary in-place rewrite: the From 3aa1bb1f8f369989ee67769435bd12933f914c88 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Mon, 3 Aug 2026 16:56:08 +0300 Subject: [PATCH 3/7] =?UTF-8?q?feat(tray):=20compact=20the=20glance=20?= =?UTF-8?q?=E2=80=94=20the=20chart=20bounds=20the=20menu,=20not=20error=20?= =?UTF-8?q?prose?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The menu was over 600pt wide: failed rows put a 40-char error clause on the title line next to a 34-char label, and Needs-Attention rows carried untruncated core errors. The 24h chart also spent 132pt of height on a mostly-empty plot. Width discipline: the widest thing in the menu is now the chart block — the element designed to be looked at. - Failed rows are one fact per line: title is always `label ×N — age`; the error clause takes the muted second line (the failure mark already flags the row) and the displaced reason stays in the tooltip and the VoiceOver string. Pre-14.4, with no second line, the clause rejoins the title under a tighter 28-char budget. - A row GAINING its error line is structural (defers to close), exactly like a run gaining its reason; the identity-turnover and late-failure liveness tests pin the one-line fallback where the rewrite happens in place. - Budgets: label 34→30, second line 60→44 — both sized to the chart block width at the menu font. - Chart: 248×84 incl. legend (was 260×116), item 272×96 (was 288×132) — a shape to recognise, not a plot to read values off; the Web UI has the full-size version. - Needs-Attention rows truncate to the same 44-char budget, full text in the tooltip. --- .../macos/MCPProxy/MCPProxy/MCPProxyApp.swift | 8 ++- .../Menu/Glance/ActivityHistogramView.swift | 19 +++--- .../Menu/Glance/GlanceFormatting.swift | 20 ++++-- .../MCPProxy/Menu/Glance/GlanceSection.swift | 65 ++++++++++--------- .../MCPProxyTests/GlanceFormattingTests.swift | 7 +- .../MCPProxyTests/GlanceSectionTests.swift | 63 ++++++++++++++---- 6 files changed, 121 insertions(+), 61 deletions(-) diff --git a/native/macos/MCPProxy/MCPProxy/MCPProxyApp.swift b/native/macos/MCPProxy/MCPProxy/MCPProxyApp.swift index f242ad0e..3b720caa 100644 --- a/native/macos/MCPProxy/MCPProxy/MCPProxyApp.swift +++ b/native/macos/MCPProxy/MCPProxy/MCPProxyApp.swift @@ -910,10 +910,16 @@ final class AppController: NSObject, NSApplicationDelegate, NSWindowDelegate, NS let summary = server.health?.summary ?? "" let icon = actionIcon(for: action) - let title = "\(server.name) — \(summary.isEmpty ? actionDisplayName(for: action) : summary)" + let fullTitle = "\(server.name) — \(summary.isEmpty ? actionDisplayName(for: action) : summary)" + // Same width discipline as the glance rows: an untruncated core + // error must not stretch the whole menu past the chart block. + // The full text stays in the tooltip. + let title = GlanceFormatting.tailTruncated( + fullTitle, limit: GlanceFormatting.reasonBudget) let item = NSMenuItem(title: title, action: #selector(handleAttentionAction(_:)), keyEquivalent: "") item.target = self item.representedObject = server + item.toolTip = fullTitle item.image = NSImage(systemSymbolName: icon, accessibilityDescription: action) menu.addItem(item) } diff --git a/native/macos/MCPProxy/MCPProxy/Menu/Glance/ActivityHistogramView.swift b/native/macos/MCPProxy/MCPProxy/Menu/Glance/ActivityHistogramView.swift index 2ea50d59..689132a3 100644 --- a/native/macos/MCPProxy/MCPProxy/Menu/Glance/ActivityHistogramView.swift +++ b/native/macos/MCPProxy/MCPProxy/Menu/Glance/ActivityHistogramView.swift @@ -193,12 +193,13 @@ struct ActivityHistogramView: View { AxisValueLabel(format: .dateTime.hour()) } } - // Height covers the plot AND the legend below it. Sized so the legend - // is additive: it must not buy its place by shrinking 24 bars that are - // already only ~10 pt wide apiece. - .frame(width: 260, height: 116) - .padding(.horizontal, 14) - .padding(.vertical, 8) + // Height covers the plot AND the legend below it. Deliberately shallow: + // the chart is a shape to recognise, not a plot to read values off — + // relative bar heights survive 60 pt of plot, and a menu is the wrong + // place for more (the Web UI has the full-size version). + .frame(width: 248, height: 84) + .padding(.horizontal, 12) + .padding(.vertical, 6) // One label for the whole chart: VoiceOver reading 48 unlabelled bar // marks would be worse than useless. .accessibilityElement(children: .ignore) @@ -210,10 +211,10 @@ extension ActivityHistogram { /// Size of the hosted chart item, in points. Menu items do not auto-size a /// hosting view, so the frame is explicit — and it must match the view's - /// own size, or the row grows a band of dead space. 260 + 2*14 = 288 wide, - /// 116 + 2*8 = 132 tall; measured `NSHostingView.fittingSize` agrees, and + /// own size, or the row grows a band of dead space. 248 + 2*12 = 272 wide, + /// 84 + 2*6 = 96 tall; measured `NSHostingView.fittingSize` agrees, and /// `testRealChartItemIsSizedAndLabelled` keeps the two in step. - static let chartItemSize = NSSize(width: 288, height: 132) + static let chartItemSize = NSSize(width: 272, height: 96) /// The glance's single custom item: an `NSHostingView` wrapping the chart. /// diff --git a/native/macos/MCPProxy/MCPProxy/Menu/Glance/GlanceFormatting.swift b/native/macos/MCPProxy/MCPProxy/Menu/Glance/GlanceFormatting.swift index 97bdfbb9..9915b7d3 100644 --- a/native/macos/MCPProxy/MCPProxy/Menu/Glance/GlanceFormatting.swift +++ b/native/macos/MCPProxy/MCPProxy/Menu/Glance/GlanceFormatting.swift @@ -96,17 +96,23 @@ enum GlanceFormatting { // MARK: - Budgets - /// Character budget for a row's reason subtitle (spec 090 FR-006). + /// Character budget for a row's second line (spec 090 FR-006) — the reason, + /// or on a failed row the error clause. /// /// Independent of the label's budget on purpose: the two lines are truncated /// separately, so a long `server:tool` never shortens the explanation and a /// long explanation never shortens the name of what ran. - static let reasonBudget = 60 - - /// Character budget for the error clause on a failed row's title line - /// (FR-011a). Deliberately smaller than the reason's: it shares the title - /// with the label and the age, and the full message is in the tooltip. - static let errorClauseBudget = 40 + /// + /// 44 is the menu's width discipline: at the menu font it is about the width + /// of the 24h chart block, so the widest thing in the menu is the chart — + /// the element designed to be looked at — never a backend's error prose. + static let reasonBudget = 44 + + /// Character budget for the error clause when it must share the TITLE line + /// — the pre-14.4 fallback with no subtitle mechanism. Deliberately smaller + /// than the reason's: it shares the line with the label and the age, and + /// the full message is in the tooltip. + static let errorClauseBudget = 28 /// Tail-truncate `text` to at most `limit` characters, keeping the head. /// diff --git a/native/macos/MCPProxy/MCPProxy/Menu/Glance/GlanceSection.swift b/native/macos/MCPProxy/MCPProxy/Menu/Glance/GlanceSection.swift index 993108a7..8e7c3d6a 100644 --- a/native/macos/MCPProxy/MCPProxy/Menu/Glance/GlanceSection.swift +++ b/native/macos/MCPProxy/MCPProxy/Menu/Glance/GlanceSection.swift @@ -60,11 +60,10 @@ final class GlanceSection { /// Character budget for a row label before middle truncation kicks in. /// - /// Never tightened to make room for anything else on the title line - /// (FR-011a): the error clause has its own, smaller budget and is cut first, - /// and the age — the shortest and most perishable part of the row — is never - /// cut at all. - private static let labelBudget = 34 + /// Sized so `label ×N — age` sits within the chart block's width — the + /// chart, not the longest tool name, is what bounds the menu. The age — + /// the shortest and most perishable part of the row — is never cut at all. + private static let labelBudget = 30 /// Whether rows may carry a second line. /// @@ -271,7 +270,7 @@ final class GlanceSection { // — a reason whose wording changed still occupies one line and is an // ordinary in-place rewrite. for (index, run) in zip(activityRows.indices, runs) - where (subtitleText(for: run.displayReason) == nil) != (activityRows[index].subtitleText == nil) { + where (subtitleText(for: run) == nil) != (activityRows[index].subtitleText == nil) { return false } @@ -365,34 +364,35 @@ final class GlanceSection { let age = GlanceFormatting.relativeTime(run.timestamp, now: now) let status = run.worstStatus let failed = status != "success" - // The error clause is cut to its own budget before anything else on the - // line gives way (FR-011a): a backend that answers in paragraphs must - // not be able to squeeze out the name of the tool that ran. - let detail = failed - ? Self.firstClause(of: run.errorMessage).map { - GlanceFormatting.tailTruncated($0, limit: GlanceFormatting.errorClauseBudget) - } - : nil - - // The reason is the row's second line, never part of its first: on a - // failed row the error joins the title and the reason keeps the - // subtitle, so "why it was attempted" and "how it went" are both - // readable at a glance (FR-011a). + let clause = failed ? Self.firstClause(of: run.errorMessage) : nil + + // One fact per line bounds the menu (compact revision of FR-011a): the + // title is always `label ×N — age`, and on a failed row the error + // clause takes the second line — the failure mark already flags the row + // — while the reason retreats to the tooltip. Error prose on the title + // line was what made the whole menu wider than the chart it opens with. + // + // Pre-14.4 there is no second line, so the clause rejoins the title + // under its own, smaller budget (the documented FR-005 degradation). let reason = run.displayReason - let subtitle = subtitleText(for: reason) + let subtitle = subtitleText(for: run) let title: String var accessibility: String - if let detail { + if let clause, !supportsRowSubtitles { + let detail = GlanceFormatting.tailTruncated(clause, limit: GlanceFormatting.errorClauseBudget) title = "\(label)\(countSuffix) · \(detail) — \(age)" - accessibility = "\(fullLabel)\(spokenCount), failed: \(detail), \(age) ago" } else { title = "\(label)\(countSuffix) — \(age)" + } + // Spoken in full, and spoken on every macOS version — the lines are + // where facts are *seen*, not where they live (FR-006, FR-025). + if let clause { + accessibility = "\(fullLabel)\(spokenCount), failed: \(clause), \(age) ago" + } else { accessibility = "\(fullLabel)\(spokenCount), " + "\(Self.outcomeDescription(forStatus: status)), \(age) ago" } - // Spoken in full, and spoken on every macOS version — the subtitle is - // where the reason is *seen*, not where it lives (FR-006, FR-025). if let reason { accessibility += ", reason: \(reason)" } // The tooltip is the row without any budget at all: full label, full @@ -424,11 +424,18 @@ final class GlanceSection { row.runIdentity = identity } - /// The text a row's second line would show, or nil when it has none — - /// either because the record declared no reason (FR-007) or because this - /// system has no subtitle mechanism (FR-005). - private func subtitleText(for reason: String?) -> String? { - guard supportsRowSubtitles, let reason else { return nil } + /// The text a row's second line would show, or nil when it has none. + /// + /// On a failed row the line belongs to the error clause — "how it went" + /// outranks "why it was attempted" once something is wrong, and the full + /// reason stays in the tooltip. Otherwise it is the reason (FR-006/FR-007). + /// Nil on systems without the subtitle mechanism (FR-005). + private func subtitleText(for run: GlanceRun) -> String? { + guard supportsRowSubtitles else { return nil } + if run.worstStatus != "success", let clause = Self.firstClause(of: run.errorMessage) { + return GlanceFormatting.tailTruncated(clause, limit: GlanceFormatting.reasonBudget) + } + guard let reason = run.displayReason else { return nil } return GlanceFormatting.tailTruncated(reason, limit: GlanceFormatting.reasonBudget) } diff --git a/native/macos/MCPProxy/MCPProxyTests/GlanceFormattingTests.swift b/native/macos/MCPProxy/MCPProxyTests/GlanceFormattingTests.swift index 66b71e2e..e707d306 100644 --- a/native/macos/MCPProxy/MCPProxyTests/GlanceFormattingTests.swift +++ b/native/macos/MCPProxy/MCPProxyTests/GlanceFormattingTests.swift @@ -110,8 +110,11 @@ final class GlanceFormattingTests: XCTestCase { /// the title line), so they are pinned here rather than read back from the /// implementation. func testReasonAndErrorClauseBudgetsAreTheSpecsNumbers() { - XCTAssertEqual(GlanceFormatting.reasonBudget, 60) - XCTAssertEqual(GlanceFormatting.errorClauseBudget, 40) + // 44 keeps the widest text line inside the 24h chart block's width — + // the chart, never error prose, bounds the menu. 28 is the tighter + // budget for the pre-14.4 fallback where the clause shares the title. + XCTAssertEqual(GlanceFormatting.reasonBudget, 44) + XCTAssertEqual(GlanceFormatting.errorClauseBudget, 28) } // MARK: - Relative time diff --git a/native/macos/MCPProxy/MCPProxyTests/GlanceSectionTests.swift b/native/macos/MCPProxy/MCPProxyTests/GlanceSectionTests.swift index 8396e3a4..2d40b92f 100644 --- a/native/macos/MCPProxy/MCPProxyTests/GlanceSectionTests.swift +++ b/native/macos/MCPProxy/MCPProxyTests/GlanceSectionTests.swift @@ -84,7 +84,7 @@ final class GlanceSectionTests: XCTestCase { "—", "Recent", "github:create_issue — 30s", - "jira:get_issue · auth failed — 2m", + "jira:get_issue — 2m", "Open Activity…" ]) } @@ -92,7 +92,7 @@ final class GlanceSectionTests: XCTestCase { func testActivityRowCarriesFullIdentity() { let section = Self.makeSection() let failed = section.items(for: Self.busyState(), now: Self.now)[5] - XCTAssertEqual(failed.title, "jira:get_issue · auth failed — 2m") + XCTAssertEqual(failed.title, "jira:get_issue — 2m") XCTAssertEqual(failed.representedObject as? String, "sess-b") XCTAssertEqual(failed.image?.accessibilityDescription, "failed") XCTAssertEqual(failed.toolTip, "jira:get_issue\nauth failed: token expired. retry after refresh") @@ -190,7 +190,9 @@ final class GlanceSectionTests: XCTestCase { let section = Self.makeSection() let row = section.items(for: state, now: Self.now)[4] - XCTAssertEqual(row.title, "jira:get_issue ×3 · auth failed — 30s") + XCTAssertEqual(row.title, "jira:get_issue ×3 — 30s") + XCTAssertEqual(Self.subtitle(of: row), "auth failed", + "the newest failure's clause takes the second line") XCTAssertEqual(row.image?.accessibilityDescription, "failed") XCTAssertEqual(row.toolTip, "jira:get_issue\nauth failed: token expired") XCTAssertEqual(row.accessibilityLabel(), @@ -236,6 +238,10 @@ final class GlanceSectionTests: XCTestCase { func testADifferentRunInTheSameSlotRewritesTheRowIdentity() { let state = Self.burstState() let section = Self.makeSection() + // One-line rows on purpose: with subtitles, the failing replacement + // would also gain a line (structural, deferred); the turnover itself + // is what this test pins. + section.supportsRowSubtitles = false let items = section.items(for: state, now: Self.now) let iconBefore = items[4].image XCTAssertNil(iconBefore, "precondition: the successful burst row is unmarked") @@ -260,6 +266,9 @@ final class GlanceSectionTests: XCTestCase { func testASameRunStillPicksUpALateFailure() { let state = Self.burstState() let section = Self.makeSection() + // One-line rows: with subtitles the late failure would gain a line, + // which is the structural case the next test pins. + section.supportsRowSubtitles = false let items = section.items(for: state, now: Self.now) state.glanceActivity[0] = Self.entry( @@ -272,6 +281,22 @@ final class GlanceSectionTests: XCTestCase { XCTAssertEqual(items[4].image?.accessibilityDescription, "failed") } + /// With subtitles available, a late failure ADDS the error line — a row + /// growing a line resizes the menu, so it is structural and waits for + /// close (FR-023), exactly like a run gaining its reason. + func testALateFailureGainingItsErrorLineIsStructural() { + let state = Self.burstState() + let section = Self.makeSection() + _ = section.items(for: state, now: Self.now) + + state.glanceActivity[0] = Self.entry( + id: "j1", server: "jira", tool: "get_issue", status: "error", + error: "rate limited: try later", + timestamp: "2027-01-15T07:59:30Z", session: "sess-j1") + + XCTAssertFalse(section.updateInPlace(for: state, now: Self.now)) + } + // MARK: - Reason subtitles (spec 090 US2) /// The reason is the row's standard subtitle — a subdued second line of the @@ -345,23 +370,28 @@ final class GlanceSectionTests: XCTestCase { XCTAssertEqual(Self.subtitle(of: row), "Check the ticket after the failed transition") } - /// FR-011a: on a failed row the error joins the TITLE and the reason keeps - /// the subtitle — the error never displaces the reason. - func testAFailedRowShowsTheErrorOnTheTitleAndKeepsTheReasonAsSubtitle() { + /// FR-011a (compact revision): one fact per line. On a failed row the + /// error clause takes the second line — "how it went" outranks "why it was + /// attempted" once something is wrong — and the reason stays reachable in + /// the tooltip and spoken by VoiceOver. + func testAFailedRowShowsTheErrorAsTheSubtitleAndKeepsTheReasonInTheTooltip() { let state = Self.reasonState(status: "error", error: "auth failed: token expired") let section = Self.makeSection() let row = section.items(for: state, now: Self.now)[4] - XCTAssertEqual(row.title, "jira:get_issue · auth failed — 30s") - XCTAssertEqual(Self.subtitle(of: row), "Verify the ticket is still open") + XCTAssertEqual(row.title, "jira:get_issue — 30s", + "error prose never widens the title line") + XCTAssertEqual(Self.subtitle(of: row), "auth failed") + XCTAssertTrue(row.toolTip?.contains("Verify the ticket is still open") == true, + "the displaced reason stays in the tooltip") XCTAssertEqual(row.accessibilityLabel(), "jira:get_issue, failed: auth failed, 30s ago, " + "reason: Verify the ticket is still open") } - /// FR-011a truncation precedence: the error clause is cut to its own - /// 40-character budget, the label keeps its 34-character middle-truncation - /// budget (it is never tightened to make room), and the age is never cut. + /// Truncation precedence on the pre-14.4 fallback (the only path where the + /// clause still shares the title): the clause is cut to its own budget, + /// the label keeps its middle-truncation budget, and the age is never cut. func testTheErrorClauseIsCutToItsOwnBudgetWhileTheLabelKeepsIts() { let state = Self.busyState() state.glanceActivity = [ @@ -374,13 +404,14 @@ final class GlanceSectionTests: XCTestCase { session: "sess-e1") ] let section = Self.makeSection() + section.supportsRowSubtitles = false let title = section.items(for: state, now: Self.now)[4].title let label = String(title.prefix(while: { $0 != "·" })).trimmingCharacters(in: .whitespaces) let clause = title .components(separatedBy: " · ")[1] .components(separatedBy: " — ")[0] - XCTAssertEqual(label.count, 34, "the label budget must not be tightened by a long error") + XCTAssertEqual(label.count, 30, "the label budget must not be tightened by a long error") XCTAssertEqual(clause.count, GlanceFormatting.errorClauseBudget) XCTAssertTrue(clause.hasSuffix("\u{2026}")) XCTAssertTrue(title.hasSuffix(" — 30s"), "the age is never truncated") @@ -563,7 +594,7 @@ final class GlanceSectionTests: XCTestCase { "—", "Recent", "github:create_issue — 30s", - "jira:get_issue · auth failed — 2m", + "jira:get_issue — 2m", "Open Activity…", "—", "Clients", @@ -684,6 +715,9 @@ final class GlanceSectionTests: XCTestCase { func testDifferentRecordInTheSameSlotRewritesTheIcon() { let state = Self.busyState() let section = Self.makeSection() + // One-line rows: the failing replacement would otherwise also gain a + // line (structural, deferred); the identity rewrite is what this pins. + section.supportsRowSubtitles = false let items = section.items(for: state, now: Self.now) let previousFailure = state.glanceActivity[1] XCTAssertNil(items[4].image, "precondition: the successful row is unmarked") @@ -706,6 +740,9 @@ final class GlanceSectionTests: XCTestCase { func testSameRecordStillPicksUpALateStatusCorrection() { let state = Self.busyState() let section = Self.makeSection() + // One-line rows, so the late failure rewrites in place instead of + // gaining a line (the structural case has its own test). + section.supportsRowSubtitles = false let items = section.items(for: state, now: Self.now) let previousFailure = state.glanceActivity[1] From b953a9e8fd9d54b254c27b266c54fbfceabf60f2 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Mon, 3 Aug 2026 17:02:21 +0300 Subject: [PATCH 4/7] fix(tray): speak attention rows in full; correct stale height comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-2 verify notes: the 44-char truncation of Needs-Attention rows also truncated what VoiceOver reads (tooltips are not spoken), so the row now carries the full text as its accessibility label — the same FR-025 truncated-on-screen/spoken-in-full discipline as the glance rows. Three comments still describing the chart as 132 pt updated to its actual 96 pt. --- native/macos/MCPProxy/MCPProxy/MCPProxyApp.swift | 3 +++ .../macos/MCPProxy/MCPProxy/Menu/Glance/GlanceSection.swift | 2 +- .../macos/MCPProxy/MCPProxyTests/ActivityHistogramTests.swift | 4 ++-- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/native/macos/MCPProxy/MCPProxy/MCPProxyApp.swift b/native/macos/MCPProxy/MCPProxy/MCPProxyApp.swift index 3b720caa..ba450ac2 100644 --- a/native/macos/MCPProxy/MCPProxy/MCPProxyApp.swift +++ b/native/macos/MCPProxy/MCPProxy/MCPProxyApp.swift @@ -920,6 +920,9 @@ final class AppController: NSObject, NSApplicationDelegate, NSWindowDelegate, NS item.target = self item.representedObject = server item.toolTip = fullTitle + // Truncated on screen, spoken in full — tooltips are not read + // by VoiceOver (same FR-025 discipline as the glance rows). + item.setAccessibilityLabel(fullTitle) item.image = NSImage(systemSymbolName: icon, accessibilityDescription: action) menu.addItem(item) } diff --git a/native/macos/MCPProxy/MCPProxy/Menu/Glance/GlanceSection.swift b/native/macos/MCPProxy/MCPProxy/Menu/Glance/GlanceSection.swift index 8e7c3d6a..1da6ef81 100644 --- a/native/macos/MCPProxy/MCPProxy/Menu/Glance/GlanceSection.swift +++ b/native/macos/MCPProxy/MCPProxy/Menu/Glance/GlanceSection.swift @@ -123,7 +123,7 @@ final class GlanceSection { private var builtVisible = false /// What kind of row the histogram block was last built with. A text row and - /// a 132 pt chart have different heights, so a kind change is structural; + /// a 96 pt chart have different heights, so a kind change is structural; /// content changes within a kind are in-place rewrites. private enum HistogramRowKind: Equatable { case loading, failed, chart } private var builtHistogramKind: HistogramRowKind? diff --git a/native/macos/MCPProxy/MCPProxyTests/ActivityHistogramTests.swift b/native/macos/MCPProxy/MCPProxyTests/ActivityHistogramTests.swift index b78b381a..cfce1144 100644 --- a/native/macos/MCPProxy/MCPProxyTests/ActivityHistogramTests.swift +++ b/native/macos/MCPProxy/MCPProxyTests/ActivityHistogramTests.swift @@ -367,7 +367,7 @@ final class GlanceInlineHistogramTests: XCTestCase { // MARK: - In-place rules - /// A text placeholder and a 132 pt chart have different heights, so the + /// A text placeholder and a 96 pt chart have different heights, so the /// flip cannot happen under the cursor — but it must not freeze the block /// either (the timeline loads seconds after launch, exactly when the menu /// is likely open). The update succeeds, the placeholder stays, and the @@ -588,7 +588,7 @@ final class UsageRefreshWiringTests: XCTestCase { @MainActor final class ActivityHistogramEncodingTests: XCTestCase { - /// The chart is 132 pt tall and the bottom 24 pt of that is the legend — + /// The chart is 96 pt tall and the bottom 24 pt of that is the legend — /// the band the frame grew to pay for. Everything above it is the plot. private static let legendBandHeight = 24.0 From cd060cae59ac3d70f43f463175b53db448581029 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Mon, 3 Aug 2026 18:06:29 +0300 Subject: [PATCH 5/7] feat(tray): collapse Needs Attention into a submenu; words for an idle day MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Owner feedback round 3: - Needs Attention is one row with a count and a warning icon; the per-server rows live in its submenu, a hover away. N servers no longer cost N rows of a menu that opens with a chart. The section stays absent entirely when nothing needs attention (as before). - A loaded-but-idle 24h axis renders as the muted sentence "No calls in the last 24h" instead of 24 empty bars — an all-zero chart reads as a broken widget, while the words say what the flat axis implied. Idle is a third text kind: it swaps with loading/failed in place (one-line geometry) and defers a flip to/from the chart to the next rebuild like the other text kinds. --- .../macos/MCPProxy/MCPProxy/MCPProxyApp.swift | 18 ++++-- .../MCPProxy/Menu/Glance/GlanceSection.swift | 59 +++++++++++++------ .../ActivityHistogramTests.swift | 45 ++++++++++++-- 3 files changed, 95 insertions(+), 27 deletions(-) diff --git a/native/macos/MCPProxy/MCPProxy/MCPProxyApp.swift b/native/macos/MCPProxy/MCPProxy/MCPProxyApp.swift index ba450ac2..70907673 100644 --- a/native/macos/MCPProxy/MCPProxy/MCPProxyApp.swift +++ b/native/macos/MCPProxy/MCPProxy/MCPProxyApp.swift @@ -898,12 +898,18 @@ final class AppController: NSObject, NSApplicationDelegate, NSWindowDelegate, NS menu.addItem(row) } - // Needs Attention — only auth required, connection errors, quarantine (NOT disabled) + // Needs Attention — only auth required, connection errors, quarantine + // (NOT disabled). One collapsed row: the count is the glanceable fact, + // the per-server detail is a hover away, and N servers no longer cost + // N rows of a menu that opens with a chart. Absent entirely when + // nothing needs attention. let attentionServers = appState.serversNeedingAttention if !attentionServers.isEmpty { - let header = NSMenuItem(title: "Needs Attention (\(attentionServers.count))", action: nil, keyEquivalent: "") - header.isEnabled = false - menu.addItem(header) + let parent = NSMenuItem(title: "Needs Attention (\(attentionServers.count))", + action: nil, keyEquivalent: "") + parent.image = NSImage(systemSymbolName: "exclamationmark.triangle", + accessibilityDescription: "needs attention") + let submenu = NSMenu(title: "Needs Attention") for server in attentionServers { let action = server.health?.action ?? "" @@ -924,8 +930,10 @@ final class AppController: NSObject, NSApplicationDelegate, NSWindowDelegate, NS // by VoiceOver (same FR-025 discipline as the glance rows). item.setAccessibilityLabel(fullTitle) item.image = NSImage(systemSymbolName: icon, accessibilityDescription: action) - menu.addItem(item) + submenu.addItem(item) } + parent.submenu = submenu + menu.addItem(parent) menu.addItem(.separator()) } diff --git a/native/macos/MCPProxy/MCPProxy/Menu/Glance/GlanceSection.swift b/native/macos/MCPProxy/MCPProxy/Menu/Glance/GlanceSection.swift index 1da6ef81..0ec9e630 100644 --- a/native/macos/MCPProxy/MCPProxy/Menu/Glance/GlanceSection.swift +++ b/native/macos/MCPProxy/MCPProxy/Menu/Glance/GlanceSection.swift @@ -122,10 +122,11 @@ final class GlanceSection { private var hasBuilt = false private var builtVisible = false - /// What kind of row the histogram block was last built with. A text row and - /// a 96 pt chart have different heights, so a kind change is structural; - /// content changes within a kind are in-place rewrites. - private enum HistogramRowKind: Equatable { case loading, failed, chart } + /// What kind of row the histogram block was last built with. The three + /// text kinds share one-line geometry and rewrite each other in place; a + /// text ↔ chart change is structural (a text row and a 96 pt chart have + /// different heights). + private enum HistogramRowKind: Equatable { case loading, failed, idle, chart } private var builtHistogramKind: HistogramRowKind? /// The histogram row currently installed, and — when it is the chart — the @@ -690,6 +691,16 @@ final class GlanceSection { histogramRow = item return item case .loaded(let bars): + // A loaded-but-idle day is a sentence, not a chart: 24 empty bars + // read as a broken widget, while the words say exactly what the + // flat axis would have implied. + if bars.allSatisfy({ $0.total == 0 }) { + builtHistogramKind = .idle + builtHistogramBars = nil + let item = Self.mutedItem(Self.idleHistogramTitle) + histogramRow = item + return item + } builtHistogramKind = .chart if let cached = histogramRow, builtHistogramBars == bars, builtHistogramTimeZoneID == TimeZone.current.identifier { @@ -703,11 +714,16 @@ final class GlanceSection { } } + /// The idle row's text — a statement about the last 24 hours, matching the + /// claim the accessibility summary makes for the same axis. + static let idleHistogramTitle = "No calls in the last 24h" + private static func kind(of state: HistogramState) -> HistogramRowKind { switch state { case .loading: return .loading case .failed: return .failed - case .loaded: return .chart + case .loaded(let bars): + return bars.allSatisfy { $0.total == 0 } ? .idle : .chart } } @@ -721,33 +737,40 @@ final class GlanceSection { /// next rebuild installs the right one. private func refreshHistogramRow(with histogramState: HistogramState) { guard let item = histogramRow, let builtKind = builtHistogramKind else { return } - switch (builtKind, Self.kind(of: histogramState)) { - case (.chart, .chart): + let newKind = Self.kind(of: histogramState) + switch (builtKind == .chart, newKind == .chart) { + case (true, true): guard case .loaded(let bars) = histogramState, builtHistogramBars != bars else { return } item.view = histogramChartItemFactory(bars).view builtHistogramBars = bars builtHistogramTimeZoneID = TimeZone.current.identifier - case (.loading, .loading), (.failed, .failed), (.loading, .failed), (.failed, .loading): - applyMutedHistogramText(for: histogramState, to: item) - case (.chart, _), (_, .chart): + case (false, false): + applyMutedHistogramText(for: histogramState, kind: newKind, to: item) + default: break } } /// Rewrite a text-kind histogram row to describe `histogramState`. - private func applyMutedHistogramText(for histogramState: HistogramState, to item: NSMenuItem) { - switch histogramState { + private func applyMutedHistogramText( + for histogramState: HistogramState, kind: HistogramRowKind, to item: NSMenuItem + ) { + switch kind { case .loading: Self.setMutedTitle("Activity (24h) — loading…", on: item) item.toolTip = nil - builtHistogramKind = .loading - case .failed(let message): + case .failed: Self.setMutedTitle("Activity (24h) unavailable", on: item) - if item.toolTip != message { item.toolTip = message } - builtHistogramKind = .failed - case .loaded: - break + if case .failed(let message) = histogramState, item.toolTip != message { + item.toolTip = message + } + case .idle: + Self.setMutedTitle(Self.idleHistogramTitle, on: item) + item.toolTip = nil + case .chart: + return } + builtHistogramKind = kind } /// A disabled, secondary-coloured text row. Setting `attributedTitle` diff --git a/native/macos/MCPProxy/MCPProxyTests/ActivityHistogramTests.swift b/native/macos/MCPProxy/MCPProxyTests/ActivityHistogramTests.swift index cfce1144..5135ac6f 100644 --- a/native/macos/MCPProxy/MCPProxyTests/ActivityHistogramTests.swift +++ b/native/macos/MCPProxy/MCPProxyTests/ActivityHistogramTests.swift @@ -321,10 +321,47 @@ final class GlanceInlineHistogramTests: XCTestCase { let state = connectedState() XCTAssertEqual(histogramRow(section, state).title, "Activity (24h) — loading…") + state.usageTimeline = [Fixture.bucket(start: Fixture.currentHour, calls: 3, errors: 1)] + + XCTAssertEqual(histogramRow(section, state).title, "CHART:24") + } + + /// A day with zero calls is a sentence, not 24 empty bars — an all-zero + /// chart reads as a broken widget (FR-020 spirit: say the claim outright). + func testAnIdleTimelineShowsWordsNotAnEmptyChart() { + let state = connectedState() state.usageTimeline = [] - XCTAssertEqual(histogramRow(section, state).title, "CHART:24", - "an idle timeline is a flat axis, not a loading row") + let row = histogramRow(makeSection(), state) + + XCTAssertEqual(row.title, GlanceSection.idleHistogramTitle) + XCTAssertFalse(row.isEnabled) + XCTAssertNil(row.submenu) + } + + /// Zero-call buckets are as idle as no buckets: the words appear whenever + /// the axis would have been flat. + func testAnAllZeroTimelineIsIdleToo() { + let state = connectedState() + state.usageTimeline = [Fixture.bucket(start: Fixture.currentHour, calls: 0, errors: 0)] + + XCTAssertEqual(histogramRow(makeSection(), state).title, + GlanceSection.idleHistogramTitle) + } + + /// Loading and idle share one-line geometry, so a timeline that loads + /// EMPTY while the menu is open replaces "loading…" in place — the words + /// change, the menu does not move. + func testLoadingBecomesTheIdleLabelInPlace() { + let section = makeSection() + let state = connectedState() + let row = histogramRow(section, state) + XCTAssertEqual(row.title, "Activity (24h) — loading…") + + state.usageTimeline = [] + + XCTAssertTrue(section.updateInPlace(for: state, now: Fixture.now)) + XCTAssertEqual(row.title, GlanceSection.idleHistogramTitle) } // MARK: - The eager-build cache @@ -379,7 +416,7 @@ final class GlanceInlineHistogramTests: XCTestCase { let items = section.items(for: state, now: Fixture.now) XCTAssertEqual(items[1].title, "Activity (24h) — loading…") - state.usageTimeline = [] + state.usageTimeline = [Fixture.bucket(start: Fixture.currentHour, calls: 3, errors: 1)] XCTAssertTrue(section.updateInPlace(for: state, now: Fixture.now), "the rest of the block keeps updating in place") @@ -392,7 +429,7 @@ final class GlanceInlineHistogramTests: XCTestCase { func testLosingTheTimelineKeepsTheChartUntilTheNextRebuild() { let section = makeSection() let state = connectedState() - state.usageTimeline = [] + state.usageTimeline = [Fixture.bucket(start: Fixture.currentHour, calls: 3, errors: 1)] let row = histogramRow(section, state) XCTAssertEqual(row.title, "CHART:24") From a9e4d43693c65b8538c048357c7e0a9d5856da10 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Mon, 3 Aug 2026 18:59:20 +0300 Subject: [PATCH 6/7] feat(tray): one 24h frame for every number in the glance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Owner-reported contradiction: the Recent list showed calls from days ago directly under a chart saying "No calls in the last 24h" — three windows (hour summary, 24h chart, unbounded recent list) in one block. The menu now speaks ONE frame, the 24 hours the histogram draws: - Header: "N calls in the last 24h" via new glanceCallsLast24h — the same poll+live-SSE reconciliation as the hour count (GH #934), summed over the histogram axis. A zero says nothing the idle row does not, so the segment appears only when there is something to count, and a summary with nothing to say gets no row (presence flip = structural). - Recent: rows filtered to the frame (GlancePresence.lookback — already 24h for clients); a retained record from days ago no longer appears. With nothing qualifying, the section is just "Open Activity…" — a header over an empty list explains nothing. - Clients placeholder: "No clients in the last 24h", naming the frame. - The hour-count machinery stays (callsThisHour + glanceCallsThisHour and their reconciliation tests are the tested contract the 24h getter shares its live-call state with). Header-consistency tests now pin the frame semantics: counts SURVIVE an hour rollover; only sliding off the 24h axis drops them. --- .../MCPProxy/Menu/Glance/GlanceSection.swift | 64 +++++++++++----- .../MCPProxy/MCPProxy/State/AppState.swift | 29 ++++++++ .../ActivityHistogramTests.swift | 20 +++-- .../MCPProxyTests/GlanceFixtures.swift | 2 +- .../GlanceHeaderConsistencyTests.swift | 4 +- .../MCPProxyTests/GlanceMenuPolicyTests.swift | 6 +- .../MCPProxyTests/GlanceSectionTests.swift | 73 ++++++++++++------- .../MCPProxyTests/MenuOpenNetworkTests.swift | 12 +-- .../MenuRefreshSchedulerTests.swift | 6 +- 9 files changed, 150 insertions(+), 66 deletions(-) diff --git a/native/macos/MCPProxy/MCPProxy/Menu/Glance/GlanceSection.swift b/native/macos/MCPProxy/MCPProxy/Menu/Glance/GlanceSection.swift index 0ec9e630..5edd417a 100644 --- a/native/macos/MCPProxy/MCPProxy/Menu/Glance/GlanceSection.swift +++ b/native/macos/MCPProxy/MCPProxy/Menu/Glance/GlanceSection.swift @@ -168,9 +168,15 @@ final class GlanceSection { var items: [NSMenuItem] = [] - let summary = disabledItem(titled: summaryTitle(for: state, now: now)) - summaryItem = summary - items.append(summary) + // The summary has nothing to say for an idle proxy with no clients — + // the idle histogram row already speaks for the day — so an empty + // title gets no row rather than a blank line. + let summaryText = summaryTitle(for: state, now: now) + if !summaryText.isEmpty { + let summary = disabledItem(titled: summaryText) + summaryItem = summary + items.append(summary) + } // The day before the minute (FR-021). The histogram answers "what has // been happening?" in one glyph, so it belongs beside the summary line @@ -180,11 +186,13 @@ final class GlanceSection { items.append(histogramRowItem(for: state, now: now)) items.append(.separator()) - items.append(disabledItem(titled: "Recent")) - let runs = GlanceSelection.activityRows(from: state.glanceActivity) - if runs.isEmpty { - items.append(disabledItem(titled: "No tool calls yet")) - } else { + // One frame for the whole menu: rows outside the histogram's 24 hours + // do not appear beside a chart (or an idle sentence) that says the day + // was quieter. When nothing qualifies the section is just the door to + // the full log — a header over an empty list explains nothing. + let runs = Self.recentRuns(for: state, now: now) + if !runs.isEmpty { + items.append(disabledItem(titled: "Recent")) for run in runs { var row = ActivityRow(item: actionableItem()) apply(run, to: &row, now: now) @@ -250,7 +258,12 @@ final class GlanceSection { guard isVisible(for: state) == builtVisible else { return false } guard builtVisible else { return true } - let runs = GlanceSelection.activityRows(from: state.glanceActivity) + let summary = summaryTitle(for: state, now: now) + // The summary row appears only when it has something to say, so its + // presence flipping is gaining or losing a row — structural (FR-023). + guard summary.isEmpty == (summaryItem == nil) else { return false } + + let runs = Self.recentRuns(for: state, now: now) let presence = Self.clientList(for: state, now: now) let clients = presence.rows guard runs.count == activityRows.count, @@ -275,8 +288,7 @@ final class GlanceSection { return false } - let summary = summaryTitle(for: state, now: now) - if summaryItem?.title != summary { summaryItem?.title = summary } + if let summaryItem, summaryItem.title != summary { summaryItem.title = summary } // `zip`, like the sibling loop below: indexing `entries` by // `activityRows.indices` reads out of bounds if the count guard above is // ever weakened, and zip cannot. @@ -310,6 +322,19 @@ final class GlanceSection { private static func overflowTitle(_ hidden: Int) -> String { "+\(hidden) more" } + /// The Recent section's rows: qualifying runs whose newest record falls + /// inside the menu's ONE frame — `GlancePresence.lookback`, the same 24 + /// hours the histogram draws and the header counts. A record the log still + /// retains from days ago must not sit beside a chart that says the day was + /// quiet (the inconsistency this frame exists to end). An unparseable + /// timestamp keeps its row: showing it is the safer failure. + private static func recentRuns(for state: AppState, now: Date) -> [GlanceRun] { + GlanceSelection.activityRows(from: state.glanceActivity).filter { run in + guard let at = GlanceFormatting.parseTimestamp(run.timestamp) else { return true } + return now.timeIntervalSince(at) <= GlancePresence.lookback + } + } + /// The overflow row's current text, for tests that pin it after an in-place /// update — the item itself is private, and reaching into the menu to find /// it by title would assert nothing about which row was rewritten. @@ -573,7 +598,7 @@ final class GlanceSection { /// the section can actually stand behind: with a stateless transport there /// is no such thing as a currently-connected client, and the old wording /// announced "nothing is connected" every time the last session timed out. - static let noClientsTitle = "No recent clients" + static let noClientsTitle = "No clients in the last 24h" /// The presence indicator's glyph. /// @@ -813,11 +838,16 @@ final class GlanceSection { /// headline over a section that had rows in it. private func summaryTitle(for state: AppState, now: Date = Date()) -> String { var parts: [String] = [] - // `glanceCallsThisHour`, not the raw polled `callsThisHour`: the rows - // below arrive over SSE and the poll is 30 seconds apart, so the raw - // count sat under rows it had never heard of (GH #934). - if let calls = state.glanceCallsThisHour(now: now) { - parts.append(calls == 1 ? "1 call this hour" : "\(calls) calls this hour") + // The menu speaks ONE time frame: the same 24 hours the histogram + // draws, the Recent rows are filtered to, and the presence lookback + // uses — a header counting a different window than the chart under it + // is how "no calls" ends up above rows from days ago. + // + // `glanceCallsLast24h` reconciles the poll with live SSE (GH #934), + // and a zero says nothing the idle histogram row does not already say, + // so the segment appears only when there is something to count. + if let calls = state.glanceCallsLast24h(now: now), calls > 0 { + parts.append(calls == 1 ? "1 call in the last 24h" : "\(calls) calls in the last 24h") } if let clients = state.glanceClientSummary(now: now) { parts.append(clients) } if state.glanceStale { parts.append("not updating") } diff --git a/native/macos/MCPProxy/MCPProxy/State/AppState.swift b/native/macos/MCPProxy/MCPProxy/State/AppState.swift index 71067ad0..51e04c61 100644 --- a/native/macos/MCPProxy/MCPProxy/State/AppState.swift +++ b/native/macos/MCPProxy/MCPProxy/State/AppState.swift @@ -417,6 +417,35 @@ final class AppState: ObservableObject { entry.type == "tool_call" && !(entry.toolName ?? "").isEmpty } + /// Calls recorded on the 24-hour axis ending at `now` — the same window + /// the histogram draws and `GlancePresence.lookback` uses, so every number + /// in the glance describes ONE frame. + /// + /// Buckets are UTC-hour aligned and sparse; anything whose hour has slid + /// off the axis is dropped, exactly as `ActivityHistogram.bars` drops it. + static func callsInLast24Hours(_ timeline: [UsageBucket], now: Date = Date()) -> Int { + let oldestHour = floorToHour(now).addingTimeInterval(-23 * 3600) + return timeline.reduce(0) { total, bucket in + floorToHour(bucket.start) >= oldestHour ? total + max(0, bucket.calls) : total + } + } + + /// The header's call count over the menu's 24h frame: the polled window + /// total plus the calls that arrived over SSE since that poll — the same + /// two-source reconciliation as `glanceCallsThisHour` (GH #934), on the + /// window the rest of the glance describes. Still `nil` before the first + /// usage response, so the header omits the segment rather than inventing + /// a count. + @MainActor + func glanceCallsLast24h(now: Date = Date()) -> Int? { + guard let usageTimeline else { return nil } + let base = AppState.callsInLast24Hours(usageTimeline, now: now) + // Live increments are all newer than the poll they follow; the age + // filter only matters for a menu left open across a very long gap. + let live = liveCallsSinceUsagePoll.filter { now.timeIntervalSince($0) < 24 * 3600 }.count + return base + live + } + /// The header's call count: the polled hour total plus the calls that have /// arrived over SSE since that poll and fall in the same hour. /// diff --git a/native/macos/MCPProxy/MCPProxyTests/ActivityHistogramTests.swift b/native/macos/MCPProxy/MCPProxyTests/ActivityHistogramTests.swift index 5135ac6f..9a3b777a 100644 --- a/native/macos/MCPProxy/MCPProxyTests/ActivityHistogramTests.swift +++ b/native/macos/MCPProxy/MCPProxyTests/ActivityHistogramTests.swift @@ -255,14 +255,16 @@ final class GlanceInlineHistogramTests: XCTestCase { action: #selector(ClickStub.openGlanceRow(_:))) } - /// The histogram row: always the summary line's neighbour. + /// The histogram row: the last item of the block that precedes the first + /// separator — its position relative to the summary moves with whether the + /// summary has anything to say. private func histogramRow(_ section: GlanceSection, _ state: AppState) -> NSMenuItem { let items = section.items(for: state, now: Fixture.now) - guard items.count > 1 else { + guard let separator = items.firstIndex(where: { $0.isSeparatorItem }), separator > 0 else { XCTFail("the block is hidden, so there is no histogram row") return NSMenuItem() } - return items[1] + return items[separator - 1] } // MARK: - The three kinds @@ -414,13 +416,14 @@ final class GlanceInlineHistogramTests: XCTestCase { let section = makeSection() let state = connectedState() let items = section.items(for: state, now: Fixture.now) - XCTAssertEqual(items[1].title, "Activity (24h) — loading…") + XCTAssertEqual(items[0].title, "Activity (24h) — loading…") state.usageTimeline = [Fixture.bucket(start: Fixture.currentHour, calls: 3, errors: 1)] - XCTAssertTrue(section.updateInPlace(for: state, now: Fixture.now), - "the rest of the block keeps updating in place") - XCTAssertEqual(items[1].title, "Activity (24h) — loading…", + XCTAssertFalse(section.updateInPlace(for: state, now: Fixture.now), + "the timeline also brings the header's call count into being — " + + "a new summary row is structural, and the rebuild installs the chart") + XCTAssertEqual(items[0].title, "Activity (24h) — loading…", "the row's height cannot change under the cursor") XCTAssertEqual(histogramRow(section, state).title, "CHART:24", "the next rebuild installs the chart") @@ -435,7 +438,8 @@ final class GlanceInlineHistogramTests: XCTestCase { state.usageTimeline = nil - XCTAssertTrue(section.updateInPlace(for: state, now: Fixture.now)) + XCTAssertFalse(section.updateInPlace(for: state, now: Fixture.now), + "losing the whole timeline also empties the header segment — structural") XCTAssertEqual(row.title, "CHART:24", "real (if stale) data stays on screen; the next rebuild decides") } diff --git a/native/macos/MCPProxy/MCPProxyTests/GlanceFixtures.swift b/native/macos/MCPProxy/MCPProxyTests/GlanceFixtures.swift index 746fba38..cf5eeb79 100644 --- a/native/macos/MCPProxy/MCPProxyTests/GlanceFixtures.swift +++ b/native/macos/MCPProxy/MCPProxyTests/GlanceFixtures.swift @@ -14,7 +14,7 @@ enum GlanceFixtures { let state = AppState() // coreState first: its didSet clears the glance feeds on any non-connected state. state.coreState = .connected - state.callsThisHour = 12 + state.usageTimeline = [UsageBucket(start: now, calls: 12, errors: 0, totalRespBytes: 0)] state.glanceActivity = [ entry(id: "a", server: "github", tool: "create_issue", timestamp: "2027-01-15T07:59:30Z", session: "sess-a"), diff --git a/native/macos/MCPProxy/MCPProxyTests/GlanceHeaderConsistencyTests.swift b/native/macos/MCPProxy/MCPProxyTests/GlanceHeaderConsistencyTests.swift index 67a69f3e..47da3446 100644 --- a/native/macos/MCPProxy/MCPProxyTests/GlanceHeaderConsistencyTests.swift +++ b/native/macos/MCPProxy/MCPProxyTests/GlanceHeaderConsistencyTests.swift @@ -94,7 +94,7 @@ final class GlanceHeaderConsistencyTests: XCTestCase { state.updateUsage(timeline: [Self.bucket(calls: 12)], now: Self.now, polledAt: Self.now.addingTimeInterval(-1)) XCTAssertEqual(Self.makeSection().items(for: state, now: Self.now).first?.title, - "12 calls this hour · 1 active", + "12 calls in the last 24h · 1 active", "precondition: the polled count is what the header shows") state.prependGlanceActivity( @@ -104,7 +104,7 @@ final class GlanceHeaderConsistencyTests: XCTestCase { ) XCTAssertEqual(Self.makeSection().items(for: state, now: Self.now).first?.title, - "13 calls this hour · 1 active", + "13 calls in the last 24h · 1 active", "the row is on screen, so the number above it has to include it") } diff --git a/native/macos/MCPProxy/MCPProxyTests/GlanceMenuPolicyTests.swift b/native/macos/MCPProxy/MCPProxyTests/GlanceMenuPolicyTests.swift index f0ccbc88..653a26d9 100644 --- a/native/macos/MCPProxy/MCPProxyTests/GlanceMenuPolicyTests.swift +++ b/native/macos/MCPProxy/MCPProxyTests/GlanceMenuPolicyTests.swift @@ -62,7 +62,7 @@ final class GlanceMenuPolicyTests: XCTestCase { let rows = section.items(for: state, now: GlanceFixtures.now) let summaryBefore = rows[0].title - XCTAssertEqual(summaryBefore, "12 calls this hour · 1 active") + XCTAssertEqual(summaryBefore, "12 calls in the last 24h · 1 active") var guardState = MenuRebuildGuard() guardState.menuWillOpen() @@ -71,7 +71,7 @@ final class GlanceMenuPolicyTests: XCTestCase { // count moves with it. The header is set deliberately: refusing an // update has to change *nothing*, and a header that alone kept moving // would describe a set of rows that is not the one below it. - state.callsThisHour = 99 + state.usageTimeline = [UsageBucket(start: GlanceFixtures.now, calls: 99, errors: 0, totalRespBytes: 0)] state.glanceActivity.insert( GlanceFixtures.entry(id: "c", server: "slack", tool: "post_message", timestamp: "2027-01-15T07:59:50Z", session: "sess-a"), @@ -110,7 +110,7 @@ final class GlanceMenuPolicyTests: XCTestCase { // Only the SECOND row changes shape; the first row's own text would // still move, because `later` is five minutes on. - state.callsThisHour = 99 + state.usageTimeline = [UsageBucket(start: GlanceFixtures.now, calls: 99, errors: 0, totalRespBytes: 0)] state.glanceActivity[1] = GlanceFixtures.entry( id: "b", server: "jira", tool: "get_issue", timestamp: "2027-01-15T07:58:00Z", session: nil, diff --git a/native/macos/MCPProxy/MCPProxyTests/GlanceSectionTests.swift b/native/macos/MCPProxy/MCPProxyTests/GlanceSectionTests.swift index 2d40b92f..a16c7acc 100644 --- a/native/macos/MCPProxy/MCPProxyTests/GlanceSectionTests.swift +++ b/native/macos/MCPProxy/MCPProxyTests/GlanceSectionTests.swift @@ -29,13 +29,13 @@ final class GlanceSectionTests: XCTestCase { func testHeaderShowsCallsThisHourAndClientCount() { let section = Self.makeSection() let items = section.items(for: Self.busyState(), now: Self.now) - XCTAssertEqual(items.first?.title, "12 calls this hour · 1 active") + XCTAssertEqual(items.first?.title, "12 calls in the last 24h · 1 active") XCTAssertFalse(items[0].isEnabled, "the header is a muted, non-clickable line") } func testHeaderOmitsCallCountUntilUsageLoads() { let state = Self.busyState() - state.callsThisHour = nil + state.usageTimeline = nil let section = Self.makeSection() XCTAssertEqual(section.items(for: state, now: Self.now).first?.title, "1 active") } @@ -52,7 +52,7 @@ final class GlanceSectionTests: XCTestCase { let section = Self.makeSection() XCTAssertEqual(section.items(for: state, now: Self.now).first?.title, - "12 calls this hour · 1 active · not updating") + "12 calls in the last 24h · 1 active · not updating") } /// …and stops saying so once the feeds recover, without a rebuild: the @@ -68,7 +68,7 @@ final class GlanceSectionTests: XCTestCase { state.clearGlanceFailure(.activity) XCTAssertTrue(section.updateInPlace(for: state, now: Self.now)) - XCTAssertEqual(items[0].title, "12 calls this hour · 1 active") + XCTAssertEqual(items[0].title, "12 calls in the last 24h · 1 active") } // MARK: - Recent section @@ -79,8 +79,8 @@ final class GlanceSectionTests: XCTestCase { $0.isSeparatorItem ? "—" : $0.title } XCTAssertEqual(Array(titles.prefix(7)), [ - "12 calls this hour · 1 active", - "Activity (24h) — loading…", + "12 calls in the last 24h · 1 active", + "Activity (24h)", "—", "Recent", "github:create_issue — 30s", @@ -108,13 +108,32 @@ final class GlanceSectionTests: XCTestCase { XCTAssertNotNil(items[6].action) } - func testNoActivityShowsOneMutedRow() { + /// One frame everywhere: with nothing inside the 24 hours the Recent + /// header would explain an empty list, so the section is just the door to + /// the full log. + func testNoQualifyingActivityHidesTheRecentSection() { let state = Self.busyState() state.glanceActivity = [] let section = Self.makeSection() - let row = section.items(for: state, now: Self.now)[4] - XCTAssertEqual(row.title, "No tool calls yet") - XCTAssertFalse(row.isEnabled) + let titles = section.items(for: state, now: Self.now).map(\.title) + XCTAssertFalse(titles.contains("Recent")) + XCTAssertFalse(titles.contains("No tool calls yet")) + XCTAssertTrue(titles.contains("Open Activity…"), + "the door to the full log stays") + } + + /// The frame in action: a record the log retains from days ago must not + /// sit beside a chart that says the day was quiet. + func testRowsOlderThanTheFrameAreNotShown() { + let state = Self.busyState() + state.glanceActivity = [ + Self.entry(id: "old", server: "github", tool: "create_issue", + timestamp: "2027-01-05T08:00:00Z", session: "sess-old") + ] + let section = Self.makeSection() + let titles = section.items(for: state, now: Self.now).map(\.title) + XCTAssertFalse(titles.contains { $0.hasPrefix("github:create_issue") }) + XCTAssertFalse(titles.contains("Recent")) } func testFirstClauseKeepsOnlyTheLeadingClause() { @@ -455,7 +474,7 @@ final class GlanceSectionTests: XCTestCase { let section = Self.makeSection() let row = section.items(for: state, now: Self.now)[9] - XCTAssertEqual(row.title, "No recent clients") + XCTAssertEqual(row.title, "No clients in the last 24h") XCTAssertFalse(row.isEnabled) state.glanceSessions = [ @@ -472,7 +491,7 @@ final class GlanceSectionTests: XCTestCase { state.glanceSessions = [] let section = Self.makeSection() let row = section.items(for: state, now: Self.now)[9] - XCTAssertEqual(row.title, "No recent clients") + XCTAssertEqual(row.title, "No clients in the last 24h") XCTAssertFalse(row.isEnabled) } @@ -539,7 +558,7 @@ final class GlanceSectionTests: XCTestCase { let section = Self.makeSection() let items = section.items(for: state, now: Self.now) - XCTAssertEqual(items[0].title, "12 calls this hour") + XCTAssertEqual(items[0].title, "12 calls in the last 24h") XCTAssertEqual(items[9].title, "Codex — 1 call · seen 3h") } @@ -547,8 +566,10 @@ final class GlanceSectionTests: XCTestCase { /// muted placeholder, read straight out of `items(for:)` — there is no /// submenu to open any more. func testHistogramShowsLoadingUntilUsageArrives() { + let state = Self.busyState() + state.usageTimeline = nil let section = Self.makeSection() - let histogram = section.items(for: Self.busyState(), now: Self.now)[1] + let histogram = section.items(for: state, now: Self.now)[1] XCTAssertEqual(histogram.title, "Activity (24h) — loading…") XCTAssertNil(histogram.submenu) } @@ -589,8 +610,8 @@ final class GlanceSectionTests: XCTestCase { let items = section.items(for: Self.busyState(), now: Self.now) let titles = items.map { $0.isSeparatorItem ? "—" : $0.title } XCTAssertEqual(titles, [ - "12 calls this hour · 1 active", - "Activity (24h) — loading…", + "12 calls in the last 24h · 1 active", + "Activity (24h)", "—", "Recent", "github:create_issue — 30s", @@ -610,8 +631,9 @@ final class GlanceSectionTests: XCTestCase { let section = Self.makeSection() let items = section.items(for: Self.busyState(), now: Self.now) - XCTAssertEqual(items[0].title, "12 calls this hour · 1 active") - XCTAssertEqual(items[1].title, "Activity (24h) — loading…") + XCTAssertEqual(items[0].title, "12 calls in the last 24h · 1 active") + XCTAssertEqual(items[1].title, "Activity (24h)", + "busyState's loaded timeline puts the real chart here") XCTAssertNil(items[1].submenu, "the histogram renders inline, not behind a submenu") XCTAssertTrue(items[2].isSeparatorItem) XCTAssertEqual(items[3].title, "Recent") @@ -632,10 +654,10 @@ final class GlanceSectionTests: XCTestCase { error: "auth failed: token expired. retry after refresh", timestamp: "2027-01-15T07:58:00Z", session: "sess-b") ] - state.callsThisHour = 13 + state.usageTimeline = [UsageBucket(start: Self.now, calls: 13, errors: 0, totalRespBytes: 0)] XCTAssertTrue(section.updateInPlace(for: state, now: Self.now)) - XCTAssertEqual(items[0].title, "13 calls this hour · 1 active") + XCTAssertEqual(items[0].title, "13 calls in the last 24h · 1 active") XCTAssertEqual(row.title, "obsidian:search_notes — 5s") XCTAssertEqual(row.representedObject as? String, "sess-c", "the click payload must follow the title, or the row opens the previous record's session") @@ -668,17 +690,16 @@ final class GlanceSectionTests: XCTestCase { let section = Self.makeSection() let items = section.items(for: state, now: Self.now) - state.usageTimeline = [UsageBucket(start: Self.now, calls: 12, errors: 1, totalRespBytes: 0)] - state.callsThisHour = 13 + state.usageTimeline = [UsageBucket(start: Self.now, calls: 13, errors: 0, totalRespBytes: 0)] XCTAssertTrue(section.updateInPlace(for: state, now: Self.now)) - XCTAssertEqual(items[0].title, "13 calls this hour · 1 active") + XCTAssertEqual(items[0].title, "13 calls in the last 24h · 1 active") // And it is still updating a cycle later: the freeze was for the rest // of the session, not for one tick. - state.callsThisHour = 14 + state.usageTimeline = [UsageBucket(start: Self.now, calls: 14, errors: 0, totalRespBytes: 0)] XCTAssertTrue(section.updateInPlace(for: state, now: Self.now)) - XCTAssertEqual(items[0].title, "14 calls this hour · 1 active") + XCTAssertEqual(items[0].title, "14 calls in the last 24h · 1 active") } func testUpdateInPlaceBeforeFirstBuildReportsStructural() { @@ -941,7 +962,7 @@ final class GlanceSectionTests: XCTestCase { let state = AppState() // coreState first: its didSet clears the glance feeds on any non-connected state. state.coreState = .connected - state.callsThisHour = 12 + state.usageTimeline = [UsageBucket(start: now, calls: 12, errors: 0, totalRespBytes: 0)] state.glanceActivity = [ entry(id: "a", server: "github", tool: "create_issue", timestamp: "2027-01-15T07:59:30Z", session: "sess-a"), diff --git a/native/macos/MCPProxy/MCPProxyTests/MenuOpenNetworkTests.swift b/native/macos/MCPProxy/MCPProxyTests/MenuOpenNetworkTests.swift index a887faba..c9d3d8ed 100644 --- a/native/macos/MCPProxy/MCPProxyTests/MenuOpenNetworkTests.swift +++ b/native/macos/MCPProxy/MCPProxyTests/MenuOpenNetworkTests.swift @@ -45,7 +45,7 @@ final class MenuOpenNetworkTests: XCTestCase { // menuWillOpen tomorrow fails this test whichever route it takes. state.apiClient = GlanceStubURLProtocol.makeClient() state.coreState = .connected - state.callsThisHour = 12 + state.usageTimeline = [UsageBucket(start: Date(), calls: 12, errors: 0, totalRespBytes: 0)] state.glanceActivity = [ Self.entry(id: "a", server: "github", tool: "create_issue", timestamp: "2027-01-15T07:59:30Z"), @@ -61,7 +61,7 @@ final class MenuOpenNetworkTests: XCTestCase { // The real sequence. controller.menuWillOpen(menu) // A poll lands while the user is reading the menu: the in-place branch. - state.callsThisHour = 13 + state.usageTimeline = [UsageBucket(start: Date(), calls: 13, errors: 0, totalRespBytes: 0)] controller.rebuildMenu() controller.menuDidClose(menu) @@ -97,7 +97,7 @@ final class MenuOpenNetworkTests: XCTestCase { let state = controller.appState state.coreState = .connected - state.callsThisHour = 12 + state.usageTimeline = [UsageBucket(start: Date(), calls: 12, errors: 0, totalRespBytes: 0)] state.glanceActivity = [ Self.entry(id: "a", server: "github", tool: "create_issue", timestamp: "2027-01-15T07:59:30Z") @@ -112,12 +112,12 @@ final class MenuOpenNetworkTests: XCTestCase { XCTAssertTrue(titles.contains("Recent"), "the glance block was not built at all") XCTAssertTrue(titles.contains { $0.hasPrefix("github:create_issue") }, "the activity rows were not built") - let summary = try XCTUnwrap(menu.items.first { $0.title.contains("calls this hour") }) + let summary = try XCTUnwrap(menu.items.first { $0.title.contains("calls in the last 24h") }) - state.callsThisHour = 13 + state.usageTimeline = [UsageBucket(start: Date(), calls: 13, errors: 0, totalRespBytes: 0)] controller.rebuildMenu() - XCTAssertTrue(summary.title.hasPrefix("13 calls this hour"), + XCTAssertTrue(summary.title.hasPrefix("13 calls in the last 24h"), "the open menu's rows must be rewritten in place, not left at menu-open time") XCTAssertEqual(source.totalCallCount, 0) } diff --git a/native/macos/MCPProxy/MCPProxyTests/MenuRefreshSchedulerTests.swift b/native/macos/MCPProxy/MCPProxyTests/MenuRefreshSchedulerTests.swift index c7d3eb6f..01167baa 100644 --- a/native/macos/MCPProxy/MCPProxyTests/MenuRefreshSchedulerTests.swift +++ b/native/macos/MCPProxy/MCPProxyTests/MenuRefreshSchedulerTests.swift @@ -53,7 +53,7 @@ final class MenuRefreshSchedulerTests: XCTestCase { let section = Self.makeSection() let items = section.items(for: state, now: Self.now) let header = items[0] - XCTAssertEqual(header.title, "12 calls this hour · 1 active") + XCTAssertEqual(header.title, "12 calls in the last 24h · 1 active") var rebuildGuard = MenuRebuildGuard() rebuildGuard.menuWillOpen() @@ -72,13 +72,13 @@ final class MenuRefreshSchedulerTests: XCTestCase { defer { token.cancel() } // A call lands while the user is reading the menu. - state.callsThisHour = 13 + state.usageTimeline = [UsageBucket(start: Self.now, calls: 13, errors: 0, totalRespBytes: 0)] Self.pump(.eventTracking, until: { !decisions.isEmpty }) XCTAssertEqual(decisions, [.updateInPlace], "a refresh during menu tracking must reach the section and take the in-place branch") - XCTAssertEqual(header.title, "13 calls this hour · 1 active", + XCTAssertEqual(header.title, "13 calls in the last 24h · 1 active", "the row on screen must show the new count, not the one captured at menu-open time") } From b4be59ba9fe86642e79c6e2ac97c2a83b467fea5 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Mon, 3 Aug 2026 19:03:30 +0300 Subject: [PATCH 7/7] fix(tray): align the Recent filter to the hour-aligned axis (codex critic) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex round 1 on the frame commit: Recent filtered on a raw 86,400 s cutoff while the header and chart use the hour-aligned 24-bar axis, so at e.g. 12:15 a call from yesterday 12:30 showed in Recent while the chart had already dropped its hour — recreating the contradiction one hour at a time. Recent now keeps a run only while its HOUR is still on the axis, the same rule ActivityHistogram.bars and callsInLast24Hours apply. Its other finding — the poll/SSE snapshot watermark race — is the pre-existing, deliberate GH-#934 trade-off (boundary = issue time; transient over-count self-corrects at the next 30 s poll), unchanged. --- .../MCPProxy/Menu/Glance/GlanceSection.swift | 17 ++++++++++------- .../MCPProxyTests/GlanceSectionTests.swift | 18 ++++++++++++++++++ 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/native/macos/MCPProxy/MCPProxy/Menu/Glance/GlanceSection.swift b/native/macos/MCPProxy/MCPProxy/Menu/Glance/GlanceSection.swift index 5edd417a..ea88d605 100644 --- a/native/macos/MCPProxy/MCPProxy/Menu/Glance/GlanceSection.swift +++ b/native/macos/MCPProxy/MCPProxy/Menu/Glance/GlanceSection.swift @@ -323,15 +323,18 @@ final class GlanceSection { private static func overflowTitle(_ hidden: Int) -> String { "+\(hidden) more" } /// The Recent section's rows: qualifying runs whose newest record falls - /// inside the menu's ONE frame — `GlancePresence.lookback`, the same 24 - /// hours the histogram draws and the header counts. A record the log still - /// retains from days ago must not sit beside a chart that says the day was - /// quiet (the inconsistency this frame exists to end). An unparseable - /// timestamp keeps its row: showing it is the safer failure. + /// inside the menu's ONE frame — the same HOUR-ALIGNED 24-bar axis the + /// histogram draws and `glanceCallsLast24h` sums, not a raw 86,400-second + /// cutoff. The two must agree at the edge: a run 23 h 40 m old whose hour + /// has already slid off the axis showing in Recent would recreate, one + /// hour at a time, the very contradiction this frame exists to end. A + /// record the log still retains from days ago never appears; an + /// unparseable timestamp keeps its row — showing it is the safer failure. private static func recentRuns(for state: AppState, now: Date) -> [GlanceRun] { - GlanceSelection.activityRows(from: state.glanceActivity).filter { run in + let oldestHour = AppState.floorToHour(now).addingTimeInterval(-23 * 3600) + return GlanceSelection.activityRows(from: state.glanceActivity).filter { run in guard let at = GlanceFormatting.parseTimestamp(run.timestamp) else { return true } - return now.timeIntervalSince(at) <= GlancePresence.lookback + return AppState.floorToHour(at) >= oldestHour } } diff --git a/native/macos/MCPProxy/MCPProxyTests/GlanceSectionTests.swift b/native/macos/MCPProxy/MCPProxyTests/GlanceSectionTests.swift index a16c7acc..c954d193 100644 --- a/native/macos/MCPProxy/MCPProxyTests/GlanceSectionTests.swift +++ b/native/macos/MCPProxy/MCPProxyTests/GlanceSectionTests.swift @@ -136,6 +136,24 @@ final class GlanceSectionTests: XCTestCase { XCTAssertFalse(titles.contains("Recent")) } + /// The frame is the histogram's HOUR-ALIGNED axis, not a raw 24×3600 + /// cutoff: a call 23 h 30 m old whose hour has already slid off the axis + /// is invisible to the chart and the header, so a Recent row for it would + /// recreate the contradiction one hour at a time. + func testARowWhoseHourSlidOffTheAxisIsHiddenWithIt() { + let state = Self.busyState() + state.glanceActivity = [ + // 23 h 30 m before `now` (08:00) — inside 86,400 s, but its hour + // (yesterday 08:00) is older than the axis's oldest (09:00). + Self.entry(id: "edge", server: "github", tool: "create_issue", + timestamp: "2027-01-14T08:30:00Z", session: "sess-edge") + ] + let section = Self.makeSection() + let titles = section.items(for: state, now: Self.now).map(\.title) + XCTAssertFalse(titles.contains { $0.hasPrefix("github:create_issue") }, + "Recent and the chart must agree at the window's edge") + } + func testFirstClauseKeepsOnlyTheLeadingClause() { XCTAssertEqual(GlanceSection.firstClause(of: "auth failed: token expired"), "auth failed") XCTAssertEqual(GlanceSection.firstClause(of: " boom "), "boom")