From 38c536225dbd14c1ee6257fbeebfa4096f504ba1 Mon Sep 17 00:00:00 2001 From: shanforge <299346208+shanforge@users.noreply.github.com> Date: Fri, 21 Aug 2026 10:00:51 +0530 Subject: [PATCH 01/12] feat(island): decide the island's surface rather than picking one colour The prompt pill was filled with a fixed near-opaque dark colour and the transcript above it used .ultraThinMaterial, so one island rendered in two materials with the seam between them visible whenever both were on screen. Worse, nothing pinned the island's colour scheme. Every foreground in it is white, and Material is not: in Light appearance .ultraThinMaterial is a light frost, so the transcript drew white text on a white-ish panel. IslandSurface is the rule, free of SwiftUI so the matrix is testable without mounting a view. Reduce Transparency stops the island being glass at all rather than making it slightly less glass, and takes the shadow with it; Increase Contrast may only ever deepen the veil and harden the edge, never lighten either; and the scrim can never fall below a floor, because glass with no veil over a white document is white text on a white panel. Parked on its own branch: this is #62 (part 2), not part 1. Part of #62. --- Logue.xcodeproj/project.pbxproj | 8 ++ Logue/CrossApp/IslandSurface.swift | 79 +++++++++++++++++++ LogueTests/IslandSurfaceTests.swift | 116 ++++++++++++++++++++++++++++ 3 files changed, 203 insertions(+) create mode 100644 Logue/CrossApp/IslandSurface.swift create mode 100644 LogueTests/IslandSurfaceTests.swift diff --git a/Logue.xcodeproj/project.pbxproj b/Logue.xcodeproj/project.pbxproj index b1347b6..f9a5b9e 100644 --- a/Logue.xcodeproj/project.pbxproj +++ b/Logue.xcodeproj/project.pbxproj @@ -447,6 +447,7 @@ A0E411A858F7CAF3DB117E8A /* LogueLogoLight.svg in Resources */ = {isa = PBXBuildFile; fileRef = C8F833920208ECD78C193568 /* LogueLogoLight.svg */; }; A11728D4726E858C626D681E /* CaptureSegmentTimelineTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D2856879E369BF8160EBAA1B /* CaptureSegmentTimelineTests.swift */; }; A138EE66F6C6808592F6F945 /* MarkdownDocumentFileTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B57C9B916A88B9AC13B95DA7 /* MarkdownDocumentFileTests.swift */; }; + A1A603E973000958C89D1EDF /* IslandSurfaceTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 464AB56546E36263CEB595E8 /* IslandSurfaceTests.swift */; }; A1D7482C98EE0833CBA17214 /* DocumentCardView.swift in Sources */ = {isa = PBXBuildFile; fileRef = AB549E1BD73D85EA486FAF7C /* DocumentCardView.swift */; }; A2E046B0B3D7991E9F91C9D1 /* Markdown in Frameworks */ = {isa = PBXBuildFile; productRef = 8556F56C6CC2D01AD3733177 /* Markdown */; }; A30AAC62CDAE227B36E45511 /* TextAnalysisRequestTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8D2D61B85F962E88AA1E5062 /* TextAnalysisRequestTests.swift */; }; @@ -646,6 +647,7 @@ EAB17DC89BEDD8391BA0D76E /* Toast.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0E78077A2EACC3DD4364C4CC /* Toast.swift */; }; EB4B67B6541ECF5EDB93F8AA /* InlineSpellEngine.swift in Sources */ = {isa = PBXBuildFile; fileRef = D6829731778689289CE48545 /* InlineSpellEngine.swift */; }; EC09A9CE12598BC04D96481A /* DocsHomeView.swift in Sources */ = {isa = PBXBuildFile; fileRef = A8ACC65E14034118E968BBE6 /* DocsHomeView.swift */; }; + EC1F72AE3061CEF89F456398 /* IslandSurface.swift in Sources */ = {isa = PBXBuildFile; fileRef = EF62C254D50D305971D3FCC9 /* IslandSurface.swift */; }; EC4770E6775F3DD9A8168148 /* DeepLink.swift in Sources */ = {isa = PBXBuildFile; fileRef = E91B8420A88FFD5D29FE6638 /* DeepLink.swift */; }; ECB8B236B80423C7D1A752FF /* AudioFileChunkReader.swift in Sources */ = {isa = PBXBuildFile; fileRef = AF31DF6705AF2BA5E32A51F9 /* AudioFileChunkReader.swift */; }; ED1971AC239DE930D076B294 /* ActionItemInbox.swift in Sources */ = {isa = PBXBuildFile; fileRef = 934F6981002CF41976F94D99 /* ActionItemInbox.swift */; }; @@ -903,6 +905,7 @@ 453FB30A414ED333A0858FF2 /* CategoryGradeCard.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CategoryGradeCard.swift; sourceTree = ""; }; 461D5E9CBBBFA62E23630E08 /* MeetingSpeakersPanelView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MeetingSpeakersPanelView.swift; sourceTree = ""; }; 463F7963624BB9B6B962EAAB /* RecordingSessionManager+Checkpoint.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "RecordingSessionManager+Checkpoint.swift"; sourceTree = ""; }; + 464AB56546E36263CEB595E8 /* IslandSurfaceTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = IslandSurfaceTests.swift; sourceTree = ""; }; 46B853F7D5C62F407E23AD41 /* OverviewRightColumnCards.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OverviewRightColumnCards.swift; sourceTree = ""; }; 46D5AD1551B13733D986BC37 /* NavigationHistoryTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NavigationHistoryTests.swift; sourceTree = ""; }; 46D6419EB91D515BAD26FEF7 /* ScheduledTaskManager+WeeklyReview.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "ScheduledTaskManager+WeeklyReview.swift"; sourceTree = ""; }; @@ -1366,6 +1369,7 @@ EE6D9693B41B9103DAAAB103 /* MeetingNote+Speakers.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "MeetingNote+Speakers.swift"; sourceTree = ""; }; EE89A8A3A959A42FAA7802BA /* InputDeviceKind.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = InputDeviceKind.swift; sourceTree = ""; }; EEBFA9476AB8E2BDA7306EAB /* MarkdownFrontmatterTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MarkdownFrontmatterTests.swift; sourceTree = ""; }; + EF62C254D50D305971D3FCC9 /* IslandSurface.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = IslandSurface.swift; sourceTree = ""; }; EF7B96F52DB280D23D637F33 /* MenuBarIcon.png */ = {isa = PBXFileReference; lastKnownFileType = image.png; path = MenuBarIcon.png; sourceTree = ""; }; EFE448FDFB9D86AB13035E23 /* ReleaseEntitlementsTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ReleaseEntitlementsTests.swift; sourceTree = ""; }; F06E61F5499F68059FF8775B /* TableCellTextView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TableCellTextView.swift; sourceTree = ""; }; @@ -1519,6 +1523,7 @@ F773F30D16F4FF92E56320E1 /* HomeContinueGridTests.swift */, A3600798EF27A735FC4BC80E /* HomeSuggestionsTests.swift */, 0062B04D59734428AFC49682 /* ImportFileReadingTests.swift */, + 464AB56546E36263CEB595E8 /* IslandSurfaceTests.swift */, 402D8E4447C9B7063ADE7358 /* IslandThreadTests.swift */, 7F48D46D3D40534008356F5D /* LinkIndexBuildTests.swift */, F3BC206B7D135B2017B99C1D /* LinkIndexTests.swift */, @@ -1728,6 +1733,7 @@ 865AE91088D3794F9C1CC457 /* CommandCenterChatRule.swift */, C8D8F63B0BEBD964C1409AA5 /* CommandCenterController.swift */, DD62B3D8FB4F2050661CE197 /* InlineAssistant.swift */, + EF62C254D50D305971D3FCC9 /* IslandSurface.swift */, E3AAB73BD6A8A6AF6C54DC29 /* IslandThread.swift */, 29E0387E99032708C27EA914 /* KeyboardShortcut.swift */, B3F9D423BBCD2E5ED6C361AA /* MenuBarCompanion.swift */, @@ -2743,6 +2749,7 @@ B68578CA8ABFB343EABB454C /* HomeContinueGridTests.swift in Sources */, 549C3D0D72BD69E53B7929A5 /* HomeSuggestionsTests.swift in Sources */, D0FFBA721311ED3F007A7D49 /* ImportFileReadingTests.swift in Sources */, + A1A603E973000958C89D1EDF /* IslandSurfaceTests.swift in Sources */, 2F4BA10FF02C91BDC115F449 /* IslandThreadTests.swift in Sources */, 5649718294F9E5770348BD76 /* LLMTestEvalExtensions.swift in Sources */, D480184376B6F26A9755133A /* LLMTestHarness.swift in Sources */, @@ -3058,6 +3065,7 @@ 9FC5E9AC4D7755C95151D036 /* InsightCardShell.swift in Sources */, 8621E76EC5A20A48091893C4 /* InsightsSectionView.swift in Sources */, 3C8B6B59AC5A90F65186C43C /* InsightsStatsProvider.swift in Sources */, + EC1F72AE3061CEF89F456398 /* IslandSurface.swift in Sources */, CF68588BF9879F2449BC4BEE /* IslandThread.swift in Sources */, 56F4853038E368546DCD2955 /* JavaScriptTool.swift in Sources */, CE5A69A3A3CBC569F9D8DA57 /* JumpToLatestPill.swift in Sources */, diff --git a/Logue/CrossApp/IslandSurface.swift b/Logue/CrossApp/IslandSurface.swift new file mode 100644 index 0000000..d405447 --- /dev/null +++ b/Logue/CrossApp/IslandSurface.swift @@ -0,0 +1,79 @@ +import Foundation + +/// How the Command Center island paints itself over a desktop it knows nothing about. +/// +/// The island is the only surface in Logue with no control over its own background. A window +/// sits on the app's own canvas; the island sits on whatever the user happens to have behind +/// it — a white document, a photograph, a video playing at full brightness. So the two things +/// that make a floating panel readable have to be decided rather than picked once and hoped +/// for. +/// +/// **Why the prompt pill was a slab.** It used to be filled with a fixed near-opaque dark +/// colour, which is legible everywhere and looks like a rectangle stuck to the screen. The +/// transcript above it used `.ultraThinMaterial` and looked like glass. One island, two +/// materials, and the seam between them visible whenever both were on screen. +/// +/// **Why the colour scheme is pinned rather than adaptive.** Every foreground in the island is +/// white — the bubbles, the placeholder, the icons, the chips. `Material` is not: in Light +/// appearance `.ultraThinMaterial` is a light frost, so the transcript rendered white text on +/// a white-ish panel and simply could not be read. Half a palette adapting is worse than none +/// adapting, and this is a HUD over someone else's window rather than a document, so the +/// island commits to dark and every white in it is then correct by construction. That is what +/// makes it legible in *both* system appearances: it does not change with them. +/// +/// Kept free of SwiftUI so the matrix below is testable without mounting a view — the same +/// reason `CommandCenterChatRule` is free of AppKit. +enum IslandSurface { + /// What to paint, for one set of accessibility settings. + /// + /// Opacities rather than colours: the colour is the island's, and a rule that returned + /// `Color` could not be tested without SwiftUI. + struct Treatment: Equatable { + /// Whether the desktop is allowed to show through at all. + let usesMaterial: Bool + /// The veil laid over the material. This is what buys contrast against a bright + /// wallpaper; without it, glass alone leaves white text sitting on white. + let scrimOpacity: Double + /// The hairline that separates the island from whatever is behind it. Against a + /// dark desktop the island's edge is otherwise invisible and it reads as a hole. + let strokeOpacity: Double + /// Lifts the island off the desktop. Dropped when transparency is reduced, because + /// a soft shadow under an opaque panel is the effect that setting exists to remove. + let shadowOpacity: Double + } + + /// The scrim can never go below this, whatever else is set. + /// + /// Glass with no veil is the failure this type exists to prevent: over a white document + /// `.ultraThinMaterial` is nearly white, and the island's white text disappears into it. + static let minimumScrimOpacity: Double = 0.22 + + /// What the island should paint. + /// + /// - Parameters: + /// - reduceTransparency: System Settings → Accessibility → Display → Reduce + /// transparency. The user has asked for no see-through surfaces, so the island stops + /// being glass entirely rather than becoming slightly-less-glass. + /// - increaseContrast: Accessibility → Display → Increase contrast. Deepens the veil + /// and hardens the edge; it never lightens either. + static func treatment( + reduceTransparency: Bool, + increaseContrast: Bool + ) -> Treatment { + guard !reduceTransparency else { + // Fully opaque, and the shadow goes with it. + return Treatment( + usesMaterial: false, + scrimOpacity: 1, + strokeOpacity: increaseContrast ? 0.9 : 0.35, + shadowOpacity: 0 + ) + } + return Treatment( + usesMaterial: true, + scrimOpacity: increaseContrast ? 0.55 : 0.3, + strokeOpacity: increaseContrast ? 0.55 : 0.12, + shadowOpacity: 0.35 + ) + } +} diff --git a/LogueTests/IslandSurfaceTests.swift b/LogueTests/IslandSurfaceTests.swift new file mode 100644 index 0000000..2977716 --- /dev/null +++ b/LogueTests/IslandSurfaceTests.swift @@ -0,0 +1,116 @@ +import Foundation +import Testing + +@testable import Logue + +/// The island's surface rule, which decides whether the desktop shows through and how hard +/// the veil over it has to work. +/// +/// Worth pinning because every case here is invisible in the one configuration a developer +/// actually runs — transparency on, contrast standard, a dark wallpaper. The settings that +/// break it are ones you have to go and turn on. +@Suite("IslandSurface") +struct IslandSurfaceTests { + private func treatment( + reduceTransparency: Bool = false, + increaseContrast: Bool = false + ) -> IslandSurface.Treatment { + IslandSurface.treatment( + reduceTransparency: reduceTransparency, + increaseContrast: increaseContrast + ) + } + + // MARK: - Reduce transparency + + @Test("Reduce transparency stops the island being glass at all") + func reducedTransparencyIsOpaque() { + // Not "less transparent" — the setting is a request for no see-through surfaces, and + // a panel that is merely 70% opaque still shows the desktop moving underneath it. + let reduced = treatment(reduceTransparency: true) + #expect(reduced.usesMaterial == false) + #expect(reduced.scrimOpacity == 1) + } + + @Test("An opaque island casts no shadow") + func reducedTransparencyDropsTheShadow() { + // A soft shadow is the same visual effect the setting exists to remove, so keeping it + // under an opaque panel honours the letter of the setting and not the point of it. + #expect(treatment(reduceTransparency: true).shadowOpacity == 0) + #expect(treatment().shadowOpacity > 0) + } + + @Test("Glass is the default") + func defaultIsGlass() { + let standard = treatment() + #expect(standard.usesMaterial) + #expect(standard.scrimOpacity < 1, "the desktop has to show through for it to be glass") + } + + // MARK: - The scrim floor + + @Test("Every treatment keeps the scrim above the floor") + func scrimNeverFallsBelowTheFloor() { + // The failure this guards: glass with no veil over a white document is white text on + // a white panel. Delete the scrim from `treatment` and this goes red for every case. + for reduceTransparency in [false, true] { + for increaseContrast in [false, true] { + let result = treatment( + reduceTransparency: reduceTransparency, + increaseContrast: increaseContrast + ) + #expect( + result.scrimOpacity >= IslandSurface.minimumScrimOpacity, + "transparency=\(reduceTransparency) contrast=\(increaseContrast)" + ) + } + } + } + + // MARK: - Increase contrast + + @Test("Increase contrast never lightens anything") + func increasedContrastOnlyEverDeepens() { + // Stated as a comparison rather than as numbers so that retuning the palette cannot + // quietly invert the setting — which is the mistake that would be hardest to notice, + // since the island still looks fine to anyone who has contrast off. + for reduceTransparency in [false, true] { + let standard = treatment(reduceTransparency: reduceTransparency) + let increased = treatment(reduceTransparency: reduceTransparency, increaseContrast: true) + #expect(increased.scrimOpacity >= standard.scrimOpacity) + #expect(increased.strokeOpacity > standard.strokeOpacity) + } + } + + @Test("The edge is always drawn") + func theEdgeIsAlwaysVisible() { + // Against a dark desktop an island with no stroke has no boundary and reads as a hole + // cut in the screen rather than a panel floating over it. + for reduceTransparency in [false, true] { + for increaseContrast in [false, true] { + let result = treatment( + reduceTransparency: reduceTransparency, + increaseContrast: increaseContrast + ) + #expect(result.strokeOpacity > 0) + } + } + } + + // MARK: - Bounds + + @Test("No opacity is outside 0...1") + func opacitiesAreInRange() { + for reduceTransparency in [false, true] { + for increaseContrast in [false, true] { + let result = treatment( + reduceTransparency: reduceTransparency, + increaseContrast: increaseContrast + ) + #expect((0 ... 1).contains(result.scrimOpacity)) + #expect((0 ... 1).contains(result.strokeOpacity)) + #expect((0 ... 1).contains(result.shadowOpacity)) + } + } + } +} From 58061b5bdbd32760772117165c2892eb023ee5b4 Mon Sep 17 00:00:00 2001 From: shanforge <299346208+shanforge@users.noreply.github.com> Date: Fri, 21 Aug 2026 10:23:55 +0530 Subject: [PATCH 02/12] feat(island): one glass surface, and a colour scheme it can rely on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both panels now paint through IslandSurface, so the island stops rendering in two materials — the prompt pill was filled with a fixed near-opaque colour while the transcript directly above it was frosted, and the seam showed whenever both were up. The legibility bug underneath it: nothing pinned the island's colour scheme. Every foreground in it is white and Material is not, so in Light appearance .ultraThinMaterial resolved to a light frost and the transcript drew white text on a white-ish panel. The island now commits to dark rather than half its palette adapting — it is a HUD over someone else's window, not a document — which is what makes it legible whichever appearance the system is in. Reduce Transparency and Increase Contrast are honoured for the first time here; the rule and its cases came with the previous commit. Part of #62. --- Logue.xcodeproj/project.pbxproj | 4 ++ .../CommandCenterChatView+Composer.swift | 10 +--- .../CrossApp/CommandCenterChatView.swift | 16 +++--- .../CrossApp/IslandSurfaceModifier.swift | 54 +++++++++++++++++++ 4 files changed, 67 insertions(+), 17 deletions(-) create mode 100644 Logue/Views/CrossApp/IslandSurfaceModifier.swift diff --git a/Logue.xcodeproj/project.pbxproj b/Logue.xcodeproj/project.pbxproj index f9a5b9e..ac19c11 100644 --- a/Logue.xcodeproj/project.pbxproj +++ b/Logue.xcodeproj/project.pbxproj @@ -306,6 +306,7 @@ 68E15F52C2575EED30F6D8AE /* TaskFileTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B3B356E5C7937D4D7600108E /* TaskFileTests.swift */; }; 693843E5E8F7A5A45AAD99BA /* InlineDiagramView.swift in Sources */ = {isa = PBXBuildFile; fileRef = C2A85CBFCD11CDF2FB693F73 /* InlineDiagramView.swift */; }; 697553876307B2C83EA86595 /* ReviewModels.swift in Sources */ = {isa = PBXBuildFile; fileRef = DE44EB0DEEE58EA51F73402B /* ReviewModels.swift */; }; + 69B2689283C249CBE8252ADE /* IslandSurfaceModifier.swift in Sources */ = {isa = PBXBuildFile; fileRef = 03E127468D8DFBEDBF111016 /* IslandSurfaceModifier.swift */; }; 69C377EFFD3B01B060B81835 /* SpeakerAlignment.swift in Sources */ = {isa = PBXBuildFile; fileRef = CD2339649C1FDB68C5155E1A /* SpeakerAlignment.swift */; }; 6A0C82EEC2ED811BD717F3D8 /* String+MarkdownStripping.swift in Sources */ = {isa = PBXBuildFile; fileRef = A164C7557EDE48648C650F46 /* String+MarkdownStripping.swift */; }; 6A4AA026F9B378C593F2EB60 /* DateGroupingHelper.swift in Sources */ = {isa = PBXBuildFile; fileRef = A4623149A8D1BB07A0CB7B26 /* DateGroupingHelper.swift */; }; @@ -726,6 +727,7 @@ 01DF49AA771F4050417643F2 /* DocumentType.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DocumentType.swift; sourceTree = ""; }; 0252C590E08043FE0A4200C7 /* MainWindowView+Navigation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "MainWindowView+Navigation.swift"; sourceTree = ""; }; 026499B13ACB9A5937CCC44A /* CommandCenterChatView+Composer.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "CommandCenterChatView+Composer.swift"; sourceTree = ""; }; + 03E127468D8DFBEDBF111016 /* IslandSurfaceModifier.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = IslandSurfaceModifier.swift; sourceTree = ""; }; 05509BDEDCFFC620AB9B8777 /* TranscriptTimelineView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TranscriptTimelineView.swift; sourceTree = ""; }; 058B2966E8BD330957460206 /* TranscriptTimelineView+Bookmarks.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "TranscriptTimelineView+Bookmarks.swift"; sourceTree = ""; }; 06255FC9EEF089BB08B5232B /* whatsnew-writing-editor.png */ = {isa = PBXFileReference; lastKnownFileType = image.png; path = "whatsnew-writing-editor.png"; sourceTree = ""; }; @@ -2336,6 +2338,7 @@ E7300054D056065027CC1FF3 /* CommandCenterNewMeetingView.swift */, 24578E3A172E90DC703D4C9D /* CommandCenterPIIView.swift */, 6445D0008F16B2DFE083F469 /* CommandCenterRecordingView.swift */, + 03E127468D8DFBEDBF111016 /* IslandSurfaceModifier.swift */, A297CA8A1434241BA7F14120 /* QuickComposeView.swift */, 6410A54BE5400DAAFADA2A88 /* SharedPolishComponents.swift */, 14A204E44FBCA2412FF88EFA /* WritingDiffView.swift */, @@ -3066,6 +3069,7 @@ 8621E76EC5A20A48091893C4 /* InsightsSectionView.swift in Sources */, 3C8B6B59AC5A90F65186C43C /* InsightsStatsProvider.swift in Sources */, EC1F72AE3061CEF89F456398 /* IslandSurface.swift in Sources */, + 69B2689283C249CBE8252ADE /* IslandSurfaceModifier.swift in Sources */, CF68588BF9879F2449BC4BEE /* IslandThread.swift in Sources */, 56F4853038E368546DCD2955 /* JavaScriptTool.swift in Sources */, CE5A69A3A3CBC569F9D8DA57 /* JumpToLatestPill.swift in Sources */, diff --git a/Logue/Views/CrossApp/CommandCenterChatView+Composer.swift b/Logue/Views/CrossApp/CommandCenterChatView+Composer.swift index ac1b430..b81d3d5 100644 --- a/Logue/Views/CrossApp/CommandCenterChatView+Composer.swift +++ b/Logue/Views/CrossApp/CommandCenterChatView+Composer.swift @@ -128,15 +128,7 @@ extension CommandCenterChatView { .padding(.leading, 14) .padding(.trailing, 10) .padding(.vertical, 10) - .background( - RoundedRectangle(cornerRadius: 22, style: .continuous) - .fill(Color(nsColor: NSColor(red: 0.11, green: 0.11, blue: 0.12, alpha: 0.92))) - ) - .overlay( - RoundedRectangle(cornerRadius: 22, style: .continuous) - .stroke(Color.white.opacity(0.07), lineWidth: 0.5) - ) - .shadow(color: .black.opacity(0.35), radius: 30, y: 12) + .islandSurface(cornerRadius: 22) .overlay(alignment: .top) { if !attachments.isEmpty || isWebSearchOnce || isDeepResearchOnce { attachmentChips diff --git a/Logue/Views/CrossApp/CommandCenterChatView.swift b/Logue/Views/CrossApp/CommandCenterChatView.swift index ef5e27d..6556669 100644 --- a/Logue/Views/CrossApp/CommandCenterChatView.swift +++ b/Logue/Views/CrossApp/CommandCenterChatView.swift @@ -110,6 +110,13 @@ struct CommandCenterChatView: View { promptPill } .frame(width: pillWidth) + // The island commits to dark. Every foreground in it is white and `Material` is not: + // in Light appearance `.ultraThinMaterial` is a light frost, so the transcript drew + // white text on a white-ish panel and could not be read. Half a palette adapting is + // worse than none, and this is a HUD over someone else's window rather than a + // document — so it does not follow the system, which is what makes it legible in + // both appearances. + .environment(\.colorScheme, .dark) .animation(.spring(response: 0.35, dampingFraction: 0.85), value: hasContent) .onAppear { isInputFocused = true @@ -257,14 +264,7 @@ struct CommandCenterChatView: View { } .frame(width: pillWidth) .frame(maxHeight: 420) - .background( - .ultraThinMaterial, - in: RoundedRectangle(cornerRadius: 18, style: .continuous) - ) - .overlay( - RoundedRectangle(cornerRadius: 18, style: .continuous) - .stroke(Color.white.opacity(0.08), lineWidth: 0.5) - ) + .islandSurface(cornerRadius: 18) } // MARK: - Logic diff --git a/Logue/Views/CrossApp/IslandSurfaceModifier.swift b/Logue/Views/CrossApp/IslandSurfaceModifier.swift new file mode 100644 index 0000000..012bcc4 --- /dev/null +++ b/Logue/Views/CrossApp/IslandSurfaceModifier.swift @@ -0,0 +1,54 @@ +import SwiftUI + +/// Paints a panel of the island, following `IslandSurface`. +/// +/// The rule lives next door, free of SwiftUI, so what to paint is testable; this is only how. +/// Both of the island's panels use it so they cannot drift into two materials again — which +/// is what they had, the pill filled with a fixed colour and the transcript above it frosted. +extension View { + func islandSurface(cornerRadius: CGFloat) -> some View { + modifier(IslandSurfaceModifier(cornerRadius: cornerRadius)) + } +} + +private struct IslandSurfaceModifier: ViewModifier { + let cornerRadius: CGFloat + + @Environment(\.accessibilityReduceTransparency) private var reduceTransparency + @Environment(\.colorSchemeContrast) private var contrast + + /// The colour the veil is made of — the island's own near-black, which is what the prompt + /// pill used to be filled with outright. + private static let base = Color( + nsColor: NSColor(red: 0.11, green: 0.11, blue: 0.12, alpha: 1) + ) + + private var treatment: IslandSurface.Treatment { + IslandSurface.treatment( + reduceTransparency: reduceTransparency, + increaseContrast: contrast == .increased + ) + } + + func body(content: Content) -> some View { + let shape = RoundedRectangle(cornerRadius: cornerRadius, style: .continuous) + content + .background { + ZStack { + if treatment.usesMaterial { + shape.fill(.ultraThinMaterial) + } + // Always drawn. With the material it is the veil that buys contrast over a + // bright wallpaper; without it, it is the whole background. + shape.fill(Self.base.opacity(treatment.scrimOpacity)) + } + } + .overlay( + shape.strokeBorder( + Color.white.opacity(treatment.strokeOpacity), + lineWidth: 0.5 + ) + ) + .shadow(color: .black.opacity(treatment.shadowOpacity), radius: 30, y: 12) + } +} From e20e9788f6adf4fe29692e6ca6059d3037835b9a Mon Sep 17 00:00:00 2001 From: shanforge <299346208+shanforge@users.noreply.github.com> Date: Fri, 21 Aug 2026 10:29:27 +0530 Subject: [PATCH 03/12] refactor(home): lift the starter chips' inputs out of the main window MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Which chips appear was already a tested rule in HomeSuggestions; how the workspace is read into that rule was three private computed properties inside AgentChatView. That is why the island has no starters — the rule was reachable and the inputs were not. Copying them would have been the wrong version: workspaceIsEmpty alone spans three stores, and a second copy that forgot spaces offers a returning user the first-run chips. The stores-loaded guard has the same shape of bug behind it. Part of #62. --- Logue.xcodeproj/project.pbxproj | 4 ++ Logue/Engine/HomeSuggestionsInputs.swift | 49 ++++++++++++++++++++++++ Logue/Views/Agent/AgentChatView.swift | 17 +++----- 3 files changed, 58 insertions(+), 12 deletions(-) create mode 100644 Logue/Engine/HomeSuggestionsInputs.swift diff --git a/Logue.xcodeproj/project.pbxproj b/Logue.xcodeproj/project.pbxproj index ac19c11..3dec487 100644 --- a/Logue.xcodeproj/project.pbxproj +++ b/Logue.xcodeproj/project.pbxproj @@ -250,6 +250,7 @@ 54F9DAED7E0DD30B5A16041A /* LLMEngine+Convenience.swift in Sources */ = {isa = PBXBuildFile; fileRef = ADFE072C32F14B59D0406571 /* LLMEngine+Convenience.swift */; }; 5516345F1ECA8AD048BB0889 /* AITitleGenerator.swift in Sources */ = {isa = PBXBuildFile; fileRef = 79F5EE85D7AA8E700D4C0FC3 /* AITitleGenerator.swift */; }; 556246B3C2F8B616A0CDD27B /* AskRoute.swift in Sources */ = {isa = PBXBuildFile; fileRef = 86C6E3021828B2B8589C720A /* AskRoute.swift */; }; + 558680C6A112BCC39EEC95AF /* HomeSuggestionsInputs.swift in Sources */ = {isa = PBXBuildFile; fileRef = 92E24E330864CAA8AB7B207A /* HomeSuggestionsInputs.swift */; }; 559A068B64BFBACC538549C3 /* FileAccessGate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3E86EBBF6643966395A7DE8B /* FileAccessGate.swift */; }; 5649718294F9E5770348BD76 /* LLMTestEvalExtensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 31ED31FAD045668E64BA99B1 /* LLMTestEvalExtensions.swift */; }; 56910F4D1E3DBFB9D424BE65 /* ExternalChangePlan.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1C141390FECB3AC097A5F95B /* ExternalChangePlan.swift */; }; @@ -1117,6 +1118,7 @@ 9226B7B10683E42ABEB71810 /* HighlightMarkTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HighlightMarkTests.swift; sourceTree = ""; }; 9230215EEC7592F9378CA66C /* ModelManager+Discovery.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "ModelManager+Discovery.swift"; sourceTree = ""; }; 92DB0F899AF6F08FEAF8AA46 /* AgentConversationListView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AgentConversationListView.swift; sourceTree = ""; }; + 92E24E330864CAA8AB7B207A /* HomeSuggestionsInputs.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HomeSuggestionsInputs.swift; sourceTree = ""; }; 92E9FCC3C656E8D24AC7A610 /* LLMTestHarness.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LLMTestHarness.swift; sourceTree = ""; }; 93368B5F8A112B6802BF4001 /* AISettingsTab.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AISettingsTab.swift; sourceTree = ""; }; 934F6981002CF41976F94D99 /* ActionItemInbox.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ActionItemInbox.swift; sourceTree = ""; }; @@ -1971,6 +1973,7 @@ 1EE7A71E63405D18365880DF /* HomeAskPrompts.swift */, F26F4F42013205EAE3A14D9D /* HomeContinueGrid.swift */, BB5DF28B62DFDA51D847EB70 /* HomeSuggestions.swift */, + 92E24E330864CAA8AB7B207A /* HomeSuggestionsInputs.swift */, 39AC4C78B6DA5F62A04B49C3 /* InlineAttributeVisitor.swift */, 358D99D88D3FB7C39EE0C7E7 /* InlineRewriteEngine.swift */, D6829731778689289CE48545 /* InlineSpellEngine.swift */, @@ -3048,6 +3051,7 @@ 44E47FDFA97164AD97F8B8E2 /* HomeLandingView.swift in Sources */, 8116FACB27C74191337D74FE /* HomeSpacesSection.swift in Sources */, 07D07554D07FE31EE59D028A /* HomeSuggestions.swift in Sources */, + 558680C6A112BCC39EEC95AF /* HomeSuggestionsInputs.swift in Sources */, 276FE55FD853C4367F2AE751 /* HorizontalInspectorTabBar.swift in Sources */, F1C5EFB9959BBAB77986EBBA /* HoverToolbar.swift in Sources */, 3078B745D4379CB1523EA5D2 /* IconToolbarView.swift in Sources */, diff --git a/Logue/Engine/HomeSuggestionsInputs.swift b/Logue/Engine/HomeSuggestionsInputs.swift new file mode 100644 index 0000000..3601fdd --- /dev/null +++ b/Logue/Engine/HomeSuggestionsInputs.swift @@ -0,0 +1,49 @@ +import Foundation + +/// Reading the workspace into the shape `HomeSuggestions.chips(for:)` wants. +/// +/// Kept out of `HomeSuggestions.swift` so that file stays pure — the rules there are +/// testable precisely because they touch no store. This is the other half: which stores the +/// inputs come from, stated once. +/// +/// It was three private computed properties inside `AgentChatView`, which is why the island +/// had no starters at all. Copying them would have been the easy version and the wrong one: +/// `workspaceIsEmpty` alone spans three stores, and a second copy that forgot spaces would +/// offer a returning user the first-run chips. +extension HomeSuggestions { + /// Whether every store the chips depend on has finished loading. + /// + /// Greeting a returning user as a new one because their library was still being read is + /// the loudest wrong answer this can give, so nothing renders until all three report. + @MainActor + static var storesAreLoaded: Bool { + DocumentStore.shared.isLoaded + && MeetingStore.shared.isLoaded + && SpaceStore.shared.isLoaded + } + + /// The workspace as the chip rules see it. + /// + /// - Parameter overdueCount: from `InsightsStatsProvider`, which each surface owns; it is + /// derived rather than stored, so it is passed in rather than read from a fourth store. + @MainActor + static func currentInputs(overdueCount: Int) -> Inputs { + let meetings = MeetingStore.shared.activeMeetings + let unsummarized = meetings + .filter { ($0.summary ?? "").isEmpty && !$0.isArchived } + .max { $0.createdAt < $1.createdAt } + + // One definition of "empty", used by every surface. Two definitions is how a + // workspace with spaces but no documents gets first-run chips. + let isEmpty = DocumentStore.shared.activeDocuments.isEmpty + && meetings.isEmpty + && SpaceStore.shared.topLevelSpaces.isEmpty + + return Inputs( + unsummarizedMeetingTitle: unsummarized?.title, + overdueCount: overdueCount, + meetingsToday: MeetingStore.shared.todaysMeetings.count, + hasAnyContent: !isEmpty + ) + } +} diff --git a/Logue/Views/Agent/AgentChatView.swift b/Logue/Views/Agent/AgentChatView.swift index ef8d292..a269e17 100644 --- a/Logue/Views/Agent/AgentChatView.swift +++ b/Logue/Views/Agent/AgentChatView.swift @@ -253,19 +253,12 @@ struct AgentChatView: View { && spaceStore.topLevelSpaces.isEmpty } - /// Derived fresh each render from the stores — no inference, no caching. The rules - /// live in `HomeSuggestions` so they can be tested without a view. + /// Derived fresh each render from the stores — no inference, no caching. The rules live + /// in `HomeSuggestions` and the reading of the workspace in `+Inputs`, so the island can + /// offer the same chips rather than growing its own list. private var suggestionChips: [HomeSuggestions.Chip] { - let unsummarized = meetingStore.activeMeetings - .filter { ($0.summary ?? "").isEmpty && !$0.isArchived } - .max { $0.createdAt < $1.createdAt } - return HomeSuggestions.chips( - for: HomeSuggestions.Inputs( - unsummarizedMeetingTitle: unsummarized?.title, - overdueCount: insights.actionItemStats.overdue, - meetingsToday: meetingStore.todaysMeetings.count, - hasAnyContent: !workspaceIsEmpty - ) + HomeSuggestions.chips( + for: HomeSuggestions.currentInputs(overdueCount: insights.actionItemStats.overdue) ) } From e8303082cb7a000d7d664bd706f4267d81963ef8 Mon Sep 17 00:00:00 2001 From: shanforge <299346208+shanforge@users.noreply.github.com> Date: Fri, 21 Aug 2026 10:32:30 +0530 Subject: [PATCH 04/12] feat(island): starters, a level meter, and what Return will do MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three things the island never said, all of them about the moment before a send. Starters are the chips Home already offers, from the same rule and the same reading of the workspace — so an empty island suggests summarising the meeting you have not summarised rather than a hardcoded list that goes stale. Held back until the stores report, because offering first-run chips to a returning user is worse than offering nothing. The level meter is the indicator the two in-app chat panels already mount. The island streams the partial transcript straight into the field so it can be edited before sending, so the indicator is handed none — what it adds is the level, which is the only thing that says the mic is hearing anything, and a stop target bigger than the mic glyph. The Return hint appears only once there is something to send. A hint that is always there is chrome, and the island has one line to spend. Part of #62. --- .../CommandCenterChatView+Composer.swift | 54 +++++++++++++++++++ .../CrossApp/CommandCenterChatView.swift | 23 ++++++++ 2 files changed, 77 insertions(+) diff --git a/Logue/Views/CrossApp/CommandCenterChatView+Composer.swift b/Logue/Views/CrossApp/CommandCenterChatView+Composer.swift index b81d3d5..007a74c 100644 --- a/Logue/Views/CrossApp/CommandCenterChatView+Composer.swift +++ b/Logue/Views/CrossApp/CommandCenterChatView+Composer.swift @@ -100,6 +100,20 @@ extension CommandCenterChatView { .disabled(isGenerating) .help(voiceManager.isRecording ? "Stop voice input" : "Voice input") + // What Return will do, once it will do anything. + // + // Shown only when there is something to send: a hint that is always there is + // chrome, and the island has one line to spend. Shift-Return for a newline is + // deliberately not advertised — it is the escape hatch from the hint, not a + // second thing to learn. + if canSend, !isGenerating { + Text("↩") + .font(.caption2.weight(.semibold)) + .foregroundStyle(.white.opacity(0.35)) + .transition(.opacity) + .accessibilityHidden(true) + } + // Send / Stop if isGenerating { Button(action: stopStreaming) { @@ -125,6 +139,7 @@ extension CommandCenterChatView { .keyboardShortcut(.return, modifiers: .command) } } + .animation(.easeOut(duration: 0.12), value: canSend) .padding(.leading, 14) .padding(.trailing, 10) .padding(.vertical, 10) @@ -147,6 +162,45 @@ extension CommandCenterChatView { } } + /// Something to ask, on an island with nothing in it yet. + /// + /// The same chips Home offers, from the same rule and the same reading of the workspace — + /// so the island suggests summarising the meeting you have not summarised rather than a + /// hardcoded list that goes stale. Hidden until the stores report, because offering + /// first-run chips to a returning user is worse than offering nothing. + var starters: some View { + VStack(spacing: 0) { + if HomeSuggestions.storesAreLoaded, !chips.isEmpty { + HStack(spacing: 6) { + ForEach(chips) { chip in + Button { + inputText = chip.prompt + isInputFocused = true + } label: { + Text(chip.label) + .font(.caption) + .foregroundStyle(.white.opacity(0.85)) + .lineLimit(1) + .padding(.horizontal, 10) + .padding(.vertical, 5) + .background(Capsule().fill(Color.white.opacity(0.10))) + .overlay(Capsule().strokeBorder(Color.white.opacity(0.10), lineWidth: 0.5)) + } + .buttonStyle(.plain) + .help(chip.prompt) + } + } + .padding(.bottom, 10) + } + } + } + + private var chips: [HomeSuggestions.Chip] { + HomeSuggestions.chips( + for: HomeSuggestions.currentInputs(overdueCount: insights.actionItemStats.overdue) + ) + } + /// What is staged for the next send, with a way to take each one back off. private var attachmentChips: some View { HStack(spacing: 6) { diff --git a/Logue/Views/CrossApp/CommandCenterChatView.swift b/Logue/Views/CrossApp/CommandCenterChatView.swift index 6556669..d67af98 100644 --- a/Logue/Views/CrossApp/CommandCenterChatView.swift +++ b/Logue/Views/CrossApp/CommandCenterChatView.swift @@ -40,6 +40,12 @@ struct CommandCenterChatView: View { var isDeepResearchOnce: Bool = false // Extension-visible: +Composer @State var deepResearch = DeepResearchCoordinator.shared + // Extension-visible: +Composer + /// Built the way `MainWindowView` builds its own, off the same singletons. The island is + /// an `NSPanel` and never receives the main window's environment, so a shared instance + /// would have to be reached for globally; two providers over one set of stores answer + /// the same question. + @State var insights = InsightsStatsProvider(meetingStore: .shared, documentStore: .shared) // Extension-visible: +Bubbles @State var copiedMessageID: UUID? // Extension-visible: +Bubbles @@ -105,6 +111,23 @@ struct CommandCenterChatView: View { messagesPanel .padding(.bottom, 10) .transition(.opacity.combined(with: .move(edge: .bottom))) + } else { + starters + } + + if voiceManager.isRecording { + // The same indicator the two in-app chat panels mount. The island streams the + // partial transcript straight into the field so it can be edited before + // sending, so the indicator is handed none — what it adds here is the level, + // which is the only thing that says the mic is actually hearing anything, and + // a stop target bigger than the mic glyph. + VoiceInputIndicator( + audioLevel: voiceManager.audioLevel, + partialTranscript: "", + onStop: { voiceManager.stopListening() } + ) + .padding(.bottom, 8) + .transition(.opacity) } promptPill From c5b64dd655a25fa4b31f6fbd1471a86f569da140 Mon Sep 17 00:00:00 2001 From: shanforge <299346208+shanforge@users.noreply.github.com> Date: Fri, 21 Aug 2026 10:45:54 +0530 Subject: [PATCH 05/12] fix(agent): a tool card's argument line is ordered and bounded MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defects in formatArguments that only became visible at island width, where the card is 700pt rather than a full window and the row has to hold the summary, the status and the Approve and Deny buttons at once. It mapped over a Dictionary, which has no order, so the same call rendered `query: standup, limit: 5` on one pass and `limit: 5, query: standup` on the next — and with the line truncated, the argument you could actually read changed as the view re-rendered. Keys are sorted now: alphabetical is arbitrary, but it is the same arbitrary every time. And it was unbounded. update_document carries the whole new body as an argument, so the string handed to Text was the length of a document — 20,009 characters in the test that now guards it. lineLimit(1) hid the tail but the text was still laid out, pushing everything after it on the row for room that was never going to be used. Values are also flattened onto one line, so a multi-paragraph argument stops looking like a short one. The approval buttons no longer compress: how to refuse must never be what gives way, and the summary beside them is already truncating. Part of #62. --- Logue.xcodeproj/project.pbxproj | 8 ++ Logue/Agent/ToolArgumentSummary.swift | 69 ++++++++++++++ Logue/Views/Agent/ToolExecutionCard.swift | 19 ++-- LogueTests/ToolArgumentSummaryTests.swift | 108 ++++++++++++++++++++++ 4 files changed, 195 insertions(+), 9 deletions(-) create mode 100644 Logue/Agent/ToolArgumentSummary.swift create mode 100644 LogueTests/ToolArgumentSummaryTests.swift diff --git a/Logue.xcodeproj/project.pbxproj b/Logue.xcodeproj/project.pbxproj index 3dec487..49fba23 100644 --- a/Logue.xcodeproj/project.pbxproj +++ b/Logue.xcodeproj/project.pbxproj @@ -260,6 +260,7 @@ 5786F7445BF8968E46CF8CCA /* WritingGoalMode.swift in Sources */ = {isa = PBXBuildFile; fileRef = CE4B9329DFE86A8D2B1629DE /* WritingGoalMode.swift */; }; 5823F6C9E41B7DC8DBB86CF7 /* WikiLinkCompletionTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2463FF4541A0C2197CBD646C /* WikiLinkCompletionTests.swift */; }; 58383E0A1CA89E1D4BC10EC4 /* StopGeneratingPill.swift in Sources */ = {isa = PBXBuildFile; fileRef = A0D79E3FE30ED849DAD9A68C /* StopGeneratingPill.swift */; }; + 58612790F2A91BCB594BA5BA /* ToolArgumentSummary.swift in Sources */ = {isa = PBXBuildFile; fileRef = EED88025342982955D643920 /* ToolArgumentSummary.swift */; }; 58CA6E17E4069C3C5F06F846 /* ModelsSettingsTab.swift in Sources */ = {isa = PBXBuildFile; fileRef = B8D03A33B16F78827FBD8EDC /* ModelsSettingsTab.swift */; }; 5997B3181E9E4EAC962C305D /* TimelineAudioConversionTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 31F21D2EC731B88B30A3B044 /* TimelineAudioConversionTests.swift */; }; 59AA097C69B46F3E88E42062 /* MLXLMTransformers in Frameworks */ = {isa = PBXBuildFile; productRef = CC75CD4CA4D3EADBB868CBF3 /* MLXLMTransformers */; }; @@ -458,6 +459,7 @@ A456F646E0BBEB74239CAACD /* StatusFilterChip.swift in Sources */ = {isa = PBXBuildFile; fileRef = 132AE87705CAF5786CEA7086 /* StatusFilterChip.swift */; }; A4651241FB99B99206090D29 /* DocumentTypeTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5B22ABF6CCA6E8E7D64ADF54 /* DocumentTypeTests.swift */; }; A4AEBA0FBDA0E9CE695009FD /* DocumentListPane.swift in Sources */ = {isa = PBXBuildFile; fileRef = D41328D9F51917FEDDC04492 /* DocumentListPane.swift */; }; + A4CBA7E4FE51027AC7623E48 /* ToolArgumentSummaryTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C8CE401568185598F580365F /* ToolArgumentSummaryTests.swift */; }; A4DA7AB242DC1D409392FF8D /* Speech.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 772CEEDFC80AF7FA18DF316F /* Speech.framework */; }; A4DC27FCF3FB55E71EB66A21 /* MeetingStore+Persistence.swift in Sources */ = {isa = PBXBuildFile; fileRef = 68D54A7FA1A1D4039E6E3B90 /* MeetingStore+Persistence.swift */; }; A5725AE738CE0852F088E454 /* RecordingSessionManager+AudioStream.swift in Sources */ = {isa = PBXBuildFile; fileRef = 11FACAFA240AC911725D7538 /* RecordingSessionManager+AudioStream.swift */; }; @@ -1264,6 +1266,7 @@ C83704AFDBD5FA20519C4067 /* WritingStatsBarView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WritingStatsBarView.swift; sourceTree = ""; }; C883BA071AA2D1AB25CF189C /* TrustChip.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TrustChip.swift; sourceTree = ""; }; C8A0088E863B5A3056A85771 /* AIContentTools.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AIContentTools.swift; sourceTree = ""; }; + C8CE401568185598F580365F /* ToolArgumentSummaryTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ToolArgumentSummaryTests.swift; sourceTree = ""; }; C8D8F63B0BEBD964C1409AA5 /* CommandCenterController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CommandCenterController.swift; sourceTree = ""; }; C8F833920208ECD78C193568 /* LogueLogoLight.svg */ = {isa = PBXFileReference; path = LogueLogoLight.svg; sourceTree = ""; }; CA29CBB4724CFEE0030E5FE3 /* AgentDictationService.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AgentDictationService.swift; sourceTree = ""; }; @@ -1373,6 +1376,7 @@ EE6D9693B41B9103DAAAB103 /* MeetingNote+Speakers.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "MeetingNote+Speakers.swift"; sourceTree = ""; }; EE89A8A3A959A42FAA7802BA /* InputDeviceKind.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = InputDeviceKind.swift; sourceTree = ""; }; EEBFA9476AB8E2BDA7306EAB /* MarkdownFrontmatterTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MarkdownFrontmatterTests.swift; sourceTree = ""; }; + EED88025342982955D643920 /* ToolArgumentSummary.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ToolArgumentSummary.swift; sourceTree = ""; }; EF62C254D50D305971D3FCC9 /* IslandSurface.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = IslandSurface.swift; sourceTree = ""; }; EF7B96F52DB280D23D637F33 /* MenuBarIcon.png */ = {isa = PBXFileReference; lastKnownFileType = image.png; path = MenuBarIcon.png; sourceTree = ""; }; EFE448FDFB9D86AB13035E23 /* ReleaseEntitlementsTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ReleaseEntitlementsTests.swift; sourceTree = ""; }; @@ -1590,6 +1594,7 @@ 5967FC0A683B57D96AAAB8CF /* TaskTriageTests.swift */, 8D2D61B85F962E88AA1E5062 /* TextAnalysisRequestTests.swift */, 31F21D2EC731B88B30A3B044 /* TimelineAudioConversionTests.swift */, + C8CE401568185598F580365F /* ToolArgumentSummaryTests.swift */, 542E11E3E63665F36931F23F /* TranscriptGutterTests.swift */, 57C7DA3D5F0B7598F71D9D15 /* TranscriptionGateTests.swift */, 1396470F3C9D5EA09515233B /* TranscriptRealignmentTests.swift */, @@ -2219,6 +2224,7 @@ 417436A70F2C0F1DDA347DC4 /* AttachmentIntake.swift */, 4B2F874FCD0AF25F7B37D3FC /* MalformedToolCall.swift */, B55FA53C32F4D2968C07CEE5 /* MessageActions.swift */, + EED88025342982955D643920 /* ToolArgumentSummary.swift */, 6711FB8D13DD53809A655A14 /* WritingAgentGraph.swift */, 90E4CF4E9246459183E66FD7 /* WritingAgentState.swift */, ); @@ -2823,6 +2829,7 @@ 25A2038F3D3B1815856A1913 /* TaskTriageTests.swift in Sources */, A30AAC62CDAE227B36E45511 /* TextAnalysisRequestTests.swift in Sources */, 5997B3181E9E4EAC962C305D /* TimelineAudioConversionTests.swift in Sources */, + A4CBA7E4FE51027AC7623E48 /* ToolArgumentSummaryTests.swift in Sources */, AC96E93D8663897BDD9091F9 /* TranscriptGutterTests.swift in Sources */, 06AA8A863594D0566529704C /* TranscriptRealignmentTests.swift in Sources */, D1686AF670A747CE0BECAC80 /* TranscriptReplacementTests.swift in Sources */, @@ -3327,6 +3334,7 @@ 1D96BDEBAD706C75ACA2C818 /* TimelineContributionLog.swift in Sources */, EAB17DC89BEDD8391BA0D76E /* Toast.swift in Sources */, 1C1775454781744474E68510 /* ToolApprovalButtons.swift in Sources */, + 58612790F2A91BCB594BA5BA /* ToolArgumentSummary.swift in Sources */, 2CDF845EA2BF63AC732BD8BA /* ToolExecutionCard.swift in Sources */, 924E8B180D4472C8BA2AA086 /* TranscriptGutter.swift in Sources */, 9C249ED08B43DEEA769A0651 /* TranscriptRealignment.swift in Sources */, diff --git a/Logue/Agent/ToolArgumentSummary.swift b/Logue/Agent/ToolArgumentSummary.swift new file mode 100644 index 0000000..55f2091 --- /dev/null +++ b/Logue/Agent/ToolArgumentSummary.swift @@ -0,0 +1,69 @@ +import Foundation + +/// The one line a tool card shows for what a tool was called with. +/// +/// Lifted out of `ToolExecutionCard.formatArguments`, which had two problems that only +/// became visible at island width. +/// +/// **It was unordered.** It mapped over a `Dictionary`, and Swift dictionaries have no order +/// — so the same call rendered `query: standup, limit: 5` on one pass and `limit: 5, query: +/// standup` on the next. In a 700pt card with the line truncated, that means the argument you +/// can actually read changes as the view re-renders. +/// +/// **It was unbounded.** `update_document` carries the whole new body as an argument, so the +/// string handed to `Text` was the length of a document. `lineLimit(1)` hid the tail but the +/// text was still laid out, and everything after it on that row — the status badge, the +/// Approve and Deny buttons — got pushed for room that was never going to be used. +/// +/// Free of SwiftUI, so both the ordering and the bounds are testable. +enum ToolArgumentSummary { + /// Longest a single value may be before it is cut. + static let maxValueLength = 48 + /// Longest the whole line may be, however many arguments there are. + static let maxTotalLength = 160 + + /// What to show for `json`, or an empty string when there is nothing worth showing. + /// + /// Keys are sorted so the line is stable across renders. Alphabetical is arbitrary but it + /// is *the same* arbitrary every time, which is the property that matters — the reader is + /// looking at a line they may have to compare with the one above it. + static func summary(fromJSON json: String) -> String { + let trimmed = json.trimmingCharacters(in: .whitespacesAndNewlines) + guard !trimmed.isEmpty, trimmed != "{}" else { return "" } + + guard let data = trimmed.data(using: .utf8), + let dict = try? JSONSerialization.jsonObject(with: data) as? [String: Any] + else { + // Not JSON we can read. Show it anyway — it is still what the tool was called + // with — but bounded, which is the whole point of this type. + return clamp(flatten(trimmed), to: maxTotalLength) + } + + let pairs = dict.keys.sorted().map { key in + "\(key): \(clamp(flatten(String(describing: dict[key] ?? "")), to: maxValueLength))" + } + return clamp(pairs.joined(separator: ", "), to: maxTotalLength) + } + + /// Puts a value on one line. + /// + /// A document body arrives with its newlines intact, and a `Text` limited to one line + /// renders the first of them and hides the rest — so a multi-paragraph argument looked + /// like a short one. Collapsing whitespace makes the truncation honest. + private static func flatten(_ value: String) -> String { + value + .components(separatedBy: .whitespacesAndNewlines) + .filter { !$0.isEmpty } + .joined(separator: " ") + } + + /// Cuts to `limit`, marking that something was cut. + /// + /// The ellipsis is inside the budget rather than added to it, so the result is never + /// longer than the caller asked for. + private static func clamp(_ value: String, to limit: Int) -> String { + guard value.count > limit else { return value } + guard limit > 1 else { return String(value.prefix(limit)) } + return String(value.prefix(limit - 1)) + "…" + } +} diff --git a/Logue/Views/Agent/ToolExecutionCard.swift b/Logue/Views/Agent/ToolExecutionCard.swift index f1d1f68..31fe979 100644 --- a/Logue/Views/Agent/ToolExecutionCard.swift +++ b/Logue/Views/Agent/ToolExecutionCard.swift @@ -58,8 +58,14 @@ struct ToolExecutionCard: View { conversationID: conversationID, clearance: toolCall.clearance ) + // How to refuse must never be what gets compressed. The summary beside + // it is already truncating, so it is the one that should give way. + .fixedSize() + .layoutPriority(1) } else { statusBadge + .fixedSize() + .layoutPriority(1) } } .padding(.vertical, 6) @@ -82,11 +88,13 @@ struct ToolExecutionCard: View { .fontDesign(.monospaced) .foregroundStyle(.secondary) - if !toolCall.arguments.isEmpty, toolCall.arguments != "{}" { - Text(formatArguments(toolCall.arguments)) + let arguments = ToolArgumentSummary.summary(fromJSON: toolCall.arguments) + if !arguments.isEmpty { + Text(arguments) .font(.caption) .foregroundStyle(.tertiary) .lineLimit(1) + .truncationMode(.middle) } } @@ -346,11 +354,4 @@ struct ToolExecutionCard: View { } // MARK: - Helpers - - private func formatArguments(_ json: String) -> String { - guard let data = json.data(using: .utf8), - let dict = try? JSONSerialization.jsonObject(with: data) as? [String: Any] - else { return json } - return dict.map { "\($0.key): \($0.value)" }.joined(separator: ", ") - } } diff --git a/LogueTests/ToolArgumentSummaryTests.swift b/LogueTests/ToolArgumentSummaryTests.swift new file mode 100644 index 0000000..618b752 --- /dev/null +++ b/LogueTests/ToolArgumentSummaryTests.swift @@ -0,0 +1,108 @@ +import Foundation +import Testing + +@testable import Logue + +/// The line a tool card shows for what a tool was called with. +/// +/// Both rules here were bugs before the type existed, and both only show at island width — +/// where the card is 700pt rather than a full window and the row has to hold the summary, +/// the status and the Approve and Deny buttons at once. +@Suite("ToolArgumentSummary") +struct ToolArgumentSummaryTests { + // MARK: - Ordering + + @Test("The same arguments always produce the same line") + func orderIsStable() { + // It mapped over a Dictionary, which has no order, so the same call rendered + // `query: standup, limit: 5` on one pass and `limit: 5, query: standup` on the next. + // With the line truncated, the argument you can read changed as the view re-rendered. + // + // This case states the intent but does not on its own catch the regression: a + // dictionary's iteration order is seeded per process, so within one test run the old + // code answered consistently too. `keysAreSorted` below is the one that goes red when + // the sort is removed — verified by removing it. + let json = #"{"query":"standup","limit":5,"space":"Work","after":"2026-01-01"}"# + let first = ToolArgumentSummary.summary(fromJSON: json) + for _ in 0 ..< 20 { + #expect(ToolArgumentSummary.summary(fromJSON: json) == first) + } + } + + @Test("Keys are ordered by name") + func keysAreSorted() { + let summary = ToolArgumentSummary.summary(fromJSON: #"{"zebra":1,"alpha":2,"mango":3}"#) + #expect(summary == "alpha: 2, mango: 3, zebra: 1") + } + + // MARK: - Bounds + + @Test("A huge argument cannot make the line huge") + func totalLengthIsBounded() { + // `update_document` carries the whole new body as an argument. lineLimit(1) hid the + // tail, but the text was still laid out and pushed the status badge and the approval + // buttons off the end of the row. + let body = String(repeating: "a", count: 20_000) + let summary = ToolArgumentSummary.summary(fromJSON: #"{"content":"\#(body)"}"#) + #expect(summary.count <= ToolArgumentSummary.maxTotalLength) + } + + @Test("One long value does not crowd out the others") + func eachValueIsBoundedSeparately() { + let long = String(repeating: "b", count: 500) + let summary = ToolArgumentSummary.summary(fromJSON: #"{"aaa":"\#(long)","zzz":"visible"}"#) + #expect(summary.hasPrefix("aaa: ")) + #expect(summary.contains("zzz: visible"), "the short one is still readable") + } + + @Test("Unparseable arguments are shown, but bounded") + func rawFallbackIsBounded() { + // Still what the tool was called with, so it is worth showing — but the reason this + // type exists is that nothing on this row may be unbounded. + let notJSON = String(repeating: "x", count: 5000) + let summary = ToolArgumentSummary.summary(fromJSON: notJSON) + #expect(summary.isEmpty == false) + #expect(summary.count <= ToolArgumentSummary.maxTotalLength) + } + + @Test("Truncation never exceeds the budget it was given") + func ellipsisFitsInsideTheBudget() { + // The ellipsis is inside the budget rather than added to it, so a caller that sized a + // row from the limit is not handed one character more. + for length in [1, 2, 47, 48, 49, 200, 5000] { + let value = String(repeating: "c", count: length) + let summary = ToolArgumentSummary.summary(fromJSON: #"{"k":"\#(value)"}"#) + #expect(summary.count <= ToolArgumentSummary.maxTotalLength) + } + } + + // MARK: - One line + + @Test("A multi-line value is flattened onto one line") + func newlinesAreCollapsed() { + // A document body arrives with its newlines intact, and a Text limited to one line + // renders the first of them and hides the rest — so a multi-paragraph argument looked + // like a short one, with no ellipsis to say otherwise. + let summary = ToolArgumentSummary.summary(fromJSON: #"{"body":"first\nsecond\n\nthird"}"#) + #expect(summary.contains("\n") == false) + #expect(summary == "body: first second third") + } + + @Test("Runs of whitespace collapse to one space") + func whitespaceIsCollapsed() { + let summary = ToolArgumentSummary.summary(fromJSON: #"{"t":"a b\t\tc"}"#) + #expect(summary == "t: a b c") + } + + // MARK: - Nothing to show + + @Test("No arguments produce no line") + func emptyArgumentsAreEmpty() { + // The card checked for these itself. Answering it here means one place decides + // whether there is anything to draw. + #expect(ToolArgumentSummary.summary(fromJSON: "{}").isEmpty) + #expect(ToolArgumentSummary.summary(fromJSON: "").isEmpty) + #expect(ToolArgumentSummary.summary(fromJSON: " ").isEmpty) + #expect(ToolArgumentSummary.summary(fromJSON: " {} ").isEmpty) + } +} From f313629995ecf51c29426c62e69db0a35616a8f5 Mon Sep 17 00:00:00 2001 From: shanforge <299346208+shanforge@users.noreply.github.com> Date: Fri, 21 Aug 2026 11:02:56 +0530 Subject: [PATCH 06/12] fix(agent): an approval prompt names what it is about to act on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every destructive tool takes a UUID. The card had five hand-written sentences and a fallback of "Agent wants to run ", and none of them named the target — so "Agent wants to delete a document" was the whole of what the user was told before being asked for Touch ID. Which document was not knowable from the card at all. ToolApprovalPrompt is the wording and which argument holds the target, pure so both are testable; ToolApprovalTargetResolver is the half that has to ask the stores. Reminders and calendar events deliberately resolve to nothing: both live behind an EventKit permission, and prompting for access as a side effect of drawing an approval card is not acceptable in the one card whose job is to be trustworthy. They show the action alone, which is honest, rather than a UUID, which is not. The coverage test walks the real registry, so a destructive tool with no sentence is a red build. It found two on its first run — fetch_contacts and get_location, both reading private data, both previously rendering as "Agent wants to run get_location". AgentCoordinator gained allKnownTools() so that walk cannot go stale; the registry is built from it rather than from a second list. The approval sentence also renders as primary text over two lines. A title cut in half is how you approve the wrong thing; everything else on a card is a record of something that already happened and stays on one line. Part of #62. --- Logue.xcodeproj/project.pbxproj | 12 ++ Logue/Agent/AgentCoordinator.swift | 42 +++-- Logue/Agent/ToolApprovalPrompt.swift | 168 ++++++++++++++++++ .../Agent/ToolApprovalTargetResolver.swift | 32 ++++ Logue/Views/Agent/ToolExecutionCard.swift | 29 +-- LogueTests/ToolApprovalPromptTests.swift | 145 +++++++++++++++ 6 files changed, 404 insertions(+), 24 deletions(-) create mode 100644 Logue/Agent/ToolApprovalPrompt.swift create mode 100644 Logue/Views/Agent/ToolApprovalTargetResolver.swift create mode 100644 LogueTests/ToolApprovalPromptTests.swift diff --git a/Logue.xcodeproj/project.pbxproj b/Logue.xcodeproj/project.pbxproj index 49fba23..a58ea94 100644 --- a/Logue.xcodeproj/project.pbxproj +++ b/Logue.xcodeproj/project.pbxproj @@ -253,6 +253,7 @@ 558680C6A112BCC39EEC95AF /* HomeSuggestionsInputs.swift in Sources */ = {isa = PBXBuildFile; fileRef = 92E24E330864CAA8AB7B207A /* HomeSuggestionsInputs.swift */; }; 559A068B64BFBACC538549C3 /* FileAccessGate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3E86EBBF6643966395A7DE8B /* FileAccessGate.swift */; }; 5649718294F9E5770348BD76 /* LLMTestEvalExtensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 31ED31FAD045668E64BA99B1 /* LLMTestEvalExtensions.swift */; }; + 565CE023D3242EDD8DE16D27 /* ToolApprovalPromptTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = A2B422B1EA792B27E116DA2C /* ToolApprovalPromptTests.swift */; }; 56910F4D1E3DBFB9D424BE65 /* ExternalChangePlan.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1C141390FECB3AC097A5F95B /* ExternalChangePlan.swift */; }; 56F4853038E368546DCD2955 /* JavaScriptTool.swift in Sources */ = {isa = PBXBuildFile; fileRef = 31F56338AE354AB4607F3597 /* JavaScriptTool.swift */; }; 572EFABAA476D44DB7AB8748 /* ReminderManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6ED5A7948FEFC3B58D925DFC /* ReminderManager.swift */; }; @@ -269,6 +270,7 @@ 5AAAF36F01B586D73D6A35EF /* ContactsTool.swift in Sources */ = {isa = PBXBuildFile; fileRef = AFA4DB05B86C453078B1E5E6 /* ContactsTool.swift */; }; 5AB6B868E9F97AEEEC43AFC5 /* MeetingTimeTrendCard.swift in Sources */ = {isa = PBXBuildFile; fileRef = 23DED725F174E9C0F6E8F30E /* MeetingTimeTrendCard.swift */; }; 5ABF0C44B73774A18BE7F417 /* SuggestionPanelView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 126A669ACEBF8395600EC8EE /* SuggestionPanelView.swift */; }; + 5ACB9C77AA0778CCD8ADB94B /* ToolApprovalTargetResolver.swift in Sources */ = {isa = PBXBuildFile; fileRef = 123B91365DC7DD41845FAF3D /* ToolApprovalTargetResolver.swift */; }; 5B40660F3A0659483AC6ED30 /* StatItem.swift in Sources */ = {isa = PBXBuildFile; fileRef = B1C70855FA4AA096BC6DBC4B /* StatItem.swift */; }; 5B40672923AE9211B7EE0387 /* HomeContextBar.swift in Sources */ = {isa = PBXBuildFile; fileRef = 42BFEBF99163808EE4126279 /* HomeContextBar.swift */; }; 5B446C6C266FB883611B2664 /* FileSystemTools.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8E65B9D58A6CC515006C9A20 /* FileSystemTools.swift */; }; @@ -657,6 +659,7 @@ ED1971AC239DE930D076B294 /* ActionItemInbox.swift in Sources */ = {isa = PBXBuildFile; fileRef = 934F6981002CF41976F94D99 /* ActionItemInbox.swift */; }; ED283783D6A185FB3914A331 /* ApprovalGate.swift in Sources */ = {isa = PBXBuildFile; fileRef = B1CF9F695CCB56896DC9A02C /* ApprovalGate.swift */; }; ED7BBA11866E8887D849AD84 /* ChatInputField.swift in Sources */ = {isa = PBXBuildFile; fileRef = B80061FBEC30F04EF2176F88 /* ChatInputField.swift */; }; + EDAA35D422C230DCE495A175 /* ToolApprovalPrompt.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4542B2571154754E0D0E8783 /* ToolApprovalPrompt.swift */; }; EE067F20A9652C38BFC02800 /* BlockTextView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 381AAF180053F03E9FBAC2A3 /* BlockTextView.swift */; }; EE4CB15C53C082C939662511 /* HomeAttentionCard.swift in Sources */ = {isa = PBXBuildFile; fileRef = 47D6D28A5F055B59944316D6 /* HomeAttentionCard.swift */; }; EE8E41D5DDCB41B4B8AB8087 /* BrowserBridgeServer+Handlers.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6B09E81F8469DC3A84D5353E /* BrowserBridgeServer+Handlers.swift */; }; @@ -774,6 +777,7 @@ 11C3F8ADFF573D2924827697 /* WhatsNewView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WhatsNewView.swift; sourceTree = ""; }; 11F90CF8BD9ABA35C96B0E76 /* DiagnosticsReport.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DiagnosticsReport.swift; sourceTree = ""; }; 11FACAFA240AC911725D7538 /* RecordingSessionManager+AudioStream.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "RecordingSessionManager+AudioStream.swift"; sourceTree = ""; }; + 123B91365DC7DD41845FAF3D /* ToolApprovalTargetResolver.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ToolApprovalTargetResolver.swift; sourceTree = ""; }; 126A669ACEBF8395600EC8EE /* SuggestionPanelView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SuggestionPanelView.swift; sourceTree = ""; }; 12A71C8E402261DE0D93DA2A /* ModelAction.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ModelAction.swift; sourceTree = ""; }; 12C9A281A8D252F78714C155 /* DashboardActionItem.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DashboardActionItem.swift; sourceTree = ""; }; @@ -908,6 +912,7 @@ 43D628E66EC44675B3D52230 /* EditTemplateSheet.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EditTemplateSheet.swift; sourceTree = ""; }; 448C84F276994CA3F884806B /* WritingLayoutManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WritingLayoutManager.swift; sourceTree = ""; }; 453FB30A414ED333A0858FF2 /* CategoryGradeCard.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CategoryGradeCard.swift; sourceTree = ""; }; + 4542B2571154754E0D0E8783 /* ToolApprovalPrompt.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ToolApprovalPrompt.swift; sourceTree = ""; }; 461D5E9CBBBFA62E23630E08 /* MeetingSpeakersPanelView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MeetingSpeakersPanelView.swift; sourceTree = ""; }; 463F7963624BB9B6B962EAAB /* RecordingSessionManager+Checkpoint.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "RecordingSessionManager+Checkpoint.swift"; sourceTree = ""; }; 464AB56546E36263CEB595E8 /* IslandSurfaceTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = IslandSurfaceTests.swift; sourceTree = ""; }; @@ -1163,6 +1168,7 @@ A1873C1B26D58D0BB783B664 /* RecordingRecoveryTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RecordingRecoveryTests.swift; sourceTree = ""; }; A26676AA09CFF5DF1EE25431 /* LinkIndex.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LinkIndex.swift; sourceTree = ""; }; A297CA8A1434241BA7F14120 /* QuickComposeView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = QuickComposeView.swift; sourceTree = ""; }; + A2B422B1EA792B27E116DA2C /* ToolApprovalPromptTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ToolApprovalPromptTests.swift; sourceTree = ""; }; A2E43C27D86830F81B1776EE /* RichTextEditorHelpers.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RichTextEditorHelpers.swift; sourceTree = ""; }; A3600798EF27A735FC4BC80E /* HomeSuggestionsTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HomeSuggestionsTests.swift; sourceTree = ""; }; A3A9B8AAE15A27CE425D28BC /* ScanSafetyTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ScanSafetyTests.swift; sourceTree = ""; }; @@ -1594,6 +1600,7 @@ 5967FC0A683B57D96AAAB8CF /* TaskTriageTests.swift */, 8D2D61B85F962E88AA1E5062 /* TextAnalysisRequestTests.swift */, 31F21D2EC731B88B30A3B044 /* TimelineAudioConversionTests.swift */, + A2B422B1EA792B27E116DA2C /* ToolApprovalPromptTests.swift */, C8CE401568185598F580365F /* ToolArgumentSummaryTests.swift */, 542E11E3E63665F36931F23F /* TranscriptGutterTests.swift */, 57C7DA3D5F0B7598F71D9D15 /* TranscriptionGateTests.swift */, @@ -2071,6 +2078,7 @@ D1A491C56C1501F3D06F7734 /* PulsingDot.swift */, 2B203FDECC75AFF647507580 /* SourcesPanelView.swift */, A4DB5AD462B4A863201D5D61 /* ToolApprovalButtons.swift */, + 123B91365DC7DD41845FAF3D /* ToolApprovalTargetResolver.swift */, 43877B1F81F7AE214808F86A /* ToolExecutionCard.swift */, ); path = Agent; @@ -2224,6 +2232,7 @@ 417436A70F2C0F1DDA347DC4 /* AttachmentIntake.swift */, 4B2F874FCD0AF25F7B37D3FC /* MalformedToolCall.swift */, B55FA53C32F4D2968C07CEE5 /* MessageActions.swift */, + 4542B2571154754E0D0E8783 /* ToolApprovalPrompt.swift */, EED88025342982955D643920 /* ToolArgumentSummary.swift */, 6711FB8D13DD53809A655A14 /* WritingAgentGraph.swift */, 90E4CF4E9246459183E66FD7 /* WritingAgentState.swift */, @@ -2829,6 +2838,7 @@ 25A2038F3D3B1815856A1913 /* TaskTriageTests.swift in Sources */, A30AAC62CDAE227B36E45511 /* TextAnalysisRequestTests.swift in Sources */, 5997B3181E9E4EAC962C305D /* TimelineAudioConversionTests.swift in Sources */, + 565CE023D3242EDD8DE16D27 /* ToolApprovalPromptTests.swift in Sources */, A4CBA7E4FE51027AC7623E48 /* ToolArgumentSummaryTests.swift in Sources */, AC96E93D8663897BDD9091F9 /* TranscriptGutterTests.swift in Sources */, 06AA8A863594D0566529704C /* TranscriptRealignmentTests.swift in Sources */, @@ -3334,6 +3344,8 @@ 1D96BDEBAD706C75ACA2C818 /* TimelineContributionLog.swift in Sources */, EAB17DC89BEDD8391BA0D76E /* Toast.swift in Sources */, 1C1775454781744474E68510 /* ToolApprovalButtons.swift in Sources */, + EDAA35D422C230DCE495A175 /* ToolApprovalPrompt.swift in Sources */, + 5ACB9C77AA0778CCD8ADB94B /* ToolApprovalTargetResolver.swift in Sources */, 58612790F2A91BCB594BA5BA /* ToolArgumentSummary.swift in Sources */, 2CDF845EA2BF63AC732BD8BA /* ToolExecutionCard.swift in Sources */, 924E8B180D4472C8BA2AA086 /* TranscriptGutter.swift in Sources */, diff --git a/Logue/Agent/AgentCoordinator.swift b/Logue/Agent/AgentCoordinator.swift index 5ddb8b1..6a6ab64 100644 --- a/Logue/Agent/AgentCoordinator.swift +++ b/Logue/Agent/AgentCoordinator.swift @@ -120,12 +120,13 @@ final class AgentCoordinator { /// when either the Settings master toggle is on or a per-send override /// (`oneShotIncludeWebTools`) is set. private func buildToolRegistry() -> [any AgentTool] { - var tools = Self.readOnlyTools() - + Self.writeTools() - + Self.aiContentTools() - + Self.appleNativeTools() - + Self.computeAndDialogTools() - + Self.fileSystemTools() + // Web search tools — registered when the Settings master toggle is on OR when a + // per-send override (the input bar's one-shot Search toggle) is active for this run. + let userOptedIn = UserDefaults.standard.bool(forKey: AppConstants.UserDefaultsKeys.webSearchEnabled) + let includeWebTools = userOptedIn || oneShotIncludeWebTools + let webNames = Set(Self.webTools().map(\.name)) + + var tools = Self.allKnownTools().filter { includeWebTools || !webNames.contains($0.name) } // Phase A: per-tool enable/disable filter. The AISettingsTab persists // a set of tool names the user has explicitly turned off (e.g. "I @@ -135,17 +136,30 @@ final class AgentCoordinator { if !disabledNames.isEmpty { tools.removeAll { disabledNames.contains($0.name) } } - // Web search tools — included when the Settings master toggle is on - // OR when a per-send override (the input bar's one-shot Search toggle) - // is active for this run. - let userOptedIn = UserDefaults.standard.bool(forKey: AppConstants.UserDefaultsKeys.webSearchEnabled) - if userOptedIn || oneShotIncludeWebTools { - tools.append(WebSearchTool()) - tools.append(FetchWebPageTool()) - } return tools } + /// Every tool the app can build, before any user filter. + /// + /// Exists so the registry has one definition rather than a second list kept in step by + /// hand — `ToolApprovalPromptTests` walks this to prove nothing that asks for approval + /// lacks a sentence explaining what it is about to do. A separate list would go stale + /// exactly when it mattered: the day someone adds a destructive tool. + static func allKnownTools() -> [any AgentTool] { + readOnlyTools() + + writeTools() + + aiContentTools() + + appleNativeTools() + + computeAndDialogTools() + + fileSystemTools() + + webTools() + } + + /// Reaching off the machine, so registered only on an explicit opt-in. + static func webTools() -> [any AgentTool] { + [WebSearchTool(), FetchWebPageTool()] + } + // MARK: - Tool registry shards // diff --git a/Logue/Agent/ToolApprovalPrompt.swift b/Logue/Agent/ToolApprovalPrompt.swift new file mode 100644 index 0000000..4694233 --- /dev/null +++ b/Logue/Agent/ToolApprovalPrompt.swift @@ -0,0 +1,168 @@ +import Foundation + +/// What an approval card says is about to happen, and to what. +/// +/// The card used to answer this with five hand-written sentences and a fallback of +/// "Agent wants to run \(toolName)". None of them named the thing being acted on, which is +/// the half that matters: every destructive tool here takes a **UUID**, so "Agent wants to +/// delete a document" is the whole of what the user was told before being asked for Touch ID. +/// Which document was not knowable from the card at all. +/// +/// Pure, so the wording and the target extraction are testable without a view or a store. The +/// names themselves have to be looked up, which is what `resolve` is for — a caller on the +/// main actor asks the stores; a test passes a stub. +enum ToolApprovalPrompt { + /// What kind of thing an id points at, so a caller knows which store to ask. + enum TargetKind: Equatable { + case document + case space + case reminder + case calendarEvent + } + + /// An id carried in the arguments, and what it points at. + struct Reference: Equatable { + let kind: TargetKind + let id: UUID + } + + /// Where the name of the thing being acted on comes from. + private enum TargetSource { + /// Nothing identifies a target — the action is the whole sentence. + case none + /// An argument holding a name, path, address or query, usable as written. + case literal(String) + /// An argument holding a UUID, which has to be resolved to something a person + /// recognises before it is worth showing. + case reference(TargetKind, String) + } + + private struct Rule { + let action: String + let target: TargetSource + } + + /// Longest target we will show. A path or a title can be arbitrarily long, and a prompt + /// that wraps to five lines is one people stop reading — which is the failure mode an + /// approval prompt can least afford. + static let maxTargetLength = 64 + + /// One entry per tool that can ask for approval. + /// + /// `ToolApprovalPromptTests` walks the registry and fails when a tool needing approval has + /// no entry here, so adding a destructive tool without saying what it does is a red build + /// rather than a card reading "Agent wants to run delete_everything". + private static let rules: [String: Rule] = [ + // Documents + "create_document": Rule(action: "Create a document", target: .literal("title")), + "update_document": Rule(action: "Edit", target: .reference(.document, "documentID")), + "delete_document": Rule(action: "Delete", target: .reference(.document, "documentID")), + "move_document": Rule(action: "Move", target: .reference(.document, "documentID")), + "add_document_tag": Rule(action: "Tag", target: .reference(.document, "documentID")), + "export_document_pdf": Rule(action: "Export as PDF", target: .reference(.document, "documentID")), + "create_document_from_template": Rule(action: "Create a document", target: .literal("title")), + // Spaces + "create_space": Rule(action: "Create a space", target: .literal("name")), + "rename_space": Rule(action: "Rename", target: .reference(.space, "spaceID")), + "delete_space": Rule( + action: "Delete, with everything in it,", + target: .reference(.space, "spaceID") + ), + // Calendar + "create_calendar_event": Rule(action: "Create a calendar event", target: .literal("title")), + "update_calendar_event": Rule(action: "Change", target: .reference(.calendarEvent, "eventID")), + "delete_calendar_event": Rule(action: "Delete", target: .reference(.calendarEvent, "eventID")), + // Reminders + "add_reminder": Rule(action: "Add a reminder", target: .literal("title")), + "update_reminder": Rule(action: "Change", target: .reference(.reminder, "reminderID")), + "delete_reminder": Rule(action: "Delete", target: .reference(.reminder, "reminderID")), + // The filesystem, where the path is the target and is already readable + "list_directory": Rule(action: "List", target: .literal("path")), + "read_file_at_path": Rule(action: "Read", target: .literal("path")), + "write_text_to_file": Rule(action: "Write to", target: .literal("path")), + "delete_file_at_path": Rule(action: "Delete the file", target: .literal("path")), + // The user's own data, held by macOS rather than by Logue. Neither takes an id and + // neither has a target worth naming — what matters is that the card says plainly + // which private thing is about to be read, which "Agent wants to run get_location" + // did not. Found by the coverage test below, not by hand. + "fetch_contacts": Rule(action: "Read your contacts", target: .literal("name")), + "get_location": Rule(action: "Read your current location", target: .none), + // Off the machine + "draft_email": Rule(action: "Draft an email to", target: .literal("to")), + "web_search": Rule(action: "Search the web for", target: .literal("query")), + "fetch_web_page": Rule(action: "Open", target: .literal("url")), + ] + + /// Whether this tool has a prompt written for it. + static func knows(toolNamed name: String) -> Bool { + rules[name] != nil + } + + /// The id this call will act on, if it acts on one that has to be looked up. + static func reference(toolNamed name: String, arguments: String) -> Reference? { + guard case let .reference(kind, key)? = rules[name]?.target, + let raw = value(of: key, in: arguments), + let id = UUID(uuidString: raw) + else { return nil } + return Reference(kind: kind, id: id) + } + + /// The sentence to show above Approve and Reject. + /// + /// - Parameter resolve: turns a `Reference` into something a person recognises. Returning + /// `nil` — the object is gone, or the id was invented — leaves the action standing on + /// its own rather than showing a UUID, which tells the user nothing and looks like a + /// bug at the exact moment they are deciding whether to trust the agent. + static func sentence( + toolNamed name: String, + arguments: String, + resolve: (Reference) -> String? + ) -> String { + guard let rule = rules[name] else { + // An unknown tool is still asking for permission, so say so plainly rather than + // inventing a description of something we do not have a rule for. + return "Run \(clamp(flatten(name)))" + } + + let target: String? = switch rule.target { + case .none: + nil + case let .literal(key): + value(of: key, in: arguments).map { clamp(flatten($0)) } + case .reference: + reference(toolNamed: name, arguments: arguments) + .flatMap(resolve) + .map { clamp(flatten($0)) } + } + + guard let target, !target.isEmpty else { return rule.action } + return "\(rule.action) “\(target)”" + } + + // MARK: - Reading arguments + + private static func value(of key: String, in json: String) -> String? { + guard let data = json.data(using: .utf8), + let dict = try? JSONSerialization.jsonObject(with: data) as? [String: Any], + let raw = dict[key] + else { return nil } + let string = String(describing: raw).trimmingCharacters(in: .whitespacesAndNewlines) + return string.isEmpty ? nil : string + } + + /// Puts a target on one line. + /// + /// Titles are user-authored and a path can contain anything. A newline in either would + /// split the sentence in half and leave the verb sitting alone above Approve. + private static func flatten(_ value: String) -> String { + value + .components(separatedBy: .whitespacesAndNewlines) + .filter { !$0.isEmpty } + .joined(separator: " ") + } + + private static func clamp(_ value: String) -> String { + guard value.count > maxTargetLength else { return value } + return String(value.prefix(maxTargetLength - 1)) + "…" + } +} diff --git a/Logue/Views/Agent/ToolApprovalTargetResolver.swift b/Logue/Views/Agent/ToolApprovalTargetResolver.swift new file mode 100644 index 0000000..735f963 --- /dev/null +++ b/Logue/Views/Agent/ToolApprovalTargetResolver.swift @@ -0,0 +1,32 @@ +import Foundation + +/// Turning the id in a tool call into the name of the thing it points at. +/// +/// The other half of `ToolApprovalPrompt`: that file decides the wording and which argument +/// holds the target, and this is the part that has to touch the stores. Split so the wording +/// stays testable without them. +/// +/// Reminders and calendar events are deliberately not resolved. Both live in EventKit behind +/// a permission the user may not have granted, and reaching for one while an approval card is +/// on screen would either block the main actor or prompt for access as a side effect of +/// *drawing* — neither of which is acceptable in a card whose whole job is to be trustworthy. +/// They fall back to the action alone, which is honest: "Delete" with nothing after it says +/// we do not know the name, rather than showing a UUID that says nothing at all. +@MainActor +enum ToolApprovalTargetResolver { + static func name(for reference: ToolApprovalPrompt.Reference) -> String? { + switch reference.kind { + case .document: + DocumentStore.shared.documents.first { $0.id == reference.id }?.title + case .space: + SpaceStore.shared.space(for: reference.id)?.name + case .reminder, .calendarEvent: + nil + } + } + + /// The sentence for a call, with names filled in where we have them. + static func sentence(toolNamed name: String, arguments: String) -> String { + ToolApprovalPrompt.sentence(toolNamed: name, arguments: arguments, resolve: self.name(for:)) + } +} diff --git a/Logue/Views/Agent/ToolExecutionCard.swift b/Logue/Views/Agent/ToolExecutionCard.swift index 31fe979..efb42f3 100644 --- a/Logue/Views/Agent/ToolExecutionCard.swift +++ b/Logue/Views/Agent/ToolExecutionCard.swift @@ -46,8 +46,16 @@ struct ToolExecutionCard: View { // Summary text Text(summaryText) .font(.callout) - .foregroundStyle(.secondary) - .lineLimit(1) + .foregroundStyle( + toolCall.status == .needsConfirmation ? .primary : .secondary + ) + .fontWeight(toolCall.status == .needsConfirmation ? .medium : .regular) + // An approval sentence names a thing the user owns, and a title cut + // in half is how you approve the wrong one. Everything else is a + // record of something that already happened, and stays on one line. + .lineLimit(toolCall.status == .needsConfirmation ? 2 : 1) + .multilineTextAlignment(.leading) + .fixedSize(horizontal: false, vertical: true) Spacer() @@ -142,15 +150,16 @@ struct ToolExecutionCard: View { return completedSummary(from: result.output) } + /// What is about to happen, and to what. + /// + /// Every destructive tool takes a UUID, so the old wording — "Agent wants to delete a + /// document" — was the whole of what the user was told before being asked for Touch ID. + /// Which document was not knowable from the card. private var pendingApprovalText: String { - switch toolCall.toolName { - case "update_document": "Agent wants to update a document" - case "delete_document": "Agent wants to delete a document" - case "rename_space": "Agent wants to rename a space" - case "delete_space": "Agent wants to delete a space" - case "create_calendar_event": "Agent wants to create a calendar event" - default: "Agent wants to run \(toolCall.toolName)" - } + ToolApprovalTargetResolver.sentence( + toolNamed: toolCall.toolName, + arguments: toolCall.arguments + ) } private var runningText: String { diff --git a/LogueTests/ToolApprovalPromptTests.swift b/LogueTests/ToolApprovalPromptTests.swift new file mode 100644 index 0000000..ec90d6b --- /dev/null +++ b/LogueTests/ToolApprovalPromptTests.swift @@ -0,0 +1,145 @@ +import Foundation +import Testing + +@testable import Logue + +/// What an approval card says is about to happen, and to what. +/// +/// The card answered this with five hand-written sentences and a fallback of "Agent wants to +/// run ". None named the thing being acted on — and every destructive tool takes a +/// UUID, so "Agent wants to delete a document" was the whole of what the user was told before +/// being asked for Touch ID. +@Suite("ToolApprovalPrompt") +struct ToolApprovalPromptTests { + private func json(_ pairs: [String: String]) -> String { + let body = pairs.keys.sorted() + .map { "\"\($0)\": \"\(pairs[$0] ?? "")\"" } + .joined(separator: ", ") + return "{\(body)}" + } + + private func sentence( + _ tool: String, + _ arguments: [String: String], + resolving name: String? = nil + ) -> String { + ToolApprovalPrompt.sentence( + toolNamed: tool, + arguments: json(arguments), + resolve: { _ in name } + ) + } + + // MARK: - Coverage + + @Test("Every tool that can ask for approval has something to say") + @MainActor + func everyGatedToolHasAPrompt() { + // Walks the real registry rather than a list kept in step by hand, so adding a + // destructive tool without a sentence is a red build rather than a card reading + // "Run delete_everything" over a Touch ID button. + let gated = AgentCoordinator.allKnownTools().filter { $0.clearance != .regular } + #expect(gated.isEmpty == false, "if this is empty the walk found nothing and proves nothing") + + let missing = gated.map(\.name).filter { !ToolApprovalPrompt.knows(toolNamed: $0) } + #expect(missing.isEmpty, "no approval prompt for: \(missing.sorted())") + } + + // MARK: - Naming the target + + @Test("A document is named, not referred to by its id") + func documentIsNamed() { + let id = UUID().uuidString + let result = sentence("delete_document", ["documentID": id], resolving: "Q3 Planning") + #expect(result == "Delete “Q3 Planning”") + #expect(result.contains(id) == false) + } + + @Test("Deleting a space says what else goes with it") + func spaceDeletionSaysWhatItTakes() { + // delete_space trashes every document and meeting inside it and its children. A + // sentence reading "Delete “Work”" describes a fraction of what the button does. + let result = sentence("delete_space", ["spaceID": UUID().uuidString], resolving: "Work") + #expect(result == "Delete, with everything in it, “Work”") + } + + @Test("A target carried as a literal is used as written") + func literalTargetsAreUsed() { + #expect(sentence("write_text_to_file", ["path": "~/notes/todo.md"]) == "Write to “~/notes/todo.md”") + #expect(sentence("draft_email", ["to": "sam@example.com"]) == "Draft an email to “sam@example.com”") + #expect(sentence("web_search", ["query": "swift actors"]) == "Search the web for “swift actors”") + } + + // MARK: - When the name is not available + + @Test("An unresolvable id leaves the action alone rather than showing a UUID") + func unresolvedTargetShowsNoID() { + // The document is gone, or the model invented the id. A UUID on the card tells the + // user nothing and reads as a bug at the exact moment they are deciding whether to + // trust the agent. + let id = UUID().uuidString + let result = sentence("delete_document", ["documentID": id], resolving: nil) + #expect(result == "Delete") + #expect(result.contains(id) == false) + } + + @Test("A malformed id is not shown either") + func malformedIDIsNotShown() { + let result = sentence("delete_document", ["documentID": "not-a-uuid"], resolving: "Should not appear") + #expect(result == "Delete") + } + + @Test("A missing argument leaves the action alone") + func missingArgumentIsSafe() { + #expect(sentence("delete_document", [:], resolving: "Nope") == "Delete") + #expect(sentence("write_text_to_file", [:]) == "Write to") + } + + @Test("An unknown tool still says it wants to run") + func unknownToolIsHonest() { + // It is still asking for permission, so say so plainly rather than inventing a + // description of something there is no rule for. + #expect(sentence("some_future_tool", [:]) == "Run some_future_tool") + } + + // MARK: - Targets are user-authored text + + @Test("A multi-line title becomes one line") + func targetsAreFlattened() { + // A document title is whatever the user typed, and a newline in it would split the + // sentence in half and leave the verb sitting alone above Approve. + let result = sentence("delete_document", ["documentID": UUID().uuidString], resolving: "Draft\n\nPart two") + #expect(result == "Delete “Draft Part two”") + } + + @Test("A very long target is cut") + func targetsAreBounded() { + let long = String(repeating: "n", count: 400) + let result = sentence("delete_document", ["documentID": UUID().uuidString], resolving: long) + #expect(result.count < long.count) + #expect(result.contains("…")) + } + + // MARK: - References + + @Test("The reference names which store to ask") + func referenceCarriesItsKind() { + let id = UUID() + let document = ToolApprovalPrompt.reference( + toolNamed: "delete_document", + arguments: json(["documentID": id.uuidString]) + ) + #expect(document == ToolApprovalPrompt.Reference(kind: .document, id: id)) + + let space = ToolApprovalPrompt.reference( + toolNamed: "rename_space", + arguments: json(["spaceID": id.uuidString, "newName": "Archive"]) + ) + #expect(space?.kind == .space) + } + + @Test("A tool whose target is a literal has no reference to resolve") + func literalToolsHaveNoReference() { + #expect(ToolApprovalPrompt.reference(toolNamed: "web_search", arguments: json(["query": "x"])) == nil) + } +} From f35e432ce1388a1598cf8879a587f8824c90e068 Mon Sep 17 00:00:00 2001 From: shanforge <299346208+shanforge@users.noreply.github.com> Date: Fri, 21 Aug 2026 11:15:59 +0530 Subject: [PATCH 07/12] feat(island): fill the gap between a send and the first token MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The island had nothing there. Between pressing Return and the first token it showed the user's own bubble and empty space — which is exactly when someone concludes the send did not land and presses Return again. Once the assistant message existed it showed a literal "..." pushed through the markdown renderer, in a grey bubble. It now shows the pulsing row the main window shows, saying what the agent is actually doing rather than a static "Thinking…". The wording already had one definition in UICopy.Status.describe. When to show it did not — it was written out longhand at each place that needed it, and came out differently at each. AgentThinkingState is that half, and both surfaces read it. The input worth naming is pendingAnswerText: what has arrived of the answer being produced *now*, not the last assistant message. The latter is the previous answer and is non-empty for the whole of every later gap, so an indicator keyed on it appears once and never again. There is a case pinning that. Part of #62. --- Logue.xcodeproj/project.pbxproj | 8 +++ Logue/Agent/AgentThinkingState.swift | 44 ++++++++++++ .../Views/Agent/AgentChatView+Messages.swift | 12 +++- .../CommandCenterChatView+Bubbles.swift | 26 +++++-- .../CrossApp/CommandCenterChatView.swift | 39 +++++++++++ LogueTests/AgentThinkingStateTests.swift | 67 +++++++++++++++++++ 6 files changed, 189 insertions(+), 7 deletions(-) create mode 100644 Logue/Agent/AgentThinkingState.swift create mode 100644 LogueTests/AgentThinkingStateTests.swift diff --git a/Logue.xcodeproj/project.pbxproj b/Logue.xcodeproj/project.pbxproj index a58ea94..bad6ac5 100644 --- a/Logue.xcodeproj/project.pbxproj +++ b/Logue.xcodeproj/project.pbxproj @@ -223,6 +223,7 @@ 4C25F8B167705F018F107EEB /* LogueLogo.svg in Resources */ = {isa = PBXBuildFile; fileRef = EC3417F4F42C4F317FA8FD4C /* LogueLogo.svg */; }; 4CBDF92105360575ED4D7688 /* WikiLinkParserTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 019756686F6D3BBAAE229D8F /* WikiLinkParserTests.swift */; }; 4CD2029761EFC71408D044F2 /* TaskItem.swift in Sources */ = {isa = PBXBuildFile; fileRef = D00B89A0E069A596FE5D1AFB /* TaskItem.swift */; }; + 4D0466330331B9B96AC2888F /* AgentThinkingStateTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5F4AFA0A1028BA82C3A9DA06 /* AgentThinkingStateTests.swift */; }; 4DB90578B4CA008D808824C1 /* BrowserBridgeRoute.swift in Sources */ = {isa = PBXBuildFile; fileRef = B0F5C6BB36E68365D56EF996 /* BrowserBridgeRoute.swift */; }; 4DFBBF9A94D043AEAE9573D1 /* SpaceFolderWriteTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7F13A88F235D33A5674EC6CB /* SpaceFolderWriteTests.swift */; }; 4E544BEADC120DA66F931B07 /* MarkdownTextView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5CB6607FD58EE7074846C4DC /* MarkdownTextView.swift */; }; @@ -329,6 +330,7 @@ 6E33CC2D3A37790D824199A3 /* MeetingStore+Diarization.swift in Sources */ = {isa = PBXBuildFile; fileRef = E7FD01A02C6FB9C64A0238CF /* MeetingStore+Diarization.swift */; }; 6E577F8B48F4A11B36E16B66 /* SelectableOptionCard.swift in Sources */ = {isa = PBXBuildFile; fileRef = 54288BA1E89F95FD74442B71 /* SelectableOptionCard.swift */; }; 6E66E9B993B63623F605E552 /* BlockPasteTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 88C42C0A1E60F8FC22AC2EF3 /* BlockPasteTests.swift */; }; + 6E8580FFC37B8084BB4F18EE /* AgentThinkingState.swift in Sources */ = {isa = PBXBuildFile; fileRef = 92E025A4B7180ED3F4CECE52 /* AgentThinkingState.swift */; }; 6EE6B461C7E6CA280E8C8661 /* HierarchicalSpaceMenu.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4714BA50A68B3093B932CB4 /* HierarchicalSpaceMenu.swift */; }; 6EEE4BC7A3149F86FCEB9A15 /* BrowserBridgeSettings.swift in Sources */ = {isa = PBXBuildFile; fileRef = D02332BE58A290E7D19E4D7D /* BrowserBridgeSettings.swift */; }; 6EF0995F3E06CB5ED24FD625 /* DocumentListContentView.swift in Sources */ = {isa = PBXBuildFile; fileRef = A864D7C5937FF77CB9835242 /* DocumentListContentView.swift */; }; @@ -973,6 +975,7 @@ 5E6CEBB82218835592057440 /* SpeakerModels.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SpeakerModels.swift; sourceTree = ""; }; 5E9170536ADC0631BED34CB3 /* MeetingRecordingBar.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MeetingRecordingBar.swift; sourceTree = ""; }; 5EC0BFF05793C2090F8A90C1 /* SidebarWidthLimitTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SidebarWidthLimitTests.swift; sourceTree = ""; }; + 5F4AFA0A1028BA82C3A9DA06 /* AgentThinkingStateTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AgentThinkingStateTests.swift; sourceTree = ""; }; 5F635405A40E8F0C32B93039 /* DetachedBufferTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DetachedBufferTests.swift; sourceTree = ""; }; 5F81FD369FAF345E6BB8A696 /* LibraryPanelToggle.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LibraryPanelToggle.swift; sourceTree = ""; }; 5FBB6345B454F1E296E6DE8D /* AudioTimelineMixerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AudioTimelineMixerTests.swift; sourceTree = ""; }; @@ -1125,6 +1128,7 @@ 9226B7B10683E42ABEB71810 /* HighlightMarkTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HighlightMarkTests.swift; sourceTree = ""; }; 9230215EEC7592F9378CA66C /* ModelManager+Discovery.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "ModelManager+Discovery.swift"; sourceTree = ""; }; 92DB0F899AF6F08FEAF8AA46 /* AgentConversationListView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AgentConversationListView.swift; sourceTree = ""; }; + 92E025A4B7180ED3F4CECE52 /* AgentThinkingState.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AgentThinkingState.swift; sourceTree = ""; }; 92E24E330864CAA8AB7B207A /* HomeSuggestionsInputs.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HomeSuggestionsInputs.swift; sourceTree = ""; }; 92E9FCC3C656E8D24AC7A610 /* LLMTestHarness.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LLMTestHarness.swift; sourceTree = ""; }; 93368B5F8A112B6802BF4001 /* AISettingsTab.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AISettingsTab.swift; sourceTree = ""; }; @@ -1492,6 +1496,7 @@ 79C6BA7C0C06C035AC3BFBD1 /* LLMIntegration */, AC241A87AE9D64BF9E0D2B66 /* ActionItemInboxTests.swift */, 86029AD56EBB4317FFDFB4E4 /* AgentRunStateTests.swift */, + 5F4AFA0A1028BA82C3A9DA06 /* AgentThinkingStateTests.swift */, 7E4CC91DCF127659514C38D2 /* AgentToolTimelineTests.swift */, 4F21C87791C6ECF443D14823 /* AppVersionTests.swift */, 0A4CB54883E92F9A6F98BE77 /* AskRouteTests.swift */, @@ -2226,6 +2231,7 @@ A488F97BC5DBB0C094B1C74F /* AgentCoordinator.swift */, 0027C5C3EE6CAEEB887EAEC7 /* AgentCoordinator+Approval.swift */, 62CA343B98C1F62D9F89B0B6 /* AgentRunState.swift */, + 92E025A4B7180ED3F4CECE52 /* AgentThinkingState.swift */, B2B3921979A175ECA3CF569F /* AgentToolTimeline.swift */, B1CF9F695CCB56896DC9A02C /* ApprovalGate.swift */, 86C6E3021828B2B8589C720A /* AskRoute.swift */, @@ -2723,6 +2729,7 @@ files = ( C7E2AEAF97B253EDD23DA5B5 /* ActionItemInboxTests.swift in Sources */, AAEF3C6465DC0A9EA911FE93 /* AgentRunStateTests.swift in Sources */, + 4D0466330331B9B96AC2888F /* AgentThinkingStateTests.swift in Sources */, CD29FF65F8DE5950B719187A /* AgentToolTimelineTests.swift in Sources */, B71E1192276BBC1904238D1A /* AppVersionTests.swift in Sources */, 85FB21E5E26B2D3D46093D47 /* AskRouteTests.swift in Sources */, @@ -2891,6 +2898,7 @@ 35592A04877BCC129AE90A1E /* AgentDictationService.swift in Sources */, 88A664FB0F648002373F7656 /* AgentReadAloudService.swift in Sources */, 7978A00C56B10809E1BFBD1C /* AgentRunState.swift in Sources */, + 6E8580FFC37B8084BB4F18EE /* AgentThinkingState.swift in Sources */, 17F7BAF0DBA60523B8EFD5C7 /* AgentTool.swift in Sources */, A8432BB73EAB05259ECC7E6E /* AgentToolSpec.swift in Sources */, 845CFBCB97942BA0E22F7D14 /* AgentToolTimeline.swift in Sources */, diff --git a/Logue/Agent/AgentThinkingState.swift b/Logue/Agent/AgentThinkingState.swift new file mode 100644 index 0000000..89cf95c --- /dev/null +++ b/Logue/Agent/AgentThinkingState.swift @@ -0,0 +1,44 @@ +import Foundation + +/// Whether a surface should say it is working, rather than showing an answer. +/// +/// There is a gap between a send and the first token — the model is loading, the context is +/// being built, the loop has not produced anything yet — and it is the moment a user is most +/// likely to conclude nothing happened and press the button again. The main window filled it +/// with a pulsing dot and a status line. The island filled it with a literal `"..."` rendered +/// as markdown, and before the assistant message existed at all it filled it with nothing. +/// +/// The wording already had one definition in `UICopy.Status.describe(toolName:)`. This is the +/// other half — *when* to show it — which was written out longhand at each place that needed +/// it, and therefore came out differently at each place. +/// +/// Free of SwiftUI so the matrix is testable without a view. +enum AgentThinkingState { + /// - Parameters: + /// - isProcessing: a run is in flight for this conversation. + /// - isStreaming: tokens are being delivered for this conversation. + /// - pendingAnswerText: what has arrived of the answer being produced *now*. Empty + /// while nothing has. Deliberately not "the last assistant message", which is the + /// previous answer and is non-empty for the whole of the next gap. + /// - hasActiveToolCard: a tool card is on screen saying what is happening. Two things + /// claiming to explain the same pause is worse than one. + static func showsThinking( + isProcessing: Bool, + isStreaming: Bool, + pendingAnswerText: String, + hasActiveToolCard: Bool + ) -> Bool { + guard isProcessing || isStreaming else { return false } + guard !hasActiveToolCard else { return false } + return pendingAnswerText.isEmpty + } + + /// What the row should say, given whatever the agent is doing. + /// + /// Delegates rather than restating: the strings are `UICopy`'s, and a second copy of the + /// mapping is how one surface starts saying "Thinking…" while the other says "Searching + /// the web…" about the same run. + static func label(activeToolName: String?) -> String { + UICopy.Status.describe(toolName: activeToolName) + } +} diff --git a/Logue/Views/Agent/AgentChatView+Messages.swift b/Logue/Views/Agent/AgentChatView+Messages.swift index 929de77..c3de885 100644 --- a/Logue/Views/Agent/AgentChatView+Messages.swift +++ b/Logue/Views/Agent/AgentChatView+Messages.swift @@ -46,11 +46,17 @@ extension AgentChatView { .id("active-\(call.id.uuidString)") } - // Processing indicator (only when not streaming and no tool calls active) - if isProcessing, activeToolCalls.isEmpty, !isStreaming { + // When to show this, and what it says, are both shared with the + // island — see `AgentThinkingState`. + if AgentThinkingState.showsThinking( + isProcessing: isProcessing, + isStreaming: isStreaming, + pendingAnswerText: streamingText, + hasActiveToolCard: !activeToolCalls.isEmpty + ) { HStack(spacing: 8) { PulsingDot() - Text("Thinking…") + Text(AgentThinkingState.label(activeToolName: activeToolCalls.last?.toolName)) .font(.callout) .foregroundStyle(.secondary) } diff --git a/Logue/Views/CrossApp/CommandCenterChatView+Bubbles.swift b/Logue/Views/CrossApp/CommandCenterChatView+Bubbles.swift index bdfb7e7..71102bd 100644 --- a/Logue/Views/CrossApp/CommandCenterChatView+Bubbles.swift +++ b/Logue/Views/CrossApp/CommandCenterChatView+Bubbles.swift @@ -43,7 +43,7 @@ extension CommandCenterChatView { RoundedRectangle(cornerRadius: 14, style: .continuous) .fill(AppThemeConstants.brandPrimary) ) - } else { + } else if !message.content.isEmpty { markdownContent(message) .padding(.horizontal, 14) .padding(.vertical, 10) @@ -64,10 +64,28 @@ extension CommandCenterChatView { } } - @ViewBuilder + /// The pulsing row the main window shows in the gap before the first token. + /// + /// The island had nothing here. Between a send and the first token it showed the user's + /// own bubble and empty space — which is the moment someone concludes the send did not + /// land and presses Return again. + func thinkingRow(toolName: String?) -> some View { + HStack(spacing: 8) { + PulsingDot() + Text(AgentThinkingState.label(activeToolName: toolName)) + .font(.callout) + .foregroundStyle(.secondary) + } + .frame(maxWidth: .infinity, alignment: .leading) + .accessibilityElement(children: .combine) + .accessibilityLabel(AgentThinkingState.label(activeToolName: toolName)) + } + private func markdownContent(_ message: EphemeralChatMessage) -> some View { - let displayText = message.content.isEmpty && message.isStreaming ? "..." : message.content - StructuredText(markdown: displayText) + // No placeholder. An empty streaming bubble used to render a literal "..." through + // the markdown renderer; the thinking row says the same thing properly, and saying it + // twice put a grey box under it with three dots in it. + StructuredText(markdown: message.content) .font(AppThemeConstants.chatMessageFont) .textual.structuredTextStyle(.gitHub) .textual.inlineStyle(.gitHub) diff --git a/Logue/Views/CrossApp/CommandCenterChatView.swift b/Logue/Views/CrossApp/CommandCenterChatView.swift index d67af98..4d391c7 100644 --- a/Logue/Views/CrossApp/CommandCenterChatView.swift +++ b/Logue/Views/CrossApp/CommandCenterChatView.swift @@ -247,6 +247,12 @@ struct CommandCenterChatView: View { islandRow(row) .id(row.id) } + + if showsThinking { + thinkingRow(toolName: activeToolName) + .id("island-thinking") + .transition(.opacity) + } } .padding(.horizontal, 20) .padding(.vertical, 12) @@ -384,6 +390,39 @@ struct CommandCenterChatView: View { return AgentToolTimeline.awaitingApproval(in: conversation.messages) } + /// Whether to say the island is working rather than show an answer. + /// + /// The same rule the main window uses. Note what is passed as the pending answer: the + /// *last* message's text only when it is the one being streamed. Reading the last + /// assistant message unconditionally would be the previous answer, which is non-empty for + /// the whole of every later gap — so the indicator would never appear again after the + /// first reply. + private var showsThinking: Bool { + guard let conversationID else { return false } + let streaming = coordinator.isStreaming(in: conversationID) + let pending: String = if case let .message(message)? = rows.last, message.isStreaming { + message.content + } else { + "" + } + return AgentThinkingState.showsThinking( + isProcessing: coordinator.isProcessing(in: conversationID), + isStreaming: streaming, + pendingAnswerText: pending, + hasActiveToolCard: !activeToolCalls.isEmpty + ) + } + + /// The tool the island is currently running, which is what the thinking row names. + private var activeToolName: String? { + activeToolCalls.last?.toolName + } + + private var activeToolCalls: [AgentToolCall] { + guard let conversationID else { return [] } + return coordinator.activeToolCalls(in: conversationID) + } + /// The error for this thread, if the last run left one. /// /// Scoped like every other read: an error from a run the main window started is that diff --git a/LogueTests/AgentThinkingStateTests.swift b/LogueTests/AgentThinkingStateTests.swift new file mode 100644 index 0000000..8b484dc --- /dev/null +++ b/LogueTests/AgentThinkingStateTests.swift @@ -0,0 +1,67 @@ +import Foundation +import Testing + +@testable import Logue + +/// When a surface should say it is working. +/// +/// The gap between a send and the first token is the moment a user is most likely to decide +/// nothing happened and press the button again, and it was the gap the island did not fill. +@Suite("AgentThinkingState") +struct AgentThinkingStateTests { + private func shows( + processing: Bool = false, + streaming: Bool = false, + pending: String = "", + toolCard: Bool = false + ) -> Bool { + AgentThinkingState.showsThinking( + isProcessing: processing, + isStreaming: streaming, + pendingAnswerText: pending, + hasActiveToolCard: toolCard + ) + } + + @Test("The gap between a send and the first token is filled") + func theGapIsFilled() { + // Before any assistant message exists at all. The island showed nothing here. + #expect(shows(processing: true)) + // And once it exists but is still empty. + #expect(shows(processing: true, streaming: true, pending: "")) + } + + @Test("It stops as soon as there is something to read") + func stopsOnFirstToken() { + #expect(shows(processing: true, streaming: true, pending: "The answer is") == false) + } + + @Test("An idle conversation says nothing") + func idleSaysNothing() { + #expect(shows() == false) + #expect(shows(pending: "an old answer") == false) + } + + @Test("A tool card already explains the pause") + func toolCardWins() { + // Two things claiming to explain the same pause is worse than one — and the card is + // the more specific of them, since it names the tool. + #expect(shows(processing: true, toolCard: true) == false) + #expect(shows(processing: true, streaming: true, pending: "", toolCard: true) == false) + } + + @Test("The previous answer does not suppress the next gap") + func previousAnswerIsNotThePendingOne() { + // The bug this input shape avoids: reading "the last assistant message" instead of + // the answer being produced now means the indicator never shows again after the + // first reply, because that message is non-empty for the whole of every later gap. + #expect(shows(processing: true, pending: "")) + } + + @Test("The label follows what the agent is doing") + func labelTracksTheTool() { + #expect(AgentThinkingState.label(activeToolName: nil) == UICopy.Status.thinking) + #expect(AgentThinkingState.label(activeToolName: "web_search") == UICopy.Status.searching) + #expect(AgentThinkingState.label(activeToolName: "get_transcript") == UICopy.Status.reading) + } +} From 9a20eb90340e546c09b79cf74c55a468181d5b8f Mon Sep 17 00:00:00 2001 From: shanforge <299346208+shanforge@users.noreply.github.com> Date: Fri, 21 Aug 2026 11:29:51 +0530 Subject: [PATCH 08/12] fix(island): the chip row is part of the layout, and it is bounded MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The island floated its chips in an overlay offset 34pt above the pill. That meant two things: they took no layout space, so with a conversation on screen they drew over the bottom of the transcript; and nothing bounded them, so attaching six files ran the row past the island's width and off both ends. The row is now in the VStack, and ComposerChipRow decides what fits. The ordering rule is the part worth stating: modes are never hidden. A mode chip says what the send is about to do — search the web, spend minutes on Deep Research — and a hidden one is a send the user did not know they were making. Attachments overflow into a counter instead, which loses nothing, because the files are still attached. The counter carries the names it hides, as a tooltip and as its VoiceOver label. A chip that hides a file with no way to find out which is worse than a row that overflows. Modes are a value now rather than two booleans added up wherever a count was needed. Part of #62. --- Logue.xcodeproj/project.pbxproj | 8 + Logue/Agent/ComposerChipRow.swift | 52 +++++++ .../CommandCenterChatView+Composer.swift | 138 ++++++++++++------ .../CrossApp/CommandCenterChatView.swift | 11 ++ LogueTests/ComposerChipRowTests.swift | 91 ++++++++++++ 5 files changed, 259 insertions(+), 41 deletions(-) create mode 100644 Logue/Agent/ComposerChipRow.swift create mode 100644 LogueTests/ComposerChipRowTests.swift diff --git a/Logue.xcodeproj/project.pbxproj b/Logue.xcodeproj/project.pbxproj index bad6ac5..699bf5a 100644 --- a/Logue.xcodeproj/project.pbxproj +++ b/Logue.xcodeproj/project.pbxproj @@ -124,6 +124,7 @@ 24AEFED6B0C5ECFA408B9C00 /* BrowserExtensionPromo.swift in Sources */ = {isa = PBXBuildFile; fileRef = A00FDC51A0E097F93FBEB05E /* BrowserExtensionPromo.swift */; }; 24C4E391A8A690C60D26E51A /* CanvasController.swift in Sources */ = {isa = PBXBuildFile; fileRef = AB988721E93C1D96F2216DF0 /* CanvasController.swift */; }; 24C5EF79903E1BC7FC98A62E /* WikiLinkCompletion.swift in Sources */ = {isa = PBXBuildFile; fileRef = 24994CE786E8D4635390440F /* WikiLinkCompletion.swift */; }; + 24FDEFC218364FE01BBB362D /* ComposerChipRow.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8C5F05B096345A4B230EC91C /* ComposerChipRow.swift */; }; 255EB07018B299213FC6BED4 /* whatsnew-transcription.png in Resources */ = {isa = PBXBuildFile; fileRef = 73C82679F61FC2AC07FC9220 /* whatsnew-transcription.png */; }; 2588490FBA16F79B755839A9 /* SuggestionParser.swift in Sources */ = {isa = PBXBuildFile; fileRef = FCEBD05BEF5831C92DDDE40E /* SuggestionParser.swift */; }; 25A2038F3D3B1815856A1913 /* TaskTriageTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5967FC0A683B57D96AAAB8CF /* TaskTriageTests.swift */; }; @@ -357,6 +358,7 @@ 784D6D17FB7448EB715CD529 /* HighlightAnchor.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7D7288BD26E235EF2434855B /* HighlightAnchor.swift */; }; 79784E41E35CF492424E5E02 /* BrowserBridgeTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6DF44F4AF480BE5D761ACE31 /* BrowserBridgeTests.swift */; }; 7978A00C56B10809E1BFBD1C /* AgentRunState.swift in Sources */ = {isa = PBXBuildFile; fileRef = 62CA343B98C1F62D9F89B0B6 /* AgentRunState.swift */; }; + 79C47DDA34BDF3E7A521DCCC /* ComposerChipRowTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 91F82D156EB771A80BD3BF0A /* ComposerChipRowTests.swift */; }; 7A30C41E7B8898E2FF5CA750 /* TemplateStore.swift in Sources */ = {isa = PBXBuildFile; fileRef = 851703C5995A9B2FA0C07975 /* TemplateStore.swift */; }; 7A79E80EA56AABEECB3D5DED /* MeetingNote.swift in Sources */ = {isa = PBXBuildFile; fileRef = D928297CC59CB70B20090442 /* MeetingNote.swift */; }; 7B000982F90A41A9148CD0F7 /* AnthropicClient.swift in Sources */ = {isa = PBXBuildFile; fileRef = 80F43E54BB04A47E0876AED0 /* AnthropicClient.swift */; }; @@ -1109,6 +1111,7 @@ 89D2D180775E0F9C8735345F /* ShortcutsSettingsTab.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ShortcutsSettingsTab.swift; sourceTree = ""; }; 8A5EAC4820F6E13CFDE208D0 /* MeetingAIChatPanelView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MeetingAIChatPanelView.swift; sourceTree = ""; }; 8AED2C469157178636079073 /* PDFExportService.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PDFExportService.swift; sourceTree = ""; }; + 8C5F05B096345A4B230EC91C /* ComposerChipRow.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ComposerChipRow.swift; sourceTree = ""; }; 8CCF492C5BB1691EC0946B25 /* InlineLaTeXView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = InlineLaTeXView.swift; sourceTree = ""; }; 8D147FADD3CE97D983829125 /* VoiceInputIndicator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = VoiceInputIndicator.swift; sourceTree = ""; }; 8D204801C0C2BADD68D406D7 /* BlockMoveTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BlockMoveTests.swift; sourceTree = ""; }; @@ -1124,6 +1127,7 @@ 9018379C22E39E1C199D81C6 /* EmbeddingService.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EmbeddingService.swift; sourceTree = ""; }; 90E4CF4E9246459183E66FD7 /* WritingAgentState.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WritingAgentState.swift; sourceTree = ""; }; 9130B8EDD12207674F5B4323 /* DocumentIconTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DocumentIconTests.swift; sourceTree = ""; }; + 91F82D156EB771A80BD3BF0A /* ComposerChipRowTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ComposerChipRowTests.swift; sourceTree = ""; }; 920C02E8AB5AA086F500AFC5 /* CategorySidebarView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CategorySidebarView.swift; sourceTree = ""; }; 9226B7B10683E42ABEB71810 /* HighlightMarkTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HighlightMarkTests.swift; sourceTree = ""; }; 9230215EEC7592F9378CA66C /* ModelManager+Discovery.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "ModelManager+Discovery.swift"; sourceTree = ""; }; @@ -1515,6 +1519,7 @@ 4AB2A7A4F24E2BC67AEE4319 /* CalloutRoundTripEdgeTests.swift */, D2856879E369BF8160EBAA1B /* CaptureSegmentTimelineTests.swift */, 136ABA45F11F81E853897C17 /* CommandCenterChatRuleTests.swift */, + 91F82D156EB771A80BD3BF0A /* ComposerChipRowTests.swift */, 83C272112D79CC98FD132288 /* ContentNavigatorTests.swift */, 263E3C59B8D03346E04E7C92 /* DeepLinkRoutingTests.swift */, EB27F11A2333C941E28C916E /* DeepLinkTests.swift */, @@ -2236,6 +2241,7 @@ B1CF9F695CCB56896DC9A02C /* ApprovalGate.swift */, 86C6E3021828B2B8589C720A /* AskRoute.swift */, 417436A70F2C0F1DDA347DC4 /* AttachmentIntake.swift */, + 8C5F05B096345A4B230EC91C /* ComposerChipRow.swift */, 4B2F874FCD0AF25F7B37D3FC /* MalformedToolCall.swift */, B55FA53C32F4D2968C07CEE5 /* MessageActions.swift */, 4542B2571154754E0D0E8783 /* ToolApprovalPrompt.swift */, @@ -2748,6 +2754,7 @@ B27F60023417DD2C91BC326C /* CalloutRoundTripEdgeTests.swift in Sources */, A11728D4726E858C626D681E /* CaptureSegmentTimelineTests.swift in Sources */, 3A1F8B924E2BAA7DD5C45A6A /* CommandCenterChatRuleTests.swift in Sources */, + 79C47DDA34BDF3E7A521DCCC /* ComposerChipRowTests.swift in Sources */, 2EBF59EE30602911A0C7236E /* ContentNavigatorTests.swift in Sources */, 8837715694AEB1053618212A /* DeepLinkRoutingTests.swift in Sources */, 52B089BBC06F1A4DC8CFA5CE /* DeepLinkTests.swift in Sources */, @@ -2977,6 +2984,7 @@ 2206F8D1450B6DA3886A850F /* CommandCenterRecordingView.swift in Sources */, 03C16AA3E0D4260F0D389809 /* CommandPaletteView.swift in Sources */, 3FEF62A652D6865C8B97DD35 /* CommunityDetector.swift in Sources */, + 24FDEFC218364FE01BBB362D /* ComposerChipRow.swift in Sources */, 8AFFF64C0EC6F5AA2678504A /* ConferencingAppDetector.swift in Sources */, 5AAAF36F01B586D73D6A35EF /* ContactsTool.swift in Sources */, DF6799F0DCAEEA99A301E63E /* ContentLoadingView.swift in Sources */, diff --git a/Logue/Agent/ComposerChipRow.swift b/Logue/Agent/ComposerChipRow.swift new file mode 100644 index 0000000..db6f7bd --- /dev/null +++ b/Logue/Agent/ComposerChipRow.swift @@ -0,0 +1,52 @@ +import Foundation + +/// How many chips a composer shows before it starts counting the rest. +/// +/// The island floated its chips in an overlay offset above the pill, which meant two things. +/// They took no layout space, so with a conversation on screen they drew over the bottom of +/// the transcript; and nothing bounded them, so attaching six files ran the row past the +/// island's width and off both ends. +/// +/// The ordering rule is the part worth stating: **modes are never hidden.** A mode chip says +/// what the send is about to *do* — search the web, spend minutes on Deep Research — and a +/// hidden one is a send the user did not know they were making. An attachment chip only says +/// what is going with it, and "+3 more" loses nothing that matters, because the files are +/// still attached and still listed the moment one is removed. +/// +/// Free of SwiftUI so the arithmetic is testable without a bar to put it in. +enum ComposerChipRow { + /// What to draw. + struct Layout: Equatable { + /// Mode chips to render. Always every one of them. + let modes: Int + /// Attachment chips to render, oldest first. + let attachments: Int + /// Attachments not rendered, summarised by a single counter chip. + let hidden: Int + + var showsOverflow: Bool { + hidden > 0 + } + } + + /// Chips the island's single row can hold before it looks like a list. + static let islandLimit = 4 + + static func layout(modeCount: Int, attachmentCount: Int, limit: Int = islandLimit) -> Layout { + let modes = max(0, modeCount) + let attachments = max(0, attachmentCount) + + // Modes first, and they are not subject to the limit — see the note above. A limit + // smaller than the number of modes is a layout that cannot be honoured, and dropping + // a mode is the wrong way to honour it. + let remaining = max(0, limit - modes) + guard attachments > remaining else { + return Layout(modes: modes, attachments: attachments, hidden: 0) + } + + // One of the remaining slots goes to the counter itself, so the row does not grow by + // adding the thing that says the row is full. + let shown = max(0, remaining - 1) + return Layout(modes: modes, attachments: shown, hidden: attachments - shown) + } +} diff --git a/Logue/Views/CrossApp/CommandCenterChatView+Composer.swift b/Logue/Views/CrossApp/CommandCenterChatView+Composer.swift index 007a74c..bcc4d31 100644 --- a/Logue/Views/CrossApp/CommandCenterChatView+Composer.swift +++ b/Logue/Views/CrossApp/CommandCenterChatView+Composer.swift @@ -144,12 +144,6 @@ extension CommandCenterChatView { .padding(.trailing, 10) .padding(.vertical, 10) .islandSurface(cornerRadius: 22) - .overlay(alignment: .top) { - if !attachments.isEmpty || isWebSearchOnce || isDeepResearchOnce { - attachmentChips - .offset(y: -34) - } - } // Dropping onto the pill is the same intake as the picker, so a file arrives the // same way whichever route the user takes. .onDrop(of: [.fileURL], isTargeted: nil) { providers in @@ -202,47 +196,109 @@ extension CommandCenterChatView { } /// What is staged for the next send, with a way to take each one back off. - private var attachmentChips: some View { - HStack(spacing: 6) { - if isDeepResearchOnce { - ModeChip( + /// + /// Part of the island's layout rather than an overlay floating above the pill. As an + /// overlay it took no space, so with a conversation on screen it drew over the bottom of + /// the transcript — and nothing bounded it, so six files ran the row off both ends. + var stagedChips: some View { + let layout = ComposerChipRow.layout( + modeCount: activeModes.count, + attachmentCount: attachments.count + ) + let shown = attachments.prefix(layout.attachments) + + return HStack(spacing: 6) { + ForEach(activeModes) { mode in + ModeChip(title: mode.title, systemImage: mode.systemImage, tint: mode.tint) { + mode.turnOff() + } + } + ForEach(shown) { attachment in + attachmentChip(attachment) + } + if layout.showsOverflow { + Text("+\(layout.hidden) more") + .font(.caption2.weight(.medium)) + .foregroundStyle(.white.opacity(0.6)) + .padding(.horizontal, 8) + .padding(.vertical, 4) + .background(Capsule().fill(Color.white.opacity(0.10))) + .help(hiddenNames(after: layout.attachments)) + .accessibilityLabel("\(layout.hidden) more attachments: \(hiddenNames(after: layout.attachments))") + } + Spacer(minLength: 0) + } + .padding(.bottom, 6) + } + + /// The names behind the counter, so nothing is unreachable — hover, or VoiceOver, reads + /// them out. A chip that hides a file with no way to find out which one is worse than a + /// row that overflows. + private func hiddenNames(after shown: Int) -> String { + attachments.dropFirst(shown).map(\.displayName).joined(separator: ", ") + } + + private func attachmentChip(_ attachment: TempAttachment) -> some View { + HStack(spacing: 4) { + Image(systemName: "doc") + .font(.caption2) + Text(attachment.displayName) + .font(.caption2) + .lineLimit(1) + .truncationMode(.middle) + Button { + attachments.removeAll { $0.id == attachment.id } + } label: { + Image(systemName: "xmark") + .font(.system(size: 8, weight: .bold)) + } + .buttonStyle(.plain) + .accessibilityLabel("Remove \(attachment.displayName)") + } + .foregroundStyle(.white.opacity(0.8)) + .padding(.horizontal, 8) + .padding(.vertical, 4) + .background(Capsule().fill(Color.white.opacity(0.12))) + } + + /// The per-send modes that are on, in the order they are drawn. + /// + /// Modelled so the count is a number `ComposerChipRow` can be handed rather than two + /// booleans the row has to remember to add up. + private var activeModes: [ComposerMode] { + var modes: [ComposerMode] = [] + if isDeepResearchOnce { + modes.append( + ComposerMode( + id: "deepResearch", title: "Deep Research", systemImage: "sparkle.magnifyingglass", tint: AppThemeConstants.brandPrimary - ) { - isDeepResearchOnce = false - } - } - if isWebSearchOnce { - ModeChip( + ) { isDeepResearchOnce = false } + ) + } + if isWebSearchOnce { + modes.append( + ComposerMode( + id: "search", title: "Search", systemImage: "globe", tint: AppThemeConstants.brandPrimary - ) { - isWebSearchOnce = false - } - } - ForEach(attachments) { attachment in - HStack(spacing: 4) { - Image(systemName: "doc") - .font(.caption2) - Text(attachment.displayName) - .font(.caption2) - .lineLimit(1) - Button { - attachments.removeAll { $0.id == attachment.id } - } label: { - Image(systemName: "xmark") - .font(.system(size: 8, weight: .bold)) - } - .buttonStyle(.plain) - .accessibilityLabel("Remove \(attachment.displayName)") - } - .foregroundStyle(.white.opacity(0.8)) - .padding(.horizontal, 8) - .padding(.vertical, 4) - .background(Capsule().fill(Color.white.opacity(0.12))) - } + ) { isWebSearchOnce = false } + ) } + return modes } } + +/// One per-send mode, as the chip row draws it. +/// +/// A value rather than two booleans read in three places, so the row can count them and the +/// order they appear in is stated once. +struct ComposerMode: Identifiable { + let id: String + let title: String + let systemImage: String + let tint: Color + let turnOff: () -> Void +} diff --git a/Logue/Views/CrossApp/CommandCenterChatView.swift b/Logue/Views/CrossApp/CommandCenterChatView.swift index 4d391c7..cc8f904 100644 --- a/Logue/Views/CrossApp/CommandCenterChatView.swift +++ b/Logue/Views/CrossApp/CommandCenterChatView.swift @@ -130,6 +130,12 @@ struct CommandCenterChatView: View { .transition(.opacity) } + // In the layout rather than floating over it: as an overlay offset above the + // pill, the chip row took no space and drew over the bottom of the transcript. + if hasStagedChips { + stagedChips + } + promptPill } .frame(width: pillWidth) @@ -390,6 +396,11 @@ struct CommandCenterChatView: View { return AgentToolTimeline.awaitingApproval(in: conversation.messages) } + /// Whether anything is staged for the next send. + private var hasStagedChips: Bool { + !attachments.isEmpty || isWebSearchOnce || isDeepResearchOnce + } + /// Whether to say the island is working rather than show an answer. /// /// The same rule the main window uses. Note what is passed as the pending answer: the diff --git a/LogueTests/ComposerChipRowTests.swift b/LogueTests/ComposerChipRowTests.swift new file mode 100644 index 0000000..bf1d1b7 --- /dev/null +++ b/LogueTests/ComposerChipRowTests.swift @@ -0,0 +1,91 @@ +import Foundation +import Testing + +@testable import Logue + +/// How many chips a composer shows before it starts counting the rest. +@Suite("ComposerChipRow") +struct ComposerChipRowTests { + private func layout(modes: Int, attachments: Int, limit: Int = ComposerChipRow.islandLimit) -> ComposerChipRow.Layout { + ComposerChipRow.layout(modeCount: modes, attachmentCount: attachments, limit: limit) + } + + @Test("A row that fits shows everything") + func everythingFits() { + let result = layout(modes: 1, attachments: 2) + #expect(result == ComposerChipRow.Layout(modes: 1, attachments: 2, hidden: 0)) + #expect(result.showsOverflow == false) + } + + @Test("Exactly filling the row still hides nothing") + func exactFitHidesNothing() { + // The boundary that invites an off-by-one: at the limit there is nothing to count, so + // spending a slot on a counter reading "+0 more" would be worse than not having one. + let result = layout(modes: 0, attachments: 4) + #expect(result.attachments == 4) + #expect(result.hidden == 0) + } + + @Test("Overflow is counted, and the counter pays for its own slot") + func overflowTakesASlot() { + // Four slots, six files: three chips and "+3 more" — not four chips and "+2 more", + // which would be five things in a four-slot row. + let result = layout(modes: 0, attachments: 6) + #expect(result.attachments == 3) + #expect(result.hidden == 3) + #expect(result.attachments + 1 <= ComposerChipRow.islandLimit) + } + + @Test("Nothing is ever lost") + func everyAttachmentIsAccountedFor() { + for attachments in 0 ... 40 { + for modes in 0 ... 2 { + let result = layout(modes: modes, attachments: attachments) + #expect(result.attachments + result.hidden == attachments) + #expect(result.modes == modes) + } + } + } + + // MARK: - Modes are never hidden + + @Test("Modes survive a row too small to hold them") + func modesAreNeverDropped() { + // A mode chip says what the send is about to *do*. A hidden Deep Research chip is a + // send the user did not know they were making — so a limit that cannot be honoured is + // exceeded rather than met by dropping one. + let result = layout(modes: 2, attachments: 0, limit: 1) + #expect(result.modes == 2) + } + + @Test("Modes take their slots before attachments do") + func modesComeFirst() { + let withoutModes = layout(modes: 0, attachments: 5) + let withModes = layout(modes: 2, attachments: 5) + #expect(withModes.attachments < withoutModes.attachments) + #expect(withModes.hidden > withoutModes.hidden) + } + + @Test("A row that is all modes hides every attachment behind a counter") + func modesCanConsumeTheRow() { + let result = layout(modes: 4, attachments: 3) + #expect(result.modes == 4) + #expect(result.attachments == 0) + #expect(result.hidden == 3, "still counted, never silently dropped") + } + + // MARK: - Degenerate inputs + + @Test("Nothing in, nothing out") + func emptyIsEmpty() { + #expect(layout(modes: 0, attachments: 0) == ComposerChipRow.Layout(modes: 0, attachments: 0, hidden: 0)) + } + + @Test("Negative counts cannot produce negative chips") + func negativesAreClamped() { + let result = layout(modes: -3, attachments: -7) + #expect(result.modes == 0) + #expect(result.attachments == 0) + #expect(result.hidden == 0) + } +} From 59f7055ca73be385f7d708ad81d1e044ee625727 Mon Sep 17 00:00:00 2001 From: shanforge <299346208+shanforge@users.noreply.github.com> Date: Fri, 21 Aug 2026 11:46:15 +0530 Subject: [PATCH 09/12] fix(island): every control says what it is, not what its icon is called MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Almost every control in the island carried only .help(…). On macOS that becomes the accessibility hint, not the label — so VoiceOver fell back to the only other thing it had, the SF Symbol name. The send button announced "arrow up"; the close button announced "xmark"; and the two mode toggles announced nothing about being on, which is the single thing a toggle exists to convey. IslandControlCopy holds the name, the state and the tooltip together so they cannot drift, and .islandControl applies all three at once — writing them out separately at each call site is exactly how one gets forgotten, which is how this happened. Send and Stop are named differently rather than being one control in two states. A button that keeps the name "Send" while it cancels is how someone stops a run they meant to let finish. The test walks every control in every state and rejects a dotted, lowercase label. Restoring "arrow.up" on the send button turns it red. Part of #62. --- Logue.xcodeproj/project.pbxproj | 8 ++ Logue/UI/IslandControlCopy.swift | 95 ++++++++++++++++++ .../CommandCenterChatView+Composer.swift | 16 ++-- .../CrossApp/CommandCenterChatView.swift | 4 +- .../CrossApp/IslandSurfaceModifier.swift | 14 +++ LogueTests/IslandControlCopyTests.swift | 96 +++++++++++++++++++ 6 files changed, 222 insertions(+), 11 deletions(-) create mode 100644 Logue/UI/IslandControlCopy.swift create mode 100644 LogueTests/IslandControlCopyTests.swift diff --git a/Logue.xcodeproj/project.pbxproj b/Logue.xcodeproj/project.pbxproj index 699bf5a..a227bbe 100644 --- a/Logue.xcodeproj/project.pbxproj +++ b/Logue.xcodeproj/project.pbxproj @@ -256,6 +256,7 @@ 559A068B64BFBACC538549C3 /* FileAccessGate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3E86EBBF6643966395A7DE8B /* FileAccessGate.swift */; }; 5649718294F9E5770348BD76 /* LLMTestEvalExtensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 31ED31FAD045668E64BA99B1 /* LLMTestEvalExtensions.swift */; }; 565CE023D3242EDD8DE16D27 /* ToolApprovalPromptTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = A2B422B1EA792B27E116DA2C /* ToolApprovalPromptTests.swift */; }; + 5688E339841DC63FA2F61E8E /* IslandControlCopy.swift in Sources */ = {isa = PBXBuildFile; fileRef = 86BACB5F9085F6C7DB5B4CD8 /* IslandControlCopy.swift */; }; 56910F4D1E3DBFB9D424BE65 /* ExternalChangePlan.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1C141390FECB3AC097A5F95B /* ExternalChangePlan.swift */; }; 56F4853038E368546DCD2955 /* JavaScriptTool.swift in Sources */ = {isa = PBXBuildFile; fileRef = 31F56338AE354AB4607F3597 /* JavaScriptTool.swift */; }; 572EFABAA476D44DB7AB8748 /* ReminderManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6ED5A7948FEFC3B58D925DFC /* ReminderManager.swift */; }; @@ -439,6 +440,7 @@ 98D77D5ABC4EB705A7CB0F58 /* MeetingSpeakersPanelView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 461D5E9CBBBFA62E23630E08 /* MeetingSpeakersPanelView.swift */; }; 99798008D35E27D8502DAE0F /* SpaceContentPane.swift in Sources */ = {isa = PBXBuildFile; fileRef = 40381D10206F56D057943756 /* SpaceContentPane.swift */; }; 9999C3BBEDA4DC345CD53950 /* ShortcutsSettingsTab.swift in Sources */ = {isa = PBXBuildFile; fileRef = 89D2D180775E0F9C8735345F /* ShortcutsSettingsTab.swift */; }; + 99B0158F57D74A593DAFF903 /* IslandControlCopyTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D21D2BCBEBA7DC11B30D5CAB /* IslandControlCopyTests.swift */; }; 99CD79938A011D1BD5FDAAC7 /* MeetingListPane.swift in Sources */ = {isa = PBXBuildFile; fileRef = C51643D004E005B44B955343 /* MeetingListPane.swift */; }; 99E3A34E78B854F7D17BC4CC /* ScratchDefaults.swift in Sources */ = {isa = PBXBuildFile; fileRef = 87A4DEDD1EB5AD63B3449584 /* ScratchDefaults.swift */; }; 99FCB8FEB17A1DA884E6892C /* CommandCenterController.swift in Sources */ = {isa = PBXBuildFile; fileRef = C8D8F63B0BEBD964C1409AA5 /* CommandCenterController.swift */; }; @@ -1098,6 +1100,7 @@ 86029AD56EBB4317FFDFB4E4 /* AgentRunStateTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AgentRunStateTests.swift; sourceTree = ""; }; 860DFF869B9C6C39BE468431 /* DiagnosticsReportTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DiagnosticsReportTests.swift; sourceTree = ""; }; 865AE91088D3794F9C1CC457 /* CommandCenterChatRule.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CommandCenterChatRule.swift; sourceTree = ""; }; + 86BACB5F9085F6C7DB5B4CD8 /* IslandControlCopy.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = IslandControlCopy.swift; sourceTree = ""; }; 86C6E3021828B2B8589C720A /* AskRoute.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AskRoute.swift; sourceTree = ""; }; 86F393DC727834A59AB47A62 /* AIChatPanelView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AIChatPanelView.swift; sourceTree = ""; }; 8756576B17C7D359AC9FC6A0 /* VerifyPanelView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = VerifyPanelView.swift; sourceTree = ""; }; @@ -1306,6 +1309,7 @@ D109C6B8E49B6D4DEC709DB5 /* WorkspaceTransferable.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WorkspaceTransferable.swift; sourceTree = ""; }; D13D5598C230DA3644325779 /* FolderStore.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FolderStore.swift; sourceTree = ""; }; D1A491C56C1501F3D06F7734 /* PulsingDot.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PulsingDot.swift; sourceTree = ""; }; + D21D2BCBEBA7DC11B30D5CAB /* IslandControlCopyTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = IslandControlCopyTests.swift; sourceTree = ""; }; D228A8EDB45295F064F77732 /* PromptRegistry+Agent.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "PromptRegistry+Agent.swift"; sourceTree = ""; }; D238B8169934A609E62E8D9E /* SavedViewFilterTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SavedViewFilterTests.swift; sourceTree = ""; }; D245AD8F8FF9957DC52A4ADC /* AppIcon.icns */ = {isa = PBXFileReference; lastKnownFileType = image.icns; path = AppIcon.icns; sourceTree = ""; }; @@ -1547,6 +1551,7 @@ F773F30D16F4FF92E56320E1 /* HomeContinueGridTests.swift */, A3600798EF27A735FC4BC80E /* HomeSuggestionsTests.swift */, 0062B04D59734428AFC49682 /* ImportFileReadingTests.swift */, + D21D2BCBEBA7DC11B30D5CAB /* IslandControlCopyTests.swift */, 464AB56546E36263CEB595E8 /* IslandSurfaceTests.swift */, 402D8E4447C9B7063ADE7358 /* IslandThreadTests.swift */, 7F48D46D3D40534008356F5D /* LinkIndexBuildTests.swift */, @@ -1708,6 +1713,7 @@ 141A999128E664D7D97120BB /* CharCounter.swift */, 207FDDB5AF3841198B2438E4 /* ChatHomeEmptyState.swift */, CFB4EE100316EB840B5D93EE /* HoverToolbar.swift */, + 86BACB5F9085F6C7DB5B4CD8 /* IslandControlCopy.swift */, 2666524E8E079ED62C748893 /* JumpToLatestPill.swift */, E28B906C35C686A1F97C984E /* ModelChip.swift */, 3E2383DBE96586FC70FE52CB /* Motion.swift */, @@ -2784,6 +2790,7 @@ B68578CA8ABFB343EABB454C /* HomeContinueGridTests.swift in Sources */, 549C3D0D72BD69E53B7929A5 /* HomeSuggestionsTests.swift in Sources */, D0FFBA721311ED3F007A7D49 /* ImportFileReadingTests.swift in Sources */, + 99B0158F57D74A593DAFF903 /* IslandControlCopyTests.swift in Sources */, A1A603E973000958C89D1EDF /* IslandSurfaceTests.swift in Sources */, 2F4BA10FF02C91BDC115F449 /* IslandThreadTests.swift in Sources */, 5649718294F9E5770348BD76 /* LLMTestEvalExtensions.swift in Sources */, @@ -3105,6 +3112,7 @@ 9FC5E9AC4D7755C95151D036 /* InsightCardShell.swift in Sources */, 8621E76EC5A20A48091893C4 /* InsightsSectionView.swift in Sources */, 3C8B6B59AC5A90F65186C43C /* InsightsStatsProvider.swift in Sources */, + 5688E339841DC63FA2F61E8E /* IslandControlCopy.swift in Sources */, EC1F72AE3061CEF89F456398 /* IslandSurface.swift in Sources */, 69B2689283C249CBE8252ADE /* IslandSurfaceModifier.swift in Sources */, CF68588BF9879F2449BC4BEE /* IslandThread.swift in Sources */, diff --git a/Logue/UI/IslandControlCopy.swift b/Logue/UI/IslandControlCopy.swift new file mode 100644 index 0000000..5018e2f --- /dev/null +++ b/Logue/UI/IslandControlCopy.swift @@ -0,0 +1,95 @@ +import Foundation + +/// What each of the island's controls is called, and what it says about its state. +/// +/// Almost every control in the island carried only `.help(…)`. On macOS that becomes the +/// accessibility *hint*, not the label — so VoiceOver fell back to the only other thing it +/// had, the SF Symbol name. The send button announced "arrow up", the close button announced +/// "xmark", and the two toggles announced nothing at all about being on, which is the single +/// thing a toggle exists to convey. +/// +/// A label is not the tooltip. The tooltip explains; the label names. Both are here so they +/// cannot drift apart, and both are pure, so a control that ships announcing its icon is a +/// failing test rather than something only a VoiceOver user finds out about. +enum IslandControlCopy { + /// One control, as the accessibility tree should see it. + struct Control: Equatable { + /// What it is. Never a symbol name, never a sentence. + let label: String + /// What state it is in, for controls that have one. + let value: String? + /// What it will do. Also the tooltip. + let hint: String + } + + static let on = "On" + static let off = "Off" + + static func attach(isBusy: Bool) -> Control { + Control( + label: "Attach files", + value: nil, + hint: isBusy ? "Not available while Logue is answering" : "Attach files to this message" + ) + } + + static func webSearch(isOn: Bool) -> Control { + Control( + label: "Web search", + value: isOn ? on : off, + hint: isOn ? "Web search is on for this message" : "Search the web for this message" + ) + } + + static func deepResearch(isOn: Bool) -> Control { + Control( + label: "Deep Research", + value: isOn ? on : off, + hint: isOn + ? "Deep Research is on for this message" + : "Research this in depth before answering" + ) + } + + static func microphone(isRecording: Bool) -> Control { + Control( + label: "Voice input", + value: isRecording ? "Recording" : "Off", + hint: isRecording ? "Stop voice input" : "Dictate this message" + ) + } + + /// The send button, which is a stop button mid-run. + /// + /// Two different actions rather than one in two states, so they are named differently — + /// a control that keeps the name "Send" while it cancels is how someone stops a run they + /// meant to let finish. + static func send(canSend: Bool, isGenerating: Bool) -> Control { + guard !isGenerating else { + return Control(label: "Stop", value: nil, hint: "Stop generating this response") + } + return Control( + label: "Send", + value: nil, + hint: canSend ? "Send this message" : "Type a message first" + ) + } + + static let openInLogue = Control( + label: "Open in Logue", + value: nil, + hint: "Continue this conversation in the main window" + ) + + static let close = Control( + label: "Close", + value: nil, + hint: "Put the island away" + ) + + static let newConversation = Control( + label: "New conversation", + value: nil, + hint: "Start a new thread. The current one stays in Logue" + ) +} diff --git a/Logue/Views/CrossApp/CommandCenterChatView+Composer.swift b/Logue/Views/CrossApp/CommandCenterChatView+Composer.swift index bcc4d31..f87c03b 100644 --- a/Logue/Views/CrossApp/CommandCenterChatView+Composer.swift +++ b/Logue/Views/CrossApp/CommandCenterChatView+Composer.swift @@ -49,7 +49,7 @@ extension CommandCenterChatView { } .buttonStyle(.plain) .disabled(isGenerating) - .help("Attach files") + .islandControl(IslandControlCopy.attach(isBusy: isGenerating)) // Web search for this turn Button { @@ -64,7 +64,7 @@ extension CommandCenterChatView { } .buttonStyle(.plain) .disabled(isGenerating) - .help(isWebSearchOnce ? "Web search is on for this message" : "Search the web for this message") + .islandControl(IslandControlCopy.webSearch(isOn: isWebSearchOnce)) // Deep Research for this turn Button { @@ -79,13 +79,7 @@ extension CommandCenterChatView { } .buttonStyle(.plain) .disabled(isGenerating) - .help( - isDeepResearchOnce - ? "Deep Research is on for this message" - : "Research this in depth before answering" - ) - .accessibilityLabel("Deep Research") - .accessibilityValue(isDeepResearchOnce ? "On" : "Off") + .islandControl(IslandControlCopy.deepResearch(isOn: isDeepResearchOnce)) // Mic button Button { @@ -98,7 +92,7 @@ extension CommandCenterChatView { } .buttonStyle(.plain) .disabled(isGenerating) - .help(voiceManager.isRecording ? "Stop voice input" : "Voice input") + .islandControl(IslandControlCopy.microphone(isRecording: voiceManager.isRecording)) // What Return will do, once it will do anything. // @@ -124,6 +118,7 @@ extension CommandCenterChatView { .background(Circle().fill(AppThemeConstants.error)) } .buttonStyle(.plain) + .islandControl(IslandControlCopy.send(canSend: canSend, isGenerating: true)) } else { Button(action: sendMessage) { Image(systemName: "arrow.up") @@ -137,6 +132,7 @@ extension CommandCenterChatView { .buttonStyle(.plain) .disabled(!canSend || LLMEngineStatus.shared.isBusy) .keyboardShortcut(.return, modifiers: .command) + .islandControl(IslandControlCopy.send(canSend: canSend, isGenerating: false)) } } .animation(.easeOut(duration: 0.12), value: canSend) diff --git a/Logue/Views/CrossApp/CommandCenterChatView.swift b/Logue/Views/CrossApp/CommandCenterChatView.swift index cc8f904..3dd2af5 100644 --- a/Logue/Views/CrossApp/CommandCenterChatView.swift +++ b/Logue/Views/CrossApp/CommandCenterChatView.swift @@ -211,6 +211,7 @@ struct CommandCenterChatView: View { .background(Capsule().fill(Color.primary.opacity(0.06))) } .buttonStyle(.plain) + .islandControl(IslandControlCopy.newConversation) Spacer() @@ -230,7 +231,7 @@ struct CommandCenterChatView: View { .background(Capsule().fill(Color.primary.opacity(0.06))) } .buttonStyle(.plain) - .help("Continue this conversation in the main window") + .islandControl(IslandControlCopy.openInLogue) } Button(action: onDismiss) { @@ -241,6 +242,7 @@ struct CommandCenterChatView: View { .background(Circle().fill(Color.primary.opacity(0.06))) } .buttonStyle(.plain) + .islandControl(IslandControlCopy.close) } .padding(.horizontal, 16) .padding(.top, 10) diff --git a/Logue/Views/CrossApp/IslandSurfaceModifier.swift b/Logue/Views/CrossApp/IslandSurfaceModifier.swift index 012bcc4..16a46d5 100644 --- a/Logue/Views/CrossApp/IslandSurfaceModifier.swift +++ b/Logue/Views/CrossApp/IslandSurfaceModifier.swift @@ -52,3 +52,17 @@ private struct IslandSurfaceModifier: ViewModifier { .shadow(color: .black.opacity(treatment.shadowOpacity), radius: 30, y: 12) } } + +/// Applies a control's name, state and tooltip in one place. +/// +/// A modifier rather than three lines at each call site: the failure this box is about is a +/// control that got the tooltip and not the label, which is exactly what happens when the +/// three are written out separately and one is forgotten. +extension View { + func islandControl(_ control: IslandControlCopy.Control) -> some View { + accessibilityLabel(control.label) + .accessibilityValue(control.value ?? "") + .accessibilityHint(control.hint) + .help(control.hint) + } +} diff --git a/LogueTests/IslandControlCopyTests.swift b/LogueTests/IslandControlCopyTests.swift new file mode 100644 index 0000000..aa6cb64 --- /dev/null +++ b/LogueTests/IslandControlCopyTests.swift @@ -0,0 +1,96 @@ +import Foundation +import Testing + +@testable import Logue + +/// What the island's controls announce. +/// +/// Almost every one carried only `.help(…)`, which on macOS is the accessibility hint rather +/// than the label — so VoiceOver fell back to the SF Symbol name. The send button announced +/// "arrow up". +@Suite("IslandControlCopy") +struct IslandControlCopyTests { + /// Every control, in every state it has. + private var allControls: [IslandControlCopy.Control] { + [ + IslandControlCopy.attach(isBusy: false), + IslandControlCopy.attach(isBusy: true), + IslandControlCopy.webSearch(isOn: false), + IslandControlCopy.webSearch(isOn: true), + IslandControlCopy.deepResearch(isOn: false), + IslandControlCopy.deepResearch(isOn: true), + IslandControlCopy.microphone(isRecording: false), + IslandControlCopy.microphone(isRecording: true), + IslandControlCopy.send(canSend: false, isGenerating: false), + IslandControlCopy.send(canSend: true, isGenerating: false), + IslandControlCopy.send(canSend: false, isGenerating: true), + IslandControlCopy.openInLogue, + IslandControlCopy.close, + IslandControlCopy.newConversation, + ] + } + + @Test("Nothing announces its icon") + func noControlAnnouncesASymbolName() { + // The regression this exists to catch. An SF Symbol name is dotted and lowercase — + // "arrow.up", "sparkle.magnifyingglass", "xmark.circle.fill" — and reaches VoiceOver + // whenever a Button's only content is an Image with no label of its own. + for control in allControls { + #expect(control.label.contains(".") == false, "symbol-shaped label: \(control.label)") + #expect(control.label.isEmpty == false) + #expect(control.label.first?.isUppercase == true, "not a name: \(control.label)") + } + } + + @Test("Every control says what it will do") + func everyControlHasAHint() { + for control in allControls { + #expect(control.hint.isEmpty == false, "no hint for \(control.label)") + } + } + + // MARK: - State + + @Test("A toggle announces which way it is set") + func togglesCarryTheirState() { + // The single thing a toggle exists to convey, and the thing `.help` could not say. + #expect(IslandControlCopy.webSearch(isOn: true).value == IslandControlCopy.on) + #expect(IslandControlCopy.webSearch(isOn: false).value == IslandControlCopy.off) + #expect(IslandControlCopy.deepResearch(isOn: true).value == IslandControlCopy.on) + #expect(IslandControlCopy.deepResearch(isOn: false).value == IslandControlCopy.off) + } + + @Test("The two states of a toggle are told apart") + func toggleStatesDiffer() { + for pair in [ + (IslandControlCopy.webSearch(isOn: true), IslandControlCopy.webSearch(isOn: false)), + (IslandControlCopy.deepResearch(isOn: true), IslandControlCopy.deepResearch(isOn: false)), + (IslandControlCopy.microphone(isRecording: true), IslandControlCopy.microphone(isRecording: false)), + ] { + #expect(pair.0 != pair.1) + #expect(pair.0.label == pair.1.label, "the name does not change with the state") + } + } + + @Test("Stop is not called Send") + func stopIsNamedForWhatItDoes() { + // A control that keeps the name "Send" while it cancels is how someone stops a run + // they meant to let finish. + let sending = IslandControlCopy.send(canSend: true, isGenerating: false) + let stopping = IslandControlCopy.send(canSend: false, isGenerating: true) + #expect(sending.label == "Send") + #expect(stopping.label == "Stop") + } + + @Test("A send that cannot fire says why") + func disabledSendExplainsItself() { + let idle = IslandControlCopy.send(canSend: false, isGenerating: false) + let ready = IslandControlCopy.send(canSend: true, isGenerating: false) + #expect(idle.hint != ready.hint) + } + + @Test("Attach says why it is unavailable mid-run") + func busyAttachExplainsItself() { + #expect(IslandControlCopy.attach(isBusy: true).hint != IslandControlCopy.attach(isBusy: false).hint) + } +} From 82c9fa60149398f456d7b49c4fbf0e378f4d9bc1 Mon Sep 17 00:00:00 2001 From: shanforge <299346208+shanforge@users.noreply.github.com> Date: Fri, 21 Aug 2026 12:00:52 +0530 Subject: [PATCH 10/12] feat(island): honour Reduce Motion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The island is the most animated surface in Logue — it slides in over another app, springs open on the first message, scales its chips in and out, and runs two forever pulses. None of it consulted Accessibility → Display → Reduce motion, so someone who asked the system for less movement got the most of it here. IslandMotion is the policy, and it turns on decoration versus information. A spring, a slide and a scale say nothing the layout does not already say, so they go — replaced by a cross-fade rather than by nothing, since removing the transition outright turns every panel into a jump cut. A pulsing dot is the harder case: it is the only thing on screen saying the agent is still working. It stops anyway, but only because every row it appears in also says so in words. The level meter is deliberately untouched — its movement is the reading, and a still meter is not a calmer meter, it is a broken one. Springs are removed rather than shortened. A spring is the movement; making it quick makes it a flinch. PulsingDot reads the setting itself rather than at each call site, so a dot added somewhere new cannot reintroduce a forever-repeating animation. Part of #62. --- Logue.xcodeproj/project.pbxproj | 8 +++ Logue/UI/IslandMotion.swift | 60 +++++++++++++++++++ Logue/Views/Agent/ModeChip.swift | 9 ++- Logue/Views/Agent/PulsingDot.swift | 11 ++++ .../Components/VoiceInputIndicator.swift | 12 +++- .../CommandCenterChatView+Composer.swift | 10 +++- .../CrossApp/CommandCenterChatView.swift | 18 ++++-- LogueTests/IslandMotionTests.swift | 59 ++++++++++++++++++ 8 files changed, 176 insertions(+), 11 deletions(-) create mode 100644 Logue/UI/IslandMotion.swift create mode 100644 LogueTests/IslandMotionTests.swift diff --git a/Logue.xcodeproj/project.pbxproj b/Logue.xcodeproj/project.pbxproj index a227bbe..3435fd3 100644 --- a/Logue.xcodeproj/project.pbxproj +++ b/Logue.xcodeproj/project.pbxproj @@ -63,6 +63,7 @@ 0D4341603B1E69D6890BD16A /* GraphRetriever.swift in Sources */ = {isa = PBXBuildFile; fileRef = F4837D7E60145F15B37A2310 /* GraphRetriever.swift */; }; 0D825C8BE5FEA23AA4BEFFBB /* TaskDrafts.swift in Sources */ = {isa = PBXBuildFile; fileRef = E4E3CD415CB79A60753F21DC /* TaskDrafts.swift */; }; 0DF9D55AFF9C181065A4C69E /* SourceMeetingChip.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5BF8F4CE67B450542E7CFAD3 /* SourceMeetingChip.swift */; }; + 0E2EB8FE3379AEF959988D62 /* IslandMotion.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7390A75F79EE786998273DA7 /* IslandMotion.swift */; }; 0EA7AB55C23D022F1C35E001 /* RewritePanelLLMTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C6922E834A41AFDEB9150115 /* RewritePanelLLMTests.swift */; }; 0F40B34FE9CF24DC1B55D9F0 /* SpaceContentPane+Cards.swift in Sources */ = {isa = PBXBuildFile; fileRef = B21302B0871DD6955B6F5148 /* SpaceContentPane+Cards.swift */; }; 0FB26798FD61EEBC32BFD3A7 /* SpeechTranscriberEngine.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0D270AE60AD2503E159E4A50 /* SpeechTranscriberEngine.swift */; }; @@ -115,6 +116,7 @@ 204FCF6880ABDA5F9C7684FF /* MeetingExportPanelView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6DE0AF8C2141C4DDEC98CB32 /* MeetingExportPanelView.swift */; }; 2097A69FB625B8274E022216 /* DocumentTemplate.swift in Sources */ = {isa = PBXBuildFile; fileRef = FE07F5F09706EA33B61BEF24 /* DocumentTemplate.swift */; }; 217939881B4BBD3104148F19 /* WritingStatsTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 51BF2A584F3FE0BCC2C793B7 /* WritingStatsTests.swift */; }; + 21A331E075E178E69671F1AC /* IslandMotionTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 346072038493F695836F34BD /* IslandMotionTests.swift */; }; 22025C227E49A8B48BD70654 /* Sparkle in Frameworks */ = {isa = PBXBuildFile; productRef = CA636898FBC407FB4FB69C61 /* Sparkle */; }; 2206F8D1450B6DA3886A850F /* CommandCenterRecordingView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6445D0008F16B2DFE083F469 /* CommandCenterRecordingView.swift */; }; 22EACBCCB7BD304F9AE40B4E /* QuickPromptChip.swift in Sources */ = {isa = PBXBuildFile; fileRef = 840A12365F18257475A34827 /* QuickPromptChip.swift */; }; @@ -868,6 +870,7 @@ 33AE1CBE332B50E515AAA7EE /* RecordingSessionManager+Recovery.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "RecordingSessionManager+Recovery.swift"; sourceTree = ""; }; 3423F242140F7A2AE832B0FD /* DocumentFilenameTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DocumentFilenameTests.swift; sourceTree = ""; }; 344DDE65E3765B845016619D /* ExternalModelsSection.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ExternalModelsSection.swift; sourceTree = ""; }; + 346072038493F695836F34BD /* IslandMotionTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = IslandMotionTests.swift; sourceTree = ""; }; 3474D653C2A7624A299D8F8F /* MeetingMemoryIndex.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MeetingMemoryIndex.swift; sourceTree = ""; }; 347E49226D27CD7AF6B329EE /* LinkRenamer.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LinkRenamer.swift; sourceTree = ""; }; 34E69663E0AE733F0F38C648 /* MeetingStore+AI.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "MeetingStore+AI.swift"; sourceTree = ""; }; @@ -1041,6 +1044,7 @@ 71CCC37048141456C9BA5AA3 /* DocumentSearchBar.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DocumentSearchBar.swift; sourceTree = ""; }; 7250A0C394AFABC041C58D68 /* DeepResearchCoordinator+Helpers.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "DeepResearchCoordinator+Helpers.swift"; sourceTree = ""; }; 72EE330160AA0276B2907F2A /* MeetingRowCompact.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MeetingRowCompact.swift; sourceTree = ""; }; + 7390A75F79EE786998273DA7 /* IslandMotion.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = IslandMotion.swift; sourceTree = ""; }; 739A71E158624FCF8E71C075 /* PostRecordingPipeline.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PostRecordingPipeline.swift; sourceTree = ""; }; 73A0577F01A5B33292317D52 /* SummaryNarrationService.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SummaryNarrationService.swift; sourceTree = ""; }; 73C82679F61FC2AC07FC9220 /* whatsnew-transcription.png */ = {isa = PBXFileReference; lastKnownFileType = image.png; path = "whatsnew-transcription.png"; sourceTree = ""; }; @@ -1552,6 +1556,7 @@ A3600798EF27A735FC4BC80E /* HomeSuggestionsTests.swift */, 0062B04D59734428AFC49682 /* ImportFileReadingTests.swift */, D21D2BCBEBA7DC11B30D5CAB /* IslandControlCopyTests.swift */, + 346072038493F695836F34BD /* IslandMotionTests.swift */, 464AB56546E36263CEB595E8 /* IslandSurfaceTests.swift */, 402D8E4447C9B7063ADE7358 /* IslandThreadTests.swift */, 7F48D46D3D40534008356F5D /* LinkIndexBuildTests.swift */, @@ -1714,6 +1719,7 @@ 207FDDB5AF3841198B2438E4 /* ChatHomeEmptyState.swift */, CFB4EE100316EB840B5D93EE /* HoverToolbar.swift */, 86BACB5F9085F6C7DB5B4CD8 /* IslandControlCopy.swift */, + 7390A75F79EE786998273DA7 /* IslandMotion.swift */, 2666524E8E079ED62C748893 /* JumpToLatestPill.swift */, E28B906C35C686A1F97C984E /* ModelChip.swift */, 3E2383DBE96586FC70FE52CB /* Motion.swift */, @@ -2791,6 +2797,7 @@ 549C3D0D72BD69E53B7929A5 /* HomeSuggestionsTests.swift in Sources */, D0FFBA721311ED3F007A7D49 /* ImportFileReadingTests.swift in Sources */, 99B0158F57D74A593DAFF903 /* IslandControlCopyTests.swift in Sources */, + 21A331E075E178E69671F1AC /* IslandMotionTests.swift in Sources */, A1A603E973000958C89D1EDF /* IslandSurfaceTests.swift in Sources */, 2F4BA10FF02C91BDC115F449 /* IslandThreadTests.swift in Sources */, 5649718294F9E5770348BD76 /* LLMTestEvalExtensions.swift in Sources */, @@ -3113,6 +3120,7 @@ 8621E76EC5A20A48091893C4 /* InsightsSectionView.swift in Sources */, 3C8B6B59AC5A90F65186C43C /* InsightsStatsProvider.swift in Sources */, 5688E339841DC63FA2F61E8E /* IslandControlCopy.swift in Sources */, + 0E2EB8FE3379AEF959988D62 /* IslandMotion.swift in Sources */, EC1F72AE3061CEF89F456398 /* IslandSurface.swift in Sources */, 69B2689283C249CBE8252ADE /* IslandSurfaceModifier.swift in Sources */, CF68588BF9879F2449BC4BEE /* IslandThread.swift in Sources */, diff --git a/Logue/UI/IslandMotion.swift b/Logue/UI/IslandMotion.swift new file mode 100644 index 0000000..2ef514d --- /dev/null +++ b/Logue/UI/IslandMotion.swift @@ -0,0 +1,60 @@ +import SwiftUI + +/// Which of the island's animations survive Reduce Motion, and what replaces the rest. +/// +/// The island is the most animated surface in Logue: it slides in over another app, its panel +/// springs open on the first message, chips scale in and out, and two indicators pulse +/// forever. None of it consulted Accessibility → Display → Reduce motion, so a user who has +/// asked the system for less movement got the most of it here. +/// +/// The distinction the policy turns on is **decoration versus information**. A spring, a +/// slide and a scale say nothing that the layout does not already say, so they go. A pulsing +/// dot is different: it is the only thing on screen saying the agent is still working. It is +/// still removed — but only because the row it sits in also carries the words "Thinking…", +/// so the information survives without it. Removing the pulse from somewhere with no such +/// text would be removing the signal, not the decoration. +/// +/// The level meter is deliberately untouched. Its movement *is* the reading — a still meter +/// is not a calmer meter, it is a broken one. +enum IslandMotion { + /// How something arriving or leaving should be drawn. + enum Entrance: Equatable { + /// Slides from an edge as it fades. + case slideAndFade + /// Fades only. A cross-fade is motion the setting permits, and dropping the + /// animation entirely would make things appear as jump cuts. + case fadeOnly + } + + static func entrance(reduceMotion: Bool) -> Entrance { + reduceMotion ? .fadeOnly : .slideAndFade + } + + /// Whether an indicator may pulse forever. + /// + /// Only ever false where the thing it decorates is also stated in words. + static func allowsPulse(reduceMotion: Bool) -> Bool { + !reduceMotion + } + + /// The animation for a layout change — a panel opening, a chip row appearing. + /// + /// `nil` rather than a shorter spring: a spring is the movement, and making it quick + /// makes it a flinch. The content still cross-fades through its transition. + static func layout(reduceMotion: Bool) -> Animation? { + reduceMotion ? nil : .spring(response: 0.35, dampingFraction: 0.85) + } + + /// The animation for a small control changing state — the Return hint, a mode toggling. + static func control(reduceMotion: Bool) -> Animation? { + reduceMotion ? nil : .easeOut(duration: 0.15) + } + + /// The transition for something entering or leaving the island. + static func transition(reduceMotion: Bool, edge: Edge = .bottom) -> AnyTransition { + switch entrance(reduceMotion: reduceMotion) { + case .slideAndFade: .move(edge: edge).combined(with: .opacity) + case .fadeOnly: .opacity + } + } +} diff --git a/Logue/Views/Agent/ModeChip.swift b/Logue/Views/Agent/ModeChip.swift index 1d52c3c..52a92db 100644 --- a/Logue/Views/Agent/ModeChip.swift +++ b/Logue/Views/Agent/ModeChip.swift @@ -14,6 +14,8 @@ struct ModeChip: View { /// that set it. let onDismiss: () -> Void + @Environment(\.accessibilityReduceMotion) private var reduceMotion + var body: some View { HStack(spacing: 4) { Image(systemName: systemImage) @@ -32,6 +34,11 @@ struct ModeChip: View { .foregroundStyle(tint) .background(Capsule().fill(tint.opacity(0.14))) .overlay(Capsule().strokeBorder(tint.opacity(0.45), lineWidth: 0.5)) - .transition(.scale.combined(with: .opacity)) + // A scale is a transition, not a pulse — so it follows `entrance`. + .transition( + IslandMotion.entrance(reduceMotion: reduceMotion) == .slideAndFade + ? .scale.combined(with: .opacity) + : .opacity + ) } } diff --git a/Logue/Views/Agent/PulsingDot.swift b/Logue/Views/Agent/PulsingDot.swift index 30c443b..b125800 100644 --- a/Logue/Views/Agent/PulsingDot.swift +++ b/Logue/Views/Agent/PulsingDot.swift @@ -8,6 +8,12 @@ struct PulsingDot: View { @State private var scale: CGFloat = 1 @State private var opacity: Double = 0.7 + /// Honoured here rather than at each call site, so a dot added somewhere new cannot + /// reintroduce a forever-repeating animation for someone who asked for less movement. + /// Safe to stop entirely because every row this dot appears in also says, in words, what + /// is happening — see `IslandMotion`. + @Environment(\.accessibilityReduceMotion) private var reduceMotion + var body: some View { Circle() .fill(color) @@ -15,6 +21,11 @@ struct PulsingDot: View { .scaleEffect(scale) .opacity(opacity) .onAppear { + guard IslandMotion.allowsPulse(reduceMotion: reduceMotion) else { + // Still visible, just still. + opacity = 1 + return + } withAnimation(.easeInOut(duration: 0.9).repeatForever(autoreverses: true)) { scale = 1.25 opacity = 1.0 diff --git a/Logue/Views/Components/VoiceInputIndicator.swift b/Logue/Views/Components/VoiceInputIndicator.swift index 737d910..c82db07 100644 --- a/Logue/Views/Components/VoiceInputIndicator.swift +++ b/Logue/Views/Components/VoiceInputIndicator.swift @@ -9,13 +9,23 @@ struct VoiceInputIndicator: View { @State private var pulse = false + /// The dot pulses to say the mic is live. "Listening..." beside it says the same thing, + /// so the pulse can stop. The level meter below is left alone deliberately — its movement + /// *is* the reading, and a still meter is not a calmer meter, it is a broken one. + @Environment(\.accessibilityReduceMotion) private var reduceMotion + var body: some View { HStack(spacing: 8) { Circle() .fill(AppThemeConstants.error) .frame(width: 8, height: 8) .opacity(pulse ? 1 : 0.4) - .animation(.easeInOut(duration: 0.8).repeatForever(autoreverses: true), value: pulse) + .animation( + IslandMotion.allowsPulse(reduceMotion: reduceMotion) + ? .easeInOut(duration: 0.8).repeatForever(autoreverses: true) + : nil, + value: pulse + ) .onAppear { pulse = true } Text(partialTranscript.isEmpty ? "Listening..." : partialTranscript) diff --git a/Logue/Views/CrossApp/CommandCenterChatView+Composer.swift b/Logue/Views/CrossApp/CommandCenterChatView+Composer.swift index f87c03b..ac77dd9 100644 --- a/Logue/Views/CrossApp/CommandCenterChatView+Composer.swift +++ b/Logue/Views/CrossApp/CommandCenterChatView+Composer.swift @@ -53,7 +53,9 @@ extension CommandCenterChatView { // Web search for this turn Button { - withAnimation(.easeOut(duration: 0.15)) { isWebSearchOnce.toggle() } + withAnimation(IslandMotion.control(reduceMotion: reduceMotion)) { + isWebSearchOnce.toggle() + } } label: { Image(systemName: "globe") .font(.subheadline.weight(.medium)) @@ -68,7 +70,9 @@ extension CommandCenterChatView { // Deep Research for this turn Button { - withAnimation(.easeOut(duration: 0.15)) { isDeepResearchOnce.toggle() } + withAnimation(IslandMotion.control(reduceMotion: reduceMotion)) { + isDeepResearchOnce.toggle() + } } label: { Image(systemName: "sparkle.magnifyingglass") .font(.subheadline.weight(.medium)) @@ -135,7 +139,7 @@ extension CommandCenterChatView { .islandControl(IslandControlCopy.send(canSend: canSend, isGenerating: false)) } } - .animation(.easeOut(duration: 0.12), value: canSend) + .animation(IslandMotion.control(reduceMotion: reduceMotion), value: canSend) .padding(.leading, 14) .padding(.trailing, 10) .padding(.vertical, 10) diff --git a/Logue/Views/CrossApp/CommandCenterChatView.swift b/Logue/Views/CrossApp/CommandCenterChatView.swift index 3dd2af5..8b09b6f 100644 --- a/Logue/Views/CrossApp/CommandCenterChatView.swift +++ b/Logue/Views/CrossApp/CommandCenterChatView.swift @@ -55,6 +55,10 @@ struct CommandCenterChatView: View { @State private var synthesizer = AVSpeechSynthesizer() // Extension-visible: +Composer @FocusState var isInputFocused: Bool + // Extension-visible: +Composer + /// Accessibility → Display → Reduce motion. The island slides in over another app, + /// springs open on the first message and scales its chips; none of it consulted this. + @Environment(\.accessibilityReduceMotion) var reduceMotion // Extension-visible: +Composer var voiceManager: VoicePushToTalkManager { @@ -110,7 +114,7 @@ struct CommandCenterChatView: View { if hasContent { messagesPanel .padding(.bottom, 10) - .transition(.opacity.combined(with: .move(edge: .bottom))) + .transition(IslandMotion.transition(reduceMotion: reduceMotion)) } else { starters } @@ -146,7 +150,7 @@ struct CommandCenterChatView: View { // document — so it does not follow the system, which is what makes it legible in // both appearances. .environment(\.colorScheme, .dark) - .animation(.spring(response: 0.35, dampingFraction: 0.85), value: hasContent) + .animation(IslandMotion.layout(reduceMotion: reduceMotion), value: hasContent) .onAppear { isInputFocused = true voiceManager.onTranscriptReady = { transcript in @@ -267,7 +271,7 @@ struct CommandCenterChatView: View { } .onChange(of: rows) { _, newRows in if let last = newRows.last { - withAnimation(.easeOut(duration: 0.15)) { + withAnimation(IslandMotion.control(reduceMotion: reduceMotion)) { proxy.scrollTo(last.id, anchor: .bottom) } } @@ -286,7 +290,7 @@ struct CommandCenterChatView: View { } .padding(.horizontal, 16) .padding(.bottom, 10) - .transition(.move(edge: .bottom).combined(with: .opacity)) + .transition(IslandMotion.transition(reduceMotion: reduceMotion)) } // Mounted, not redrawn: the island shows the same seven-step strip the main @@ -296,7 +300,7 @@ struct CommandCenterChatView: View { if let error = currentError { errorBanner(error) - .transition(.move(edge: .bottom).combined(with: .opacity)) + .transition(IslandMotion.transition(reduceMotion: reduceMotion)) } } .frame(width: pillWidth) @@ -471,7 +475,9 @@ struct CommandCenterChatView: View { Spacer(minLength: 0) Button { - withAnimation { coordinator.dismissError() } + withAnimation(IslandMotion.control(reduceMotion: reduceMotion)) { + coordinator.dismissError() + } } label: { Image(systemName: "xmark") .font(.caption2.weight(.semibold)) diff --git a/LogueTests/IslandMotionTests.swift b/LogueTests/IslandMotionTests.swift new file mode 100644 index 0000000..a90e076 --- /dev/null +++ b/LogueTests/IslandMotionTests.swift @@ -0,0 +1,59 @@ +import SwiftUI +import Testing + +@testable import Logue + +/// What Reduce Motion does to the island. +/// +/// The island is the most animated surface in Logue and consulted the setting nowhere, so a +/// user who asked the system for less movement got the most of it here. +@Suite("IslandMotion") +struct IslandMotionTests { + @Test("Reduce Motion turns movement into a fade") + func movementBecomesAFade() { + // A cross-fade is motion the setting permits. Dropping the transition altogether + // would make panels appear as jump cuts, which is not what was asked for. + #expect(IslandMotion.entrance(reduceMotion: true) == .fadeOnly) + #expect(IslandMotion.entrance(reduceMotion: false) == .slideAndFade) + } + + @Test("Reduce Motion stops the pulses") + func pulsesStop() { + // Safe only because every place the island pulses also says what is happening in + // words. A pulse with no such text is the signal, not the decoration. + #expect(IslandMotion.allowsPulse(reduceMotion: true) == false) + #expect(IslandMotion.allowsPulse(reduceMotion: false)) + } + + @Test("A spring is removed rather than shortened") + func springsAreRemovedNotHurried() { + // A spring *is* the movement; making it quick makes it a flinch rather than calm. + #expect(IslandMotion.layout(reduceMotion: true) == nil) + #expect(IslandMotion.layout(reduceMotion: false) != nil) + } + + @Test("Control changes stop animating too") + func controlAnimationsStop() { + #expect(IslandMotion.control(reduceMotion: true) == nil) + #expect(IslandMotion.control(reduceMotion: false) != nil) + } + + @Test("Nothing animates at all when motion is reduced") + func nothingSurvivesUnnoticed() { + // A sweep, so a new animation added to the policy without a reduced branch fails + // here rather than shipping. + #expect(IslandMotion.layout(reduceMotion: true) == nil) + #expect(IslandMotion.control(reduceMotion: true) == nil) + #expect(IslandMotion.allowsPulse(reduceMotion: true) == false) + #expect(IslandMotion.entrance(reduceMotion: true) == .fadeOnly) + } + + @Test("Ordinary settings keep every animation") + func normalSettingsAreUnchanged() { + // The other half: this must not quietly flatten the island for everyone. + #expect(IslandMotion.layout(reduceMotion: false) != nil) + #expect(IslandMotion.control(reduceMotion: false) != nil) + #expect(IslandMotion.allowsPulse(reduceMotion: false)) + #expect(IslandMotion.entrance(reduceMotion: false) == .slideAndFade) + } +} From f762b288df33489bd1ca72e372c000b1b28b8766 Mon Sep 17 00:00:00 2001 From: shanforge <299346208+shanforge@users.noreply.github.com> Date: Fri, 4 Sep 2026 11:34:50 +0530 Subject: [PATCH 11/12] fix: what the first review round found MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four findings, most severe first. **A refused send showed nothing (blocking).** `localError` was set on both refusal paths and rendered by none: the banner read `currentError`, which is `coordinator.lastError(in: conversationID)`. The globally-busy guard in `sendMessage` returns *before* `ensureConversation()`, so on that path `conversationID` is still nil and `currentError` can only ever answer nil. Start a run in the main window, open the island, type, press Return: the guard fires, sets the message, and nothing appears — which is verbatim the failure its own comment says it exists to prevent. The banner now reads `displayedError`, local first, and a send that goes ahead clears the last refusal so a stale "busy elsewhere" cannot sit over a send that worked. **A document title could reverse the sentence naming it (major).** `flatten` collapsed whitespace but let control and format characters through. A title carrying U+202E — which can arrive in a `.md` file dropped into the markdown folder, or from a `create_document` call a prompt-injected model made — reverses the display of everything after it, so `Delete “report.txt”` can be made to name a different file on the one card whose whole job is to be true, directly above a Touch ID prompt. `CLAUDE.md` already requires stripping control characters from any user string reaching a prompt or label, and `AgentArgs.sanitize` does it in eight other places. `ToolApprovalPrompt` and `ToolArgumentSummary` had a copy each of `flatten` and `clamp`, so the fix is a shared `DisplayText` rather than a third copy — a control that exists twice is a control that gets fixed once. The order inside it is the part worth stating: **a newline is itself a control character** (U+000A is Cc), so stripping controls before splitting on whitespace turned `first\nsecond` into `firstsecond` — a different value, silently. The existing suite caught exactly that. Whitespace is exempted from the strip and the split handles it. Mutation-checked: removing the strip turns six cases red across both files. **Two definitions of "the workspace is empty" (minor).** The branch extracted `storesAreLoaded` and the emptiness rule into `HomeSuggestions` so the island could share them, and left `AgentChatView`'s copies in use — each commented as being the one definition. Change the rule in one and `HomeLandingView` says empty while the chips say otherwise: first-run landing above returning-user chips, the drift both comments warn of. The view now reads the extracted rule. No runtime change — `LogueApp` injects the same singletons the extraction reads. **Reduce Motion was read once (minor).** `PulsingDot` consulted it in `onAppear` only, so turning it on while the island was streaming left the forever-repeating animation running until the view was rebuilt — the moment the setting is most likely to be reached for. Now also on change, and the scale resets rather than freezing mid-pulse. Verified: build succeeds, 1670 tests in 146 suites pass, SwiftFormat --lint clean over 544 files, SwiftLint --strict 0 violations in 689 files. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01M3Wpnj9ZmWPKYdFPB1AVY3 --- Logue.xcodeproj/project.pbxproj | 8 ++ Logue/Agent/DisplayText.swift | 49 +++++++++ Logue/Agent/ToolApprovalPrompt.swift | 12 +-- Logue/Agent/ToolArgumentSummary.swift | 18 +--- Logue/Views/Agent/AgentChatView.swift | 18 ++-- Logue/Views/Agent/PulsingDot.swift | 33 ++++--- .../CrossApp/CommandCenterChatView.swift | 18 +++- LogueTests/DisplayTextTests.swift | 99 +++++++++++++++++++ LogueTests/ToolApprovalPromptTests.swift | 30 ++++++ 9 files changed, 240 insertions(+), 45 deletions(-) create mode 100644 Logue/Agent/DisplayText.swift create mode 100644 LogueTests/DisplayTextTests.swift diff --git a/Logue.xcodeproj/project.pbxproj b/Logue.xcodeproj/project.pbxproj index fbdb0c5..ab6dcbe 100644 --- a/Logue.xcodeproj/project.pbxproj +++ b/Logue.xcodeproj/project.pbxproj @@ -179,6 +179,7 @@ 35CBDE38DC02F798AE51C76F /* DocumentWidthModeTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = AE8030D02974A04A80B48065 /* DocumentWidthModeTests.swift */; }; 35FC159E499EA14E07D6304B /* PreRollBufferTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = DA91866185B2C46BE60A76CB /* PreRollBufferTests.swift */; }; 362E757F0940BDB7CB5E560A /* BlockMoveTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8D204801C0C2BADD68D406D7 /* BlockMoveTests.swift */; }; + 3644EF3779F653692AF66911 /* DisplayTextTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 88BB102BC13D324CE38E77E2 /* DisplayTextTests.swift */; }; 372D7C7E58F4EF0B8305E096 /* BlockRowView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 61150569A2C3758E03DC0666 /* BlockRowView.swift */; }; 37DEDF1E37B6221AC2F38912 /* KeychainHelper.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0E812DD5E7EE68ED58521109 /* KeychainHelper.swift */; }; 382265ED81A9CA7B18C98FBB /* TaskEditTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = CFEEA2965A9DBF97DA170575 /* TaskEditTests.swift */; }; @@ -636,6 +637,7 @@ E22FBD5A4C277D1F262060D5 /* SpeakerRenameTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 090F10872AED47868B63F897 /* SpeakerRenameTests.swift */; }; E26DD5D68ADEFAC8D3EF3F3A /* LibraryPanelToggle.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5F81FD369FAF345E6BB8A696 /* LibraryPanelToggle.swift */; }; E2C79A46521D2379FF1EA047 /* AppIcon.icns in Resources */ = {isa = PBXBuildFile; fileRef = D245AD8F8FF9957DC52A4ADC /* AppIcon.icns */; }; + E3549A4FBC52738DCCAA763A /* DisplayText.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9CF656F23AE2CDAF7F51B822 /* DisplayText.swift */; }; E36CF22C957E1B5063A87B99 /* katex.min.js in Resources */ = {isa = PBXBuildFile; fileRef = 6217AE8765C37EAAE87C7F54 /* katex.min.js */; }; E37BD26415388D727ACC60A6 /* CanvasPaneView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4B4D41D14C5F7B6352AC692F /* CanvasPaneView.swift */; }; E4AB21C58F0BCE5EF7C8E037 /* LLMEngine+WritingAnalysis.swift in Sources */ = {isa = PBXBuildFile; fileRef = DDA3B12D86E29B9012E0B96B /* LLMEngine+WritingAnalysis.swift */; }; @@ -1121,6 +1123,7 @@ 88018F4CBB121A88EF363BBE /* MeetingListContentView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MeetingListContentView.swift; sourceTree = ""; }; 8876D7837082707205CBFC6D /* LogueApp.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LogueApp.swift; sourceTree = ""; }; 88A1A4DAACB05EBD1C285AF9 /* MessageActionsTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MessageActionsTests.swift; sourceTree = ""; }; + 88BB102BC13D324CE38E77E2 /* DisplayTextTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DisplayTextTests.swift; sourceTree = ""; }; 88C42C0A1E60F8FC22AC2EF3 /* BlockPasteTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BlockPasteTests.swift; sourceTree = ""; }; 89916892E9BF400CDC89E1FF /* WebSearchTools.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WebSearchTools.swift; sourceTree = ""; }; 89D2D180775E0F9C8735345F /* ShortcutsSettingsTab.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ShortcutsSettingsTab.swift; sourceTree = ""; }; @@ -1174,6 +1177,7 @@ 9C91F95BC414F31D9357EB2D /* ModelDownloadRow.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ModelDownloadRow.swift; sourceTree = ""; }; 9CB78CD4382582C122C7FF5F /* DocumentStore+AITitle.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "DocumentStore+AITitle.swift"; sourceTree = ""; }; 9CBB79FEF396D25431AACAE0 /* SpeakerShortLabel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SpeakerShortLabel.swift; sourceTree = ""; }; + 9CF656F23AE2CDAF7F51B822 /* DisplayText.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DisplayText.swift; sourceTree = ""; }; 9D62B7E5FDE2C0D83FFFD13E /* DroppedFileImportTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DroppedFileImportTests.swift; sourceTree = ""; }; 9DDCC3C3CFEDA93D9D945B57 /* WritingStats.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WritingStats.swift; sourceTree = ""; }; 9E8CCC12AA344F074862CF57 /* WritingNSTextView+Table.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "WritingNSTextView+Table.swift"; sourceTree = ""; }; @@ -1548,6 +1552,7 @@ 17F31B485FED361E8702AA2D /* DeviceLossPolicyTests.swift */, 860DFF869B9C6C39BE468431 /* DiagnosticsReportTests.swift */, B7D76EDEE9C20C5C390BAF10 /* DiagramMathBlockTests.swift */, + 88BB102BC13D324CE38E77E2 /* DisplayTextTests.swift */, 3423F242140F7A2AE832B0FD /* DocumentFilenameTests.swift */, 9130B8EDD12207674F5B4323 /* DocumentIconTests.swift */, 0E1246768E9657B7146D7062 /* DocumentPersistenceSplitTests.swift */, @@ -2269,6 +2274,7 @@ A38FEAE5E714CB8DAD9D7263 /* AskStopTarget.swift */, 417436A70F2C0F1DDA347DC4 /* AttachmentIntake.swift */, 8C5F05B096345A4B230EC91C /* ComposerChipRow.swift */, + 9CF656F23AE2CDAF7F51B822 /* DisplayText.swift */, 4B2F874FCD0AF25F7B37D3FC /* MalformedToolCall.swift */, B55FA53C32F4D2968C07CEE5 /* MessageActions.swift */, 4542B2571154754E0D0E8783 /* ToolApprovalPrompt.swift */, @@ -2792,6 +2798,7 @@ 2AF2DBAEFCA1E332ECBA2A1D /* DeviceLossPolicyTests.swift in Sources */, D5975A82DEA279851186013D /* DiagnosticsReportTests.swift in Sources */, 861F5D959175FAC61CA02A55 /* DiagramMathBlockTests.swift in Sources */, + 3644EF3779F653692AF66911 /* DisplayTextTests.swift in Sources */, 9608E20746E44DEC17BA379A /* DocumentFilenameTests.swift in Sources */, 1AC0C886212A4F3ADC9A48D1 /* DocumentIconTests.swift in Sources */, 53C6F90254BA856A74CD343A /* DocumentLLMTests.swift in Sources */, @@ -3040,6 +3047,7 @@ 09A677787DE7FF238366729D /* DiarizationManager+BatchASR.swift in Sources */, 3040F141C008F5162215FC11 /* DiarizationManager+LongRecording.swift in Sources */, 2FD721F2491FFD83810DB23C /* DiarizationManager.swift in Sources */, + E3549A4FBC52738DCCAA763A /* DisplayText.swift in Sources */, EC09A9CE12598BC04D96481A /* DocsHomeView.swift in Sources */, A1D7482C98EE0833CBA17214 /* DocumentCardView.swift in Sources */, F6B3634D89D31E335DE866BC /* DocumentFilename.swift in Sources */, diff --git a/Logue/Agent/DisplayText.swift b/Logue/Agent/DisplayText.swift new file mode 100644 index 0000000..d99ba75 --- /dev/null +++ b/Logue/Agent/DisplayText.swift @@ -0,0 +1,49 @@ +import Foundation + +/// Turning a string that came from somewhere else into one line it is safe to show. +/// +/// Two places need exactly this — the sentence on an approval card and the argument line on +/// a tool card — and both show text the app did not write: a document title, a filesystem +/// path, an argument a model produced. They had a copy each, which is one copy too many for +/// something that is partly a security control. +enum DisplayText { + /// One line, with everything that could make that line lie taken out. + /// + /// **Control and format characters go first**, and that is the half that matters. A title + /// is not always something the user typed: it can arrive in a `.md` file dropped into the + /// markdown folder, or from a call a prompt-injected model made. A bidirectional override + /// (U+202E) reverses the display of everything after it, so `Delete “report.txt”` can be + /// made to read as a different file — and on an approval card that sits directly above a + /// Touch ID prompt. `CharacterSet.controlCharacters` is Unicode categories Cc *and* Cf, + /// which is what makes it the right set rather than merely a plausible one. + /// + /// Whitespace is then collapsed, so a multi-paragraph value looks as truncated as it is + /// rather than like a short one, and a newline cannot split a sentence in half and leave + /// the verb sitting alone above Approve. + static func singleLine(_ value: String) -> String { + // Whitespace is exempted from the strip and handled by the split below, because a + // newline is itself a control character (U+000A is Cc). Removing it here rather than + // splitting on it would run the words either side of it together — `first\nsecond` + // becoming `firstsecond`, which reads as a different value rather than a flattened + // one. So: take out the controls that carry no meaning, then let the split turn the + // ones that do into a single space. + let scrubbed = value.unicodeScalars.filter { scalar in + !CharacterSet.controlCharacters.contains(scalar) + || CharacterSet.whitespacesAndNewlines.contains(scalar) + } + return String(String.UnicodeScalarView(scrubbed)) + .components(separatedBy: .whitespacesAndNewlines) + .filter { !$0.isEmpty } + .joined(separator: " ") + } + + /// Cuts to `limit`, marking that something was cut. + /// + /// The ellipsis is inside the budget rather than added to it, so the result is never + /// longer than the caller asked for. + static func clamp(_ value: String, to limit: Int) -> String { + guard value.count > limit else { return value } + guard limit > 1 else { return String(value.prefix(limit)) } + return String(value.prefix(limit - 1)) + "…" + } +} diff --git a/Logue/Agent/ToolApprovalPrompt.swift b/Logue/Agent/ToolApprovalPrompt.swift index 4694233..b264b97 100644 --- a/Logue/Agent/ToolApprovalPrompt.swift +++ b/Logue/Agent/ToolApprovalPrompt.swift @@ -150,19 +150,11 @@ enum ToolApprovalPrompt { return string.isEmpty ? nil : string } - /// Puts a target on one line. - /// - /// Titles are user-authored and a path can contain anything. A newline in either would - /// split the sentence in half and leave the verb sitting alone above Approve. private static func flatten(_ value: String) -> String { - value - .components(separatedBy: .whitespacesAndNewlines) - .filter { !$0.isEmpty } - .joined(separator: " ") + DisplayText.singleLine(value) } private static func clamp(_ value: String) -> String { - guard value.count > maxTargetLength else { return value } - return String(value.prefix(maxTargetLength - 1)) + "…" + DisplayText.clamp(value, to: maxTargetLength) } } diff --git a/Logue/Agent/ToolArgumentSummary.swift b/Logue/Agent/ToolArgumentSummary.swift index 55f2091..b87fe81 100644 --- a/Logue/Agent/ToolArgumentSummary.swift +++ b/Logue/Agent/ToolArgumentSummary.swift @@ -45,25 +45,11 @@ enum ToolArgumentSummary { return clamp(pairs.joined(separator: ", "), to: maxTotalLength) } - /// Puts a value on one line. - /// - /// A document body arrives with its newlines intact, and a `Text` limited to one line - /// renders the first of them and hides the rest — so a multi-paragraph argument looked - /// like a short one. Collapsing whitespace makes the truncation honest. private static func flatten(_ value: String) -> String { - value - .components(separatedBy: .whitespacesAndNewlines) - .filter { !$0.isEmpty } - .joined(separator: " ") + DisplayText.singleLine(value) } - /// Cuts to `limit`, marking that something was cut. - /// - /// The ellipsis is inside the budget rather than added to it, so the result is never - /// longer than the caller asked for. private static func clamp(_ value: String, to limit: Int) -> String { - guard value.count > limit else { return value } - guard limit > 1 else { return String(value.prefix(limit)) } - return String(value.prefix(limit - 1)) + "…" + DisplayText.clamp(value, to: limit) } } diff --git a/Logue/Views/Agent/AgentChatView.swift b/Logue/Views/Agent/AgentChatView.swift index b68a614..ac4fb5a 100644 --- a/Logue/Views/Agent/AgentChatView.swift +++ b/Logue/Views/Agent/AgentChatView.swift @@ -241,16 +241,20 @@ struct AgentChatView: View { /// library was still being read is the loudest possible wrong answer this screen can /// give, so nothing derived from store contents renders until all three report. private var storesAreLoaded: Bool { - documentStore.isLoaded && meetingStore.isLoaded && spaceStore.isLoaded + HomeSuggestions.storesAreLoaded } - /// One definition, used by both the header and the cards. Two definitions is how a - /// workspace with spaces but no documents gets first-run chips above a set of cards - /// that have all self-hidden. + /// One definition, used by both the header and the cards — and now by the island too, + /// which is why it lives in `HomeSuggestions` rather than here. Two definitions is how a + /// workspace with spaces but no documents gets first-run chips above a set of cards that + /// have all self-hidden; leaving a copy behind after extracting the rule is how the two + /// would have drifted back apart. + /// + /// The environment stores and the singletons are the same objects — `LogueApp` injects + /// `.shared` — so reading them through `HomeSuggestions` changes nothing at runtime and + /// removes the second definition. private var workspaceIsEmpty: Bool { - documentStore.activeDocuments.isEmpty - && meetingStore.activeMeetings.isEmpty - && spaceStore.topLevelSpaces.isEmpty + !HomeSuggestions.currentInputs(overdueCount: 0).hasAnyContent } /// Derived fresh each render from the stores — no inference, no caching. The rules live diff --git a/Logue/Views/Agent/PulsingDot.swift b/Logue/Views/Agent/PulsingDot.swift index b125800..fbbde26 100644 --- a/Logue/Views/Agent/PulsingDot.swift +++ b/Logue/Views/Agent/PulsingDot.swift @@ -20,17 +20,28 @@ struct PulsingDot: View { .frame(width: size, height: size) .scaleEffect(scale) .opacity(opacity) - .onAppear { - guard IslandMotion.allowsPulse(reduceMotion: reduceMotion) else { - // Still visible, just still. - opacity = 1 - return - } - withAnimation(.easeInOut(duration: 0.9).repeatForever(autoreverses: true)) { - scale = 1.25 - opacity = 1.0 - } - } + .onAppear { applyPulse() } + // Also on change, not only on appear. The dot outlives the setting: someone who + // turns Reduce Motion on while the island is streaming would otherwise keep the + // forever-repeating animation until the view was rebuilt, which is the one case + // where they are most likely to be looking at it. + .onChange(of: reduceMotion) { _, _ in applyPulse() } .accessibilityHidden(true) } + + private func applyPulse() { + guard IslandMotion.allowsPulse(reduceMotion: reduceMotion) else { + // Still visible, just still. Reset the scale too — turning the setting on + // mid-pulse would otherwise freeze the dot at whatever size it had reached. + withAnimation(.default) { + scale = 1 + opacity = 1 + } + return + } + withAnimation(.easeInOut(duration: 0.9).repeatForever(autoreverses: true)) { + scale = 1.25 + opacity = 1.0 + } + } } diff --git a/Logue/Views/CrossApp/CommandCenterChatView.swift b/Logue/Views/CrossApp/CommandCenterChatView.swift index 0c34829..95eca05 100644 --- a/Logue/Views/CrossApp/CommandCenterChatView.swift +++ b/Logue/Views/CrossApp/CommandCenterChatView.swift @@ -352,7 +352,7 @@ struct CommandCenterChatView: View { // there. DeepResearchProgressView(conversationID: conversationID) - if let error = currentError { + if let error = displayedError { errorBanner(error) .transition(IslandMotion.transition(reduceMotion: reduceMotion)) } @@ -387,6 +387,10 @@ struct CommandCenterChatView: View { ) guard let route, !isGenerating else { return } + // A new attempt supersedes the last refusal. Without this the "busy elsewhere" + // banner outlives the condition that raised it and sits over a send that worked. + localError = nil + // `isGenerating` is scoped to this conversation, which is right for the spinner and // wrong for this: `AgentCoordinator.send` refuses on a *global* busy check. Without // this guard a send made while the main window was mid-run cleared the field, the @@ -530,6 +534,18 @@ struct CommandCenterChatView: View { return coordinator.lastError(in: conversationID) } + /// What the banner shows. + /// + /// `localError` first, because a refusal is the most recent thing that happened and it is + /// the only one of the two that can exist without a conversation: `sendMessage` refuses a + /// globally-busy send *before* `ensureConversation()`, so `conversationID` is still nil + /// and `currentError` — which needs one — can only ever answer nil on that path. Reading + /// `currentError` alone meant the refusal set a message nothing rendered, and the send + /// appeared to vanish, which is precisely the failure the guard that sets it describes. + private var displayedError: String? { + localError ?? currentError + } + /// Says when a send failed. /// /// The island had this and lost it when it moved onto the coordinator: the old bare diff --git a/LogueTests/DisplayTextTests.swift b/LogueTests/DisplayTextTests.swift new file mode 100644 index 0000000..716ca5a --- /dev/null +++ b/LogueTests/DisplayTextTests.swift @@ -0,0 +1,99 @@ +import Testing +@testable import Logue + +/// What a string picked up somewhere else may look like once it is shown. +/// +/// The approval card is the reason these are not merely cosmetic: it names the thing a +/// destructive tool is about to act on, directly above a Touch ID prompt, and the name it +/// shows can come from a `.md` file dropped into the markdown folder or from a call a +/// prompt-injected model made. +@Suite("Display text") +struct DisplayTextTests { + // MARK: - The part that is a security control + + @Test("A bidirectional override cannot survive into a label") + func stripsBidiOverride() { + // U+202E reverses the display of everything after it. Left in a document title it + // lets `Delete “report.txt”` render as a different filename on the one card whose + // job is to be true. + let spoofed = "report\u{202E}gnp.txt" + let shown = DisplayText.singleLine(spoofed) + #expect(shown.unicodeScalars.contains { $0.value == 0x202E } == false) + #expect(shown == "reportgnp.txt") + } + + @Test("Every bidirectional control is stripped, not only the one we thought of") + func stripsEveryBidiControl() { + // Pinning the *set*, not one example: `CharacterSet.controlCharacters` is Unicode + // categories Cc and Cf, and this is what makes that claim testable rather than + // merely plausible. LRO, RLO, LRE, RLE, PDF, LRI, RLI, FSI, PDI. + for scalar in [0x202A, 0x202B, 0x202C, 0x202D, 0x202E, 0x2066, 0x2067, 0x2068, 0x2069] { + let unicode = Unicode.Scalar(scalar) + #expect(unicode != nil) + guard let unicode else { continue } + let shown = DisplayText.singleLine("a\(String(Character(unicode)))b") + #expect( + shown.unicodeScalars.contains { $0.value == UInt32(scalar) } == false, + "U+\(String(scalar, radix: 16, uppercase: true)) survived" + ) + } + } + + @Test("C0 control characters are stripped") + func stripsC0Controls() { + let shown = DisplayText.singleLine("a\u{0000}b\u{0007}c\u{001B}d") + #expect(shown == "abcd") + } + + @Test("A zero-width joiner cannot pad a name out of view") + func stripsZeroWidth() { + #expect(DisplayText.singleLine("a\u{200B}\u{200D}\u{FEFF}b").contains("\u{200B}") == false) + } + + // MARK: - One line + + @Test("A multi-line value becomes one line") + func collapsesNewlines() { + // The newline becomes a space rather than disappearing. A newline is itself a + // control character, so stripping controls before splitting produced `firstsecond` + // — a different value, silently. That is the mistake this shape invites. + #expect(DisplayText.singleLine("first\nsecond\n\nthird") == "first second third") + #expect(DisplayText.singleLine("a\tb") == "a b") + #expect(DisplayText.singleLine("a\r\nb") == "a b") + } + + @Test("Runs of whitespace collapse to a single space") + func collapsesRuns() { + #expect(DisplayText.singleLine(" a \t\t b ") == "a b") + } + + @Test("Ordinary text is left exactly as it is") + func leavesTextAlone() { + #expect(DisplayText.singleLine("Q3 Planning — notes (v2)") == "Q3 Planning — notes (v2)") + } + + @Test("Text that is only control characters comes back empty, not mangled") + func allControls() { + #expect(DisplayText.singleLine("\u{202E}\u{0007}").isEmpty) + } + + // MARK: - Clamping + + @Test("The ellipsis is inside the budget, never added to it") + func clampNeverExceedsItsLimit() { + for limit in 0 ... 40 { + let clamped = DisplayText.clamp(String(repeating: "x", count: 100), to: limit) + #expect(clamped.count <= limit, "limit \(limit) produced \(clamped.count)") + } + } + + @Test("A value inside the limit is returned untouched") + func shortValueUnchanged() { + #expect(DisplayText.clamp("short", to: 10) == "short") + } + + @Test("A cut value says it was cut") + func cutValueIsMarked() { + #expect(DisplayText.clamp("abcdefghij", to: 5) == "abcd…") + } +} diff --git a/LogueTests/ToolApprovalPromptTests.swift b/LogueTests/ToolApprovalPromptTests.swift index ec90d6b..7b396de 100644 --- a/LogueTests/ToolApprovalPromptTests.swift +++ b/LogueTests/ToolApprovalPromptTests.swift @@ -45,6 +45,36 @@ struct ToolApprovalPromptTests { #expect(missing.isEmpty, "no approval prompt for: \(missing.sorted())") } + // MARK: - A target cannot lie about itself + + @Test("A title cannot reverse the sentence it is shown in") + func targetCannotSpoofWithBidi() { + // The card sits above a Touch ID prompt. A title carrying U+202E — which can arrive + // in a .md file dropped into the markdown folder, or from a create_document call a + // prompt-injected model made — would otherwise render the filename backwards, so the + // card names a different document than the one about to be deleted. + let id = UUID().uuidString + let result = sentence("delete_document", ["documentID": id], resolving: "report\u{202E}gnp.txt") + #expect(result.unicodeScalars.contains { $0.value == 0x202E } == false) + #expect(result == "Delete \u{201C}reportgnp.txt\u{201D}") + } + + @Test("A literal argument is stripped too, not only a resolved name") + func literalTargetIsStripped() { + // `write_text_to_file` takes the path straight from the model's arguments — there is + // no store lookup to launder it, so this is the shorter path to the same card. + let result = sentence("write_text_to_file", ["path": "~/notes\u{202E}dm.txt"], resolving: nil) + #expect(result.unicodeScalars.contains { $0.value == 0x202E } == false) + } + + @Test("A newline in a target cannot split the sentence") + func targetStaysOnOneLine() { + let id = UUID().uuidString + let result = sentence("delete_document", ["documentID": id], resolving: "Q3\nPlanning") + #expect(result.contains("\n") == false) + #expect(result == "Delete \u{201C}Q3 Planning\u{201D}") + } + // MARK: - Naming the target @Test("A document is named, not referred to by its id") From 7b5da68d070a204294c6af2f4be08fb936a1e986 Mon Sep 17 00:00:00 2001 From: shanforge <299346208+shanforge@users.noreply.github.com> Date: Fri, 4 Sep 2026 11:38:54 +0530 Subject: [PATCH 12/12] fix: what the second review round found, all of it in the first round's fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The second round reviewed the first round's changes, which is where the new defects were. **`workspaceIsEmpty` asked for work it then threw away.** The fix routed it through `HomeSuggestions.currentInputs(overdueCount: 0)` — a dummy argument, passed to reach a function that also finds the most recent unsummarized meeting and counts today's, none of which a boolean needs. `AgentChatView` then called `currentInputs` twice per render, once here and once for the chips. It now reads it once as `suggestionInputs` and derives both from it, which is also what makes the empty state and the chips provably agree rather than merely currently agree. **Three `@Environment` stores were left dead by that fix.** `meetingStore`, `documentStore` and `spaceStore` had no remaining reader in `AgentChatView` or its extensions. Removed — and the comment above them, which said the view read from the environment "rather than reaching for the shared singletons", had become the opposite of what the code does. It now says which two are still injected and why, and where the other three went. **`localError` was marked extension-visible and is not.** No file but `CommandCenterChatView` mentions it, so the `// Extension-visible: +Composer` marker named an extension that does not use it — the marker is how a reader knows the wider access is deliberate, so a wrong one is worse than none. Now `private`. Refuted this round, recorded so they are not re-examined from scratch: the `minimumScrimOpacity` floor is unenforced in `IslandSurface` but *is* swept by `IslandSurfaceTests` across the whole matrix, which is this codebase's stated way of holding a pure rule to account; and `IslandMotion.allowsPulse` stopping every pulse looked like it could remove the signal rather than the decoration, but all five `PulsingDot` call sites pair it with words, so the claim its doc makes holds today. Verified: build succeeds, 1670 tests in 146 suites pass, SwiftFormat --lint clean over 544 files, SwiftLint --strict 0 violations in 689 files. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01M3Wpnj9ZmWPKYdFPB1AVY3 --- Logue/Views/Agent/AgentChatView.swift | 38 ++++++++++--------- .../CrossApp/CommandCenterChatView.swift | 3 +- 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/Logue/Views/Agent/AgentChatView.swift b/Logue/Views/Agent/AgentChatView.swift index ac4fb5a..6a3a40a 100644 --- a/Logue/Views/Agent/AgentChatView.swift +++ b/Logue/Views/Agent/AgentChatView.swift @@ -13,14 +13,15 @@ struct AgentChatView: View { /// Incremented to pull focus into the input after a card fills it. @State private var focusRequest = 0 - // Injected by `MainWindowView`. Read from the environment rather than reaching for - // the shared singletons again, so this view observes the same instances the rest of - // the window does. - @Environment(MeetingStore.self) private var meetingStore - @Environment(DocumentStore.self) private var documentStore + // Injected by `MainWindowView`. Both are per-window state rather than a global rule, + // which is why they stay in the environment: `insights` is derived and each surface + // owns its own, and `modelManager` is read for what this window is showing. + // + // The document, meeting and space stores used to sit here too. The three things this + // view asked them are now `HomeSuggestions.storesAreLoaded` and `currentInputs`, + // because the island needs the same answers — see the note on `suggestionInputs`. @Environment(InsightsStatsProvider.self) private var insights @Environment(ModelManager.self) private var modelManager - @Environment(SpaceStore.self) private var spaceStore /// Reveals a space created from Home's first-run card. `AgentChatView` cannot set the /// sidebar selection itself, so `MainWindowView` supplies the one line that can. @@ -244,26 +245,27 @@ struct AgentChatView: View { HomeSuggestions.storesAreLoaded } - /// One definition, used by both the header and the cards — and now by the island too, - /// which is why it lives in `HomeSuggestions` rather than here. Two definitions is how a - /// workspace with spaces but no documents gets first-run chips above a set of cards that - /// have all self-hidden; leaving a copy behind after extracting the rule is how the two - /// would have drifted back apart. + /// The workspace as the chip rules see it. /// - /// The environment stores and the singletons are the same objects — `LogueApp` injects - /// `.shared` — so reading them through `HomeSuggestions` changes nothing at runtime and - /// removes the second definition. + /// Read once and used for both the empty state and the chips, so the two cannot + /// disagree. They were separate readings until the island needed the same rule: two + /// definitions is how a workspace with spaces but no documents gets first-run chips + /// above a set of cards that have all self-hidden. + private var suggestionInputs: HomeSuggestions.Inputs { + HomeSuggestions.currentInputs(overdueCount: insights.actionItemStats.overdue) + } + + /// One definition, used by both the header and the cards — and by the island, which is + /// why the rule lives in `HomeSuggestions` rather than here. private var workspaceIsEmpty: Bool { - !HomeSuggestions.currentInputs(overdueCount: 0).hasAnyContent + !suggestionInputs.hasAnyContent } /// Derived fresh each render from the stores — no inference, no caching. The rules live /// in `HomeSuggestions` and the reading of the workspace in `+Inputs`, so the island can /// offer the same chips rather than growing its own list. private var suggestionChips: [HomeSuggestions.Chip] { - HomeSuggestions.chips( - for: HomeSuggestions.currentInputs(overdueCount: insights.actionItemStats.overdue) - ) + HomeSuggestions.chips(for: suggestionInputs) } /// Post-first-message layout: scrolling message list + bottom-anchored input diff --git a/Logue/Views/CrossApp/CommandCenterChatView.swift b/Logue/Views/CrossApp/CommandCenterChatView.swift index 95eca05..0e0d748 100644 --- a/Logue/Views/CrossApp/CommandCenterChatView.swift +++ b/Logue/Views/CrossApp/CommandCenterChatView.swift @@ -516,13 +516,12 @@ struct CommandCenterChatView: View { return coordinator.activeToolCalls(in: conversationID) } - // Extension-visible: +Composer /// An error the island raised itself, as opposed to one a run left behind. /// /// Separate from the coordinator's `lastError` because a refused send never reaches a /// coordinator — there is no run to own the message — and because it must clear as soon /// as the user does anything. - @State var localError: String? + @State private var localError: String? /// The error for this thread, if the last run left one. ///