diff --git a/native/macos/MCPProxy/MCPProxy/MCPProxyApp.swift b/native/macos/MCPProxy/MCPProxy/MCPProxyApp.swift index 9bbaeca2..70907673 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 @@ -903,25 +898,42 @@ 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 ?? "" 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 + // 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) + submenu.addItem(item) } + parent.submenu = submenu + menu.addItem(parent) menu.addItem(.separator()) } diff --git a/native/macos/MCPProxy/MCPProxy/Menu/Glance/ActivityHistogramView.swift b/native/macos/MCPProxy/MCPProxy/Menu/Glance/ActivityHistogramView.swift index 00b7028f..689132a3 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 { @@ -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,12 +211,12 @@ 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 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 +235,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/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 1342a0a9..ea88d605 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 @@ -49,21 +51,19 @@ 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 /// 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. /// @@ -122,16 +122,23 @@ 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? - - /// 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? + /// 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 + /// 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]? + /// 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 @@ -155,33 +162,37 @@ final class GlanceSection { activityRows = [] clientRows = [] clientOverflowItem = nil - histogramItem = nil hasBuilt = true builtVisible = isVisible(for: state) guard builtVisible else { return [] } 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 - // 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")) - 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) @@ -234,18 +245,25 @@ 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 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 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, @@ -266,12 +284,11 @@ 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 } - 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. @@ -283,6 +300,9 @@ final class GlanceSection { let title = Self.overflowTitle(presence.hidden) if clientOverflowItem.title != title { clientOverflowItem.title = title } } + refreshHistogramRow(with: ActivityHistogram.state(timeline: state.usageTimeline, + errorMessage: state.usageError, + now: now)) return true } @@ -302,6 +322,22 @@ 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 — 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] { + 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 AppState.floorToHour(at) >= oldestHour + } + } + /// 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. @@ -357,34 +393,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 @@ -416,11 +453,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) } @@ -557,7 +601,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. /// @@ -644,36 +688,140 @@ final class GlanceSection { // MARK: Histogram - /// The "Activity (24h)" item and its (initially empty) submenu. + /// 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'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 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): + // 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 { + return cached + } + let item = histogramChartItemFactory(bars) + histogramRow = item + builtHistogramBars = bars + builtHistogramTimeZoneID = TimeZone.current.identifier + return item + } + } + + /// 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(let bars): + return bars.allSatisfy { $0.total == 0 } ? .idle : .chart + } + } + + /// 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, let builtKind = builtHistogramKind else { return } + 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 (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, kind: HistogramRowKind, to item: NSMenuItem + ) { + switch kind { + case .loading: + Self.setMutedTitle("Activity (24h) — loading…", on: item) + item.toolTip = nil + case .failed: + Self.setMutedTitle("Activity (24h) unavailable", on: item) + 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` + /// leaves `title` intact, so the plain string stays available to tests and + /// to accessibility. + static func mutedItem(_ title: String) -> NSMenuItem { + // 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 + ]) + } + // MARK: Header /// The header line, plus an admission when the numbers in it have stopped @@ -693,11 +841,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 21a41c7f..9a3b777a 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,255 @@ 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: 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 let item = items.first(where: { $0.title == "Activity (24h)" }) else { - XCTFail("no Activity (24h) item in the block") + 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 item + return items[separator - 1] } - /// 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) + // MARK: - The three kinds + + func testLoadingRowWhileTheTimelineIsNil() { + let row = histogramRow(makeSection(), connectedState()) + + 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) } - /// 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()) + func testErrorRowWhenTheFetchFailedBeforeAnyTimelineArrived() { + let state = connectedState() + state.recordUsageFailure("connection refused") - XCTAssertEqual(item.submenu?.numberOfItems, 0) + let row = histogramRow(makeSection(), state) + + 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) } - /// `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()) + /// 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)] + + let row = histogramRow(makeSection(), state) - XCTAssertNotNil(item.submenu?.delegate, - "the section must retain the submenu delegate") + XCTAssertEqual(row.title, "CHART:24", "the factory receives the shaped 24-hour axis") + XCTAssertNil(row.submenu) } - /// 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() { + /// Real data beats a stale failure. + func testChartRowWinsOverAStaleFailure() { + let state = connectedState() + state.recordUsageFailure("connection refused") + state.usageTimeline = [Fixture.bucket(start: Fixture.currentHour, calls: 3, errors: 1)] + + let row = histogramRow(makeSection(), state) + + XCTAssertEqual(row.title, "CHART:24") + } + + /// A timeline that arrives while the menu sits closed is charted by the + /// next rebuild — `menuWillOpen` runs one before the menu is drawn, so the + /// next open never shows a stale loading row. + func testATimelineArrivingWhileClosedIsChartedOnTheNextRebuild() { let section = makeSection() - let item = histogramItem(section, connectedState()) + let state = connectedState() + XCTAssertEqual(histogramRow(section, state).title, "Activity (24h) — loading…") - let delegate = item.submenu?.delegate - XCTAssertNotNil(delegate) - XCTAssertFalse(delegate === Self.clickStub) - XCTAssertFalse(delegate === section as AnyObject) + state.usageTimeline = [Fixture.bucket(start: Fixture.currentHour, calls: 3, errors: 1)] + + XCTAssertEqual(histogramRow(section, state).title, "CHART:24") } - func testLoadingRowWhileTheTimelineIsNil() { - let menu = histogramItem(makeSection(), connectedState()).submenu! + /// 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 = [] - open(menu) + let row = histogramRow(makeSection(), state) - 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(attributes[.foregroundColor] as? NSColor, NSColor.secondaryLabelColor) + XCTAssertEqual(row.title, GlanceSection.idleHistogramTitle) + XCTAssertFalse(row.isEnabled) + XCTAssertNil(row.submenu) } - func testErrorRowWhenTheFetchFailedBeforeAnyTimelineArrived() { + /// 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.recordUsageFailure("connection refused") - let menu = histogramItem(makeSection(), state).submenu! + state.usageTimeline = [Fixture.bucket(start: Fixture.currentHour, calls: 0, errors: 0)] + + XCTAssertEqual(histogramRow(makeSection(), state).title, + GlanceSection.idleHistogramTitle) + } - open(menu) + /// 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…") - 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(attributes[.foregroundColor] as? NSColor, NSColor.secondaryLabelColor) + state.usageTimeline = [] + + XCTAssertTrue(section.updateInPlace(for: state, now: Fixture.now)) + XCTAssertEqual(row.title, GlanceSection.idleHistogramTitle) } - /// 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() { + // MARK: - The eager-build cache + + /// `items(for:)` runs on every debounced rebuild, menu open or closed; the + /// chart must not be re-rendered when the shaped axis has not moved. + func testTheChartItemIsCachedAcrossRebuildsWithUnchangedBars() { + let section = makeBareSection() + var factoryCalls = 0 + section.histogramChartItemFactory = { bars in + factoryCalls += 1 + return NSMenuItem(title: "CHART:\(bars.count)", action: nil, keyEquivalent: "") + } let state = connectedState() state.usageTimeline = [Fixture.bucket(start: Fixture.currentHour, calls: 3, errors: 1)] - for _ in 0..