diff --git a/CHANGELOG.md b/CHANGELOG.md index e4975753..fdbd5bac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -194,12 +194,15 @@ heading, and (b) bumping `pubspec.yaml` `version:` in the same commit. ### Changed -- The conversation stream's **collapsible cards** (activity runs, edit runs, - sub-agent runs) now share one `ClideCollapserCard` primitive with consistent - chrome: the collapsed ticker leads with the card's label, the step/edit count - sits in a fixed-width slot, and the status tick (spinner / check / cross) is - pinned hard against the card's right edge while the chevron hugs the left. A - `color` drives the border + label tint per card type. (T-305) +- **Every tool use in the conversation is now a collapsible card** on one + `ClideCollapserCard` primitive — activity runs, edit runs, sub-agent runs, and + each individual tool call (a single tool is a collapser over a one-item list). + Consistent chrome throughout: the collapsed ticker leads with the label + + echoed last line, the count sits in a fixed-width slot, and the status tick + (spinner while running, check / cross once done) is pinned hard against the + right edge while the chevron hugs the left; a `color` drives the border + + label tint. The inner content card carries its own per-item status, with even + padding on all sides. (T-305) - The Claude composer's **slash typeahead**, the team-chat **@-mention** list, and the status-bar **theme switcher** now ride the shared `ClideAnchoredOverlay` + `ClideMenu` popover primitive, alongside the menu diff --git a/lib/builtin/claude/src/conversation_card.dart b/lib/builtin/claude/src/conversation_card.dart index 571dfc2f..cb6e5967 100644 --- a/lib/builtin/claude/src/conversation_card.dart +++ b/lib/builtin/claude/src/conversation_card.dart @@ -61,8 +61,14 @@ class ConversationCard extends StatefulWidget { this.borderColor, this.status = ConversationCardStatus.none, this.extraSegments = const [], + this.margin = const EdgeInsets.only(bottom: 14), }); + /// Outer margin below the card. The stream rhythm is `bottom: 14` (T-282); + /// a card used as a collapser's inner item passes [EdgeInsets.zero] so the + /// collapser owns the surrounding padding evenly (T-305). + final EdgeInsetsGeometry margin; + final ConversationCardVariant variant; final Color accent; final String label; @@ -185,7 +191,7 @@ class _ConversationCardState extends State { ], ); return Padding( - padding: const EdgeInsets.only(bottom: 14), + padding: widget.margin, child: MouseRegion( onEnter: (_) => setState(() => _hover = true), onExit: (_) => setState(() => _hover = false), diff --git a/lib/builtin/claude/src/conversation_view.dart b/lib/builtin/claude/src/conversation_view.dart index 25ba8120..466dc54d 100644 --- a/lib/builtin/claude/src/conversation_view.dart +++ b/lib/builtin/claude/src/conversation_view.dart @@ -289,6 +289,7 @@ class _ConversationViewState extends State { key: ValueKey('turn.${item.uuid}'), item: item, tokens: tokens, + collapseTools: true, toolUseOutcomes: widget.toolUseOutcomes, toolUseById: widget.controller.toolUseById, resultByToolUseId: resultByToolUseId, @@ -371,6 +372,7 @@ class _ConversationTurn extends StatelessWidget { super.key, required this.item, required this.tokens, + this.collapseTools = false, this.toolUseOutcomes = const {}, this.toolUseById = const {}, this.resultByToolUseId = const {}, @@ -380,6 +382,16 @@ class _ConversationTurn extends StatelessWidget { final ConversationItem item; final SurfaceTokens tokens; + + /// When true (top-level stream items), a tool use renders as its own + /// collapser over a one-item list (T-305). When false (already inside a run / + /// edit collapser), it renders the bare inner content card so collapsers + /// don't nest. + final bool collapseTools; + + /// Card bottom margin: the stream rhythm (14) at top level, or the collapser's + /// even inner spacing (10) when this turn is a collapser child (T-305). + EdgeInsetsGeometry get _childMargin => collapseTools ? const EdgeInsets.only(bottom: 14) : const EdgeInsets.only(bottom: kClideCardHeaderPadH); final Map toolUseOutcomes; /// Index from toolUseId → AssistantToolUse, for result-card pairing (T-168). @@ -414,12 +426,14 @@ class _ConversationTurn extends StatelessWidget { collapsible: true, collapsedByDefault: true, collapsedSummary: _firstLine(i.text), + margin: _childMargin, body: ClideText(i.text, muted: true, fontSize: clideFontMeta), ), UserMessage() => ConversationCard( accent: tokens.globalFocus, label: 'you', copyText: i.text, + margin: _childMargin, // Pasted-image @path tokens render as inline thumbnails that open the // lightbox (T-236/T-254); copyText keeps the original text verbatim. body: ClideMarkdown( @@ -436,6 +450,7 @@ class _ConversationTurn extends StatelessWidget { accent: i.isSidechain ? tokens.globalTextMuted : claudeAccent, label: i.isSidechain ? 'agent' : 'claude', copyText: i.text, + margin: _childMargin, body: ClideMarkdown(i.text, onRecordTap: (id) => _openRecord(context, id), onLinkTap: (url) => _openUrl(context, url)), ), AssistantThinkingMessage() => ConversationCard( @@ -445,9 +460,10 @@ class _ConversationTurn extends StatelessWidget { copyText: i.thinking, collapsible: true, collapsedByDefault: true, + margin: _childMargin, body: ClideText(i.thinking, muted: true, fontSize: clideFontMeta), ), - AssistantToolUse() => _toolUse(i), + AssistantToolUse() => collapseTools ? _toolUseCollapser(i) : _toolContentCard(i), ToolResultMessage() => _toolResult(i), ImageMessage() => _image(context, i), }; @@ -528,78 +544,30 @@ class _ConversationTurn extends StatelessWidget { ), ); - Widget _toolUse(AssistantToolUse t) { - // T-262: fold the paired result into this card. A successful result becomes - // a "result" segment below the call + a green header check; a failed result - // stamps a red header cross but stays a separate prominent error card (the - // result is not suppressed — see _visibleItems). No result yet (in-flight) → - // no mark, no segment. - final result = resultByToolUseId[t.toolUseId]; - final succeeded = result != null && !result.isError; - final status = result == null ? ConversationCardStatus.none : (result.isError ? ConversationCardStatus.error : ConversationCardStatus.success); - - // T-264: an Agent/Task call nests its whole sub-agent run in a holder below - // the card. When a run is shown, the returned-result segment would just - // duplicate the run's final output, so drop it (note E) — but keep it when - // there's no captured run, so the output is never lost. - final isAgent = _isAgentTool(t.name); - final runItems = isAgent ? (runByToolUseId[t.toolUseId] ?? const []) : const []; - final hasRun = runItems.isNotEmpty; - - // T-263: an Agent/Task card folds its sub-agent prompt(s) in. Layered order - // when expanded (note E): call input (body) → prompt → returned result. - final segments = [ - for (final p in promptsByToolUseId[t.toolUseId] ?? const []) - CardSegment(label: 'prompt', child: ClideText(p.text, muted: true, fontSize: clideFontMeta)), - if (succeeded && !(isAgent && hasRun)) CardSegment(label: 'result', child: ClideCodeBlock(source: result.content, language: _resultLanguage(t))), - ]; - - // A resolved permission-prompted call: collapsed, green if approved / red - // if denied — a quiet record of what was permitted (D-78). It still folds - // its result + outcome check like any other merged card (T-262). + /// A standalone tool use (T-305): every tool use is a collapser over a + /// one-item list. The collapser carries the echoed last line, the count, and + /// the aggregate status (spinner while in-flight, check / cross once resolved) + /// — pushed up from the item; the inner card holds the call body + segments + /// and its own per-item mark. An Agent/Task call also nests its visible + /// sub-agent run in a second collapser below (T-264). + Widget _toolUseCollapser(AssistantToolUse t) { final outcome = toolUseOutcomes[t.toolUseId]; - final ConversationCard card; - if (outcome != null) { - final color = outcome ? tokens.statusSuccess : tokens.statusError; - card = ConversationCard( - variant: ConversationCardVariant.bordered, - accent: color, - borderColor: color, - label: t.name, - copyText: const JsonEncoder.withIndent(' ').convert(t.input), - collapsible: true, - collapsedByDefault: true, - collapsedSummary: _toolUseSummary(t), - status: status, - body: toolInputBody(tokens, t.name, t.input), - extraSegments: segments, - ); - } else { - // Per-tool body rendering (T-168): Bash → command block, Edit/Write → - // diff, Read/Grep/LS → path label, others → indented JSON. Always - // collapsible so a bulky write body doesn't dominate the scroll. - card = ConversationCard( - variant: ConversationCardVariant.bordered, - accent: tokens.globalFocus, - label: t.name, - copyText: const JsonEncoder.withIndent(' ').convert(t.input), - collapsible: true, - collapsedByDefault: true, - collapsedSummary: _toolUseSummary(t), - status: status, - body: toolInputBody(tokens, t.name, t.input), - extraSegments: segments, - ); - } + final color = outcome == null ? tokens.globalFocus : (outcome ? tokens.statusSuccess : tokens.statusError); + final collapser = ClideCollapserCard( + label: t.name, + color: color, + collapsedSummary: _toolUseSummary(t), + counter: '1 step', + status: _toolRunStatus(t), + children: [_toolContentCard(t)], + ); - if (!hasRun) return card; - // T-264: nest the sub-agent run in a holder UNDER the Agent card, so a - // reader can tell where the sub-agent work begins and ends. The run stays - // VISIBLE (not folded away) — this is attribution + containment. + final runItems = _isAgentTool(t.name) ? (runByToolUseId[t.toolUseId] ?? const []) : const []; + if (runItems.isEmpty) return collapser; return Column( crossAxisAlignment: CrossAxisAlignment.stretch, children: [ - card, + collapser, Padding( padding: const EdgeInsets.only(left: 12), child: ClideCollapserCard( @@ -625,6 +593,61 @@ class _ConversationTurn extends StatelessWidget { ); } + /// The inner content card for a tool use (T-305): the call body + folded + /// CALL/PROMPT/RESULT segments + its own per-item status mark, with NO own + /// collapse caret — the enclosing collapser owns collapse. Used both as a + /// run/edit child and as the single child of a standalone tool's collapser. + Widget _toolContentCard(AssistantToolUse t) { + // T-262: fold the paired result into this card. A successful result becomes + // a "result" segment below the call + a green header check; a failed result + // stamps a red header cross. No result yet (in-flight) → no mark, no segment. + final result = resultByToolUseId[t.toolUseId]; + final succeeded = result != null && !result.isError; + final status = result == null ? ConversationCardStatus.none : (result.isError ? ConversationCardStatus.error : ConversationCardStatus.success); + + // T-264: an Agent/Task call's run is shown in its own collapser, so the + // returned-result segment would just duplicate the run's final output — + // drop it (note E) when there's a captured run, keep it otherwise. + final isAgent = _isAgentTool(t.name); + final runItems = isAgent ? (runByToolUseId[t.toolUseId] ?? const []) : const []; + final hasRun = runItems.isNotEmpty; + + // T-263: an Agent/Task card folds its sub-agent prompt(s) in. Layered order + // (note E): call input (body) → prompt → returned result. + final segments = [ + for (final p in promptsByToolUseId[t.toolUseId] ?? const []) + CardSegment(label: 'prompt', child: ClideText(p.text, muted: true, fontSize: clideFontMeta)), + if (succeeded && !(isAgent && hasRun)) CardSegment(label: 'result', child: ClideCodeBlock(source: result.content, language: _resultLanguage(t))), + ]; + + // A resolved permission-prompted call is tinted green if approved / red if + // denied — a quiet record of what was permitted (D-78). + final outcome = toolUseOutcomes[t.toolUseId]; + final accent = outcome == null ? tokens.globalFocus : (outcome ? tokens.statusSuccess : tokens.statusError); + return ConversationCard( + variant: ConversationCardVariant.bordered, + accent: accent, + borderColor: outcome == null ? null : accent, + label: t.name, + copyText: const JsonEncoder.withIndent(' ').convert(t.input), + status: status, + body: toolInputBody(tokens, t.name, t.input), + extraSegments: segments, + // Inside a collapser the surrounding padding is even on all sides + // (T-305): a matching bottom margin is the canvas's bottom inset and the + // inter-item gap in a multi-item run. + margin: const EdgeInsets.only(bottom: kClideCardHeaderPadH), + ); + } + + /// Aggregate run status for a tool's collapser tick: a spinner while the call + /// is in-flight, settling to a check or cross once the result lands (T-305). + ClideRunStatus _toolRunStatus(AssistantToolUse t) { + final r = resultByToolUseId[t.toolUseId]; + if (r == null) return ClideRunStatus.running; + return r.isError ? ClideRunStatus.error : ClideRunStatus.success; + } + /// Per-tool language for the folded result code block (T-262): Read shows the /// file's content, so colorize by the file's grammar; Bash output is shell; /// everything else (Grep/LS/Write/Edit confirmations/…) falls back to plain. diff --git a/lib/widgets/src/clide_collapser_card.dart b/lib/widgets/src/clide_collapser_card.dart index 47eeede1..9ffdc6b0 100644 --- a/lib/widgets/src/clide_collapser_card.dart +++ b/lib/widgets/src/clide_collapser_card.dart @@ -187,8 +187,12 @@ class _ClideCollapserCardState extends State { crossAxisAlignment: CrossAxisAlignment.stretch, children: [ _headerRow(tokens), + // Even padding around the inner item canvas (T-305): the sides + + // top match, and each inner item carries a matching bottom margin + // (so the last item's margin is the bottom inset and items in a + // multi-item run are evenly separated) — hence bottom 0 here. Padding( - padding: const EdgeInsets.fromLTRB(kClideCardHeaderPadH, 0, kClideCardHeaderPadH, 8), + padding: const EdgeInsets.fromLTRB(kClideCardHeaderPadH, kClideCardHeaderPadH, kClideCardHeaderPadH, 0), child: Column( crossAxisAlignment: CrossAxisAlignment.stretch, children: [ diff --git a/test/builtin/claude/conversation_view_test.dart b/test/builtin/claude/conversation_view_test.dart index 87133831..86bd2099 100644 --- a/test/builtin/claude/conversation_view_test.dart +++ b/test/builtin/claude/conversation_view_test.dart @@ -190,7 +190,17 @@ void main() { final stream = StreamController.broadcast(); final c = ConversationController(stream: stream.stream); addTearDown(c.dispose); - await tester.pumpWidget(harness(f, ConversationView(controller: c, foldLevel: FoldLevel.none))); + await tester.pumpWidget(harness( + f, + Builder( + // The in-flight Bash collapser shows a live spinner (T-305); disable + // animations so it renders static and pumpAndSettle can settle. + builder: (ctx) => MediaQuery( + data: MediaQuery.of(ctx).copyWith(disableAnimations: true), + child: ConversationView(controller: c, foldLevel: FoldLevel.none), + ), + ), + )); stream.add(AssistantToolUse(uuid: 'A', timestamp: _t, isSidechain: false, toolUseId: 'A', name: 'Bash', input: const {'command': 'echo a'})); stream.add(AssistantThinkingMessage(uuid: 'B', timestamp: _t, isSidechain: false, thinking: 'thinking body')); await tester.pumpAndSettle(); @@ -413,8 +423,8 @@ void main() { // Collapsed by default: the prompt is hidden. expect(find.text('find all the widgets'), findsNothing); - // Expand the Agent card → a "prompt" segment reveals the folded prompt. - await tester.tap(find.bySemanticsLabel('Expand')); + // Expand the Agent collapser → a "prompt" segment reveals the folded prompt. + await tester.tap(find.bySemanticsLabel('Task, 1 step, collapsed')); await tester.pumpAndSettle(); expect(find.text('prompt'), findsOneWidget); // segment sub-label expect(find.text('find all the widgets'), findsOneWidget); @@ -438,9 +448,9 @@ void main() { UserMessage(uuid: 'pB', timestamp: _t, isSidechain: true, parentUuid: 'mB', text: 'PROMPT FOR B'), UserMessage(uuid: 'pA', timestamp: _t, isSidechain: true, parentUuid: 'mA', text: 'PROMPT FOR A'), ]); - // Two collapsed Agent cards; the first Expand caret belongs to card A. + // Two collapsed Agent collapsers; the first belongs to card A. expect(find.text('you'), findsNothing); - await tester.tap(find.bySemanticsLabel('Expand').first); + await tester.tap(find.bySemanticsLabel('Task, 1 step, collapsed').first); await tester.pumpAndSettle(); // Only card A is expanded → its prompt (A) shows; B's stays folded away. // Nearest-preceding would have put A's prompt under B, revealing nothing. @@ -475,8 +485,8 @@ void main() { // The Task's returned result (main chain) — equals the run's final prose. ToolResultMessage(uuid: 'tr', timestamp: _t, isSidechain: false, parentUuid: 'mA', toolUseId: 'tA', content: 'THE FINAL ANSWER', isError: false), ]); - // Expand the Agent card (its "result" segment would show here if kept)… - await tester.tap(find.bySemanticsLabel('Expand')); + // Expand the Agent collapser (its "result" segment would show here if kept)… + await tester.tap(find.bySemanticsLabel('Task, 1 step, collapsed')); await tester.pumpAndSettle(); // …and the nested run. await tester.tap(find.bySemanticsLabel('agent run, 1 step, collapsed')); @@ -563,10 +573,12 @@ void main() { toolUseOutcomes: {'x1': true}, // approved ); final handle = tester.ensureSemantics(); - expect(find.text('Write'), findsOneWidget); // shown (resolved) - expect(find.bySemanticsLabel('Expand'), findsOneWidget); // collapsed caret - // T-262: the resolved card also folds its result + a success check. - expect(find.bySemanticsLabel('succeeded'), findsOneWidget); + expect(find.text('Write'), findsOneWidget); // shown (resolved) — collapser label + // T-305: the resolved tool is its own collapser, collapsed by default. + expect(find.bySemanticsLabel('Write, 1 step, collapsed'), findsOneWidget); + // T-262: the resolved card folds its result + carries a success check on + // the collapser (the inner card is hidden while collapsed). + expect(find.byWidgetPredicate((w) => w is ClideStatusIndicator && w.status == ClideRunStatus.success), findsOneWidget); handle.dispose(); }); @@ -583,10 +595,10 @@ void main() { await pumpWith(tester, [ _tool('Bash', {'command': 'ls -la'}) ]); - // Card starts collapsed — the command appears as the collapsed summary. + // Collapser starts collapsed — the command appears as the echoed summary. expect(find.text('ls -la'), findsOneWidget); - // Expand to verify the body is a bash code block. - await tester.tap(find.byType(ClideIcon)); + // Expand the collapser to verify the inner body is a bash code block. + await tester.tap(find.bySemanticsLabel('Bash, 1 step, collapsed')); await tester.pump(); final blocks = tester.widgetList(find.byType(ClideCodeBlock)).toList(); expect(blocks.any((b) => b.language == 'bash' && b.source.contains('ls -la')), isTrue); @@ -608,14 +620,14 @@ void main() { ]); // No standalone success result card — it's folded into the Read card. expect(find.text('Read · result'), findsNothing); - // The merged card shows a success check. - expect(find.bySemanticsLabel('succeeded'), findsOneWidget); + // The collapser shows a success check while collapsed. + expect(find.byWidgetPredicate((w) => w is ClideStatusIndicator && w.status == ClideRunStatus.success), findsOneWidget); // Collapsed by default: neither the call body nor the result is shown yet. expect(find.text('final answer = 42;'), findsNothing); // Expand: the call segment, a "result" sub-label, and the folded result // as a colorized code block (Read → grammar from the .dart path). - await tester.tap(find.bySemanticsLabel('Expand')); + await tester.tap(find.bySemanticsLabel('Read, 1 step, collapsed')); await tester.pumpAndSettle(); expect(find.text('result'), findsOneWidget); // segment sub-label final blocks = tester.widgetList(find.byType(ClideCodeBlock)).toList(); @@ -639,7 +651,7 @@ void main() { _tool('Bash', {'command': 'echo hi'}), _result('hi\nthere'), ]); - await tester.tap(find.bySemanticsLabel('Expand')); + await tester.tap(find.bySemanticsLabel('Bash, 1 step, collapsed')); await tester.pumpAndSettle(); final blocks = tester.widgetList(find.byType(ClideCodeBlock)).toList(); expect(blocks.any((b) => b.language == 'bash' && b.source == 'hi\nthere'), isTrue); @@ -654,8 +666,8 @@ void main() { // The error stays a separate prominent card… expect(find.text('Bash · error'), findsOneWidget); expect(find.text('permission denied'), findsOneWidget); - // …and the call card carries a red failure mark for symmetry (note C). - expect(find.bySemanticsLabel('failed'), findsOneWidget); + // …and the call collapser carries a red failure mark for symmetry (note C). + expect(find.byWidgetPredicate((w) => w is ClideStatusIndicator && w.status == ClideRunStatus.error), findsOneWidget); handle.dispose(); }); diff --git a/test/goldens/collapser_card_goldens_test.dart b/test/goldens/collapser_card_goldens_test.dart index b6322fb8..35460da8 100644 --- a/test/goldens/collapser_card_goldens_test.dart +++ b/test/goldens/collapser_card_goldens_test.dart @@ -15,7 +15,7 @@ void main() { // A stand-in inner item card: content + its own per-item status mark (the // collapser carries the aggregate; items keep their own — T-305). Widget item(String label, ClideRunStatus status) => Padding( - padding: const EdgeInsets.only(bottom: 8), + padding: const EdgeInsets.only(bottom: 10), child: DecoratedBox( decoration: BoxDecoration( border: Border.all(color: const Color(0xFF393E48)), diff --git a/test/goldens/goldens/linux/collapser_card.png b/test/goldens/goldens/linux/collapser_card.png index 2b00cf5a..b15b6629 100644 Binary files a/test/goldens/goldens/linux/collapser_card.png and b/test/goldens/goldens/linux/collapser_card.png differ diff --git a/test/goldens/goldens/linux/tool_collapser.png b/test/goldens/goldens/linux/tool_collapser.png new file mode 100644 index 00000000..035d075a Binary files /dev/null and b/test/goldens/goldens/linux/tool_collapser.png differ diff --git a/test/goldens/tool_collapser_goldens_test.dart b/test/goldens/tool_collapser_goldens_test.dart new file mode 100644 index 00000000..bb2e9046 --- /dev/null +++ b/test/goldens/tool_collapser_goldens_test.dart @@ -0,0 +1,74 @@ +import 'package:alchemist/alchemist.dart'; +import 'package:clide/builtin/claude/src/conversation_card.dart'; +import 'package:clide/widgets/widgets.dart'; +import 'package:flutter/widgets.dart'; +import 'package:flutter_test/flutter_test.dart'; + +import '../helpers/kernel_fixture.dart'; +import '../helpers/widget_harness.dart'; + +/// Visualises the T-305 single-tool rendering: every tool use is a collapser +/// over a one-item list. Collapsed → ticker (label + echoed line + 1 step + +/// status). Expanded → the inner content card (body + folded result + its own +/// per-item mark; the collapser carries the aggregate). +void main() { + late KernelFixture f; + setUp(() async => f = await KernelFixture.create()); + tearDown(() async => f.dispose()); + + Widget bashContent() => const ConversationCard( + variant: ConversationCardVariant.bordered, + accent: Color(0xFF4C9AFF), + label: 'Bash', + status: ConversationCardStatus.success, + body: Text('npm test', style: TextStyle(fontSize: 12, color: Color(0xFF9DA5B4)), textDirection: TextDirection.ltr), + extraSegments: [ + CardSegment( + label: 'result', child: Text('All 42 tests passed', style: TextStyle(fontSize: 12, color: Color(0xFF9DA5B4)), textDirection: TextDirection.ltr)), + ], + ); + + goldenTest( + 'single tool collapser (T-305): collapsed ticker + expanded inner card', + fileName: 'tool_collapser', + builder: () => GoldenTestGroup( + columns: 1, + children: [ + GoldenTestScenario( + name: 'collapsed — Bash, 1 step', + child: SizedBox( + width: 420, + child: harness( + f, + ClideCollapserCard( + label: 'Bash', + color: const Color(0xFF4C9AFF), + collapsedSummary: 'npm test', + counter: '1 step', + status: ClideRunStatus.success, + children: [bashContent()], + ), + ), + ), + ), + GoldenTestScenario( + name: 'expanded — inner content card', + child: SizedBox( + width: 420, + child: harness( + f, + ClideCollapserCard( + label: 'Bash', + color: const Color(0xFF4C9AFF), + counter: '1 step', + status: ClideRunStatus.success, + initiallyExpanded: true, + children: [bashContent()], + ), + ), + ), + ), + ], + ), + ); +}