diff --git a/.pql/changelog/ticket_history/2026-06.sql b/.pql/changelog/ticket_history/2026-06.sql index 82188f4d..c31bab58 100644 --- a/.pql/changelog/ticket_history/2026-06.sql +++ b/.pql/changelog/ticket_history/2026-06.sql @@ -784,3 +784,4 @@ INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-267', 'status', 'backlog', 'ready', NULL, '2026-06-07 16:25:28', '2026-06-07 16:25:28', '2026-06-07 16:25:28', NULL, 'f89b4bd8614ca3f405f7f888e789d56a', 1) ON CONFLICT(hash) DO NOTHING; INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-267', 'status', 'ready', 'in_progress', NULL, '2026-06-07 17:22:06', '2026-06-07 17:22:06', '2026-06-07 17:22:06', NULL, 'bd4730367ab0af0d33d145a56748b2ab', 1) ON CONFLICT(hash) DO NOTHING; INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-262', 'status', 'backlog', 'in_progress', NULL, '2026-06-07 17:23:39', '2026-06-07 17:23:39', '2026-06-07 17:23:39', NULL, 'dd562ebdd3cb176d4d58bdf07190924a', 1) ON CONFLICT(hash) DO NOTHING; +INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-262', 'status', 'in_progress', 'done', NULL, '2026-06-07 17:41:11', '2026-06-07 17:41:11', '2026-06-07 17:41:11', NULL, '2e581770ff1d438497379beaa530b9c6', 1) ON CONFLICT(hash) DO NOTHING; diff --git a/.pql/changelog/tickets/2026-06.sql b/.pql/changelog/tickets/2026-06.sql index d8e4ae2f..1cf73f0d 100644 --- a/.pql/changelog/tickets/2026-06.sql +++ b/.pql/changelog/tickets/2026-06.sql @@ -1703,3 +1703,37 @@ C — Error-card symmetry decision: this ticket merges only SUCCESS into the too D — Activity-cluster coupling: activity_cluster.groupConversation currently folds a tool CALL and its RESULT as TWO separate foldable items into a cluster (activity_cluster.dart ~L100-109, classifying each by toolUseId). Once this ticket makes call+result a single self-contained card, the grouping pass must treat the tool call as ONE unit (its result is part of the card, no longer a separate foldable item) or the result will double-render (once folded into the card, once as a cluster item). Update _isFoldable / the pairing logic accordingly and add a test that a merged tool card is not double-counted.', 'in_progress', 'medium', NULL, NULL, 'D-78', '2026-06-07 08:10:42', '2026-06-07 17:23:39', NULL, '04fae1762c477c0e883f426080c90b53', 1) ON CONFLICT(id) DO UPDATE SET type=excluded.type, parent_id=excluded.parent_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); INSERT INTO tickets (id, type, parent_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-273', 'task', NULL, 'turn indicator font color → Claude accent orange', 'Change the animated running/turn indicator''s text color to the Claude accent orange (the brand accent), instead of its current color. Lives in lib/builtin/claude/src/running_indicator.dart (the rotating-verb turn indicator, T-255). Use the appropriate accent token from SurfaceTokens (ui-design skill: confirm which token maps to Claude accent orange) — no hardcoded hex. Update/adjust any golden or widget test that asserts the indicator color.', 'backlog', 'low', NULL, NULL, NULL, '2026-06-07 17:39:10', '2026-06-07 17:39:10', NULL, '2f09f004ad10014a7e881fe28d9e2f15', 1) ON CONFLICT(id) DO UPDATE SET type=excluded.type, parent_id=excluded.parent_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); +INSERT INTO tickets (id, type, parent_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-262', 'story', 'T-267', 'Merge tool-call + success result into one card with header status check', 'Today a Claude tool call renders as TWO stacked cards in the conversation log: the tool-use card (e.g. "Write " with a collapsible call body) and a separate success/result card ("Write · result / File created successfully..."). Collapse the successful pair into ONE card; failure keeps the current two-card interaction. + +Current code: lib/builtin/claude/src/conversation_view.dart — _toolUse (~L294) renders AssistantToolUse, _toolResult (~L329) renders ToolResultMessage; the two ConversationItems are linked by toolUseId (ToolResultMessage.isError marks failure). ConversationCard header is built in lib/builtin/claude/src/conversation_card.dart _header (~L200). Builds on T-168 (per-tool body rendering). + +Desired behavior (success): +- One card per successful tool call — the tool-use card. The standalone success result card is suppressed (folded in), not rendered separately. +- A success-green check mark sits at the RIGHT END of the card header row (reuse tokens.statusSuccess + a check glyph: PhosphorIcons.check or CheckIcon). No new theme tokens. +- Collapsed by default: header shows the existing summary + the check. Expanding the card reveals the CALL segment (the input — path / diff / command, as today) and, BELOW it, the RESULT segment (the swallowed output). This applies to ALL tools, not just Write/Edit: for Bash/Read/Grep/LS the real output relocates into the expandable body so nothing is lost; for Write/Edit the trivial confirmation folds in the same way. +- Render the result segment as an inline ClideCodeBlock (lib/widgets/src/clide_code_block.dart) WITH editor syntax colorization. ClideCodeBlock already colorizes via TreeSitterService when handed a language — so the work is inferring/passing the right language per tool: Read → from the file extension/path (language_map.grammarForPath), Bash → bash, Grep/LS/others → text fallback. Where a path is available, prefer it so highlighting matches the editor. + +Desired behavior (failure) — UNCHANGED: +- Leave the current interaction as-is: the tool-use card plus the prominent red "· error" result card (borderColor statusError, expanded-by-default). Optionally show a red status mark on the tool-use header for symmetry, but do NOT fold the error into one card. + +What to build: +1. ConversationCard: a trailing header status slot (e.g. status: success|error|none) rendered in _header between the Spacer/summary and the hover action buttons. +2. ConversationCard: support a second body segment so an expanded card can show CALL then RESULT with a clear visual separator / sub-label (e.g. a divider or a muted "result" label) so the user can tell the call from its output. +3. conversation_view: a result-by-toolUseId lookup (reverse of the existing toolUseById) so a tool-use card knows its outcome; on a successful pair, stamp the check + fold the result body and SUPPRESS the standalone success ToolResultMessage from the rendered list. Errors render both cards as today. +4. Per-tool result language inference for the folded code block. + +Edge cases to handle: +- In-flight tool-use with no result yet: render the call card as today (no check, no folded result); status/result appear once the result arrives. +- Orphan result with no paired tool-use: keep rendering it standalone. +- Permission-resolved tool-use cards (conversation_view.dart ~L297, already green/red bordered + collapsed): reconcile so the merged-card + check treatment is consistent and not duplicated with the existing border-outcome styling. +- Result items may not be strictly adjacent to their tool-use in the list; suppression must be keyed by toolUseId, not list position. + +Tests: unit/widget coverage for the merged success card (collapsed shows check; expanded shows call + colorized result), language inference per tool, the unchanged error path, and the in-flight/orphan cases. Add a golden for the merged success card. + +Refs: D-78 (interaction zone / display-only conversation widgets). Builds on T-168 (per-tool tool-use/result body rendering). + +IMPLEMENTATION NOTES (from streamlining analysis): + +C — Error-card symmetry decision: this ticket merges only SUCCESS into the tool card; the error path intentionally stays a separate prominent red "· error" card (conversation_view.dart ~L337) for failure visibility. Decide explicitly: keep that asymmetry (success folds, failure stays two-card) OR also stamp a red status mark on the tool-use header for visual symmetry while still keeping the separate red card. Default leaning: keep the separate red card; optionally add the red header mark. + +D — Activity-cluster coupling: activity_cluster.groupConversation currently folds a tool CALL and its RESULT as TWO separate foldable items into a cluster (activity_cluster.dart ~L100-109, classifying each by toolUseId). Once this ticket makes call+result a single self-contained card, the grouping pass must treat the tool call as ONE unit (its result is part of the card, no longer a separate foldable item) or the result will double-render (once folded into the card, once as a cluster item). Update _isFoldable / the pairing logic accordingly and add a test that a merged tool card is not double-counted.', 'done', 'medium', NULL, NULL, 'D-78', '2026-06-07 08:10:42', '2026-06-07 17:41:11', NULL, '536c793d50d899e4d403b5a2a246f6a5', 1) ON CONFLICT(id) DO UPDATE SET type=excluded.type, parent_id=excluded.parent_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); diff --git a/CHANGELOG.md b/CHANGELOG.md index 161ddc62..e1f01af9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -144,6 +144,11 @@ heading, and (b) bumping `pubspec.yaml` `version:` in the same commit. ### Changed +- A successful tool call now renders as **one merged card** instead of a + separate call + result pair: a green check sits at the header's right edge and + the output folds in as a colorized code block (Read → file grammar, Bash → + shell) revealed on expand. Failures keep their prominent red card, now with a + matching header mark. (T-262) - The in-flight turn indicator now feels alive: instead of a static gray `running…`, it shows a rotating curated status verb (`Pondering…`, `Conjuring…`, …) with an animated ellipsis. Respects reduced-motion (static diff --git a/lib/builtin/claude/src/conversation_card.dart b/lib/builtin/claude/src/conversation_card.dart index 45b32e84..f6869cd2 100644 --- a/lib/builtin/claude/src/conversation_card.dart +++ b/lib/builtin/claude/src/conversation_card.dart @@ -31,6 +31,21 @@ class MessageAction { enum ConversationCardVariant { stripe, bordered, bare } +/// A trailing status mark shown at the right end of the header (T-262): a +/// green check for a succeeded tool call, a red cross for a failure. [none] +/// renders no mark (the default for non-tool cards). +enum ConversationCardStatus { none, success, error } + +/// An extra labelled body segment shown below the primary [ConversationCard.body] +/// when the card is expanded (T-262). Lets one card lay out CALL → RESULT (or, +/// for the Agent card, CALL → PROMPT → RESULT — T-263) with a muted sub-label + +/// divider between segments so the reader can tell the parts apart. +class CardSegment { + const CardSegment({required this.label, required this.child}); + final String label; + final Widget child; +} + class ConversationCard extends StatefulWidget { const ConversationCard({ super.key, @@ -44,6 +59,8 @@ class ConversationCard extends StatefulWidget { this.collapsedByDefault = false, this.collapsedSummary, this.borderColor, + this.status = ConversationCardStatus.none, + this.extraSegments = const [], }); final ConversationCardVariant variant; @@ -51,6 +68,14 @@ class ConversationCard extends StatefulWidget { final String label; final Widget body; + /// Trailing header status mark (T-262) — a success check or error cross at + /// the right end of the header. [ConversationCardStatus.none] shows nothing. + final ConversationCardStatus status; + + /// Extra labelled segments rendered below [body] when expanded (T-262/T-263); + /// each gets a muted sub-label + divider so CALL/PROMPT/RESULT read apart. + final List extraSegments; + /// Raw text the copy action yields; no copy button when null. final String? copyText; @@ -149,7 +174,14 @@ class _ConversationCardState extends State { crossAxisAlignment: CrossAxisAlignment.start, children: [ _header(tokens), - if (!_collapsed) ...[const SizedBox(height: 4), widget.body], + if (!_collapsed) ...[ + const SizedBox(height: 4), + widget.body, + for (final seg in widget.extraSegments) ...[ + _segmentLabel(tokens, seg.label), + seg.child, + ], + ], ], ); return Padding( @@ -215,6 +247,9 @@ class _ConversationCardState extends State { ), ] else const Spacer(), + // Trailing status mark (T-262): success check / error cross, sitting + // between the summary/spacer and the action bar. + if (widget.status != ConversationCardStatus.none) _statusMark(tokens), // Actions are always in the tree (keyboard/AT always reachable). // Opacity reveals them on hover or keyboard focus; opacity-0 keeps // them layout-present but visually hidden so they don't distract. @@ -253,6 +288,47 @@ class _ConversationCardState extends State { ); } + /// The trailing success/error mark (T-262). Carries a Semantics label so the + /// outcome is announced, not just colour-coded. + Widget _statusMark(SurfaceTokens tokens) { + final ClideIconPainter icon; + final Color color; + final String label; + switch (widget.status) { + case ConversationCardStatus.success: + icon = const CheckIcon(); + color = tokens.statusSuccess; + label = 'succeeded'; + case ConversationCardStatus.error: + icon = const CloseIcon(); + color = tokens.statusError; + label = 'failed'; + case ConversationCardStatus.none: + return const SizedBox.shrink(); + } + return Semantics( + label: label, + container: true, + child: Padding( + padding: const EdgeInsets.only(left: 8), + child: ClideIcon(icon, size: 12, color: color), + ), + ); + } + + /// A muted sub-label + hairline divider introducing an [CardSegment] below + /// the primary body (T-262), so CALL/PROMPT/RESULT read as distinct parts. + Widget _segmentLabel(SurfaceTokens tokens, String label) => Padding( + padding: const EdgeInsets.only(top: 8, bottom: 4), + child: Row( + children: [ + ClideText(label, fontSize: clideFontMeta, color: tokens.globalTextMuted, fontFamily: clideMonoFamily), + const SizedBox(width: 8), + Expanded(child: Container(height: 1, color: tokens.panelBorder)), + ], + ), + ); + List _actions(SurfaceTokens tokens) { final items = <_ActionItem>[]; if (widget.copyText != null) items.add(_ActionItem('copy', _copy)); diff --git a/lib/builtin/claude/src/conversation_view.dart b/lib/builtin/claude/src/conversation_view.dart index c7c5ee12..21648b7a 100644 --- a/lib/builtin/claude/src/conversation_view.dart +++ b/lib/builtin/claude/src/conversation_view.dart @@ -18,6 +18,7 @@ import 'package:clide/builtin/claude/src/conversation_controller.dart'; import 'package:clide/builtin/claude/src/prompt_card.dart'; import 'package:clide/builtin/claude/src/transcript_reader.dart'; import 'package:clide/kernel/src/facade.dart'; +import 'package:clide/kernel/src/syntax/language_map.dart'; import 'package:clide/kernel/src/theme/controller.dart'; import 'package:clide/kernel/src/theme/tokens.dart'; import 'package:clide/widgets/widgets.dart'; @@ -99,14 +100,31 @@ class _ConversationViewState extends State { if (it is AssistantToolUse && it.name == 'AskUserQuestion') it.toolUseId, }; final outcomes = widget.toolUseOutcomes; + final toolUseById = { + for (final it in items) + if (it is AssistantToolUse) it.toolUseId: it, + }; + + // A tool-use is dropped (not rendered as a card) when it's an + // AskUserQuestion or a still-pending permission prompt. + bool toolUseDropped(AssistantToolUse t) { + if (t.name == 'AskUserQuestion') return true; + return hidden.contains(t.toolUseId) && !outcomes.containsKey(t.toolUseId); + } + bool drop(ConversationItem it) { - if (it is AssistantToolUse) { - if (it.name == 'AskUserQuestion') return true; - // Permission-prompted: hide only while pending; once resolved it shows - // collapsed with a green/red border. - return hidden.contains(it.toolUseId) && !outcomes.containsKey(it.toolUseId); + if (it is AssistantToolUse) return toolUseDropped(it); + if (it is ToolResultMessage) { + if (auqIds.contains(it.toolUseId)) return true; // AUQ result echo — noise + // T-262: a successful result whose paired tool-use is going to render + // folds INTO that card — suppress the standalone result here so it + // doesn't double-render. Errors stay standalone (prominent red card); + // orphan results (no paired tool-use) stay standalone too. + if (it.isError) return false; + final tu = toolUseById[it.toolUseId]; + if (tu == null) return false; + return !toolUseDropped(tu); } - if (it is ToolResultMessage) return auqIds.contains(it.toolUseId); // AUQ result only; keep permission results return false; } @@ -139,6 +157,14 @@ class _ConversationViewState extends State { ); } + // Reverse pairing (T-262): toolUseId → its result, so a tool-use card can + // fold a successful result in. Built from the full item list (not the + // visible one — the success result is suppressed from `items`). + final resultByToolUseId = { + for (final it in widget.controller.items) + if (it is ToolResultMessage) it.toolUseId: it, + }; + // Fold runs of meta items into collapsible activity cards (T-230); sticky // items (user/prose/surfaced errors) render first-class as before. final groups = groupConversation(items, widget.foldLevel); @@ -156,12 +182,14 @@ class _ConversationViewState extends State { tokens: tokens, toolUseOutcomes: widget.toolUseOutcomes, toolUseById: widget.controller.toolUseById, + resultByToolUseId: resultByToolUseId, ), FoldedCluster(:final items) => _ActivityCard( items: items, tokens: tokens, toolUseOutcomes: widget.toolUseOutcomes, toolUseById: widget.controller.toolUseById, + resultByToolUseId: resultByToolUseId, ), }; }, @@ -185,6 +213,7 @@ class _ConversationTurn extends StatelessWidget { required this.tokens, this.toolUseOutcomes = const {}, this.toolUseById = const {}, + this.resultByToolUseId = const {}, }); final ConversationItem item; @@ -194,6 +223,10 @@ class _ConversationTurn extends StatelessWidget { /// Index from toolUseId → AssistantToolUse, for result-card pairing (T-168). final Map toolUseById; + /// Index from toolUseId → its result, so a tool-use card can fold a + /// successful result into one merged card (T-262). + final Map resultByToolUseId; + @override Widget build(BuildContext context) { final i = item; @@ -315,8 +348,20 @@ 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); + final segments = + succeeded ? [CardSegment(label: 'result', child: ClideCodeBlock(source: result.content, language: _resultLanguage(t)))] : const []; + // A resolved permission-prompted call: collapsed, green if approved / red - // if denied — a quiet record of what was permitted (D-78). + // 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). final outcome = toolUseOutcomes[t.toolUseId]; if (outcome != null) { final color = outcome ? tokens.statusSuccess : tokens.statusError; @@ -329,7 +374,9 @@ class _ConversationTurn extends StatelessWidget { collapsible: true, collapsedByDefault: true, collapsedSummary: _toolUseSummary(t), + status: status, body: toolInputBody(tokens, t.name, t.input), + extraSegments: segments, ); } // Per-tool body rendering (T-168): Bash → command block, Edit/Write → diff, @@ -345,10 +392,27 @@ class _ConversationTurn extends StatelessWidget { collapsible: true, collapsedByDefault: true, collapsedSummary: summary, + status: status, body: body, + extraSegments: segments, ); } + /// 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. + String _resultLanguage(AssistantToolUse t) { + switch (t.name) { + case 'Read': + final path = (t.input['file_path'] ?? t.input['path']) as String?; + return (path != null && path.isNotEmpty ? grammarForPath(path) : null) ?? 'text'; + case 'Bash': + return 'bash'; + default: + return 'text'; + } + } + Widget _toolResult(ToolResultMessage t) { final paired = toolUseById[t.toolUseId]; final accent = t.isError ? tokens.statusError : tokens.globalTextMuted; @@ -430,12 +494,14 @@ class _ActivityCard extends StatefulWidget { required this.tokens, required this.toolUseOutcomes, required this.toolUseById, + required this.resultByToolUseId, }); final List items; final SurfaceTokens tokens; final Map toolUseOutcomes; final Map toolUseById; + final Map resultByToolUseId; @override State<_ActivityCard> createState() => _ActivityCardState(); @@ -503,6 +569,7 @@ class _ActivityCardState extends State<_ActivityCard> { tokens: tokens, toolUseOutcomes: widget.toolUseOutcomes, toolUseById: widget.toolUseById, + resultByToolUseId: widget.resultByToolUseId, ), ], ), diff --git a/test/builtin/claude/conversation_card_test.dart b/test/builtin/claude/conversation_card_test.dart index 7597cd5b..31e4a9e4 100644 --- a/test/builtin/claude/conversation_card_test.dart +++ b/test/builtin/claude/conversation_card_test.dart @@ -283,6 +283,68 @@ void main() { expect(find.text('result'), findsOneWidget); }); + testWidgets('status: success renders a "succeeded" semantics mark in the header (T-262)', (tester) async { + final handle = tester.ensureSemantics(); + await tester.pumpWidget(harness( + f, + const ConversationCard( + variant: ConversationCardVariant.bordered, + accent: Color(0xFFFFFFFF), + label: 'Bash', + status: ConversationCardStatus.success, + body: Text('body', textDirection: TextDirection.ltr), + ), + )); + await tester.pump(); + expect(find.bySemanticsLabel('succeeded'), findsOneWidget); + expect(find.bySemanticsLabel('failed'), findsNothing); + handle.dispose(); + }); + + testWidgets('status: error renders a "failed" semantics mark; none renders nothing (T-262)', (tester) async { + final handle = tester.ensureSemantics(); + await tester.pumpWidget(harness( + f, + const ConversationCard( + variant: ConversationCardVariant.bordered, + accent: Color(0xFFFFFFFF), + label: 'Bash', + status: ConversationCardStatus.error, + body: Text('body', textDirection: TextDirection.ltr), + ), + )); + await tester.pump(); + expect(find.bySemanticsLabel('failed'), findsOneWidget); + expect(find.bySemanticsLabel('succeeded'), findsNothing); + handle.dispose(); + }); + + testWidgets('extraSegments render below the body with their sub-label when expanded (T-262)', (tester) async { + await tester.pumpWidget(harness( + f, + const ConversationCard( + variant: ConversationCardVariant.bordered, + accent: Color(0xFFFFFFFF), + label: 'Read', + collapsible: true, + collapsedByDefault: true, + body: Text('the call', textDirection: TextDirection.ltr), + extraSegments: [CardSegment(label: 'result', child: Text('the output', textDirection: TextDirection.ltr))], + ), + )); + await tester.pump(); + // Collapsed: neither the body nor the segment shows. + expect(find.text('the call'), findsNothing); + expect(find.text('result'), findsNothing); + expect(find.text('the output'), findsNothing); + + await tester.tap(find.bySemanticsLabel('Expand')); + await tester.pump(); + expect(find.text('the call'), findsOneWidget); // primary body + expect(find.text('result'), findsOneWidget); // segment sub-label + expect(find.text('the output'), findsOneWidget); // segment child + }); + testWidgets('bare variant renders content without frame decoration', (tester) async { await tester.pumpWidget(harness( f, diff --git a/test/builtin/claude/conversation_view_test.dart b/test/builtin/claude/conversation_view_test.dart index ead7d8a2..2833617d 100644 --- a/test/builtin/claude/conversation_view_test.dart +++ b/test/builtin/claude/conversation_view_test.dart @@ -177,7 +177,9 @@ void main() { tester, [ _tool('Bash', const {'command': 'echo hi'}), - _result('hi'), + // A second, distinct in-flight tool call (T-262 folds a success + // result into its call card, so two *calls* are what make 2 steps). + AssistantToolUse(uuid: 'tu2', timestamp: _t, isSidechain: false, toolUseId: 'x2', name: 'Read', input: const {'file_path': '/a'}), ], foldLevel: FoldLevel.tools); // Collapsed by default: one card with a step count, not the raw rows. @@ -189,6 +191,21 @@ void main() { expect(find.bySemanticsLabel('Activity, 2 steps, expanded'), findsOneWidget); }); + testWidgets('a folded success result is not a separate step — merged into its call (T-262 note D)', (tester) async { + await pumpWith( + tester, + [ + _tool('Bash', const {'command': 'echo hi'}), + _result('hi there'), + ], + foldLevel: FoldLevel.tools); + // The call + its success result is ONE unit now: 1 step, not 2. + expect(find.text('1 step'), findsOneWidget); + expect(find.text('2 steps'), findsNothing); + // The result is not double-rendered: no standalone result card. + expect(find.text('Bash · result'), findsNothing); + }); + testWidgets('an image card renders with the "image" label, the file, and its caption (T-249)', (tester) async { await pumpWith(tester, [_image('/no/such/file.png', caption: 'before the fix')]); expect(find.text('image'), findsOneWidget); @@ -247,8 +264,9 @@ void main() { expect(find.text('claude'), findsOneWidget); expect(find.text('thinking'), findsOneWidget); expect(find.text('Bash'), findsOneWidget); - // Result labels now include the paired tool name (T-168). - expect(find.text('Bash · result'), findsOneWidget); + // T-262: a successful result folds into the tool card (no standalone + // "Bash · result"); a failed result keeps its prominent standalone card. + expect(find.text('Bash · result'), findsNothing); expect(find.text('Bash · error'), findsOneWidget); }); @@ -289,8 +307,12 @@ void main() { hiddenToolUseIds: {'x1'}, toolUseOutcomes: {'x1': true}, // approved ); + final handle = tester.ensureSemantics(); expect(find.text('Write'), findsOneWidget); // shown (resolved) - expect(find.byType(ClideIcon), findsOneWidget); // collapsed caret + expect(find.bySemanticsLabel('Expand'), findsOneWidget); // collapsed caret + // T-262: the resolved card also folds its result + a success check. + expect(find.bySemanticsLabel('succeeded'), findsOneWidget); + handle.dispose(); }); testWidgets('an injected user message renders as a muted "context" card, not "you"', (tester) async { @@ -323,12 +345,63 @@ void main() { expect(find.text('/foo/bar.dart'), findsOneWidget); }); - testWidgets('result label includes paired tool name (T-168)', (tester) async { + testWidgets('a successful result folds into the tool-use card with a check (T-262)', (tester) async { + final handle = tester.ensureSemantics(); await pumpWith(tester, [ - _tool('Read', {'file_path': '/x'}), - _result('file content'), + _tool('Read', {'file_path': '/foo/bar.dart'}), + _result('final answer = 42;'), ]); - expect(find.text('Read · result'), findsOneWidget); + // 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); + // 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.pumpAndSettle(); + expect(find.text('result'), findsOneWidget); // segment sub-label + final blocks = tester.widgetList(find.byType(ClideCodeBlock)).toList(); + expect(blocks.any((b) => b.language == 'dart' && b.source == 'final answer = 42;'), isTrue); + handle.dispose(); + }); + + testWidgets('Bash result folds as a bash code block; in-flight call has no check (T-262)', (tester) async { + final handle = tester.ensureSemantics(); + // In-flight: a call with no result yet → no check, no folded result. + await pumpWith(tester, [ + _tool('Bash', {'command': 'ls'}) + ]); + expect(find.bySemanticsLabel('succeeded'), findsNothing); + expect(find.bySemanticsLabel('failed'), findsNothing); + handle.dispose(); + }); + + testWidgets('Bash success result infers the bash language for the folded block (T-262)', (tester) async { + await pumpWith(tester, [ + _tool('Bash', {'command': 'echo hi'}), + _result('hi\nthere'), + ]); + await tester.tap(find.bySemanticsLabel('Expand')); + await tester.pumpAndSettle(); + final blocks = tester.widgetList(find.byType(ClideCodeBlock)).toList(); + expect(blocks.any((b) => b.language == 'bash' && b.source == 'hi\nthere'), isTrue); + }); + + testWidgets('a failed result is NOT folded: standalone error card + red header mark (T-262)', (tester) async { + final handle = tester.ensureSemantics(); + await pumpWith(tester, [ + _tool('Bash', {'command': 'boom'}), + _result('permission denied', isError: true), + ]); + // 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); + handle.dispose(); }); testWidgets('error result label includes paired tool name (T-168)', (tester) async { diff --git a/test/goldens/conversation_card_goldens_test.dart b/test/goldens/conversation_card_goldens_test.dart new file mode 100644 index 00000000..4a44d54a --- /dev/null +++ b/test/goldens/conversation_card_goldens_test.dart @@ -0,0 +1,78 @@ +import 'package:alchemist/alchemist.dart'; +import 'package:clide/builtin/claude/src/conversation_card.dart'; +import 'package:flutter/widgets.dart'; +import 'package:flutter_test/flutter_test.dart'; + +import '../helpers/kernel_fixture.dart'; +import '../helpers/widget_harness.dart'; + +void main() { + late KernelFixture f; + + setUp(() async => f = await KernelFixture.create()); + tearDown(() async => f.dispose()); + + goldenTest( + 'ConversationCard merged tool card (T-262): status mark + folded result segment', + fileName: 'conversation_card_merged', + builder: () => GoldenTestGroup( + columns: 1, + children: [ + GoldenTestScenario( + name: 'success / collapsed (check + summary)', + child: _wrap( + f, + const ConversationCard( + variant: ConversationCardVariant.bordered, + accent: Color(0xFF4C9AFF), + label: 'Read', + status: ConversationCardStatus.success, + collapsible: true, + collapsedByDefault: true, + collapsedSummary: '/lib/main.dart', + body: Text('/lib/main.dart', textDirection: TextDirection.ltr), + extraSegments: [CardSegment(label: 'result', child: Text('void main() {}', textDirection: TextDirection.ltr))], + ), + ), + ), + GoldenTestScenario( + name: 'success / expanded (call → result segment)', + child: _wrap( + f, + const ConversationCard( + variant: ConversationCardVariant.bordered, + accent: Color(0xFF4C9AFF), + label: 'Read', + status: ConversationCardStatus.success, + collapsible: true, + collapsedByDefault: false, + body: Text('/lib/main.dart', textDirection: TextDirection.ltr), + extraSegments: [CardSegment(label: 'result', child: Text('void main() {}', textDirection: TextDirection.ltr))], + ), + ), + ), + GoldenTestScenario( + name: 'error / header mark (call card stays separate from the red result card)', + child: _wrap( + f, + const ConversationCard( + variant: ConversationCardVariant.bordered, + accent: Color(0xFF4C9AFF), + label: 'Bash', + status: ConversationCardStatus.error, + collapsible: true, + collapsedByDefault: true, + collapsedSummary: 'cat nonexistent', + body: Text('cat nonexistent', textDirection: TextDirection.ltr), + ), + ), + ), + ], + ), + ); +} + +Widget _wrap(KernelFixture f, Widget child) => SizedBox( + width: 360, + child: harness(f, child), + ); diff --git a/test/goldens/goldens/linux/conversation_card_merged.png b/test/goldens/goldens/linux/conversation_card_merged.png new file mode 100644 index 00000000..a78ae04d Binary files /dev/null and b/test/goldens/goldens/linux/conversation_card_merged.png differ