diff --git a/CLAUDE.md b/CLAUDE.md index a5b2c4a..b03385f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -137,7 +137,16 @@ decided what Logue was.** These rules exist so that cannot happen again. - **Live run state belongs to a conversation, not to the app.** `AgentRunState` scopes `isProcessing`, `isStreaming`, `streamingText`, `activeToolCalls` and `lastError` to the conversation that started the run. A global flag means a run on one surface paints its - spinner, its streaming text and its tool cards onto the other's thread. + spinner, its streaming text and its tool cards onto the other's thread. This holds for + *every* pipeline, not only the agent loop: `DeepResearchCoordinator` runs one at a time + app-wide, which is precisely what made a global `isRunning` look sufficient for as long as + only one surface could start a run. It owns `runningConversationID` and is read through + `isRunning(in:)`. +- **What the agent did is part of the answer.** Both surfaces render tool calls from + `AgentToolTimeline`, which pairs a call with the result that answers it and settles what to + show when the stored status and the result disagree. The island hid tool turns entirely, + which was fine while it had no tools and became the island claiming credit for work it + would not show. - **An error outlives its run but never its owner.** `lastError` persists after the run ends, so the banner survives until acknowledged — which is why `conversationID` survives `finish()` too. An error with no owner paints on every surface. diff --git a/Logue.xcodeproj/project.pbxproj b/Logue.xcodeproj/project.pbxproj index 2b99f8d..122c0cc 100644 --- a/Logue.xcodeproj/project.pbxproj +++ b/Logue.xcodeproj/project.pbxproj @@ -87,6 +87,7 @@ 13B9900A31E52DA797308A98 /* MLXLMCommon in Frameworks */ = {isa = PBXBuildFile; productRef = 1D568EAA2001718431FEF06A /* MLXLMCommon */; }; 141F4110E598AC7277B8AA8E /* FaviconCache.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7F10E442D4BA9DD990D8CEBA /* FaviconCache.swift */; }; 14575BB38E5D155A9BF7415B /* CommandCenterPIIView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 24578E3A172E90DC703D4C9D /* CommandCenterPIIView.swift */; }; + 14669001D4941A2DF4E783C2 /* AskStopTarget.swift in Sources */ = {isa = PBXBuildFile; fileRef = A38FEAE5E714CB8DAD9D7263 /* AskStopTarget.swift */; }; 14AF54A07D25FA4EE8DA4DC7 /* ScoreRingView.swift in Sources */ = {isa = PBXBuildFile; fileRef = E188450986288699B1B8A252 /* ScoreRingView.swift */; }; 14BD5D00F6C5353221E2E913 /* GraphEntity.swift in Sources */ = {isa = PBXBuildFile; fileRef = 73D1A15BDB3690A5CF5AB97E /* GraphEntity.swift */; }; 15CC6613C14DCCA50A5BC133 /* DiagramMathBlockView.swift in Sources */ = {isa = PBXBuildFile; fileRef = E1C8C014AA23FE44D9ED97AE /* DiagramMathBlockView.swift */; }; @@ -99,6 +100,7 @@ 17F7BAF0DBA60523B8EFD5C7 /* AgentTool.swift in Sources */ = {isa = PBXBuildFile; fileRef = F520732BF6E86EA62DAC3E2B /* AgentTool.swift */; }; 17F88171613BB35BB0AF2A2A /* HomeContinueSection.swift in Sources */ = {isa = PBXBuildFile; fileRef = 105A1B21D0379DCB157BC451 /* HomeContinueSection.swift */; }; 18B116F18A40D8062423489E /* RecordingSessionManager+Recovery.swift in Sources */ = {isa = PBXBuildFile; fileRef = 33AE1CBE332B50E515AAA7EE /* RecordingSessionManager+Recovery.swift */; }; + 18B4AC4D2D7DAF44809171DB /* ComposerPlusMenu.swift in Sources */ = {isa = PBXBuildFile; fileRef = DB7A59F7C65882E22FFD424D /* ComposerPlusMenu.swift */; }; 1928EADD3A8BEEA0B72553EF /* SharedPolishComponents.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6410A54BE5400DAAFADA2A88 /* SharedPolishComponents.swift */; }; 1AC0C886212A4F3ADC9A48D1 /* DocumentIconTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9130B8EDD12207674F5B4323 /* DocumentIconTests.swift */; }; 1BE965CE5953BCA256111BEA /* MeetingRowCompact.swift in Sources */ = {isa = PBXBuildFile; fileRef = 72EE330160AA0276B2907F2A /* MeetingRowCompact.swift */; }; @@ -155,6 +157,7 @@ 2EBF59EE30602911A0C7236E /* ContentNavigatorTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 83C272112D79CC98FD132288 /* ContentNavigatorTests.swift */; }; 2ED027BF90138612DDE85A2C /* CalloutBlockTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 46DB0259534855A640246B2A /* CalloutBlockTests.swift */; }; 2F188F551FC6C5738BDB23B2 /* QuickOpen.swift in Sources */ = {isa = PBXBuildFile; fileRef = D732A7B7C4D529BBD960BDC9 /* QuickOpen.swift */; }; + 2F4BA10FF02C91BDC115F449 /* IslandThreadTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 402D8E4447C9B7063ADE7358 /* IslandThreadTests.swift */; }; 2FD721F2491FFD83810DB23C /* DiarizationManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6518F90C57E0A10C7DBF1328 /* DiarizationManager.swift */; }; 3040F141C008F5162215FC11 /* DiarizationManager+LongRecording.swift in Sources */ = {isa = PBXBuildFile; fileRef = 65926C816DF6F8E6E334194A /* DiarizationManager+LongRecording.swift */; }; 3078B745D4379CB1523EA5D2 /* IconToolbarView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0FF6C4FACAA2A06BB2C5306A /* IconToolbarView.swift */; }; @@ -186,6 +189,7 @@ 3BB74E6C769B50816B912D28 /* BulletRow.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2B9F22B7392C28686BD5CE60 /* BulletRow.swift */; }; 3C20C03EB550A70C46FFAD7F /* MeetingPromptBuilder.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3794E1F92314F0474FC7B317 /* MeetingPromptBuilder.swift */; }; 3C8B6B59AC5A90F65186C43C /* InsightsStatsProvider.swift in Sources */ = {isa = PBXBuildFile; fileRef = 97081E3100BD2ABC8E14C65B /* InsightsStatsProvider.swift */; }; + 3D7EA126595AC2DEE512D052 /* ComposerPlusMenuTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 113C0BD21D2ED30E81FE0C42 /* ComposerPlusMenuTests.swift */; }; 3DE02709418A72736AFE5D68 /* SuggestionCardView.swift in Sources */ = {isa = PBXBuildFile; fileRef = F4D0B06CDDC1184FD7958F9A /* SuggestionCardView.swift */; }; 3FEF62A652D6865C8B97DD35 /* CommunityDetector.swift in Sources */ = {isa = PBXBuildFile; fileRef = E72E4F906B12DB58475B9460 /* CommunityDetector.swift */; }; 40A5F92F28F50D4CE947C97F /* ModelDownloadRow.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9C91F95BC414F31D9357EB2D /* ModelDownloadRow.swift */; }; @@ -275,6 +279,7 @@ 5CA3F8E612944FDC80FE2A00 /* OverviewRecentActivityCard.swift in Sources */ = {isa = PBXBuildFile; fileRef = FD9DB88569857E33E72CCFAE /* OverviewRecentActivityCard.swift */; }; 5CBFF3B40F91960EC8178989 /* CommandCenterComposeView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7ABC89CED798F0088E00B682 /* CommandCenterComposeView.swift */; }; 5CEA326BB1396B276764A6EB /* WhatsNewView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 11C3F8ADFF573D2924827697 /* WhatsNewView.swift */; }; + 5F47672A876BDA67388F15DF /* AskStopTargetTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4AFBE4944E79C2F3975627B4 /* AskStopTargetTests.swift */; }; 5F91A42038BFDFE88AF5D2FF /* MeetingStore+Reminders.swift in Sources */ = {isa = PBXBuildFile; fileRef = C1C5465515D27E6AB7556636 /* MeetingStore+Reminders.swift */; }; 5FCA28149B189B61283A387C /* DocumentIcon.swift in Sources */ = {isa = PBXBuildFile; fileRef = DBB768463E40E11BC5922145 /* DocumentIcon.swift */; }; 5FCC04535B46B4197AE5B771 /* HomeContentColumn.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1631B02D9AF07049775AF024 /* HomeContentColumn.swift */; }; @@ -367,6 +372,7 @@ 8305B9B5810624EA6AD19520 /* MarkdownStyler.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6D5F001CEA6CB89B25C75055 /* MarkdownStyler.swift */; }; 83D4B061F474B55E93422576 /* DocumentRowCompact.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1B122ABAFDC2934FB106C9C6 /* DocumentRowCompact.swift */; }; 83F129EF9A0320C6AD1EF039 /* HandCursorArea.swift in Sources */ = {isa = PBXBuildFile; fileRef = 387EC57EC8BA655296463A43 /* HandCursorArea.swift */; }; + 845CFBCB97942BA0E22F7D14 /* AgentToolTimeline.swift in Sources */ = {isa = PBXBuildFile; fileRef = B2B3921979A175ECA3CF569F /* AgentToolTimeline.swift */; }; 855530A64C4C8BBBC9C995EE /* BlockFrameStore.swift in Sources */ = {isa = PBXBuildFile; fileRef = D0946FCC63B694398DDCA143 /* BlockFrameStore.swift */; }; 859BD1BDE5D6617990A2352C /* OfficeExtractor.swift in Sources */ = {isa = PBXBuildFile; fileRef = F686C58A65228625D6D45D32 /* OfficeExtractor.swift */; }; 85BFFE25C6715ABAE40DFB2C /* RightIconToolbar.swift in Sources */ = {isa = PBXBuildFile; fileRef = 088E6D7B300F24ADC23373E8 /* RightIconToolbar.swift */; }; @@ -545,10 +551,12 @@ CBFB8E3030659A2F66D8A54A /* AgentCoordinator+Approval.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0027C5C3EE6CAEEB887EAEC7 /* AgentCoordinator+Approval.swift */; }; CCA2748ACA502E36CD99A964 /* MeetingGridCardView.swift in Sources */ = {isa = PBXBuildFile; fileRef = AD6FE3B101005521F444593D /* MeetingGridCardView.swift */; }; CCD1A1E6938412C9D24E066E /* LangGraph in Frameworks */ = {isa = PBXBuildFile; productRef = 7EFECA56D85500BA67B0D907 /* LangGraph */; }; + CD29FF65F8DE5950B719187A /* AgentToolTimelineTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7E4CC91DCF127659514C38D2 /* AgentToolTimelineTests.swift */; }; CDA217D545BB79B7978CFCF1 /* DocumentRelationshipTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4B27EF7809E2AAC8C8F7E6DC /* DocumentRelationshipTests.swift */; }; CDA2BBE893DF12C9FC4A103C /* MeetingStore+Metadata.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3D36503D92C1D2637BBC06BF /* MeetingStore+Metadata.swift */; }; CE08246128090E026CD9895A /* MeetingStore+Search.swift in Sources */ = {isa = PBXBuildFile; fileRef = 81156E078CE31A1267D150EE /* MeetingStore+Search.swift */; }; CE5A69A3A3CBC569F9D8DA57 /* JumpToLatestPill.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2666524E8E079ED62C748893 /* JumpToLatestPill.swift */; }; + CF68588BF9879F2449BC4BEE /* IslandThread.swift in Sources */ = {isa = PBXBuildFile; fileRef = E3AAB73BD6A8A6AF6C54DC29 /* IslandThread.swift */; }; CF7EF95C9DA5C2830CE64C33 /* MeetingStore+Transcript.swift in Sources */ = {isa = PBXBuildFile; fileRef = 709692D702A29C9D5A0C4C9D /* MeetingStore+Transcript.swift */; }; CF82951A70FC5F643DD8F1CD /* whatsnew-privacy.png in Resources */ = {isa = PBXBuildFile; fileRef = 9C6A8A41EE02367332FFCEEE /* whatsnew-privacy.png */; }; CFC1E2D409AE2B588FE6A9AD /* OverviewQuickActionsCard.swift in Sources */ = {isa = PBXBuildFile; fileRef = ED35C64E018F0B223D5FE487 /* OverviewQuickActionsCard.swift */; }; @@ -696,6 +704,7 @@ FE2793A4972BDC2CEB93C018 /* MenuBarIcon.png in Resources */ = {isa = PBXBuildFile; fileRef = EF7B96F52DB280D23D637F33 /* MenuBarIcon.png */; }; FE2FB582956D1251285E5C43 /* BatchTranscriptFilterTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 505AF0501A911C8BE2CB13CC /* BatchTranscriptFilterTests.swift */; }; FEA46F7D5BB9F4D69448D52D /* TableBlockData.swift in Sources */ = {isa = PBXBuildFile; fileRef = DB031A0FCAED343F68FB194A /* TableBlockData.swift */; }; + FEED3F76A6F1EDCD3B643928 /* DeepResearchOwnershipTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = CE1A7250F0233D29C4DC9960 /* DeepResearchOwnershipTests.swift */; }; FF11912FAC2B1F75C7ED19E6 /* OpenAICompatibleClient.swift in Sources */ = {isa = PBXBuildFile; fileRef = 249472506C451094FA86F049 /* OpenAICompatibleClient.swift */; }; FF22452C6C48807D20EFB494 /* BrowserBridgeServer.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0CF329AEF9DA7EDE1B1CF4E6 /* BrowserBridgeServer.swift */; }; FF8EF47C24090CCE60A0090C /* PromptRegistry.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6430E324276EBE1BD45A48BD /* PromptRegistry.swift */; }; @@ -759,6 +768,7 @@ 1073F096F16AFC2BD1908BE7 /* AssistantActionRow.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AssistantActionRow.swift; sourceTree = ""; }; 1083BC25237ADD22FEE5A277 /* BackupManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BackupManager.swift; sourceTree = ""; }; 112B70D4C5B4DF51BEFCFB36 /* TaskInspectorPanel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TaskInspectorPanel.swift; sourceTree = ""; }; + 113C0BD21D2ED30E81FE0C42 /* ComposerPlusMenuTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ComposerPlusMenuTests.swift; sourceTree = ""; }; 1153D35EF20882A425B5D836 /* SectionHeader.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SectionHeader.swift; sourceTree = ""; }; 11C3F8ADFF573D2924827697 /* WhatsNewView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WhatsNewView.swift; sourceTree = ""; }; 11F90CF8BD9ABA35C96B0E76 /* DiagnosticsReport.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DiagnosticsReport.swift; sourceTree = ""; }; @@ -884,6 +894,7 @@ 3E3BFD10323B282DEB7E0A9A /* whatsnew-smart-minutes.png */ = {isa = PBXFileReference; lastKnownFileType = image.png; path = "whatsnew-smart-minutes.png"; sourceTree = ""; }; 3E86EBBF6643966395A7DE8B /* FileAccessGate.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FileAccessGate.swift; sourceTree = ""; }; 3FF85EA0F9822CBD660E9AAB /* MarkdownStorageWarningSheet.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MarkdownStorageWarningSheet.swift; sourceTree = ""; }; + 402D8E4447C9B7063ADE7358 /* IslandThreadTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = IslandThreadTests.swift; sourceTree = ""; }; 40381D10206F56D057943756 /* SpaceContentPane.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SpaceContentPane.swift; sourceTree = ""; }; 4066081DEC2821193FE7D882 /* WikiLinkURLTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WikiLinkURLTests.swift; sourceTree = ""; }; 417436A70F2C0F1DDA347DC4 /* AttachmentIntake.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AttachmentIntake.swift; sourceTree = ""; }; @@ -908,6 +919,7 @@ 493337C75A0D4F073E22C76F /* BlockEditorDocument.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BlockEditorDocument.swift; sourceTree = ""; }; 4A96015B635CF7C9D07F84DA /* PermissionsSettingsTab.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PermissionsSettingsTab.swift; sourceTree = ""; }; 4AB2A7A4F24E2BC67AEE4319 /* CalloutRoundTripEdgeTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CalloutRoundTripEdgeTests.swift; sourceTree = ""; }; + 4AFBE4944E79C2F3975627B4 /* AskStopTargetTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AskStopTargetTests.swift; sourceTree = ""; }; 4B27EF7809E2AAC8C8F7E6DC /* DocumentRelationshipTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DocumentRelationshipTests.swift; sourceTree = ""; }; 4B2F874FCD0AF25F7B37D3FC /* MalformedToolCall.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MalformedToolCall.swift; sourceTree = ""; }; 4B4D41D14C5F7B6352AC692F /* CanvasPaneView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CanvasPaneView.swift; sourceTree = ""; }; @@ -1048,6 +1060,7 @@ 7DF36BD14718CAF328990D0A /* FolderStore+SeedData.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "FolderStore+SeedData.swift"; sourceTree = ""; }; 7E01E332D859D172446E5AE2 /* Assets.xcassets */ = {isa = PBXFileReference; lastKnownFileType = folder.assetcatalog; path = Assets.xcassets; sourceTree = ""; }; 7E2E2735D98AE330FF3804B4 /* HomeSpacesSection.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HomeSpacesSection.swift; sourceTree = ""; }; + 7E4CC91DCF127659514C38D2 /* AgentToolTimelineTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AgentToolTimelineTests.swift; sourceTree = ""; }; 7E67F5E0205204A2F59C5F8D /* AgentToolSpec.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AgentToolSpec.swift; sourceTree = ""; }; 7EE17DC9C617B8DC71B33171 /* SpaceFolderLayout.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SpaceFolderLayout.swift; sourceTree = ""; }; 7F10E442D4BA9DD990D8CEBA /* FaviconCache.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FaviconCache.swift; sourceTree = ""; }; @@ -1151,6 +1164,7 @@ A297CA8A1434241BA7F14120 /* QuickComposeView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = QuickComposeView.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 = ""; }; + A38FEAE5E714CB8DAD9D7263 /* AskStopTarget.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AskStopTarget.swift; sourceTree = ""; }; A3A9B8AAE15A27CE425D28BC /* ScanSafetyTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ScanSafetyTests.swift; sourceTree = ""; }; A4623149A8D1BB07A0CB7B26 /* DateGroupingHelper.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DateGroupingHelper.swift; sourceTree = ""; }; A488F97BC5DBB0C094B1C74F /* AgentCoordinator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AgentCoordinator.swift; sourceTree = ""; }; @@ -1193,6 +1207,7 @@ B20F66869474AA1C0D3640A9 /* CaptureSegmentTimeline.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CaptureSegmentTimeline.swift; sourceTree = ""; }; B21302B0871DD6955B6F5148 /* SpaceContentPane+Cards.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "SpaceContentPane+Cards.swift"; sourceTree = ""; }; B287EB86FD4781645F744C86 /* HighlightAnchorTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HighlightAnchorTests.swift; sourceTree = ""; }; + B2B3921979A175ECA3CF569F /* AgentToolTimeline.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AgentToolTimeline.swift; sourceTree = ""; }; B37427E70D5B05A2E6C5B253 /* QuickOpenTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = QuickOpenTests.swift; sourceTree = ""; }; B3B356E5C7937D4D7600108E /* TaskFileTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TaskFileTests.swift; sourceTree = ""; }; B3C7E5CF5FB74C62BE34845D /* SlashCommandView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SlashCommandView.swift; sourceTree = ""; }; @@ -1262,6 +1277,7 @@ CCC4C9A99648838E46779A4D /* EditorZoom.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EditorZoom.swift; sourceTree = ""; }; CCEE1CC67630CF84E03498D5 /* MermaidRenderer.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MermaidRenderer.swift; sourceTree = ""; }; CD2339649C1FDB68C5155E1A /* SpeakerAlignment.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SpeakerAlignment.swift; sourceTree = ""; }; + CE1A7250F0233D29C4DC9960 /* DeepResearchOwnershipTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DeepResearchOwnershipTests.swift; sourceTree = ""; }; CE4B9329DFE86A8D2B1629DE /* WritingGoalMode.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WritingGoalMode.swift; sourceTree = ""; }; CF540C94525BF2C21BA389BF /* LogoBrandView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LogoBrandView.swift; sourceTree = ""; }; CF81895980E378C21F96AFD6 /* DocumentLLMTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DocumentLLMTests.swift; sourceTree = ""; }; @@ -1302,6 +1318,7 @@ DB031A0FCAED343F68FB194A /* TableBlockData.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TableBlockData.swift; sourceTree = ""; }; DB53BFC5C00966DB03AB861E /* KeyboardShortcutsView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = KeyboardShortcutsView.swift; sourceTree = ""; }; DB73B7518D8547885462EA7B /* TaskFolderStore.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TaskFolderStore.swift; sourceTree = ""; }; + DB7A59F7C65882E22FFD424D /* ComposerPlusMenu.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ComposerPlusMenu.swift; sourceTree = ""; }; DB8FF31CFF720AFDAE449EBE /* RecordingResilienceReviewTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RecordingResilienceReviewTests.swift; sourceTree = ""; }; DBB768463E40E11BC5922145 /* DocumentIcon.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DocumentIcon.swift; sourceTree = ""; }; DCC5DA2719E8ECFA34A035EB /* DocumentListPane+Organise.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "DocumentListPane+Organise.swift"; sourceTree = ""; }; @@ -1326,6 +1343,7 @@ E2A54999991BFEA773747361 /* AgentReadAloudService.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AgentReadAloudService.swift; sourceTree = ""; }; E2CEF58FA0555995987FD7AE /* InputDialogTools.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = InputDialogTools.swift; sourceTree = ""; }; E2F831E25CF9FDC86465A53D /* AgentChatGraph.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AgentChatGraph.swift; sourceTree = ""; }; + E3AAB73BD6A8A6AF6C54DC29 /* IslandThread.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = IslandThread.swift; sourceTree = ""; }; E3F2421D7F17F2EE5A04EF7F /* BlockFrameStoreTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BlockFrameStoreTests.swift; sourceTree = ""; }; E3FB4C4D5FE2C19460E3DFED /* VocabularyEnhancementPanelView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = VocabularyEnhancementPanelView.swift; sourceTree = ""; }; E420BD97F84DEF6141ADEDA4 /* katex.min.css */ = {isa = PBXFileReference; lastKnownFileType = text.css; path = katex.min.css; sourceTree = ""; }; @@ -1466,8 +1484,10 @@ 79C6BA7C0C06C035AC3BFBD1 /* LLMIntegration */, AC241A87AE9D64BF9E0D2B66 /* ActionItemInboxTests.swift */, 86029AD56EBB4317FFDFB4E4 /* AgentRunStateTests.swift */, + 7E4CC91DCF127659514C38D2 /* AgentToolTimelineTests.swift */, 4F21C87791C6ECF443D14823 /* AppVersionTests.swift */, 0A4CB54883E92F9A6F98BE77 /* AskRouteTests.swift */, + 4AFBE4944E79C2F3975627B4 /* AskStopTargetTests.swift */, 9A8627FAD5FB6C9F54FE23A2 /* AttachmentIntakeTests.swift */, 2476E5FAF6705D05BF8C22F2 /* AudioFileChunkReaderTests.swift */, 5FBB6345B454F1E296E6DE8D /* AudioTimelineMixerTests.swift */, @@ -1483,9 +1503,11 @@ 4AB2A7A4F24E2BC67AEE4319 /* CalloutRoundTripEdgeTests.swift */, D2856879E369BF8160EBAA1B /* CaptureSegmentTimelineTests.swift */, 136ABA45F11F81E853897C17 /* CommandCenterChatRuleTests.swift */, + 113C0BD21D2ED30E81FE0C42 /* ComposerPlusMenuTests.swift */, 83C272112D79CC98FD132288 /* ContentNavigatorTests.swift */, 263E3C59B8D03346E04E7C92 /* DeepLinkRoutingTests.swift */, EB27F11A2333C941E28C916E /* DeepLinkTests.swift */, + CE1A7250F0233D29C4DC9960 /* DeepResearchOwnershipTests.swift */, 5F635405A40E8F0C32B93039 /* DetachedBufferTests.swift */, 17F31B485FED361E8702AA2D /* DeviceLossPolicyTests.swift */, 860DFF869B9C6C39BE468431 /* DiagnosticsReportTests.swift */, @@ -1509,6 +1531,7 @@ F773F30D16F4FF92E56320E1 /* HomeContinueGridTests.swift */, A3600798EF27A735FC4BC80E /* HomeSuggestionsTests.swift */, 0062B04D59734428AFC49682 /* ImportFileReadingTests.swift */, + 402D8E4447C9B7063ADE7358 /* IslandThreadTests.swift */, 7F48D46D3D40534008356F5D /* LinkIndexBuildTests.swift */, F3BC206B7D135B2017B99C1D /* LinkIndexTests.swift */, BABAA98AD686B90D04DEA359 /* LinkRenameTests.swift */, @@ -1718,6 +1741,7 @@ 865AE91088D3794F9C1CC457 /* CommandCenterChatRule.swift */, C8D8F63B0BEBD964C1409AA5 /* CommandCenterController.swift */, DD62B3D8FB4F2050661CE197 /* InlineAssistant.swift */, + E3AAB73BD6A8A6AF6C54DC29 /* IslandThread.swift */, 29E0387E99032708C27EA914 /* KeyboardShortcut.swift */, B3F9D423BBCD2E5ED6C361AA /* MenuBarCompanion.swift */, 8DC85DAE7DF132DD4B355F61 /* PolishEngine.swift */, @@ -2036,6 +2060,7 @@ DA41BE45973601CD371C00BB /* AgentChatView+Messages.swift */, 92DB0F899AF6F08FEAF8AA46 /* AgentConversationListView.swift */, 1073F096F16AFC2BD1908BE7 /* AssistantActionRow.swift */, + DB7A59F7C65882E22FFD424D /* ComposerPlusMenu.swift */, FEA066DE093D5AFBD76D6F0D /* DeepResearchProgressView.swift */, C2A85CBFCD11CDF2FB693F73 /* InlineDiagramView.swift */, 8CCF492C5BB1691EC0946B25 /* InlineLaTeXView.swift */, @@ -2191,8 +2216,10 @@ A488F97BC5DBB0C094B1C74F /* AgentCoordinator.swift */, 0027C5C3EE6CAEEB887EAEC7 /* AgentCoordinator+Approval.swift */, 62CA343B98C1F62D9F89B0B6 /* AgentRunState.swift */, + B2B3921979A175ECA3CF569F /* AgentToolTimeline.swift */, B1CF9F695CCB56896DC9A02C /* ApprovalGate.swift */, 86C6E3021828B2B8589C720A /* AskRoute.swift */, + A38FEAE5E714CB8DAD9D7263 /* AskStopTarget.swift */, 417436A70F2C0F1DDA347DC4 /* AttachmentIntake.swift */, 4B2F874FCD0AF25F7B37D3FC /* MalformedToolCall.swift */, B55FA53C32F4D2968C07CEE5 /* MessageActions.swift */, @@ -2684,8 +2711,10 @@ files = ( C7E2AEAF97B253EDD23DA5B5 /* ActionItemInboxTests.swift in Sources */, AAEF3C6465DC0A9EA911FE93 /* AgentRunStateTests.swift in Sources */, + CD29FF65F8DE5950B719187A /* AgentToolTimelineTests.swift in Sources */, B71E1192276BBC1904238D1A /* AppVersionTests.swift in Sources */, 85FB21E5E26B2D3D46093D47 /* AskRouteTests.swift in Sources */, + 5F47672A876BDA67388F15DF /* AskStopTargetTests.swift in Sources */, E5908FC6811FA07E101AB6F1 /* AttachmentIntakeTests.swift in Sources */, 08CC77D0F8EB0A6EAB31D544 /* AudioFileChunkReaderTests.swift in Sources */, 47F0712575338CEF0C0411F0 /* AudioTimelineMixerTests.swift in Sources */, @@ -2701,9 +2730,11 @@ B27F60023417DD2C91BC326C /* CalloutRoundTripEdgeTests.swift in Sources */, A11728D4726E858C626D681E /* CaptureSegmentTimelineTests.swift in Sources */, 3A1F8B924E2BAA7DD5C45A6A /* CommandCenterChatRuleTests.swift in Sources */, + 3D7EA126595AC2DEE512D052 /* ComposerPlusMenuTests.swift in Sources */, 2EBF59EE30602911A0C7236E /* ContentNavigatorTests.swift in Sources */, 8837715694AEB1053618212A /* DeepLinkRoutingTests.swift in Sources */, 52B089BBC06F1A4DC8CFA5CE /* DeepLinkTests.swift in Sources */, + FEED3F76A6F1EDCD3B643928 /* DeepResearchOwnershipTests.swift in Sources */, 00578DC8D44288FBDC45B33C /* DetachedBufferTests.swift in Sources */, 2AF2DBAEFCA1E332ECBA2A1D /* DeviceLossPolicyTests.swift in Sources */, D5975A82DEA279851186013D /* DiagnosticsReportTests.swift in Sources */, @@ -2729,6 +2760,7 @@ B68578CA8ABFB343EABB454C /* HomeContinueGridTests.swift in Sources */, 549C3D0D72BD69E53B7929A5 /* HomeSuggestionsTests.swift in Sources */, D0FFBA721311ED3F007A7D49 /* ImportFileReadingTests.swift in Sources */, + 2F4BA10FF02C91BDC115F449 /* IslandThreadTests.swift in Sources */, 5649718294F9E5770348BD76 /* LLMTestEvalExtensions.swift in Sources */, D480184376B6F26A9755133A /* LLMTestHarness.swift in Sources */, BDA8BA0D74D9DDA236A9B348 /* LinkIndexBuildTests.swift in Sources */, @@ -2849,6 +2881,7 @@ 7978A00C56B10809E1BFBD1C /* AgentRunState.swift in Sources */, 17F7BAF0DBA60523B8EFD5C7 /* AgentTool.swift in Sources */, A8432BB73EAB05259ECC7E6E /* AgentToolSpec.swift in Sources */, + 845CFBCB97942BA0E22F7D14 /* AgentToolTimeline.swift in Sources */, 7B000982F90A41A9148CD0F7 /* AnthropicClient.swift in Sources */, 8D189EF6BCF4EBB2D9001347 /* AppConstants.swift in Sources */, 1CCAD60D4518A9CCED75F860 /* AppDelegate.swift in Sources */, @@ -2856,6 +2889,7 @@ E6894F2CF944A02ECBF56187 /* AppVersion.swift in Sources */, ED283783D6A185FB3914A331 /* ApprovalGate.swift in Sources */, 556246B3C2F8B616A0CDD27B /* AskRoute.swift in Sources */, + 14669001D4941A2DF4E783C2 /* AskStopTarget.swift in Sources */, B62AF07AD0FDB3E3E4B31D77 /* AssistantActionRow.swift in Sources */, 1C5F8AF69527526490E8CA54 /* AttachmentIntake.swift in Sources */, ECB8B236B80423C7D1A752FF /* AudioFileChunkReader.swift in Sources */, @@ -2924,6 +2958,7 @@ 2206F8D1450B6DA3886A850F /* CommandCenterRecordingView.swift in Sources */, 03C16AA3E0D4260F0D389809 /* CommandPaletteView.swift in Sources */, 3FEF62A652D6865C8B97DD35 /* CommunityDetector.swift in Sources */, + 18B4AC4D2D7DAF44809171DB /* ComposerPlusMenu.swift in Sources */, 8AFFF64C0EC6F5AA2678504A /* ConferencingAppDetector.swift in Sources */, 5AAAF36F01B586D73D6A35EF /* ContactsTool.swift in Sources */, DF6799F0DCAEEA99A301E63E /* ContentLoadingView.swift in Sources */, @@ -3043,6 +3078,7 @@ 9FC5E9AC4D7755C95151D036 /* InsightCardShell.swift in Sources */, 8621E76EC5A20A48091893C4 /* InsightsSectionView.swift in Sources */, 3C8B6B59AC5A90F65186C43C /* InsightsStatsProvider.swift in Sources */, + CF68588BF9879F2449BC4BEE /* IslandThread.swift in Sources */, 56F4853038E368546DCD2955 /* JavaScriptTool.swift in Sources */, CE5A69A3A3CBC569F9D8DA57 /* JumpToLatestPill.swift in Sources */, D1E0942395634B019B74C872 /* KeyboardShortcut.swift in Sources */, diff --git a/Logue/Agent/AgentCoordinator+Approval.swift b/Logue/Agent/AgentCoordinator+Approval.swift index a7a4881..dd2e519 100644 --- a/Logue/Agent/AgentCoordinator+Approval.swift +++ b/Logue/Agent/AgentCoordinator+Approval.swift @@ -142,7 +142,8 @@ extension AgentCoordinator { ) } - // Append the result message (invisible in UI; read by findResult in ToolExecutionCard) + // The result is its own message rather than a field on the call, so it is paired + // back up on read — see `AgentToolTimeline`. Never rendered as a row itself. let toolResult = AgentToolResult(toolCallID: toolCallID, output: output, isError: isError) store.appendMessage( AgentMessage(role: .toolResult, content: output, toolResult: toolResult), diff --git a/Logue/Agent/AgentToolTimeline.swift b/Logue/Agent/AgentToolTimeline.swift new file mode 100644 index 0000000..fd91a1e --- /dev/null +++ b/Logue/Agent/AgentToolTimeline.swift @@ -0,0 +1,107 @@ +import Foundation + +/// What the agent did during a turn, paired up and ready to render. +/// +/// A tool call and its result are two separate messages in the conversation, linked only by +/// an id — the call is appended when the model asks for it, the result arrives whenever the +/// tool finishes. Rendering a card therefore means walking forward from a call looking for +/// the message that answers it. +/// +/// That walk lived in a `private func findResult` inside `AgentChatView+Messages`, along with +/// the rule for what status to *show* when the stored status and the result disagree. Being +/// private to a view is why the island could not show tool history at all: the island runs +/// the same agent loop and stores the same messages, and the only thing it lacked was a way +/// to read them back. #61's rule is that a feature is mounted by both surfaces rather than +/// written twice, so the pairing moved here and both surfaces ask it. +/// +/// Free of SwiftUI, so the pairing is testable without a view. +enum AgentToolTimeline { + /// One tool call with whatever is known about how it went. + struct Entry: Identifiable, Equatable { + let call: AgentToolCall + let result: AgentToolResult? + /// What the card should show, which is not always `call.status` — see + /// `displayStatus(of:result:)`. + let status: AgentToolCallStatus + + var id: UUID { + call.id + } + + static func == (lhs: Self, rhs: Self) -> Bool { + lhs.call.id == rhs.call.id + && lhs.status == rhs.status + && lhs.result?.output == rhs.result?.output + && lhs.result?.isError == rhs.result?.isError + } + } + + /// The result answering `toolCallID`, if one has arrived. + /// + /// Every message is searched rather than only those after the call. Ordering is how the + /// conversation is built, not something the store guarantees on read, and a result that + /// sorted oddly would otherwise render as a call that never finished. + static func result(for toolCallID: UUID, in messages: [AgentMessage]) -> AgentToolResult? { + for message in messages where message.role == .toolResult { + if let result = message.toolResult, result.toolCallID == toolCallID { + return result + } + } + return nil + } + + /// What to show when the stored status and the result disagree. + /// + /// A call can be persisted as `.needsConfirmation` and still have a result: the user + /// answered the prompt on the other surface, or the run was resumed. The result is the + /// later fact, so it wins — otherwise a finished call keeps rendering Approve/Deny + /// buttons for something that has already happened, which is worse on the island than + /// anywhere else because the island is where the answer was given. + static func displayStatus( + of call: AgentToolCall, + result: AgentToolResult? + ) -> AgentToolCallStatus { + guard let result else { return call.status } + return result.isError ? .failed : .completed + } + + /// The calls carried by `message`, each paired with its result. + static func entries( + in message: AgentMessage, + allMessages: [AgentMessage] + ) -> [Entry] { + message.toolCalls.map { call in + let result = result(for: call.id, in: allMessages) + return Entry( + call: AgentToolCall( + id: call.id, + toolName: call.toolName, + arguments: call.arguments, + status: displayStatus(of: call, result: result), + clearance: call.clearance + ), + result: result, + status: displayStatus(of: call, result: result) + ) + } + } + + /// Every tool call in the conversation, in order, paired with its result. + /// + /// Used by the island, which renders tool history as its own rows rather than hanging + /// cards off the message that carried them. + static func entries(in messages: [AgentMessage]) -> [Entry] { + messages.flatMap { entries(in: $0, allMessages: messages) } + } + + /// The calls still waiting on the user. + /// + /// A call whose result has arrived is never pending however it was stored, which is the + /// same disagreement `displayStatus` settles — stated once here so the approval strip and + /// the history cards cannot answer it differently. + static func awaitingApproval(in messages: [AgentMessage]) -> [AgentToolCall] { + entries(in: messages) + .filter { $0.status == .needsConfirmation } + .map(\.call) + } +} diff --git a/Logue/Agent/AskRoute.swift b/Logue/Agent/AskRoute.swift index 4931358..9cbdd17 100644 --- a/Logue/Agent/AskRoute.swift +++ b/Logue/Agent/AskRoute.swift @@ -87,7 +87,11 @@ enum AskRouter { // the agent is expected to read it. guard !trimmed.isEmpty || request.hasAttachments else { return nil } - if request.deepResearchRequested { + // Deep Research needs a question. The chip alone is not one: a send carrying only + // attachments passes the guard above, and routing that to Deep Research appended an + // empty user bubble and ran the whole seven-step pipeline on "" — with the files + // handed straight back, since research takes no attachments on either surface. + if request.deepResearchRequested, !trimmed.isEmpty { return .deepResearch } diff --git a/Logue/Agent/AskStopTarget.swift b/Logue/Agent/AskStopTarget.swift new file mode 100644 index 0000000..b7c755a --- /dev/null +++ b/Logue/Agent/AskStopTarget.swift @@ -0,0 +1,32 @@ +import Foundation + +/// What the Stop button stops. +/// +/// Two coordinators can be behind one button and they do not know about each other, so the +/// decision has to be made somewhere — and it was made inside a `View`, where nothing could +/// test it. That is the same privacy-inside-a-view problem `AgentToolTimeline` was extracted +/// to fix, one button along. +/// +/// The case that matters is `nothing`. The island's version fell back to cancelling the agent +/// loop unconditionally, and `AgentCoordinator.cancel()` is *unscoped* — it kills the single +/// global task and rejects every pending approval. So pressing "New" on an idle island +/// stopped an answer that was streaming in the main window. +enum AskStopTarget: Equatable { + case deepResearch + case agentLoop + /// Nothing this surface owns is running, so Stop stops nothing. Cancelling "just in case" + /// reaches into whatever the other surface is doing. + case nothing + + /// - Parameters: + /// - isResearchingHere: a Deep Research run owned by *this* conversation. + /// - isAgentRunningHere: an agent-loop run owned by *this* conversation. + static func target(isResearchingHere: Bool, isAgentRunningHere: Bool) -> AskStopTarget { + // Deep Research first: it is the longer and more expensive of the two, and when both + // somehow read as running it is the one the user is waiting on. + if isResearchingHere { + return .deepResearch + } + return isAgentRunningHere ? .agentLoop : .nothing + } +} diff --git a/Logue/Agent/DeepResearch/DeepResearchCoordinator.swift b/Logue/Agent/DeepResearch/DeepResearchCoordinator.swift index 4b5c131..84fd793 100644 --- a/Logue/Agent/DeepResearch/DeepResearchCoordinator.swift +++ b/Logue/Agent/DeepResearch/DeepResearchCoordinator.swift @@ -26,6 +26,27 @@ final class DeepResearchCoordinator { // MARK: - State private(set) var isRunning: Bool = false + /// The conversation this run belongs to, and it outlives the run. + /// + /// Deep Research is one-at-a-time app-wide, which made a single global `isRunning` look + /// sufficient while only the main window could start a run. The island can now start one + /// too, and a global flag means a run started in one window paints its progress — and + /// then its clarifying questions and its error — onto the other's thread. Same rule as + /// `AgentRunState`: live run state belongs to a conversation, not to the app, and the + /// owner survives the run because `lastError` and `clarifyingQuestions` do. + private(set) var runningConversationID: UUID? + + /// Which run the coordinator's state currently describes. + /// + /// `cancel()` releases `isRunning` synchronously, but the cancelled task does not unwind + /// until whatever it is awaiting returns — and MLX generation is not preemptible, so that + /// can be tens of seconds. In that window a second run legitimately starts, and then the + /// first one finally throws `CancellationError` and writes *its* terminal state over the + /// live run's: the strip reads "Cancelled", `isRunning` goes false while the second run is + /// still working, its web-tool override is cleared underneath it, and nothing can stop it. + /// + /// Every write from inside `execute` is therefore gated on still owning this. + private var runGeneration = 0 private(set) var currentStep: DeepResearchStep = .idle private(set) var sections: [ResearchSection] = [] private(set) var currentSectionIdx: Int = 0 @@ -48,31 +69,80 @@ final class DeepResearchCoordinator { private init() {} + // MARK: - Scoped reads + + /// Whether *this* conversation has a run in flight. + func isRunning(in conversationID: UUID) -> Bool { + isRunning && runningConversationID == conversationID + } + + /// Whether this conversation has anything to show — a run, questions it came back with, + /// or how it failed. The progress panel is mounted on this rather than on `isRunning`, so + /// a finished run's questions stay readable on the surface that asked for them. + func hasActivity(in conversationID: UUID) -> Bool { + guard runningConversationID == conversationID else { return false } + return isRunning + || currentStep == .failed + || !clarifyingQuestions.isEmpty + || lastError != nil + } + // MARK: - Public API + /// Puts the question in the conversation and starts a run on it. + /// + /// `run` expects the user message to be there already, which left each surface appending + /// it themselves — and the island, arriving second, would have had to rediscover that + /// contract by finding a run whose question was missing from the thread. #61's rule is + /// that both surfaces mount the same behaviour, so starting a run is one call. + /// + /// - Returns: the id of the appended question, so a caller that scrolls can scroll to it. + /// - Returns: the id of the appended question, or `nil` when a run was already in flight + /// and this one was refused. + /// + /// The refusal is checked *before* the question is appended. It used to append first and + /// let `run` drop the request, which put the user's question in the thread with nothing + /// that would ever answer it: no spinner (run state is per conversation), no progress + /// strip (it belongs to the other conversation), no error, and the composer already + /// cleared. Returning `nil` is what lets a caller say so instead. + @discardableResult + func start(prompt: String, in conversationID: UUID, oneShotWebSearch: Bool = false) -> UUID? { + guard !isRunning else { return nil } + let question = AgentMessage(role: .user, content: prompt) + AgentConversationStore.shared.appendMessage(question, to: conversationID) + run(prompt: prompt, conversationID: conversationID, oneShotWebSearch: oneShotWebSearch) + return question.id + } + /// Kicks off a Deep Research run for `prompt` and posts the final report to /// `conversationID`. The user message is expected to already be in the - /// conversation (the chat view appends it before calling). + /// conversation — prefer `start(prompt:in:oneShotWebSearch:)`, which does both. func run(prompt: String, conversationID: UUID, oneShotWebSearch: Bool = false) { guard !isRunning else { return } task?.cancel() resetState() isRunning = true + runningConversationID = conversationID // Mirror the per-send override into AgentCoordinator's tool registry so // `constrainedToolSpecs()` (which reads from there) sees web tools for // this run only. Cleanup happens in `execute()`'s defer block. if oneShotWebSearch { AgentCoordinator.shared.setOneShotIncludeWebTools(true) } + runGeneration += 1 + let generation = runGeneration task = Task { [weak self] in guard let self else { return } - await execute(prompt: prompt, conversationID: conversationID) + await execute(prompt: prompt, conversationID: conversationID, generation: generation) } } func cancel() { task?.cancel() task = nil + // The cancelled task may still be unwinding; bumping the generation makes anything + // it writes afterwards a no-op, including its own "Cancelled." bookkeeping. + runGeneration += 1 if isRunning { currentStep = .failed lastError = "Cancelled." @@ -85,18 +155,24 @@ final class DeepResearchCoordinator { func dismiss() { guard !isRunning else { return } resetState() + runningConversationID = nil } // MARK: - Pipeline // swiftlint:disable:next function_body_length - private func execute(prompt: String, conversationID: UUID) async { + private func execute(prompt: String, conversationID: UUID, generation: Int) async { defer { - isRunning = false - // Match the AgentCoordinator cleanup contract — clear any per-run - // web-search override regardless of success/error/cancellation. - if AgentCoordinator.shared.oneShotIncludeWebTools { - AgentCoordinator.shared.setOneShotIncludeWebTools(false) + // Only if this is still the run the coordinator is describing. A cancelled run + // unwinding late must not release a *newer* run's `isRunning`, nor strip its + // web-tool override. + if generation == runGeneration { + isRunning = false + // Match the AgentCoordinator cleanup contract — clear any per-run + // web-search override regardless of success/error/cancellation. + if AgentCoordinator.shared.oneShotIncludeWebTools { + AgentCoordinator.shared.setOneShotIncludeWebTools(false) + } } } @@ -110,6 +186,7 @@ final class DeepResearchCoordinator { case .sufficient: break case let .insufficient(questions): + guard generation == runGeneration else { return } clarifyingQuestions = questions currentStep = .failed lastError = "Need more detail." @@ -167,14 +244,18 @@ final class DeepResearchCoordinator { // posting throws (future refactor), we don't want the UI to claim // success while the conversation store has nothing. postReport(to: conversationID, report: finalReport) + guard generation == runGeneration else { return } currentStep = .completed } catch is CancellationError { - currentStep = .failed - lastError = "Cancelled." + // A cancelled run says nothing. `cancel()` already wrote the message, and by now + // a newer run may own the coordinator — repeating it here is how "Cancelled." + // landed on top of a run that was still going. + return } catch { + logger.error("Deep Research failed: \(error.localizedDescription, privacy: .public)") + guard generation == runGeneration else { return } currentStep = .failed lastError = error.localizedDescription - logger.error("Deep Research failed: \(error.localizedDescription, privacy: .public)") postFailure(to: conversationID, error: error.localizedDescription) } } diff --git a/Logue/App/AppConstants.swift b/Logue/App/AppConstants.swift index 20df2c2..6549950 100644 --- a/Logue/App/AppConstants.swift +++ b/Logue/App/AppConstants.swift @@ -71,6 +71,15 @@ enum AppConstants { static let oneShotWebSearch = "agent.oneShotWebSearch" /// Same idea for the per-send Deep Research toggle. static let oneShotDeepResearch = "agent.oneShotDeepResearch" + /// The island's own one-shot modes. + /// + /// Deliberately *not* the two keys above. The island clears its flags after every + /// send, so sharing the main window's keys meant a quick question here disarmed a + /// chip the user had armed over there and left on an unsent prompt — it then ran + /// without web tools and never said so. Separate keys keep the `@AppStorage` + /// binding a SwiftUI `Menu` needs without sharing the value. + static let islandOneShotWebSearch = "island.oneShotWebSearch" + static let islandOneShotDeepResearch = "island.oneShotDeepResearch" /// Optional override for the agent system prompt. Empty string = use the /// built-in default in `PromptRegistry.Agent`. static let agentSystemPromptOverride = "agent.systemPromptOverride" diff --git a/Logue/App/AppDelegate.swift b/Logue/App/AppDelegate.swift index f9e99ba..f498fe8 100644 --- a/Logue/App/AppDelegate.swift +++ b/Logue/App/AppDelegate.swift @@ -650,8 +650,20 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent } } + /// Opens Settings on the AI tab, from either composer's `+` menu. + /// + /// Goes through `activateApp` because the island floats over another application: + /// posting the notification alone opens the Settings window *behind* whatever is + /// frontmost, which reads as the menu item doing nothing. From the main window the + /// activation is a no-op. + @objc + static func openToolSettings() { + SettingsNavigator.shared.pendingTab = .ai + (NSApp.delegate as? AppDelegate)?.showSettings() + } + @objc - private func showSettings() { + fileprivate func showSettings() { activateApp { NotificationCenter.default.post(name: .openSettingsGeneral, object: nil) } diff --git a/Logue/CrossApp/CommandCenterChatRule.swift b/Logue/CrossApp/CommandCenterChatRule.swift index 6a506fa..254209a 100644 --- a/Logue/CrossApp/CommandCenterChatRule.swift +++ b/Logue/CrossApp/CommandCenterChatRule.swift @@ -3,10 +3,17 @@ import Foundation /// What the chat island is currently holding. /// /// The three are kept apart because they earn different protection. Deliberately -/// putting the island away — Esc, the close button, the shortcut — has always -/// destroyed an unsent prompt, and a conversation is what a click off the island -/// must not throw away. Switching apps is not a decision about the island at all, -/// so that leaves everything alone. +/// putting the island away — Esc pressed in it, the close button, the shortcut — +/// has always destroyed an unsent prompt, and a conversation is what a click off +/// the island must not throw away. +/// +/// `hasConversation` is *not* "has visible messages". The island owns a thread the +/// moment `ensureConversation()` runs, which is before the first reply exists and +/// before `AgentCoordinator.send` appends anything — that append happens inside a +/// `Task`, while the composer clears synchronously. Asking about visible rows left +/// a window, a frame or two wide, in which a freshly-sent island reported itself +/// completely empty and every "an empty island is disposable" rule was armed +/// against it. Clicking Send could then close the island. /// /// `hasAttachments` exists because staged files cost more to replace than typed /// text does. A dropped file is not recoverable by retyping it: the user found it @@ -14,37 +21,41 @@ import Foundation /// one kind of content the island could hold that nothing here knew about, which /// made an empty-looking island with a PDF staged on it disposable. struct CommandCenterChatContent: Equatable { - var hasMessages: Bool + /// A thread exists — whether or not anything is drawn in it yet. + var hasConversation: Bool var hasDraft: Bool var hasAttachments: Bool - init(hasMessages: Bool, hasDraft: Bool, hasAttachments: Bool = false) { - self.hasMessages = hasMessages + init(hasConversation: Bool, hasDraft: Bool, hasAttachments: Bool = false) { + self.hasConversation = hasConversation self.hasDraft = hasDraft self.hasAttachments = hasAttachments } var isEmpty: Bool { - !hasMessages && !hasDraft && !hasAttachments + !hasConversation && !hasDraft && !hasAttachments } } -/// What a Command Center trigger, or Logue losing frontmost status, means for the -/// chat island. +/// What a Command Center trigger, a click, an Esc, or Logue losing frontmost +/// status means for the chat island. /// /// Kept free of AppKit so the matrix is testable without an NSPanel: the controller /// owns the panel and only asks what to do with it. Issue #46 was several wrong /// answers to these questions, none of which a test could reach while they lived /// inside the window code. +/// +/// The dividing line every rule here draws is **deliberate versus incidental**. The +/// close button, the shortcut, and Esc pressed inside the island are someone saying +/// "put this away" — they always work. Clicking another window, switching apps, or +/// pressing Esc in a different app are not about the island at all, and may only +/// close one that holds nothing worth keeping. enum CommandCenterChatRule { /// What pressing the Ask Logue shortcut should do. enum Trigger: Equatable { - /// The chat island is up in front — the shortcut puts it away. + /// The chat island is up — the shortcut puts it away. Deliberate, so it + /// applies however much the island is holding. case dismiss - /// The island was sent behind other apps when Logue lost focus. The - /// shortcut is how you ask for it back, so bring it forward rather than - /// destroying it along with whatever was typed into it. - case raise /// Another island is up and has to go first. case replace /// Nothing is up. @@ -56,26 +67,56 @@ enum CommandCenterChatRule { enum FocusLoss: Equatable { /// Empty, so nothing is lost by closing it. case dismiss - /// It holds a conversation, or a prompt typed but not sent, and both live - /// only in the view's state. Keep it, but stop it covering the app being - /// switched to. - case sendBehindOtherApps + /// It holds something. Leave it exactly as it is. + /// + /// This used to be `sendBehindOtherApps`, which dropped the panel to + /// `.normal` level. That looked like a kindness and behaved like a + /// disappearance: the panel is `.nonactivatingPanel`, so clicking it never + /// re-activates Logue and never restored the level, leaving the island + /// occluded by the window the user had just clicked with no way back except + /// the shortcut. Users read that as "it closed". + case keep } - static func trigger( - mode: CommandCenterMode?, - isShowingPanel: Bool, - isChatBehindOtherApps: Bool - ) -> Trigger { + static func trigger(mode: CommandCenterMode?, isShowingPanel: Bool) -> Trigger { guard isShowingPanel else { return .present } if case .chat = mode { - return isChatBehindOtherApps ? .raise : .dismiss + return .dismiss } return .replace } static func focusLoss(mode: CommandCenterMode?, chatHasContent: Bool) -> FocusLoss? { guard case .chat = mode else { return nil } - return chatHasContent ? .sendBehindOtherApps : .dismiss + return chatHasContent ? .keep : .dismiss + } + + /// Whether a click that landed on some *other* window may close the island. + /// + /// A conversation or a staged file keeps it: neither is recoverable by the user + /// retyping something, and an island holding a thread is the thing the close + /// button exists for. + /// + /// A *draft* deliberately does not keep it. With no conversation there is no + /// close button — it is only drawn above the transcript — so protecting a draft + /// here would leave an island with no visible way out. Esc still closes it. + static func clickOff(_ content: CommandCenterChatContent) -> Bool { + !content.hasConversation && !content.hasAttachments + } + + /// Whether Esc should close the island. + /// + /// - Parameter pressedInIsland: the keystroke went to the island itself. That is + /// someone deliberately dismissing what they are looking at, so it always + /// works. + /// + /// Esc from another app is not about the island, so it may not close a + /// conversation. It *may* still close one holding only staged files, and that is + /// deliberate rather than an oversight: an island with no conversation draws no + /// close button, so refusing here as well would leave it with no keyboard exit at + /// all — the user would have to click into the field first to make the panel key. + /// Clicks still spare the files (`clickOff`); only an explicit Esc discards them. + static func escape(content: CommandCenterChatContent, pressedInIsland: Bool) -> Bool { + pressedInIsland || !content.hasConversation } } diff --git a/Logue/CrossApp/CommandCenterController.swift b/Logue/CrossApp/CommandCenterController.swift index 9c75ec5..b394aec 100644 --- a/Logue/CrossApp/CommandCenterController.swift +++ b/Logue/CrossApp/CommandCenterController.swift @@ -1,4 +1,5 @@ import Cocoa +import os.log import SwiftUI /// Panel mode for Command Center — chat or recording. @@ -73,6 +74,18 @@ private class TransparentContainerView: NSView { /// on turns the transparent region into a dead zone that swallows them. var claimsEmptyAreaClicks: (() -> Bool)? + /// The island's own drawn bounds, in this view's coordinates. + /// + /// "Empty area" used to mean "no SwiftUI subview claimed this point", which is + /// not the same thing at all: the pill's background, its padding, and the gap + /// between the transcript and the prompt bar are all inside the island and are + /// claimed by nothing. Clicking any of them put the island away, which is the + /// reported bug — a click *on* the island closing it. + /// + /// A click inside these bounds belongs to the island whether or not a control + /// wanted it. Only clicks outside them are clicks off the island. + var islandBounds: (() -> NSRect)? + override func draw(_ dirtyRect: NSRect) { // Fully transparent — do not draw anything. } @@ -93,15 +106,37 @@ private class TransparentContainerView: NSView { return hit } } - // No subview hit — claim it only if we will act on it, otherwise return nil - // to let the click pass through entirely. + // Inside the island but claimed by no control — its background, its padding, + // the gap between the transcript and the pill. That is still the island, so + // take the click and do nothing with it rather than letting it fall through + // to the app behind (which would activate that app and bury us). + if isInsideIsland(localPoint) { + return self + } + // Genuinely off the island: claim it only if we will act on it, otherwise + // return nil to let the click pass through entirely. return claimsEmptyAreaClicks?() == true ? self : nil } override func mouseDown(with event: NSEvent) { - // Only called when hitTest returned self (empty area click). + // Reached for two different clicks — one on the island's own inert area, one + // beside it — and only the second is a decision about the island. + let localPoint = convert(event.locationInWindow, from: nil) + guard !isInsideIsland(localPoint) else { + // Still hand it up the responder chain: `CommandCenterPanel.mouseDown` + // makes the panel key, and swallowing this meant clicking the island's own + // background no longer focused it — which silently disabled Esc, since the + // local monitor only acts while the panel is key. + super.mouseDown(with: event) + return + } onClickEmptyArea?() } + + private func isInsideIsland(_ localPoint: NSPoint) -> Bool { + guard let islandBounds else { return false } + return islandBounds().contains(localPoint) + } } /// Manages the Command Center panels — bottom-center chat island @@ -118,13 +153,14 @@ class CommandCenterController: ObservableObject { private var escMonitor: Any? private var escLocalMonitor: Any? private var clickLocalMonitor: Any? - private var chatContent = CommandCenterChatContent(hasMessages: false, hasDraft: false, hasAttachments: false) + private var chatContent = CommandCenterChatContent(hasConversation: false, hasDraft: false, hasAttachments: false) /// The meeting ID of the active recording panel, used to restore the island. private(set) var activeRecordingMeetingID: UUID? /// Whether the panel was hidden because the Logue main window became active. private var hiddenForMainWindow: Bool = false private var appActiveObserver: NSObjectProtocol? + private static let logger = Logger(subsystem: AppConstants.bundleID, category: "CommandCenter") private var appResignObserver: NSObjectProtocol? /// Suppresses the next app-became-active hide (used during panel creation). private var suppressNextActiveHide: Bool = false @@ -134,43 +170,25 @@ class CommandCenterController: ObservableObject { /// Summons the chat island, or dismisses it when it is already up, so the /// activation shortcut is the same key that puts it away again. func toggleChatPanel() { - switch CommandCenterChatRule.trigger( - mode: currentMode, - isShowingPanel: panel != nil, - isChatBehindOtherApps: panel?.level == .normal - ) { + switch CommandCenterChatRule.trigger(mode: currentMode, isShowingPanel: panel != nil) { case .dismiss: - dismissPanel() - return - case .raise: - raiseChatPanel() + dismissPanel(reason: .hotkey) return case .replace: - dismissPanel() + dismissPanel(reason: .replacedByRecording) case .present: break } currentMode = .chat - chatContent = CommandCenterChatContent(hasMessages: false, hasDraft: false) + chatContent = CommandCenterChatContent(hasConversation: false, hasDraft: false) createPanel(mode: .chat) setupAppActiveObservers() } - /// Brings an island that was sent behind other apps back to the front, with - /// whatever was typed into it still there. Reached from the shortcut while - /// another app is frontmost, where nothing else would re-promote it. - private func raiseChatPanel() { - guard let panel else { return } - panel.level = .floating - panel.orderFrontRegardless() - panel.makeKey() - NSApp.activate(ignoringOtherApps: true) - } - func showRecordingPanel(meetingID: UUID) { hiddenForMainWindow = false if panel != nil { - dismissPanel() + dismissPanel(reason: .replacedByRecording) } activeRecordingMeetingID = meetingID currentMode = .recording(meetingID: meetingID) @@ -221,8 +239,9 @@ class CommandCenterController: ObservableObject { } private func handleAppBecameActive() { + // The chat island keeps its level for its whole life now, so there is + // nothing to restore — it is simply left alone. if case .chat = currentMode { - panel?.level = .floating return } guard case .recording = currentMode else { return } @@ -243,10 +262,9 @@ class CommandCenterController: ObservableObject { private func handleAppResignedActive() { switch CommandCenterChatRule.focusLoss(mode: currentMode, chatHasContent: !chatContent.isEmpty) { case .dismiss: - dismissPanel() + dismissPanel(reason: .focusLoss) return - case .sendBehindOtherApps: - panel?.level = .normal + case .keep: return case nil: break @@ -276,7 +294,9 @@ class CommandCenterController: ObservableObject { switch mode { case .chat: let chatView = CommandCenterChatView( - onDismiss: { [weak self] in self?.dismissPanel() }, + onDismiss: { [weak self] dismissal in + self?.dismissPanel(reason: dismissal == .openInLogue ? .openInLogue : .closeButton) + }, onContentChanged: { [weak self] content in self?.chatContent = content } ) let hv = TransparentHostingView(rootView: chatView) @@ -303,8 +323,18 @@ class CommandCenterController: ObservableObject { hv.topAnchor.constraint(greaterThanOrEqualTo: container.topAnchor), ]) - container.claimsEmptyAreaClicks = { [weak self] in self?.chatContent.hasMessages == false } - container.onClickEmptyArea = { [weak self] in self?.dismissPanel() } + // The same rule the click monitor uses, so an island holding a staged file + // is not torn down by a click on the transparent area while surviving one + // on another window. + container.claimsEmptyAreaClicks = { [weak self] in + guard let self else { return false } + return CommandCenterChatRule.clickOff(chatContent) + } + // What counts as "on the island" — the hosting view is pinned to the bottom + // and is only as tall as the SwiftUI content, so its frame is exactly the + // island's drawn bounds. + container.islandBounds = { [weak hv] in hv?.frame ?? .zero } + container.onClickEmptyArea = { [weak self] in self?.dismissPanel(reason: .emptyAreaClick) } hostingView = container panelWidth = 740 @@ -314,7 +344,7 @@ class CommandCenterController: ObservableObject { let recordingView = CommandCenterRecordingView( recorder: RecordingSessionManager.shared, activeMeetingID: meetingID, - onDismiss: { [weak self] in self?.dismissPanel() }, + onDismiss: { [weak self] in self?.dismissPanel(reason: .recordingDismissed) }, onStop: { [weak self] in self?.dismissRecordingWithCollapse() }, onOpenInApp: { [weak self] in self?.openMeetingInApp(meetingID) } ) @@ -429,7 +459,9 @@ class CommandCenterController: ObservableObject { private func setupMonitors(mode: CommandCenterMode) { escMonitor = NSEvent.addGlobalMonitorForEvents(matching: .keyDown) { [weak self] event in if event.keyCode == Self.escapeKeyCode { - Task { @MainActor in self?.dismissPanel() } + // Global means the keystroke went to another app, so it is not a + // decision about the island — held to the same bar as a stray click. + Task { @MainActor in self?.dismissIfEscapeApplies(pressedInIsland: false) } } } @@ -441,7 +473,9 @@ class CommandCenterController: ObservableObject { if case .chat = mode { escLocalMonitor = NSEvent.addLocalMonitorForEvents(matching: .keyDown) { [weak self] event in guard event.keyCode == Self.escapeKeyCode, let self, panel?.isKeyWindow == true else { return event } - dismissPanel() + // The island is key, so this is someone dismissing what they are + // looking at. Always honoured, however much it holds. + dismissIfEscapeApplies(pressedInIsland: true) return nil } @@ -456,37 +490,53 @@ class CommandCenterController: ObservableObject { // landed yet, so the island was torn down before the file arrived. clickLocalMonitor = NSEvent.addLocalMonitorForEvents(matching: .leftMouseDown) { [weak self] event in - let screenPoint = Self.screenPoint(for: event) - Task { @MainActor in self?.dismissIfClickMissedPanel(screenPoint) } + // Reduced to two Sendable facts here rather than carrying the `NSWindow` + // across the actor hop, which `NSWindow` is not. + let clicked = event.window.map(ObjectIdentifier.init) + let isPanel = event.window is NSPanel + Task { @MainActor in + self?.dismissIfClickLandedElsewhere(windowID: clicked, isPanel: isPanel) + } return event } } } - /// Where a click landed, in screen coordinates. Events from a global monitor - /// carry no window and are already in screen space. - private static func screenPoint(for event: NSEvent) -> NSPoint { - guard let window = event.window else { return event.locationInWindow } - return window.convertPoint(toScreen: event.locationInWindow) - } - - /// Puts a chat island away when a click inside Logue lands anywhere but on it. + /// Puts an empty chat island away when a click lands on one of Logue's *other* + /// windows. What may be closed is `CommandCenterChatRule.clickOff`. /// - /// A conversation or a staged file keeps it. A *draft* deliberately does not — - /// with no messages there is no close button, so protecting the draft here would - /// leave an island with no visible way out. `CommandCenterChatRuleTests` states - /// that reasoning; the doc comment used to claim the opposite, and the code was - /// right. + /// Decided by window identity rather than by hit-testing a screen point against + /// `panel.frame`, which is what this used to do and got wrong twice over. The + /// point came from `event.locationInWindow` converted with the event's own + /// window — and when that window was nil the code fell back to using + /// *window-local* coordinates as though they were screen coordinates, so a click + /// on the island itself could read as a click somewhere else and close it. The + /// same test also counted clicks inside the attach picker as clicks elsewhere: + /// `NSOpenPanel.begin` is non-modal and, because Logue is unsandboxed, runs in + /// this process, so choosing a file tore down the very island it was for. /// - /// Attachments are the exception because they are not recoverable by retyping: - /// the user found that file in Finder once and would have to find it again. - private func dismissIfClickMissedPanel(_ screenPoint: NSPoint) { - guard let panel, - !chatContent.hasMessages, - !chatContent.hasAttachments, - !panel.frame.contains(screenPoint) - else { return } - dismissPanel() + /// Panels are excluded wholesale. Every panel Logue puts on screen — the picker, + /// the toast, the island itself — is either ours or transient, and none of them + /// is a user saying "I am done with the island". + private func dismissIfClickLandedElsewhere(windowID: ObjectIdentifier?, isPanel: Bool) { + guard case .chat = currentMode, let panel, let windowID, !isPanel else { return } + guard windowID != ObjectIdentifier(panel) else { return } + guard CommandCenterChatRule.clickOff(chatContent) else { return } + dismissPanel(reason: .clickOnOtherWindow) + } + + /// Handles Esc from either monitor. `pressedInIsland` is what separates a + /// deliberate dismissal from a keystroke that happened elsewhere. + private func dismissIfEscapeApplies(pressedInIsland: Bool) { + // The global Esc monitor is installed for both modes, and `chatContent` is a chat + // fact. Recording keeps the unconditional dismissal it has always had. + guard case .chat = currentMode else { + dismissPanel(reason: .escape(inIsland: pressedInIsland)) + return + } + guard panel != nil else { return } + guard CommandCenterChatRule.escape(content: chatContent, pressedInIsland: pressedInIsland) else { return } + dismissPanel(reason: .escape(inIsland: pressedInIsland)) } // MARK: - Dismiss @@ -522,7 +572,7 @@ class CommandCenterController: ObservableObject { panel = nil isVisible = false currentMode = nil - chatContent = CommandCenterChatContent(hasMessages: false, hasDraft: false) + chatContent = CommandCenterChatContent(hasConversation: false, hasDraft: false) if !keepRecordingState { activeRecordingMeetingID = nil tearDownAppActiveObservers() @@ -530,7 +580,34 @@ class CommandCenterController: ObservableObject { return dismissed } - func dismissPanel() { + /// Why an island is being put away. + /// + /// Recorded on every dismissal because the island vanishing for the wrong reason + /// is invisible after the fact — the panel is gone either way — and several paths + /// can do it. With this, the log says which one fired. + enum DismissReason: Equatable { + /// The X in the island's header. + case closeButton + /// "Open in Logue" carried the thread to the main window. + case openInLogue + /// The Ask Logue shortcut, pressed while the island was up. + case hotkey + /// A recording island took its place. + case replacedByRecording + /// The recording island's own close button. + case recordingDismissed + /// Esc. `inIsland` is false when the keystroke went to another app. + case escape(inIsland: Bool) + /// A click landed on another Logue window. + case clickOnOtherWindow + /// A click landed on the panel's transparent area, around the island. + case emptyAreaClick + /// Logue stopped being frontmost and the island was empty. + case focusLoss + } + + func dismissPanel(reason: DismissReason) { + Self.logger.notice("Command Center panel dismissed: \(String(describing: reason), privacy: .public)") removeMonitors() let isRecordingMode = if case .recording = currentMode { true diff --git a/Logue/CrossApp/IslandThread.swift b/Logue/CrossApp/IslandThread.swift new file mode 100644 index 0000000..c7250bf --- /dev/null +++ b/Logue/CrossApp/IslandThread.swift @@ -0,0 +1,106 @@ +import Foundation + +/// Why the island is putting itself away. +/// +/// The view knows the difference; the controller only knows a closure fired. Without this +/// the dismissal log attributed "Open in Logue" to the close button, which is the one thing +/// the log exists to tell apart. +enum CommandCenterChatDismissal: Equatable { + case closeButton + case openInLogue +} + +/// One turn as the island draws it. +/// +/// Named for what it was — a view model over messages the island kept in its own array and +/// threw away on dismiss. It now reads from the store like everything else; the name stayed +/// because renaming it touches every bubble and buys nothing. +struct EphemeralChatMessage: Identifiable, Equatable { + let id: UUID + var content: String + let isUser: Bool + var isStreaming: Bool + let timestamp: Date + + static func == (lhs: Self, rhs: Self) -> Bool { + lhs.id == rhs.id && lhs.content == rhs.content && lhs.isStreaming == rhs.isStreaming + } +} + +/// A single row in the island, which is either something someone said or something the agent +/// did. +enum IslandRow: Identifiable, Equatable { + case message(EphemeralChatMessage) + case tool(AgentToolTimeline.Entry) + + var id: UUID { + switch self { + case let .message(message): message.id + case let .tool(entry): entry.id + } + } +} + +/// What the island shows for a conversation. +/// +/// The island used to drop every tool turn on the floor, so a question that made the agent +/// search the web or open a document showed the answer and no sign of how it was reached. +/// That was defensible while the island was a bare completion call with no tools; once #61 +/// put it on the same agent loop it became the island claiming credit for work it would not +/// show — and the surface where trusting the answer matters most is the one floating over +/// someone else's window. +/// +/// Rows rather than a filtered message list because the ordering is the point: a card has to +/// sit between the question that caused it and the answer that used it. Free of SwiftUI so +/// that ordering is testable without mounting a panel. +enum IslandThread { + /// The rows for `messages`. + /// + /// - Parameter isStreaming: whether this conversation has a run in flight. Only the last + /// assistant message can be the one being written, so this marks that one and nothing + /// else — a spinner on an older bubble would claim a finished answer is still arriving. + static func rows(for messages: [AgentMessage], isStreaming: Bool) -> [IslandRow] { + let lastIndex = messages.count - 1 + return messages.enumerated().flatMap { index, message -> [IslandRow] in + switch message.role { + case .user: + return [.message(chatMessage(message, isUser: true, isStreaming: false))] + + case .assistant: + let live = isStreaming && index == lastIndex + // An assistant turn that only asked for tools carries no prose. Rendering it + // draws an empty bubble above the card that explains it, so it is dropped — + // unless it is the one being written, which starts empty and fills in. + guard live || !message.content.isEmpty else { return [] } + return [.message(chatMessage(message, isUser: false, isStreaming: live))] + + case .toolCall: + return AgentToolTimeline.entries(in: message, allMessages: messages) + // A call still waiting on the user is drawn by the pinned approval strip + // above the pill, where it cannot scroll out of reach. Emitting it here + // too put two cards for one call inside a 420pt panel, only one of them + // answerable, which reads as a rendering bug. + .filter { $0.status != .needsConfirmation } + .map(IslandRow.tool) + + case .toolResult: + // Rendered inside the card for the call it answers, never as a row of its own. + return [] + } + } + } + + private static func chatMessage( + _ message: AgentMessage, + isUser: Bool, + isStreaming: Bool + ) -> EphemeralChatMessage { + EphemeralChatMessage( + id: message.id, + content: message.content, + isUser: isUser, + isStreaming: isStreaming, + timestamp: message.timestamp + ) + } +} diff --git a/Logue/UI/UICopy.swift b/Logue/UI/UICopy.swift index f526f99..43d3e5f 100644 --- a/Logue/UI/UICopy.swift +++ b/Logue/UI/UICopy.swift @@ -32,8 +32,19 @@ enum UICopy { static let voice = "Dictate" static let send = "Send" static let stop = "Stop" + /// Chip titles. Shared because the island and the main window render the same + /// `ModeChip` for the same mode, and were spelling it two ways. static let webSearch = "Search" - static let deepResearch = "Deep Research" + static let deepResearch = "Deep research" + /// The `+` menu's items. One definition, because both composers mount the same + /// menu and two copies of a label is how the surfaces start naming one action + /// two different things. + static let addFiles = "Add photos & files" + static let searchTheWeb = "Search the web" + static let deepResearchMenu = "Deep research" + static let toolSettings = "Tool settings…" + static let composerMenuHelp = "Attach, search, deep research…" + static let composerMenuLabel = "More actions" static let reasoning = "Reasoning" static let footerPrivacy = "Local model · Offline-capable · No data leaves this Mac" static let charLimitNear = "Approaching context limit" diff --git a/Logue/Views/Agent/AgentChatView+Input.swift b/Logue/Views/Agent/AgentChatView+Input.swift index d9fd7e8..105d8db 100644 --- a/Logue/Views/Agent/AgentChatView+Input.swift +++ b/Logue/Views/Agent/AgentChatView+Input.swift @@ -25,12 +25,11 @@ extension AgentChatView { /// re-entrancy the counter exists to rule out. let focusRequest: Int @Binding var attachments: [TempAttachment] - /// Per-send flags use @AppStorage instead of @Binding because - /// SwiftUI's Menu silently drops Button.action closures on macOS — the - /// only reliable way to flip these from inside a Menu is to bind a - /// Toggle to a UserDefault (AppStorage). The AgentCoordinator and the - /// parent's onSend handler also read these defaults, so all three - /// surfaces (Menu, chip, send) see the same value. + /// Per-send flags bind `@AppStorage` rather than `@Binding`, which is + /// load-bearing rather than incidental — `ComposerPlusMenu` explains what macOS + /// does to a `Menu`'s toggles otherwise. The chip and the send path read the same + /// key, so all three agree; the coordinator does not read it at all, it is handed + /// the value at send time. @AppStorage(AppConstants.UserDefaultsKeys.oneShotWebSearch) var isWebSearchOnce: Bool = false @AppStorage(AppConstants.UserDefaultsKeys.oneShotDeepResearch) @@ -158,7 +157,7 @@ extension AgentChatView { HStack(spacing: 6) { if isWebSearchOnce { ModeChip( - title: "Search", + title: UICopy.Input.webSearch, systemImage: "globe", tint: AppThemeConstants.brandPrimary ) { @@ -167,7 +166,7 @@ extension AgentChatView { } if isDeepResearch { ModeChip( - title: "Deep research", + title: UICopy.Input.deepResearch, systemImage: "sparkle.magnifyingglass", tint: AppThemeConstants.brandPrimary ) { @@ -199,64 +198,17 @@ extension AgentChatView { ) } - // MARK: - + Menu (ChatGPT-style) + // MARK: - + Menu - /// Single + button on the left of the pill. Opens a popup with all the - /// per-send toggles and quick actions — collapses paperclip, web - /// search, deep research, image generation, expert activation, and - /// reasoning visibility into one discoverable surface. + /// Single + button on the left of the pill. The menu itself is + /// `ComposerPlusMenu`, mounted by the island too — see its note on why the + /// per-send flags are passed as keys rather than bindings. private var plusMenuButton: some View { - Menu { - Button { - openFilePicker() - } label: { - Label("Add photos & files", systemImage: "paperclip") - } - .keyboardShortcut("u", modifiers: .command) - - // Set explicit values (not `.toggle()`) — on macOS the - // Menu's deferred close pipeline can swallow a Bool.toggle() - // call against an `@Binding`. Hand-write the new value AND - // mirror it to a UserDefault that the AgentCoordinator reads - // at send time, so the agent sees the toggle even if a - // late-propagating Menu binding loses the race. - // Toggles inside a Menu render as native NSMenuItems with a - // checkmark when on. Binding straight to @AppStorage (instead - // of an @Binding) sidesteps the Menu close pipeline - // race that drops Button.action closures on macOS. - Toggle(isOn: $isWebSearchOnce) { - Label("Search the web", systemImage: "globe") - } - Toggle(isOn: $isDeepResearch) { - Label("Deep research", systemImage: "sparkle.magnifyingglass") - } - - Divider() - - Button { - SettingsNavigator.shared.pendingTab = .ai - NotificationCenter.default.post(name: .openSettingsGeneral, object: nil) - } label: { - Label("Tool settings…", systemImage: "slider.horizontal.3") - } - } label: { - Image(systemName: "plus") - .font(.system(size: 14, weight: .medium)) - .foregroundStyle(.primary) - .frame(width: 28, height: 28) - .background( - Circle().fill(Color.secondary.opacity(0.12)) - ) - .overlay( - Circle().strokeBorder(Color.primary.opacity(0.10), lineWidth: 0.5) - ) - .contentShape(Circle()) - } - .menuStyle(.borderlessButton) - .menuIndicator(.hidden) - .fixedSize() - .help("Attach, search, deep research…") - .disabled(isProcessing || isBusy) + ComposerPlusMenu( + surface: .mainWindow, + isDisabled: isProcessing || isBusy, + onAttach: { openFilePicker() } + ) } private var micButton: some View { diff --git a/Logue/Views/Agent/AgentChatView+Messages.swift b/Logue/Views/Agent/AgentChatView+Messages.swift index 588f0d9..36b25c3 100644 --- a/Logue/Views/Agent/AgentChatView+Messages.swift +++ b/Logue/Views/Agent/AgentChatView+Messages.swift @@ -313,26 +313,10 @@ extension AgentChatView { private func toolCallView(_ message: AgentMessage) -> some View { VStack(alignment: .leading, spacing: 4) { - ForEach(message.toolCalls) { call in - // Find matching result in subsequent messages - let result = findResult(for: call.id, in: messages) - // If the stored status is .needsConfirmation but a result has arrived, the - // approval was resolved elsewhere — treat it as completed/failed. - let displayStatus: AgentToolCallStatus = { - if let result { - return result.isError ? .failed : .completed - } - return call.status - }() + ForEach(AgentToolTimeline.entries(in: message, allMessages: messages)) { entry in ToolExecutionCard( - toolCall: AgentToolCall( - id: call.id, - toolName: call.toolName, - arguments: call.arguments, - status: displayStatus, - clearance: call.clearance - ), - result: result, + toolCall: entry.call, + result: entry.result, conversationID: conversationID ) } @@ -345,15 +329,5 @@ extension AgentChatView { // so we render them as invisible to avoid duplication EmptyView() } - - /// Finds the tool result matching a tool call ID from subsequent messages. - private func findResult(for toolCallID: UUID, in messages: [AgentMessage]) -> AgentToolResult? { - for message in messages where message.role == .toolResult { - if let result = message.toolResult, result.toolCallID == toolCallID { - return result - } - } - return nil - } } } diff --git a/Logue/Views/Agent/AgentChatView.swift b/Logue/Views/Agent/AgentChatView.swift index bd0a817..53cbd20 100644 --- a/Logue/Views/Agent/AgentChatView.swift +++ b/Logue/Views/Agent/AgentChatView.swift @@ -300,7 +300,7 @@ struct AgentChatView: View { .transition(.move(edge: .bottom).combined(with: .opacity)) } - DeepResearchProgressView() + DeepResearchProgressView(conversationID: conversation.id) inputBar .matchedGeometryEffect(id: "inputBar", in: inputBarNamespace) @@ -318,7 +318,7 @@ struct AgentChatView: View { inputText: $inputText, focusRequest: focusRequest, attachments: $inputAttachments, - isProcessing: isThisConversationProcessing || deepResearchCoordinator.isRunning, + isProcessing: isThisConversationProcessing || isThisConversationResearching, isBusy: isBusy, onSend: { let text = inputText.trimmingCharacters(in: .whitespacesAndNewlines) @@ -359,7 +359,11 @@ struct AgentChatView: View { } }, onCancel: { - if deepResearchCoordinator.isRunning { + // Same decision the island makes, from the same place. + if AskStopTarget.target( + isResearchingHere: isThisConversationResearching, + isAgentRunningHere: isThisConversationProcessing + ) == .deepResearch { deepResearchCoordinator.cancel() } else { coordinator.cancel() @@ -426,6 +430,13 @@ struct AgentChatView: View { return coordinator.isProcessing(in: id) } + /// Scoped like `isThisConversationProcessing`: a Deep Research run started from the island + /// must not make this window's input bar look busy. + private var isThisConversationResearching: Bool { + guard let id = activeConversation?.id else { return false } + return deepResearchCoordinator.isRunning(in: id) + } + private var isThisConversationStreaming: Bool { guard let id = activeConversation?.id else { return false } return coordinator.isStreaming(in: id) @@ -536,15 +547,22 @@ struct AgentChatView: View { private func startDeepResearch(_ text: String, oneShotWebSearch: Bool = false) { HapticFeedback.send() let conversationID = ensureActiveConversation() - let userMsg = AgentMessage(role: .user, content: text) - AgentConversationStore.shared.appendMessage(userMsg, to: conversationID) - scrollTargetID = userMsg.id - scrollToTopTrigger += 1 - deepResearchCoordinator.run( + guard let questionID = deepResearchCoordinator.start( prompt: text, - conversationID: conversationID, + in: conversationID, oneShotWebSearch: oneShotWebSearch ) + else { + // A run is already in flight, and since #74 scoped the indicators it may belong to + // the island — in which case nothing on this window would have said so. Put the + // question back rather than dropping it. + inputText = text + isDeepResearch = true + ToastCenter.shared.show(UICopy.Status.busyElsewhere, kind: .warning) + return + } + scrollTargetID = questionID + scrollToTopTrigger += 1 } private func regenerateFromEditedMessage(messageID: UUID, newContent: String, conversationID: UUID) { diff --git a/Logue/Views/Agent/ComposerPlusMenu.swift b/Logue/Views/Agent/ComposerPlusMenu.swift new file mode 100644 index 0000000..6336ec7 --- /dev/null +++ b/Logue/Views/Agent/ComposerPlusMenu.swift @@ -0,0 +1,123 @@ +import SwiftUI + +/// The `+` at the left of a composer: attach a file, arm a per-send mode, or open the +/// tool settings. +/// +/// Lifted out of `InputBarView` so the Command Center island offers the same menu rather +/// than a row of its own icon buttons. Before this the island spelled three of these +/// items out as separate glyphs and had no way to reach tool settings at all — the one +/// item on #61's list of shared composer parts that was still redrawn per surface. +/// +/// **Why the flags are keys rather than bindings.** On macOS a SwiftUI `Menu` drops +/// `Button.action` closures and swallows `.toggle()` against an `@Binding` in its +/// deferred-close pipeline; binding a `Toggle` straight to `@AppStorage` is the only +/// shape that reliably survives it. So this takes *key names* and declares its own +/// storage, which lets each surface keep its own value — the island must not share the +/// main window's one-shot flags, because it clears them after every send and would +/// disarm a chip the user had set up over there. +struct ComposerPlusMenu: View { + /// Colours for the `+` glyph and its backing circle, so the main window's light card + /// and the island's dark pill can each look like themselves. + struct Style { + let foreground: Color + let background: Color + let border: Color + + static let mainWindow = Style( + foreground: .primary, + background: Color.secondary.opacity(0.12), + border: Color.primary.opacity(0.10) + ) + + /// The island is a dark HUD over another app, so `.primary` would be wrong. + static let island = Style( + foreground: .white.opacity(0.85), + background: Color.white.opacity(0.10), + border: Color.white.opacity(0.12) + ) + } + + /// Which composer this menu belongs to. + /// + /// The per-send keys are chosen from this rather than passed in, so a surface cannot be + /// wired to the other one's storage by mistake — which is the regression that made the + /// island's send disarm a chip armed in the main window. + enum Surface { + case mainWindow + case island + } + + static func keys(for surface: Surface) -> (webSearch: String, deepResearch: String) { + switch surface { + case .mainWindow: + (AppConstants.UserDefaultsKeys.oneShotWebSearch, AppConstants.UserDefaultsKeys.oneShotDeepResearch) + case .island: + ( + AppConstants.UserDefaultsKeys.islandOneShotWebSearch, + AppConstants.UserDefaultsKeys.islandOneShotDeepResearch + ) + } + } + + let isDisabled: Bool + let onAttach: () -> Void + let style: Style + + @AppStorage private var isWebSearchOnce: Bool + @AppStorage private var isDeepResearchOnce: Bool + + init( + surface: Surface, + isDisabled: Bool, + style: Style = .mainWindow, + onAttach: @escaping () -> Void + ) { + self.isDisabled = isDisabled + self.style = style + self.onAttach = onAttach + let keys = Self.keys(for: surface) + _isWebSearchOnce = AppStorage(wrappedValue: false, keys.webSearch) + _isDeepResearchOnce = AppStorage(wrappedValue: false, keys.deepResearch) + } + + var body: some View { + Menu { + Button(action: onAttach) { + Label(UICopy.Input.addFiles, systemImage: "paperclip") + } + .keyboardShortcut("u", modifiers: .command) + + // Toggles inside a Menu render as native NSMenuItems with a checkmark when + // on. See the note on this type for why they bind `@AppStorage` rather than + // a `@Binding` handed in by the caller. + Toggle(isOn: $isWebSearchOnce) { + Label(UICopy.Input.searchTheWeb, systemImage: "globe") + } + Toggle(isOn: $isDeepResearchOnce) { + Label(UICopy.Input.deepResearchMenu, systemImage: "sparkle.magnifyingglass") + } + + Divider() + + Button { + AppDelegate.openToolSettings() + } label: { + Label(UICopy.Input.toolSettings, systemImage: "slider.horizontal.3") + } + } label: { + Image(systemName: "plus") + .font(.system(size: 14, weight: .medium)) + .foregroundStyle(style.foreground) + .frame(width: 28, height: 28) + .background(Circle().fill(style.background)) + .overlay(Circle().strokeBorder(style.border, lineWidth: 0.5)) + .contentShape(Circle()) + } + .menuStyle(.borderlessButton) + .menuIndicator(.hidden) + .fixedSize() + .help(UICopy.Input.composerMenuHelp) + .disabled(isDisabled) + .accessibilityLabel(UICopy.Input.composerMenuLabel) + } +} diff --git a/Logue/Views/Agent/DeepResearchProgressView.swift b/Logue/Views/Agent/DeepResearchProgressView.swift index 3eb7b83..b3c1934 100644 --- a/Logue/Views/Agent/DeepResearchProgressView.swift +++ b/Logue/Views/Agent/DeepResearchProgressView.swift @@ -5,10 +5,27 @@ import SwiftUI /// a spinner for in-progress, and an empty circle for pending. Includes a Cancel /// button so the user isn't stranded. struct DeepResearchProgressView: View { + /// The thread this strip is reporting on. + /// + /// Read globally, the strip appeared on whatever thread happened to be on screen — which + /// only ever looked right while the main window was the sole place a run could start. + /// Both surfaces can start one now, so the strip belongs to the conversation that asked. + let conversationID: UUID? + @State private var coordinator = DeepResearchCoordinator.shared + /// Whether this thread has anything to report. + /// + /// Asks `hasActivity(in:)` rather than rebuilding the rule out of `runningConversationID` + /// and `isRunning`. Two expressions for one question meant the tests pinned a predicate + /// the view did not use, so the strip could stop appearing with every assertion green. + private var isMine: Bool { + guard let conversationID else { return false } + return coordinator.hasActivity(in: conversationID) + } + var body: some View { - if coordinator.isRunning || coordinator.currentStep == .failed { + if isMine { VStack(alignment: .leading, spacing: 10) { header stepList diff --git a/Logue/Views/Agent/ModeChip.swift b/Logue/Views/Agent/ModeChip.swift index 1d52c3c..7e35aec 100644 --- a/Logue/Views/Agent/ModeChip.swift +++ b/Logue/Views/Agent/ModeChip.swift @@ -3,9 +3,10 @@ import SwiftUI /// A mode that is on for the next send, with a way to turn it off. /// /// Lifted out of `InputBarView` so the Command Center island shows the same chip rather than -/// drawing its own. The modes are stored in `UserDefaults` and read by both surfaces *and* by -/// the coordinator, so a chip that looked different on one of them would be describing the -/// same state in two voices. +/// drawing its own. Each surface owns its own one-shot keys and hands the value to the +/// coordinator at send time, so this is the only thing keeping the two from describing the +/// same mode in two voices — which is why the titles come from `UICopy` rather than being +/// typed at each call site. struct ModeChip: View { let title: String let systemImage: String diff --git a/Logue/Views/CrossApp/CommandCenterChatView+Bubbles.swift b/Logue/Views/CrossApp/CommandCenterChatView+Bubbles.swift index 7d77c62..9033c4b 100644 --- a/Logue/Views/CrossApp/CommandCenterChatView+Bubbles.swift +++ b/Logue/Views/CrossApp/CommandCenterChatView+Bubbles.swift @@ -7,6 +7,22 @@ import Textual /// error banner. Rendering is the cohesive half: the view itself is now the panel, the prompt /// pill and the wiring to the coordinator, and this is what a message looks like. extension CommandCenterChatView { + /// One row: a turn, or a card for work the agent did between turns. + /// + /// The card is the same `ToolExecutionCard` the main window uses rather than an island + /// variant, per #61's rule that a feature is mounted by both surfaces and not drawn twice. + /// It has no `conversationID`, which is what keeps it read-only: approval belongs to the + /// strip pinned above the pill, where an answer cannot scroll out of reach. + @ViewBuilder + func islandRow(_ row: IslandRow) -> some View { + switch row { + case let .message(message): + messageBubble(message) + case let .tool(entry): + ToolExecutionCard(toolCall: entry.call, result: entry.result, conversationID: nil) + } + } + /// The panel in `CommandCenterChatView` renders this, so it crosses the file boundary. func messageBubble(_ message: EphemeralChatMessage) -> some View { HStack(alignment: .top, spacing: 0) { @@ -108,12 +124,12 @@ extension CommandCenterChatView { Button { speakMessage(message) } label: { HStack(spacing: 3) { - Image(systemName: speakingMessageID == message.id ? "stop.fill" : "speaker.wave.2") + Image(systemName: readAloud.isSpeaking(content: message.content) ? "stop.fill" : "speaker.wave.2") .font(.caption2.weight(.medium)) - Text(speakingMessageID == message.id ? "Stop" : "Speak") + Text(readAloud.isSpeaking(content: message.content) ? "Stop" : "Speak") .font(.caption2.weight(.medium)) } - .foregroundStyle(speakingMessageID == message.id ? AppThemeConstants.error : Color.secondary) + .foregroundStyle(readAloud.isSpeaking(content: message.content) ? AppThemeConstants.error : Color.secondary) .padding(.horizontal, 6) .padding(.vertical, 3) .background(Capsule().fill(Color.primary.opacity(0.04))) diff --git a/Logue/Views/CrossApp/CommandCenterChatView+Composer.swift b/Logue/Views/CrossApp/CommandCenterChatView+Composer.swift index ea58249..d257147 100644 --- a/Logue/Views/CrossApp/CommandCenterChatView+Composer.swift +++ b/Logue/Views/CrossApp/CommandCenterChatView+Composer.swift @@ -17,6 +17,22 @@ extension CommandCenterChatView { .frame(width: 28, height: 28) .clipShape(RoundedRectangle(cornerRadius: 6, style: .continuous)) + // Everything that arms or attaches, in one menu — the same one the main + // window mounts. It used to be three separate glyphs here, which is the + // per-surface redraw #61 exists to stop, and left no way to reach tool + // settings from the island at all. + ComposerPlusMenu( + surface: .island, + isDisabled: isGenerating, + style: .island, + onAttach: { + Task { @MainActor in + let picked = await AttachmentIntake.pickFiles() + attachments = AttachmentIntake.merging(picked, into: attachments) + } + } + ) + // Input field TextField("What can I help you with?", text: $inputText, axis: .vertical) .font(.body) @@ -28,44 +44,16 @@ extension CommandCenterChatView { if NSEvent.modifierFlags.contains(.shift) { inputText += "\n" return .handled - } else if canSend { + } else if canSend, !LLMEngineStatus.shared.isBusy { + // The same condition the Send button is disabled on. Without it + // Return sent while the button beside it refused to, which reads as + // the button being broken. sendMessage() return .handled } return .handled } - // Attach - Button { - Task { @MainActor in - let picked = await AttachmentIntake.pickFiles() - attachments = AttachmentIntake.merging(picked, into: attachments) - } - } label: { - Image(systemName: "paperclip") - .font(.subheadline.weight(.medium)) - .foregroundStyle(.white.opacity(0.4)) - .frame(width: 28, height: 28) - } - .buttonStyle(.plain) - .disabled(isGenerating) - .help("Attach files") - - // Web search for this turn - Button { - withAnimation(.easeOut(duration: 0.15)) { isWebSearchOnce.toggle() } - } label: { - Image(systemName: "globe") - .font(.subheadline.weight(.medium)) - .foregroundStyle( - isWebSearchOnce ? AppThemeConstants.brandPrimary : .white.opacity(0.4) - ) - .frame(width: 28, height: 28) - } - .buttonStyle(.plain) - .disabled(isGenerating) - .help(isWebSearchOnce ? "Web search is on for this message" : "Search the web for this message") - // Mic button Button { voiceManager.toggle() @@ -93,10 +81,14 @@ extension CommandCenterChatView { Button(action: sendMessage) { Image(systemName: "arrow.up") .font(.subheadline.weight(.bold)) - .foregroundStyle(canSend ? .white : .white.opacity(0.25)) + .foregroundStyle(canSend && !LLMEngineStatus.shared.isBusy ? .white : .white.opacity(0.25)) .frame(width: 32, height: 32) .background( - Circle().fill(canSend ? AppThemeConstants.brandPrimary : Color.white.opacity(0.08)) + Circle().fill( + canSend && !LLMEngineStatus.shared.isBusy + ? AppThemeConstants.brandPrimary + : Color.white.opacity(0.08) + ) ) } .buttonStyle(.plain) @@ -116,12 +108,6 @@ extension CommandCenterChatView { .stroke(Color.white.opacity(0.07), lineWidth: 0.5) ) .shadow(color: .black.opacity(0.35), radius: 30, y: 12) - .overlay(alignment: .top) { - if !attachments.isEmpty || isWebSearchOnce { - 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 @@ -135,11 +121,20 @@ extension CommandCenterChatView { } /// What is staged for the next send, with a way to take each one back off. - private var attachmentChips: some View { + var attachmentChips: some View { HStack(spacing: 6) { + if isDeepResearchOnce { + ModeChip( + title: UICopy.Input.deepResearch, + systemImage: "sparkle.magnifyingglass", + tint: AppThemeConstants.brandPrimary + ) { + isDeepResearchOnce = false + } + } if isWebSearchOnce { ModeChip( - title: "Search", + title: UICopy.Input.webSearch, systemImage: "globe", tint: AppThemeConstants.brandPrimary ) { diff --git a/Logue/Views/CrossApp/CommandCenterChatView.swift b/Logue/Views/CrossApp/CommandCenterChatView.swift index 627b3ca..9a269fe 100644 --- a/Logue/Views/CrossApp/CommandCenterChatView.swift +++ b/Logue/Views/CrossApp/CommandCenterChatView.swift @@ -1,28 +1,12 @@ -import AVFoundation import SwiftUI import Textual -/// One bubble in the island, mapped from the conversation's `AgentMessage`. -/// -/// No longer ephemeral despite the name: it is a view model over stored messages, and `id` -/// is the stored message's, so bubbles keep their identity across a re-render rather than -/// being reissued every time the mapping runs. -struct EphemeralChatMessage: Identifiable, Equatable { - let id: UUID - var content: String - let isUser: Bool - var isStreaming: Bool - let timestamp: Date - - static func == (lhs: Self, rhs: Self) -> Bool { - lhs.id == rhs.id && lhs.content == rhs.content && lhs.isStreaming == rhs.isStreaming - } -} - /// Bottom-centered chat island. /// Prompt pill at bottom, messages float above with glass backdrop. struct CommandCenterChatView: View { - let onDismiss: () -> Void + /// Puts the island away. The reason is passed so the controller's log can tell a + /// hand-off apart from someone pressing the close button. + let onDismiss: (CommandCenterChatDismissal) -> Void let onContentChanged: (CommandCenterChatContent) -> Void /// The island's own thread, created on first send and never selected. @@ -48,16 +32,40 @@ struct CommandCenterChatView: View { /// window and left armed on an unsent prompt. They would never be told — the prompt just /// runs without web tools. /// - /// The coordinator takes the value as a parameter, so nothing needs the shared read. A - /// per-send mode is per surface, and this is `@State` accordingly. - @State var isWebSearchOnce: Bool = false + /// The coordinator takes the value as a parameter, so nothing needs the shared read. + /// + /// It is `@AppStorage` on the island's *own* key rather than `@State`, because the `+` + /// menu's `Toggle` has to bind one: on macOS a SwiftUI `Menu` swallows `.toggle()` + /// against a plain `@Binding` in its deferred-close pipeline. A separate key keeps the + /// per-surface isolation that matters here while satisfying that. `.onAppear` clears it, + /// so a mode still dies with the island rather than surviving to the next launch. + @AppStorage(AppConstants.UserDefaultsKeys.islandOneShotWebSearch) + var isWebSearchOnce: Bool = false + // Extension-visible: +Composer + /// Deep Research for the next send, belonging to this island alone. + /// + /// Same story as the web-search flag above, and the same fix: this bound the key the main + /// window's Deep Research toggle binds, and the island clears it after every send — so a + /// question asked here disarmed a Deep Research run the user had set up over there. The + /// consequence is larger than the search one, because that run is the expensive one they + /// deliberately chose. Same storage story as the flag above. + @AppStorage(AppConstants.UserDefaultsKeys.islandOneShotDeepResearch) + var isDeepResearchOnce: Bool = false + // Extension-visible: +Composer + @State var deepResearch = DeepResearchCoordinator.shared // Extension-visible: +Bubbles @State var copiedMessageID: UUID? // Extension-visible: +Bubbles @State var savedMessageID: UUID? // Extension-visible: +Bubbles - @State var speakingMessageID: UUID? - @State private var synthesizer = AVSpeechSynthesizer() + /// The same service the main window reads with. + /// + /// The island used to own a bare `AVSpeechSynthesizer`, which is how the two surfaces + /// ended up sounding different: `AgentReadAloudService` strips Markdown before speaking, + /// so the main window says "hello" where the island said "asterisk asterisk hello + /// asterisk asterisk". It also tracks what is playing, which removes the polling loop the + /// island needed to notice that speech had finished. + @State var readAloud = AgentReadAloudService.shared // Extension-visible: +Composer @FocusState var isInputFocused: Bool @@ -66,34 +74,47 @@ struct CommandCenterChatView: View { .shared } - /// The island's thread, as the bubbles want it. + /// The island's thread as stored. /// /// Read from the store rather than held here. That is the whole point of the change: the /// island used to keep its own array and throw it away on dismiss, so a question asked /// here had no history, no tools and no memory. It is now the same conversation the rest /// of Logue can see. - private var messages: [EphemeralChatMessage] { + private var storedMessages: [AgentMessage] { guard let conversationID, let conversation = store.conversations.first(where: { $0.id == conversationID }) else { return [] } + return conversation.messages + } - let live = coordinator.isStreaming(in: conversationID) - return conversation.messages.enumerated().compactMap { index, message in - switch message.role { - case .user, .assistant: break - // Tool and system turns are the agent's bookkeeping. The main window renders - // them as cards; the island is a pill over someone else's app and has no room. - default: return nil - } - let isLast = index == conversation.messages.count - 1 - return EphemeralChatMessage( - id: message.id, - content: message.content, - isUser: message.role == .user, - isStreaming: live && isLast && message.role == .assistant, - timestamp: message.timestamp - ) - } + /// What the island draws: the turns, and the work the agent did between them. + private var rows: [IslandRow] { + guard let conversationID else { return [] } + return IslandThread.rows( + for: storedMessages, + isStreaming: coordinator.isStreaming(in: conversationID) + ) + } + + /// Whether there is anything in the thread at all — the pill stands alone until there is. + private var hasContent: Bool { + !rows.isEmpty + } + + /// Whether anything is staged for the next send. + private var hasStagedChips: Bool { + !attachments.isEmpty || isWebSearchOnce || isDeepResearchOnce + } + + /// Whether this island owns a thread — drawn or not. + /// + /// The close button lives in `messagesPanel`, and the rules that refuse to dismiss the + /// island read `hasConversation`. Those two must be the same question or the island + /// reaches a state where nothing incidental can close it and there is no X to press: a + /// conversation exists with no rows yet, which happens between `ensureConversation()` and + /// the first row, and persists when a send is refused. + private var hasThread: Bool { + hasContent || conversationID != nil } // Extension-visible: +Composer @@ -101,13 +122,20 @@ struct CommandCenterChatView: View { var isGenerating: Bool { guard let conversationID else { return false } return coordinator.isProcessing(in: conversationID) + || deepResearch.isRunning(in: conversationID) } private let pillWidth: CGFloat = 740 private var content: CommandCenterChatContent { CommandCenterChatContent( - hasMessages: !messages.isEmpty, + // A thread the island owns counts, whether or not anything is drawn in it + // yet. `AgentCoordinator.send` appends the user message inside a `Task` + // while this view clears the composer synchronously, so asking `hasContent` + // alone left a window in which a just-sent island reported itself empty — + // and an empty island is one every rule here is allowed to throw away. + // Clicking Send could close the island. + hasConversation: hasThread, hasDraft: !inputText.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty, hasAttachments: !attachments.isEmpty ) @@ -115,16 +143,27 @@ struct CommandCenterChatView: View { var body: some View { VStack(spacing: 0) { - if !messages.isEmpty { + if hasThread { messagesPanel .padding(.bottom, 10) .transition(.opacity.combined(with: .move(edge: .bottom))) } + // In the layout rather than floating above the pill. As an `.overlay` offset + // -34pt they sat outside the hosting view's frame on a fresh island — which the + // panel clips, and which hit-testing treats as "not the island", so a staged + // file or an armed mode showed no chip at all on the one path where you most + // need to see it. + if hasStagedChips { + attachmentChips + .frame(width: pillWidth, alignment: .leading) + .padding(.bottom, 6) + } + promptPill } .frame(width: pillWidth) - .animation(.spring(response: 0.35, dampingFraction: 0.85), value: messages.isEmpty) + .animation(.spring(response: 0.35, dampingFraction: 0.85), value: hasThread) .onAppear { isInputFocused = true voiceManager.onTranscriptReady = { transcript in @@ -141,9 +180,27 @@ struct CommandCenterChatView: View { if voiceManager.isRecording { voiceManager.stopListening() } - synthesizer.stopSpeaking(at: .immediate) + // The callback lives on a shared singleton, so leaving ours installed means a + // later dictation from the editor's chat panel is delivered into this dead + // view's state and vanishes. The two in-app panels already clear theirs. + voiceManager.onTranscriptReady = nil + stopReadAloudIfOurs() + } + .onAppear { + // The controller resets its copy when it builds the panel, so tell it what + // we actually hold before any change fires. + onContentChanged(content) + // A per-send mode belongs to the question it was set for. These live in + // `UserDefaults` only because the `+` menu's toggles need somewhere bindable, + // so clear them here — otherwise an armed Deep Research would outlive the + // island and surprise the user with an expensive run days later. + isWebSearchOnce = false + isDeepResearchOnce = false } - .onChange(of: messages.count) { _, _ in + .onChange(of: conversationID) { _, _ in + onContentChanged(content) + } + .onChange(of: rows.count) { _, _ in onContentChanged(content) } // Staged files count as content, so the controller has to be told about them — @@ -155,23 +212,6 @@ struct CommandCenterChatView: View { .onChange(of: inputText) { _, _ in onContentChanged(content) } - // U7: Replaced continuous Timer.publish with onChange-triggered task - .onChange(of: speakingMessageID) { _, newValue in - if newValue != nil { - Task { - let deadline = ContinuousClock.now + .seconds(60) - while speakingMessageID != nil, synthesizer.isSpeaking { - if ContinuousClock.now >= deadline { - break - } - try? await Task.sleep(for: AppConstants.Delays.speechSynthesisPolling) - } - if speakingMessageID != nil, !synthesizer.isSpeaking { - speakingMessageID = nil - } - } - } - } } // MARK: - Messages Panel @@ -217,7 +257,7 @@ struct CommandCenterChatView: View { .help("Continue this conversation in the main window") } - Button(action: onDismiss) { + Button { onDismiss(.closeButton) } label: { Image(systemName: "xmark") .font(.caption2.weight(.bold)) .foregroundStyle(.secondary) @@ -233,16 +273,16 @@ struct CommandCenterChatView: View { ScrollViewReader { proxy in ScrollView(.vertical, showsIndicators: false) { LazyVStack(spacing: 14) { - ForEach(messages) { message in - messageBubble(message) - .id(message.id) + ForEach(rows) { row in + islandRow(row) + .id(row.id) } } .padding(.horizontal, 20) .padding(.vertical, 12) } - .onChange(of: messages) { _, newMessages in - if let last = newMessages.last { + .onChange(of: rows) { _, newRows in + if let last = newRows.last { withAnimation(.easeOut(duration: 0.15)) { proxy.scrollTo(last.id, anchor: .bottom) } @@ -265,6 +305,11 @@ struct CommandCenterChatView: View { .transition(.move(edge: .bottom).combined(with: .opacity)) } + // Mounted, not redrawn: the island shows the same seven-step strip the main + // window does, scoped to its own thread so a run started over there stays over + // there. + DeepResearchProgressView(conversationID: conversationID) + if let error = currentError { errorBanner(error) .transition(.move(edge: .bottom).combined(with: .opacity)) @@ -294,13 +339,14 @@ struct CommandCenterChatView: View { func sendMessage() { let text = inputText.trimmingCharacters(in: .whitespacesAndNewlines) - // The same routing decision the main window makes. The island has no Deep Research - // chip and no attachments yet, so those inputs are false — but it asks the one - // router rather than carrying a second opinion that can drift from it. + // The same routing decision the main window makes, on the same inputs. Every one of + // them is now reachable from here, so the island asks the one router and gets the one + // answer rather than carrying a second opinion that can drift from it. let route = AskRouter.route( for: AskRouter.Request( text: text, hasAttachments: !attachments.isEmpty, + deepResearchRequested: isDeepResearchOnce, imageIntentFires: PromptIntentClassifier.shared.shouldPresentImagePlayground(for: text) ) ) @@ -312,7 +358,10 @@ struct CommandCenterChatView: View { // staged files and the mode flags, then hit that refusal and did nothing — no bubble, // no error, and an empty conversation left behind by `ensureConversation`. guard !coordinator.isProcessingAnyConversation else { - ToastCenter.shared.show(UICopy.Status.busyElsewhere, kind: .warning) + // Not a toast: `.toastOverlay()` is mounted only on the main window, so a toast + // raised here is shown in the window behind the pill or not at all — the same + // trap `MessageActions` documents. The island has its own banner. + localError = UICopy.Status.busyElsewhere return } @@ -321,9 +370,11 @@ struct CommandCenterChatView: View { let searchThisTurn = isWebSearchOnce inputText = "" attachments = [] - // Cleared per send, like the main window's chip. Left set, a search switched on here - // would arm the next send in the other window too. + // Cleared per send: a one-shot mode must not survive the question it was set for. + // Both flags are `@AppStorage` on the island's own keys — see their declarations + // for why they are not the main window's — and are cleared again in `.onAppear`. isWebSearchOnce = false + isDeepResearchOnce = false // Re-focus input after state update Task { @@ -332,15 +383,35 @@ struct CommandCenterChatView: View { } switch route { - case .agentLoop, .deepResearch: - // Deep Research is unreachable from here — no chip lights it — and if it ever - // becomes reachable it belongs on the same coordinator, not a second pipeline. + case .agentLoop: coordinator.send( message: text, conversationID: conversationID, attachments: staged, oneShotWebSearch: searchThisTurn ) + case .deepResearch: + // The same launch the main window uses, on the island's own thread — so the + // report lands in the conversation `Open in Logue` carries across, rather than + // somewhere the island cannot show. + // + // Staged files are deliberately not passed: the research pipeline reads from the + // library and the web, and takes no attachments on either surface. Anyone who + // dropped a file in gets it back on the pill rather than silently losing it. + attachments = staged + let started = deepResearch.start( + prompt: text, + in: conversationID, + oneShotWebSearch: searchThisTurn + ) + if started == nil { + // One run at a time, app-wide — and this one belongs to the other surface, so + // none of the island's own indicators would have said anything. Put the whole + // send back, both modes included: restoring the prompt and the Deep Research + // chip while dropping the Search one is how the next attempt runs without web + // tools and never says so. + restore(text: text, webSearch: searchThisTurn, deepResearch: true, files: staged) + } case let .imagePlayground(concept): // ImagePlayground presents a sheet, which a floating pill cannot host. Answer it // as a normal turn rather than silently dropping the send. @@ -361,19 +432,26 @@ struct CommandCenterChatView: View { /// `approvalTimeoutSeconds` (five minutes), and then failed. The run looked frozen and /// there was no way to say yes. /// - /// Only calls that need an answer are rendered. Completed tool calls stay filtered out of - /// the island — the main window shows those as history, and a pill floating over another - /// app has no room for a transcript of everything the agent did. What it must show is - /// what it is blocking on. + /// This is a *second*, pinned copy of a call that `IslandThread` also emits as a row — + /// deliberately, because an answer that scrolls out of reach is an answer the user cannot + /// give. The row copy is read-only (`conversationID: nil`); this one carries the buttons. + /// `IslandThread` drops a call while it is awaiting approval so the two are never on + /// screen at once. private var pendingApprovals: [AgentToolCall] { guard let conversationID, let conversation = store.conversations.first(where: { $0.id == conversationID }) else { return [] } - return conversation.messages - .flatMap(\.toolCalls) - .filter { $0.status == .needsConfirmation } + return AgentToolTimeline.awaitingApproval(in: conversation.messages) } + // 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? + /// 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 @@ -409,7 +487,10 @@ struct CommandCenterChatView: View { Spacer(minLength: 0) Button { - withAnimation { coordinator.dismissError() } + withAnimation { + localError = nil + coordinator.dismissError() + } } label: { Image(systemName: "xmark") .font(.caption2.weight(.semibold)) @@ -446,7 +527,7 @@ struct CommandCenterChatView: View { // thread it promised to carry over unreachable until the user opened a window by hand. AppDelegate.bringMainWindowForward() NotificationCenter.default.post(name: .chatFocusInput, object: nil) - onDismiss() + onDismiss(.openInLogue) } /// The island's thread, made on first use. @@ -465,42 +546,90 @@ struct CommandCenterChatView: View { // Extension-visible: +Composer func stopStreaming() { - coordinator.cancel() + cancelWhicheverIsRunning() isInputFocused = true } + /// Stops the pipeline that is actually answering. + /// + /// Two coordinators can be behind the Stop button now, and they do not know about each + /// other. Cancelling only the agent loop while Deep Research was the one running left the + /// run going with nothing on screen able to stop it — the button reported success and the + /// report arrived minutes later. + private func cancelWhicheverIsRunning() { + guard let conversationID else { return } + switch AskStopTarget.target( + isResearchingHere: deepResearch.isRunning(in: conversationID), + isAgentRunningHere: coordinator.isProcessing(in: conversationID) + ) { + case .deepResearch: + deepResearch.cancel() + case .agentLoop: + coordinator.cancel() + case .nothing: + // Nothing of ours is running. The `else` here used to call `coordinator.cancel()` + // unconditionally, which is unscoped — so pressing "New" on the island stopped an + // answer streaming in the main window and rejected whatever it was waiting on. + break + } + } + /// Starts a fresh thread rather than erasing one. /// /// The old island discarded its messages because they existed nowhere else. They are a /// real conversation now, so "clear" means the island stops pointing at it — deleting it /// would throw away something the user can still open from the main window. private func clearSession() { - coordinator.cancel() - synthesizer.stopSpeaking(at: .immediate) - speakingMessageID = nil + cancelWhicheverIsRunning() + stopReadAloudIfOurs() conversationID = nil inputText = "" + // A staged file belonged to the question being abandoned. Leaving it meant "New" + // handed you a fresh thread that was still holding — and about to send — the + // previous one's attachment. + attachments = [] + localError = nil isInputFocused = true } + /// Puts a refused send back exactly as it was. + /// + /// `sendMessage` clears the composer before it knows whether the send was accepted, so + /// every refusal has to undo all of it. Written once because the branch that restored + /// the prompt and one mode while dropping the other is the bug this replaced. + private func restore(text: String, webSearch: Bool, deepResearch: Bool, files: [TempAttachment]) { + inputText = text + isWebSearchOnce = webSearch + isDeepResearchOnce = deepResearch + attachments = files + localError = UICopy.Status.busyElsewhere + } + + /// Stops read-aloud only when what is playing is one of this island's answers. + /// + /// `AgentReadAloudService` is shared with the main window — that is the point of + /// mounting it rather than owning a second synthesizer — so an unconditional `stop()` + /// here cut off a reply the *main window* was reading whenever the island was dismissed + /// or cleared. Same rule as every other piece of live state: it belongs to the + /// conversation that started it. + private func stopReadAloudIfOurs() { + guard rows.contains(where: { row in + if case let .message(message) = row { + return readAloud.isSpeaking(content: message.content) + } + return false + }) + else { return } + readAloud.stop() + } + // Extension-visible: +Bubbles func speakMessage(_ message: EphemeralChatMessage) { - if speakingMessageID == message.id { - synthesizer.stopSpeaking(at: .immediate) - speakingMessageID = nil - return - } - synthesizer.stopSpeaking(at: .immediate) - let utterance = AVSpeechUtterance(string: message.content) - utterance.rate = AVSpeechUtteranceDefaultSpeechRate - if let voice = AVSpeechSynthesisVoice.speechVoices() - .filter({ $0.language.hasPrefix("en") }) - .max(by: { $0.quality.rawValue < $1.quality.rawValue }) - { - utterance.voice = voice + if readAloud.isSpeaking(content: message.content) { + readAloud.stop() + } else { + readAloud.speak(message.content) } - speakingMessageID = message.id - synthesizer.speak(utterance) } // Extension-visible: +Bubbles diff --git a/LogueTests/AgentToolTimelineTests.swift b/LogueTests/AgentToolTimelineTests.swift new file mode 100644 index 0000000..7b4588d --- /dev/null +++ b/LogueTests/AgentToolTimelineTests.swift @@ -0,0 +1,135 @@ +import Foundation +import Testing + +@testable import Logue + +/// Pairing tool calls with their results, and settling what a card should show when the +/// stored status and the result disagree. +/// +/// This was a `private func` inside the main window's message list, so none of it was +/// reachable from a test or from the island. Both of those are now false. +@Suite("AgentToolTimeline") +struct AgentToolTimelineTests { + private func call( + id: UUID = .init(), + name: String = "search_web", + status: AgentToolCallStatus = .completed + ) -> AgentToolCall { + AgentToolCall(id: id, toolName: name, arguments: "{}", status: status) + } + + private func callMessage(_ calls: [AgentToolCall]) -> AgentMessage { + AgentMessage(role: .toolCall, content: "", toolCalls: calls) + } + + private func resultMessage(for id: UUID, output: String = "ok", isError: Bool = false) -> AgentMessage { + AgentMessage( + role: .toolResult, + content: "", + toolResult: AgentToolResult(toolCallID: id, output: output, isError: isError) + ) + } + + // MARK: - Pairing + + @Test("A call is paired with the result carrying its id") + func pairsOnIdentity() { + let wanted = call() + let other = call() + let messages = [ + callMessage([wanted, other]), + resultMessage(for: other.id, output: "other"), + resultMessage(for: wanted.id, output: "mine"), + ] + #expect(AgentToolTimeline.result(for: wanted.id, in: messages)?.output == "mine") + } + + @Test("A result stored before its call is still found") + func orderingIsNotAssumed() { + // Ordering is how the conversation happens to be built, not something the store + // promises on read. Search only forwards from the call and a result that sorted + // oddly renders as a call that never finished. + let subject = call() + let messages = [resultMessage(for: subject.id), callMessage([subject])] + #expect(AgentToolTimeline.result(for: subject.id, in: messages) != nil) + } + + @Test("A call with no result yet pairs with nothing") + func unfinishedCallHasNoResult() { + let subject = call(status: .running) + #expect(AgentToolTimeline.result(for: subject.id, in: [callMessage([subject])]) == nil) + } + + // MARK: - Display status + + @Test("A result outranks the stored status") + func resultWinsOverStoredStatus() { + // The case that produced it: the user approved the call on the other surface, so the + // stored status is still `.needsConfirmation` while the tool has already run. + let subject = call(status: .needsConfirmation) + let result = AgentToolResult(toolCallID: subject.id, output: "done") + #expect(AgentToolTimeline.displayStatus(of: subject, result: result) == .completed) + } + + @Test("A failed result shows as failed, not completed") + func errorResultShowsFailed() { + let subject = call(status: .running) + let result = AgentToolResult(toolCallID: subject.id, output: "boom", isError: true) + #expect(AgentToolTimeline.displayStatus(of: subject, result: result) == .failed) + } + + @Test("With no result the stored status stands") + func storedStatusStandsWithoutAResult() { + for status: AgentToolCallStatus in [.pending, .running, .needsConfirmation, .failed] { + #expect(AgentToolTimeline.displayStatus(of: call(status: status), result: nil) == status) + } + } + + // MARK: - Awaiting approval + + @Test("A call that has already run is not awaiting approval") + func answeredCallIsNotPending() { + // The island's approval strip filtered on the stored status alone, so a call answered + // in the main window kept its Approve and Deny buttons on the island — offering a + // decision about something that had already happened. + let subject = call(status: .needsConfirmation) + let messages = [callMessage([subject]), resultMessage(for: subject.id)] + #expect(AgentToolTimeline.awaitingApproval(in: messages).isEmpty) + } + + @Test("A call still waiting is reported") + func unansweredCallIsPending() { + let subject = call(name: "delete_document", status: .needsConfirmation) + let pending = AgentToolTimeline.awaitingApproval(in: [callMessage([subject])]) + #expect(pending.map(\.toolName) == ["delete_document"]) + } + + // MARK: - Entries + + @Test("Entries keep the order the calls were made in") + func entriesAreOrdered() { + let first = call(name: "read_document") + let second = call(name: "search_web") + let messages = [callMessage([first]), callMessage([second])] + #expect(AgentToolTimeline.entries(in: messages).map(\.call.toolName) == ["read_document", "search_web"]) + } + + @Test("An entry carries the display status, not the stored one") + func entryCarriesDisplayStatus() { + let subject = call(status: .needsConfirmation) + let messages = [callMessage([subject]), resultMessage(for: subject.id, isError: true)] + let entry = AgentToolTimeline.entries(in: messages).first + #expect(entry?.status == .failed) + #expect(entry?.call.status == .failed, "the card reads the call, so it has to carry it too") + } + + @Test("A conversation with no tool calls has no entries") + func plainConversationHasNoEntries() { + let messages = [ + AgentMessage(role: .user, content: "hello"), + AgentMessage(role: .assistant, content: "hi"), + ] + #expect(AgentToolTimeline.entries(in: messages).isEmpty) + #expect(AgentToolTimeline.awaitingApproval(in: messages).isEmpty) + } +} diff --git a/LogueTests/AskRouteTests.swift b/LogueTests/AskRouteTests.swift index c39ba40..cbf6fbc 100644 --- a/LogueTests/AskRouteTests.swift +++ b/LogueTests/AskRouteTests.swift @@ -126,4 +126,38 @@ struct AskRouteTests { let request = AskRouter.Request(text: "summarise my last meeting") #expect(AskRouter.route(for: request) == .agentLoop) } + + // MARK: - Deep Research needs a question + + @Test("The Deep Research chip alone is not a question") + func deepResearchNeedsText() { + // A send carrying only attachments is valid, so it passes the empty check — and + // routing that to Deep Research appended an empty user bubble and ran the whole + // seven-step pipeline on "". The files are handed back either way, because research + // takes no attachments on either surface. + let route = AskRouter.route( + for: AskRouter.Request(text: " ", hasAttachments: true, deepResearchRequested: true) + ) + #expect(route == .agentLoop) + } + + @Test("Deep Research with a question still routes to Deep Research") + func deepResearchWithTextIsUnchanged() { + let route = AskRouter.route( + for: AskRouter.Request(text: "compare A and B", deepResearchRequested: true) + ) + #expect(route == .deepResearch) + } + + @Test("Deep Research with a question and attachments still routes to Deep Research") + func deepResearchWithTextAndFiles() { + let route = AskRouter.route( + for: AskRouter.Request( + text: "compare these", + hasAttachments: true, + deepResearchRequested: true + ) + ) + #expect(route == .deepResearch) + } } diff --git a/LogueTests/AskStopTargetTests.swift b/LogueTests/AskStopTargetTests.swift new file mode 100644 index 0000000..e26029b --- /dev/null +++ b/LogueTests/AskStopTargetTests.swift @@ -0,0 +1,37 @@ +import Foundation +import Testing + +@testable import Logue + +/// What the Stop button stops. +/// +/// The decision used to live as a `private func` inside a `View`, so nothing could test it — +/// and its fallback branch reached into another surface's run. +@Suite("AskStopTarget") +struct AskStopTargetTests { + @Test("Stop stops the research run this thread owns") + func researchWins() { + #expect(AskStopTarget.target(isResearchingHere: true, isAgentRunningHere: false) == .deepResearch) + } + + @Test("Stop stops the agent run this thread owns") + func agentLoopIsStopped() { + #expect(AskStopTarget.target(isResearchingHere: false, isAgentRunningHere: true) == .agentLoop) + } + + @Test("With nothing of ours running, Stop stops nothing") + func idleStopsNothing() { + // The bug this exists to prevent: the island's fallback called + // AgentCoordinator.cancel() unconditionally, and that is unscoped — it kills the one + // global task and rejects every pending approval. Pressing "New" on an idle island + // stopped an answer streaming in the main window. + #expect(AskStopTarget.target(isResearchingHere: false, isAgentRunningHere: false) == .nothing) + } + + @Test("Research outranks the agent loop when both read as running") + func researchOutranksTheLoop() { + // It is the longer and more expensive of the two, so it is the one the user is + // waiting on. + #expect(AskStopTarget.target(isResearchingHere: true, isAgentRunningHere: true) == .deepResearch) + } +} diff --git a/LogueTests/CommandCenterChatRuleTests.swift b/LogueTests/CommandCenterChatRuleTests.swift index 56d3d9b..14d300b 100644 --- a/LogueTests/CommandCenterChatRuleTests.swift +++ b/LogueTests/CommandCenterChatRuleTests.swift @@ -14,29 +14,20 @@ import Testing struct CommandCenterChatRuleTests { // MARK: - Trigger - @Test("The shortcut closes a chat island that is up in front") + @Test("The shortcut closes a chat island however much it is holding") func triggerDismissesShowingChat() { + // Deliberate, so it is not held to the bar a stray click is — and it is the + // keyboard way out now that clicks and app switches spare a conversation. #expect( - CommandCenterChatRule.trigger(mode: .chat, isShowingPanel: true, isChatBehindOtherApps: false) + CommandCenterChatRule.trigger(mode: .chat, isShowingPanel: true) == .dismiss ) } - @Test("The shortcut brings back an island sent behind other apps") - func triggerRaisesDemotedChat() { - // Pressing the shortcut from another app is how you ask for the island - // back. Dismissing it there would destroy an unsent prompt and show - // nothing in its place — the reported symptom, reintroduced. - #expect( - CommandCenterChatRule.trigger(mode: .chat, isShowingPanel: true, isChatBehindOtherApps: true) - == .raise - ) - } - @Test("The shortcut opens the chat island when nothing is up") func triggerPresentsWhenIdle() { #expect( - CommandCenterChatRule.trigger(mode: nil, isShowingPanel: false, isChatBehindOtherApps: false) + CommandCenterChatRule.trigger(mode: nil, isShowingPanel: false) == .present ) } @@ -46,7 +37,7 @@ struct CommandCenterChatRuleTests { let recording = CommandCenterMode.recording(meetingID: UUID()) #expect( - CommandCenterChatRule.trigger(mode: recording, isShowingPanel: true, isChatBehindOtherApps: false) + CommandCenterChatRule.trigger(mode: recording, isShowingPanel: true) == .replace ) } @@ -54,7 +45,7 @@ struct CommandCenterChatRuleTests { @Test("A panel with no mode is replaced rather than left alone") func triggerReplacesWhenModeIsMissing() { #expect( - CommandCenterChatRule.trigger(mode: nil, isShowingPanel: true, isChatBehindOtherApps: false) + CommandCenterChatRule.trigger(mode: nil, isShowingPanel: true) == .replace ) } @@ -65,7 +56,7 @@ struct CommandCenterChatRuleTests { // while it still reads .chat with nothing on screen. Dismissing then would // be a shortcut that does nothing. #expect( - CommandCenterChatRule.trigger(mode: .chat, isShowingPanel: false, isChatBehindOtherApps: false) + CommandCenterChatRule.trigger(mode: .chat, isShowingPanel: false) == .present ) } @@ -77,11 +68,12 @@ struct CommandCenterChatRuleTests { #expect(CommandCenterChatRule.focusLoss(mode: .chat, chatHasContent: false) == .dismiss) } - @Test("Switching apps keeps what the island holds but stops it covering them") - func focusLossDemotesChatWithContent() { - // Content is a sent conversation or a prompt typed and not sent — the view - // reports either, so switching apps mid-sentence cannot throw the draft away. - #expect(CommandCenterChatRule.focusLoss(mode: .chat, chatHasContent: true) == .sendBehindOtherApps) + @Test("Switching apps leaves an island that holds something exactly as it is") + func focusLossKeepsChatWithContent() { + // This used to demote the panel to `.normal`, which read as "it closed": the + // panel is non-activating, so clicking it never restored the level and the + // island stayed buried under whatever the user had just clicked. + #expect(CommandCenterChatRule.focusLoss(mode: .chat, chatHasContent: true) == .keep) } @Test("The recording island is left alone whatever the chat island holds") @@ -101,10 +93,10 @@ struct CommandCenterChatRuleTests { @Test("An island is empty only with no conversation, no draft and nothing staged") func contentIsEmptyOnlyWhenAllAreAbsent() { - #expect(CommandCenterChatContent(hasMessages: false, hasDraft: false).isEmpty) - #expect(!CommandCenterChatContent(hasMessages: true, hasDraft: false).isEmpty) - #expect(!CommandCenterChatContent(hasMessages: false, hasDraft: true).isEmpty) - #expect(!CommandCenterChatContent(hasMessages: true, hasDraft: true).isEmpty) + #expect(CommandCenterChatContent(hasConversation: false, hasDraft: false).isEmpty) + #expect(!CommandCenterChatContent(hasConversation: true, hasDraft: false).isEmpty) + #expect(!CommandCenterChatContent(hasConversation: false, hasDraft: true).isEmpty) + #expect(!CommandCenterChatContent(hasConversation: true, hasDraft: true).isEmpty) } @Test("A staged file makes an island non-empty") @@ -112,7 +104,7 @@ struct CommandCenterChatRuleTests { // Attachments were the one kind of content the island could hold that nothing here // knew about, so an island holding a PDF and nothing else reported itself disposable // and was torn down by the next click or app switch. - let staged = CommandCenterChatContent(hasMessages: false, hasDraft: false, hasAttachments: true) + let staged = CommandCenterChatContent(hasConversation: false, hasDraft: false, hasAttachments: true) #expect(!staged.isEmpty) } @@ -120,34 +112,130 @@ struct CommandCenterChatRuleTests { func stagedFilesSurviveFocusLoss() { // The rule reads `isEmpty`, so this follows from the case above — which is the point: // teaching the struct about attachments is what fixes every reader at once. - let staged = CommandCenterChatContent(hasMessages: false, hasDraft: false, hasAttachments: true) - #expect(CommandCenterChatRule.focusLoss(mode: .chat, chatHasContent: !staged.isEmpty) - == .sendBehindOtherApps) + let staged = CommandCenterChatContent(hasConversation: false, hasDraft: false, hasAttachments: true) + #expect(CommandCenterChatRule.focusLoss(mode: .chat, chatHasContent: !staged.isEmpty) == .keep) } @Test("An island with nothing staged is still disposable on an app switch") func emptyIslandStillGoesAway() { // The other direction, so the fix above cannot turn into "the island never closes". - let empty = CommandCenterChatContent(hasMessages: false, hasDraft: false) + let empty = CommandCenterChatContent(hasConversation: false, hasDraft: false) #expect(CommandCenterChatRule.focusLoss(mode: .chat, chatHasContent: !empty.isEmpty) == .dismiss) } @Test("A draft survives an app switch but not a deliberate dismissal") func draftIsProtectedFromFocusLossOnly() { - let draftOnly = CommandCenterChatContent(hasMessages: false, hasDraft: true) + let draftOnly = CommandCenterChatContent(hasConversation: false, hasDraft: true) // Switching apps is not a decision about the island, so it keeps the draft. - #expect(CommandCenterChatRule.focusLoss(mode: .chat, chatHasContent: !draftOnly.isEmpty) - == .sendBehindOtherApps) + #expect(CommandCenterChatRule.focusLoss(mode: .chat, chatHasContent: !draftOnly.isEmpty) == .keep) // Clicking off it is such a decision, and has always discarded an unsent // prompt. Protecting the draft here would leave an island with no visible // way out — the close button only exists once there are messages. // // Staged files are the deliberate exception: a dropped file cannot be recovered - // by retyping it, so `dismissIfClickMissedPanel` checks `hasAttachments` while - // still ignoring `hasDraft`. Esc remains the way out in both cases. - #expect(!draftOnly.hasMessages) + // by retyping it, so `clickOff` checks `hasAttachments` while still ignoring + // `hasDraft`. Esc remains the way out in both cases. + #expect(!draftOnly.hasConversation) #expect(!draftOnly.hasAttachments) } + + @Test("A thread the island owns counts even before anything is drawn in it") + func aConversationWithNoRowsIsStillContent() { + // The bug this closes: `AgentCoordinator.send` appends the user message inside + // a `Task`, while the composer clears synchronously. For a frame or two after + // pressing Send there were no rows, no draft and no attachments — a completely + // empty island by every rule here — so a click or an app switch in that window + // destroyed a conversation that had just started. + let justSent = CommandCenterChatContent(hasConversation: true, hasDraft: false) + #expect(!justSent.isEmpty) + #expect(!CommandCenterChatRule.clickOff(justSent)) + #expect(CommandCenterChatRule.focusLoss(mode: .chat, chatHasContent: !justSent.isEmpty) == .keep) + } + + // MARK: - What a click that missed the island may close + + @Test("A click elsewhere closes an island holding nothing worth keeping") + func clickOffClosesEmptyAndDraftOnly() { + #expect(CommandCenterChatRule.clickOff( + CommandCenterChatContent(hasConversation: false, hasDraft: false) + )) + // A draft is deliberately not protected: with no conversation there is no X, + // so keeping the island here would leave it with no visible way out. + #expect(CommandCenterChatRule.clickOff( + CommandCenterChatContent(hasConversation: false, hasDraft: true) + )) + } + + @Test("A click elsewhere never closes an island holding a conversation or a file") + func clickOffSparesConversationsAndAttachments() { + #expect(!CommandCenterChatRule.clickOff( + CommandCenterChatContent(hasConversation: true, hasDraft: false) + )) + // The inconsistency this fixes: the transparent-area click used to consult only + // the conversation, so an island holding a staged PDF was torn down by a click + // beside the pill while surviving one on another window. + #expect(!CommandCenterChatRule.clickOff( + CommandCenterChatContent(hasConversation: false, hasDraft: false, hasAttachments: true) + )) + } + + // MARK: - Esc + + @Test("Esc pressed in the island always closes it") + func escapeInIslandAlwaysCloses() { + // Deliberate: the user is looking at the island and asking for it to go away. + for content in [ + CommandCenterChatContent(hasConversation: false, hasDraft: false), + CommandCenterChatContent(hasConversation: false, hasDraft: true), + CommandCenterChatContent(hasConversation: false, hasDraft: false, hasAttachments: true), + CommandCenterChatContent(hasConversation: true, hasDraft: false), + ] { + #expect(CommandCenterChatRule.escape(content: content, pressedInIsland: true)) + } + } + + @Test("Esc pressed in another app never closes a conversation") + func escapeElsewhereSparesAConversation() { + // The global monitor sees every Esc anywhere on the machine — closing a + // dictation panel in another app should not throw away a thread here. + #expect(!CommandCenterChatRule.escape( + content: CommandCenterChatContent(hasConversation: true, hasDraft: false), + pressedInIsland: false + )) + } + + @Test("Esc pressed in another app still closes an island holding nothing") + func escapeElsewhereClosesEmpty() { + // The other direction, so the fix cannot become "the island never closes". + #expect(CommandCenterChatRule.escape( + content: CommandCenterChatContent(hasConversation: false, hasDraft: false), + pressedInIsland: false + )) + } + + @Test("Neither a stray click nor a stray Esc can close a conversation") + func incidentalInputsSpareAConversation() { + // The one thing both incidental paths must agree on. + for content in [ + CommandCenterChatContent(hasConversation: true, hasDraft: false), + CommandCenterChatContent(hasConversation: true, hasDraft: true, hasAttachments: true), + ] { + #expect(!CommandCenterChatRule.clickOff(content)) + #expect(!CommandCenterChatRule.escape(content: content, pressedInIsland: false)) + } + } + + @Test("An island holding only files keeps them through a click but not through Esc") + func attachmentsSurviveClicksButNotEscape() { + // They part company here on purpose. An island with no conversation draws no close + // button, so if Esc from elsewhere also refused, a file-only island would have no + // keyboard exit at all — the user would have to click into the field first to make + // the panel key. A click is cheap to make by accident and spares the files; pressing + // Esc is not. + let staged = CommandCenterChatContent(hasConversation: false, hasDraft: false, hasAttachments: true) + #expect(!CommandCenterChatRule.clickOff(staged)) + #expect(CommandCenterChatRule.escape(content: staged, pressedInIsland: false)) + } } diff --git a/LogueTests/ComposerPlusMenuTests.swift b/LogueTests/ComposerPlusMenuTests.swift new file mode 100644 index 0000000..2ffe3c7 --- /dev/null +++ b/LogueTests/ComposerPlusMenuTests.swift @@ -0,0 +1,86 @@ +import Foundation +import Testing + +@testable import Logue + +/// The `+` menu both composers mount, and the storage that keeps them apart. +/// +/// The menu itself is a SwiftUI `Menu` and cannot be exercised headlessly. What can be — +/// and what actually regressed once already — is which `UserDefaults` key each surface +/// arms, and whether the menu has words to show. +@Suite("ComposerPlusMenu") +struct ComposerPlusMenuTests { + @Test("The island arms its own keys, never the main window's") + func islandKeysAreItsOwn() { + // The regression this guards, fixed once already in review: both surfaces bound the + // same key, and the island clears its flags after every send — so a quick question + // asked from the island disarmed a Search or Deep Research chip the user had turned + // on in the main window and left on an unsent prompt. It then ran without the tools + // and never said why. + #expect( + AppConstants.UserDefaultsKeys.islandOneShotWebSearch + != AppConstants.UserDefaultsKeys.oneShotWebSearch + ) + #expect( + AppConstants.UserDefaultsKeys.islandOneShotDeepResearch + != AppConstants.UserDefaultsKeys.oneShotDeepResearch + ) + } + + @Test("Each surface's menu is wired to that surface's keys") + func eachSurfaceUsesItsOwnKeys() { + // The previous version of this suite compared two constants, which cannot fail for + // the regression it names: the mistake happens at the *call site*, where a surface + // passes the other one's key into the menu. Routing both call sites through this + // mapping is what makes the assertion mean something. + #expect(ComposerPlusMenu.keys(for: .island).webSearch + == AppConstants.UserDefaultsKeys.islandOneShotWebSearch) + #expect(ComposerPlusMenu.keys(for: .island).deepResearch + == AppConstants.UserDefaultsKeys.islandOneShotDeepResearch) + #expect(ComposerPlusMenu.keys(for: .mainWindow).webSearch + == AppConstants.UserDefaultsKeys.oneShotWebSearch) + #expect(ComposerPlusMenu.keys(for: .mainWindow).deepResearch + == AppConstants.UserDefaultsKeys.oneShotDeepResearch) + } + + @Test("The two surfaces never share a key") + func surfacesShareNothing() { + let island = ComposerPlusMenu.keys(for: .island) + let main = ComposerPlusMenu.keys(for: .mainWindow) + #expect(island.webSearch != main.webSearch) + #expect(island.deepResearch != main.deepResearch) + } + + @Test("No two of the four one-shot keys collide") + func allFourKeysAreDistinct() { + let keys = Set([ + AppConstants.UserDefaultsKeys.oneShotWebSearch, + AppConstants.UserDefaultsKeys.oneShotDeepResearch, + AppConstants.UserDefaultsKeys.islandOneShotWebSearch, + AppConstants.UserDefaultsKeys.islandOneShotDeepResearch, + ]) + #expect(keys.count == 4) + } + + @Test("Every menu item has something to say") + func menuCopyIsPresent() { + for label in [ + UICopy.Input.addFiles, + UICopy.Input.searchTheWeb, + UICopy.Input.deepResearchMenu, + UICopy.Input.toolSettings, + UICopy.Input.composerMenuHelp, + UICopy.Input.composerMenuLabel, + ] { + #expect(!label.isEmpty) + } + } + + @Test("The menu's own label is a name, not its icon") + func menuLabelIsNotASymbolName() { + // Same rule the island's other controls follow: a Button whose only content is an + // Image hands VoiceOver the SF Symbol name unless something else names it. + #expect(!UICopy.Input.composerMenuLabel.contains(".")) + #expect(UICopy.Input.composerMenuLabel.first?.isUppercase == true) + } +} diff --git a/LogueTests/DeepResearchOwnershipTests.swift b/LogueTests/DeepResearchOwnershipTests.swift new file mode 100644 index 0000000..0d5e0a3 --- /dev/null +++ b/LogueTests/DeepResearchOwnershipTests.swift @@ -0,0 +1,158 @@ +import Foundation +import Testing + +@testable import Logue + +/// Who a Deep Research run belongs to. +/// +/// Deep Research is one-at-a-time app-wide, which is exactly what made a single global +/// `isRunning` look sufficient — and why the bug it hides only appears once a second surface +/// can start a run. These cases are the guard on that, and they need no model: ownership is +/// decided by the coordinator's own state, not by the pipeline. +@Suite("DeepResearchOwnership") +@MainActor +struct DeepResearchOwnershipTests { + /// The coordinator is a singleton, so each case leaves it as it found it. + /// + /// `run` spawns a `Task` that would reach for a model. Nothing here is `async` and every + /// body is synchronous main-actor code, so that task cannot be scheduled before the + /// `defer` cancels it — the ownership state is set synchronously by `run` itself, which + /// is the only part these cases look at. Making any of them `async` would hand the + /// pipeline a turn to start on, so don't. + private func withIdleCoordinator(_ body: (DeepResearchCoordinator) -> Void) { + let coordinator = DeepResearchCoordinator.shared + coordinator.cancel() + coordinator.dismiss() + defer { + coordinator.cancel() + coordinator.dismiss() + } + body(coordinator) + } + + @Test("An idle coordinator belongs to no conversation") + func idleOwnsNothing() { + withIdleCoordinator { coordinator in + #expect(coordinator.runningConversationID == nil) + #expect(coordinator.isRunning(in: UUID()) == false) + #expect(coordinator.hasActivity(in: UUID()) == false) + } + } + + @Test("A run is reported only to the conversation that asked for it") + func onlyTheOwnerSeesTheRun() { + // The failure this prevents: the island starts a run, and the main window's input bar + // goes busy and its progress strip appears on whatever thread it happens to be showing. + withIdleCoordinator { coordinator in + let island = UUID() + let mainWindow = UUID() + coordinator.run(prompt: "why", conversationID: island) + + #expect(coordinator.isRunning(in: island)) + #expect(coordinator.isRunning(in: mainWindow) == false) + #expect(coordinator.hasActivity(in: mainWindow) == false) + } + } + + @Test("Cancelling leaves the failure with its owner and nobody else") + func cancellationStaysWithTheOwner() { + // `lastError` outlives the run — the strip has to say why it stopped — so the owner + // has to outlive it too, or the message paints on every surface. + withIdleCoordinator { coordinator in + let owner = UUID() + let other = UUID() + coordinator.run(prompt: "why", conversationID: owner) + coordinator.cancel() + + #expect(coordinator.isRunning == false) + #expect(coordinator.lastError != nil, "a cancelled run says so") + #expect(coordinator.hasActivity(in: owner), "and says it here") + #expect(coordinator.hasActivity(in: other) == false, "not here") + } + } + + @Test("A failed run still has something to report") + func failedRunsStillReport() { + // The progress strip mounts on `hasActivity`, so this is the predicate that decides + // whether a user who cancelled sees why it stopped. It used to mount on a separate + // expression in the view, which meant these assertions could stay green while the + // strip stopped appearing. + withIdleCoordinator { coordinator in + let owner = UUID() + coordinator.run(prompt: "why", conversationID: owner) + coordinator.cancel() + #expect(coordinator.currentStep == .failed) + #expect(coordinator.hasActivity(in: owner)) + } + } + + @Test("A cancelled run cannot take the next one down with it") + func aCancelledRunDoesNotClobberItsSuccessor() { + // `cancel()` releases `isRunning` synchronously, but the cancelled task keeps + // unwinding until whatever it is awaiting returns — MLX generation is not + // preemptible, so that can be tens of seconds. A second run legitimately starts in + // that window, and the first one then wrote its own terminal state over it: the strip + // read "Cancelled", `isRunning` went false while the second was still working, and + // nothing could stop it. The generation counter is what makes those late writes + // no-ops; this pins the part of it that is observable synchronously. + withIdleCoordinator { coordinator in + let first = UUID() + let second = UUID() + + coordinator.run(prompt: "one", conversationID: first) + coordinator.cancel() + #expect(coordinator.lastError != nil, "the cancel itself still reports") + + coordinator.run(prompt: "two", conversationID: second) + #expect(coordinator.isRunning(in: second), "the second run owns the coordinator") + #expect(coordinator.runningConversationID == second) + #expect(coordinator.lastError == nil, "a fresh run starts clean") + #expect(coordinator.currentStep != .failed) + } + } + + @Test("Dismissing a finished run gives up the thread") + func dismissReleasesOwnership() { + withIdleCoordinator { coordinator in + let owner = UUID() + coordinator.run(prompt: "why", conversationID: owner) + coordinator.cancel() + coordinator.dismiss() + + #expect(coordinator.runningConversationID == nil) + #expect(coordinator.hasActivity(in: owner) == false) + } + } + + @Test("A refused start appends nothing") + func refusedStartLeavesNoQuestion() { + // `start` used to append the question and let `run` drop the request, which left the + // user's question in the thread with nothing that would ever answer it: no spinner + // (run state is per conversation), no strip (it belongs to the other conversation), + // no error, and the composer already cleared. Returning nil is what lets the caller + // put the question back instead. + withIdleCoordinator { coordinator in + let first = UUID() + coordinator.run(prompt: "one", conversationID: first) + + let refused = coordinator.start(prompt: "two", in: UUID()) + #expect(refused == nil) + #expect(coordinator.runningConversationID == first, "the first run keeps the coordinator") + } + } + + @Test("A second run cannot start while one is in flight") + func oneRunAtATime() { + // The pre-existing policy, pinned because ownership now depends on it: if a second + // run could start it would silently take the first one's thread with it. + withIdleCoordinator { coordinator in + let first = UUID() + let second = UUID() + coordinator.run(prompt: "one", conversationID: first) + coordinator.run(prompt: "two", conversationID: second) + + #expect(coordinator.runningConversationID == first) + #expect(coordinator.isRunning(in: second) == false) + } + } +} diff --git a/LogueTests/IslandThreadTests.swift b/LogueTests/IslandThreadTests.swift new file mode 100644 index 0000000..8eda211 --- /dev/null +++ b/LogueTests/IslandThreadTests.swift @@ -0,0 +1,205 @@ +import Foundation +import Testing + +@testable import Logue + +/// What the island draws for a conversation, and in what order. +/// +/// The island dropped every tool turn on the floor, so a question that made the agent search +/// the web showed the answer and no sign of how it was reached. These cases pin what replaced +/// that, and the ordering in particular: a card is only useful sitting between the question +/// that caused it and the answer that used it. +@Suite("IslandThread") +struct IslandThreadTests { + private func user(_ text: String) -> AgentMessage { + AgentMessage(role: .user, content: text) + } + + private func assistant(_ text: String) -> AgentMessage { + AgentMessage(role: .assistant, content: text) + } + + private func toolCall(_ name: String, id: UUID = .init(), status: AgentToolCallStatus = .completed) -> AgentMessage { + AgentMessage( + role: .toolCall, + content: "", + toolCalls: [AgentToolCall(id: id, toolName: name, arguments: "{}", status: status)] + ) + } + + private func toolResult(for id: UUID, isError: Bool = false) -> AgentMessage { + AgentMessage( + role: .toolResult, + content: "out", + toolResult: AgentToolResult(toolCallID: id, output: "out", isError: isError) + ) + } + + private func kinds(_ rows: [IslandRow]) -> [String] { + rows.map { row in + switch row { + case let .message(message): message.isUser ? "user" : "assistant" + case let .tool(entry): "tool:\(entry.call.toolName)" + } + } + } + + // MARK: - Ordering + + @Test("A tool card sits between the question and the answer") + func toolCardIsInPlace() { + let callID = UUID() + let rows = IslandThread.rows( + for: [ + user("what happened today"), + toolCall("search_web", id: callID), + toolResult(for: callID), + assistant("Here is what I found."), + ], + isStreaming: false + ) + #expect(kinds(rows) == ["user", "tool:search_web", "assistant"]) + } + + @Test("A tool result is never a row of its own") + func resultIsNotARow() { + // It is rendered inside the card for the call it answers. A row would show the raw + // tool output as if the assistant had said it. + let callID = UUID() + let rows = IslandThread.rows( + for: [toolCall("read_document", id: callID), toolResult(for: callID)], + isStreaming: false + ) + // Asserting the kind, not just the count: drop the call row and emit the result as an + // assistant message instead — its content is non-empty, so it would render — and a + // count of 1 still passes while raw tool output is shown as if the assistant said it. + #expect(kinds(rows) == ["tool:read_document"]) + } + + @Test("A call awaiting approval is not also drawn as a row") + func approvalPendingCallsAreLeftToTheStrip() { + // The pinned strip above the pill owns those, because an answer that scrolls out of + // reach is an answer the user cannot give. Emitting it here too put two cards for one + // call inside a 420pt panel, only one of them answerable. + let rows = IslandThread.rows( + for: [ + user("delete that document"), + toolCall("delete_document", status: .needsConfirmation), + ], + isStreaming: false + ) + #expect(kinds(rows) == ["user"]) + } + + @Test("Once approved, the call becomes a row like any other") + func answeredCallsBecomeRows() { + let callID = UUID() + let rows = IslandThread.rows( + for: [ + user("delete that document"), + toolCall("delete_document", id: callID, status: .needsConfirmation), + toolResult(for: callID), + ], + isStreaming: false + ) + #expect(kinds(rows) == ["user", "tool:delete_document"]) + } + + @Test("A card carries its result once one has arrived") + func cardCarriesItsResult() { + let callID = UUID() + let rows = IslandThread.rows( + for: [toolCall("read_document", id: callID, status: .running), toolResult(for: callID)], + isStreaming: false + ) + guard case let .tool(entry) = rows.first else { + Issue.record("expected a tool row") + return + } + #expect(entry.result != nil) + #expect(entry.status == .completed, "the result outranks the stored .running") + } + + // MARK: - Empty assistant turns + + @Test("An assistant turn that only asked for tools draws no bubble") + func emptyToolRequestingTurnIsDropped() { + // The agent loop appends an assistant message carrying the tool calls and no prose. + // Rendered, it is an empty grey bubble sitting above the card that explains it. + let callID = UUID() + let rows = IslandThread.rows( + for: [ + user("summarise my notes"), + assistant(""), + toolCall("list_documents", id: callID), + toolResult(for: callID), + assistant("You have three."), + ], + isStreaming: false + ) + #expect(kinds(rows) == ["user", "tool:list_documents", "assistant"]) + } + + @Test("The answer being written is kept even while it is empty") + func streamingEmptyAnswerSurvives() { + // It starts empty and fills in. Dropping it would make the island show nothing during + // the gap before the first token, which reads as a send that did not happen. + let rows = IslandThread.rows(for: [user("hi"), assistant("")], isStreaming: true) + #expect(kinds(rows) == ["user", "assistant"]) + } + + // MARK: - Streaming + + @Test("Only the last assistant turn is marked as streaming") + func onlyTheLastTurnStreams() { + let rows = IslandThread.rows( + for: [user("a"), assistant("first"), user("b"), assistant("second")], + isStreaming: true + ) + let streaming = rows.compactMap { row -> Bool? in + if case let .message(message) = row, !message.isUser { return message.isStreaming } + return nil + } + #expect(streaming == [false, true]) + } + + @Test("Nothing streams when no run is in flight") + func nothingStreamsWhenIdle() { + let rows = IslandThread.rows(for: [user("a"), assistant("done")], isStreaming: false) + let streaming = rows.compactMap { row -> Bool? in + if case let .message(message) = row, !message.isUser { return message.isStreaming } + return nil + } + #expect(streaming == [false]) + } + + @Test("A trailing tool call does not make the previous answer stream") + func aTrailingToolCallDoesNotStreamTheAnswer() { + // `isStreaming` marks the last message, and the last message here is not the answer. + let rows = IslandThread.rows( + for: [user("a"), assistant("thinking about it"), toolCall("search_web")], + isStreaming: true + ) + let streaming = rows.compactMap { row -> Bool? in + if case let .message(message) = row, !message.isUser { return message.isStreaming } + return nil + } + #expect(streaming == [false]) + } + + // MARK: - Identity + + @Test("A row's id is the stored message's, so rows keep identity across a re-render") + func rowIdentityIsStable() { + let messages = [user("a"), assistant("b")] + let first = IslandThread.rows(for: messages, isStreaming: false) + let second = IslandThread.rows(for: messages, isStreaming: false) + #expect(first.map(\.id) == second.map(\.id)) + #expect(first.map(\.id) == messages.map(\.id)) + } + + @Test("An empty conversation has no rows") + func emptyConversationHasNoRows() { + #expect(IslandThread.rows(for: [], isStreaming: false).isEmpty) + } +}