From 1ae5cc609cc43b30126f7016ab6f4726ea749908 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Mon, 8 Jun 2026 17:37:38 +0200 Subject: [PATCH] key conversation list items so card state survives result reshapes (T-285) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The conversation ListView.builder built its items (_ConversationTurn, _ActivityCard) with no keys, so Flutter matched the stateful subtrees inside them (ConversationCard collapse/hover/focus; ClideHolderCard expand state) to widgets by POSITION. The visible list reshapes exactly when a tool result lands — T-262 folds a success result into its call card and suppresses the standalone result, errors append a sticky card, clusters re-fold — so after a read/write completed, a card's collapse/hover state (or a cluster's identity) could reattach to the wrong card. Give each list item a stable ValueKey from its identity: sticky item by item.uuid, folded cluster by its first item's uuid (namespaced turn./cluster./ run./step. so the four call sites can't collide), plus super.key on the _ConversationTurn/_ActivityCard constructors. Tests: unfolded cards carry per-item keys; a folded cluster carries its first-item key. Co-Authored-By: Claude Opus 4.8 (1M context) --- .pql/changelog/ticket_history/2026-06.sql | 2 + .pql/changelog/tickets/2026-06.sql | 30 +++++++++++++++ CHANGELOG.md | 5 +++ lib/builtin/claude/src/conversation_view.dart | 9 +++++ .../claude/conversation_view_test.dart | 37 ++++++++++++++++++- 5 files changed, 82 insertions(+), 1 deletion(-) diff --git a/.pql/changelog/ticket_history/2026-06.sql b/.pql/changelog/ticket_history/2026-06.sql index d94b1aac..b4f3b70f 100644 --- a/.pql/changelog/ticket_history/2026-06.sql +++ b/.pql/changelog/ticket_history/2026-06.sql @@ -873,3 +873,5 @@ 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-279', 'status', 'in_progress', 'done', NULL, '2026-06-08 14:12:46', '2026-06-08 14:12:46', '2026-06-08 14:12:46', NULL, '86c50eb88cf56b7e9e9914dcab61b9c3', 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-284', 'status', 'backlog', 'in_progress', NULL, '2026-06-08 14:55:51', '2026-06-08 14:55:51', '2026-06-08 14:55:51', NULL, '088102bf2d6d9337db66c82fd7b17922', 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-284', 'status', 'in_progress', 'done', NULL, '2026-06-08 14:57:15', '2026-06-08 14:57:15', '2026-06-08 14:57:15', NULL, '7f2c5be1e2bb330dac14007350d6dec1', 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-285', 'status', 'backlog', 'in_progress', NULL, '2026-06-08 15:22:35', '2026-06-08 15:22:35', '2026-06-08 15:22:35', NULL, 'd4ed6a44830ac6e7cac90547d9a027a9', 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-285', 'status', 'in_progress', 'done', NULL, '2026-06-08 15:37:29', '2026-06-08 15:37:29', '2026-06-08 15:37:29', NULL, 'b8289629265f19b4f41318863e744804', 1) ON CONFLICT(hash) DO NOTHING; diff --git a/.pql/changelog/tickets/2026-06.sql b/.pql/changelog/tickets/2026-06.sql index ddc05dff..4ccbc4cc 100644 --- a/.pql/changelog/tickets/2026-06.sql +++ b/.pql/changelog/tickets/2026-06.sql @@ -2228,3 +2228,33 @@ Acceptance: 2. With the flag off, overflow still scrolls as today. 3. Toggling the flag at runtime starts/stops the scroll. 4. Test covering the reduced-motion (no-scroll, settles) case.', 'done', 'medium', NULL, NULL, NULL, '2026-06-08 14:55:43', '2026-06-08 14:57:15', NULL, '31d8fd91f6d556a68d35c7d25410d7ec', 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-285', 'bug', 'T-276', 'Conversation ListView items lack keys — card state mis-associates when results stream in', 'The conversation ListView.builder (conversation_view.dart:257) builds its items (_ConversationTurn, _ActivityCard) with NO keys. Each resolves to a stateful subtree (ConversationCard: _collapsed/_hover/focus nodes, with _collapsed set once and never re-synced in didUpdateWidget; ClideHolderCard owns cluster expand state). In a keyless list Flutter matches State to widgets by POSITION, so when the visible list reshapes, State attaches to the wrong logical card. + +The list reshapes exactly when a read/write result lands: T-262 folds a successful result into its call card and suppresses the standalone result via _visibleItems, errors append a sticky card, and activity clusters re-fold. So after a tool completes, collapse/hover state — and for a folded cluster, which tools/summary it shows — can jump to the wrong card. (The success/error mark itself is widget.status, recomputed per build, so it follows the widget; the stateful bits are what mis-associate.) + +Fix: give each ListView item a stable ValueKey from its identity — sticky item by item.uuid, folded cluster by its first item''s uuid (namespaced so sticky vs cluster can''t collide) — plus add super.key to the _ConversationTurn/_ActivityCard constructors. Also key the nested run items in the Agent holder + activity card so sub-agent streams reshape safely. + +Acceptance: +1. Streaming a tool-use then its success result keeps each card''s collapse state pinned to the right card (a card the user expanded stays expanded when a later tool completes). +2. A folded activity cluster keeps its identity/summary across reshapes. +3. Widget test: pump a conversation, toggle one card''s collapse, stream a new result that reshapes the list, assert the toggled card is still the one expanded.', 'backlog', 'high', NULL, NULL, NULL, '2026-06-08 15:22:32', '2026-06-08 15:22:32', NULL, '0d7ef068ea04b9d4302f04b0c3134287', 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-285', 'bug', 'T-276', 'Conversation ListView items lack keys — card state mis-associates when results stream in', 'The conversation ListView.builder (conversation_view.dart:257) builds its items (_ConversationTurn, _ActivityCard) with NO keys. Each resolves to a stateful subtree (ConversationCard: _collapsed/_hover/focus nodes, with _collapsed set once and never re-synced in didUpdateWidget; ClideHolderCard owns cluster expand state). In a keyless list Flutter matches State to widgets by POSITION, so when the visible list reshapes, State attaches to the wrong logical card. + +The list reshapes exactly when a read/write result lands: T-262 folds a successful result into its call card and suppresses the standalone result via _visibleItems, errors append a sticky card, and activity clusters re-fold. So after a tool completes, collapse/hover state — and for a folded cluster, which tools/summary it shows — can jump to the wrong card. (The success/error mark itself is widget.status, recomputed per build, so it follows the widget; the stateful bits are what mis-associate.) + +Fix: give each ListView item a stable ValueKey from its identity — sticky item by item.uuid, folded cluster by its first item''s uuid (namespaced so sticky vs cluster can''t collide) — plus add super.key to the _ConversationTurn/_ActivityCard constructors. Also key the nested run items in the Agent holder + activity card so sub-agent streams reshape safely. + +Acceptance: +1. Streaming a tool-use then its success result keeps each card''s collapse state pinned to the right card (a card the user expanded stays expanded when a later tool completes). +2. A folded activity cluster keeps its identity/summary across reshapes. +3. Widget test: pump a conversation, toggle one card''s collapse, stream a new result that reshapes the list, assert the toggled card is still the one expanded.', 'in_progress', 'high', NULL, NULL, NULL, '2026-06-08 15:22:32', '2026-06-08 15:22:35', NULL, 'f08107f9795fdf61f608e06e339a5002', 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-285', 'bug', 'T-276', 'Conversation ListView items lack keys — card state mis-associates when results stream in', 'The conversation ListView.builder (conversation_view.dart:257) builds its items (_ConversationTurn, _ActivityCard) with NO keys. Each resolves to a stateful subtree (ConversationCard: _collapsed/_hover/focus nodes, with _collapsed set once and never re-synced in didUpdateWidget; ClideHolderCard owns cluster expand state). In a keyless list Flutter matches State to widgets by POSITION, so when the visible list reshapes, State attaches to the wrong logical card. + +The list reshapes exactly when a read/write result lands: T-262 folds a successful result into its call card and suppresses the standalone result via _visibleItems, errors append a sticky card, and activity clusters re-fold. So after a tool completes, collapse/hover state — and for a folded cluster, which tools/summary it shows — can jump to the wrong card. (The success/error mark itself is widget.status, recomputed per build, so it follows the widget; the stateful bits are what mis-associate.) + +Fix: give each ListView item a stable ValueKey from its identity — sticky item by item.uuid, folded cluster by its first item''s uuid (namespaced so sticky vs cluster can''t collide) — plus add super.key to the _ConversationTurn/_ActivityCard constructors. Also key the nested run items in the Agent holder + activity card so sub-agent streams reshape safely. + +Acceptance: +1. Streaming a tool-use then its success result keeps each card''s collapse state pinned to the right card (a card the user expanded stays expanded when a later tool completes). +2. A folded activity cluster keeps its identity/summary across reshapes. +3. Widget test: pump a conversation, toggle one card''s collapse, stream a new result that reshapes the list, assert the toggled card is still the one expanded.', 'done', 'high', NULL, NULL, NULL, '2026-06-08 15:22:32', '2026-06-08 15:37:29', NULL, '0d20b26d5b7a12af99c26cb2a849e123', 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 a4e3b4e6..378fab2e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -198,6 +198,11 @@ heading, and (b) bumping `pubspec.yaml` `version:` in the same commit. ### Fixed +- Conversation cards no longer **mis-associate their state** when the message + list reshapes as a tool result streams in. The list items now carry stable + per-item keys, so a card you expanded (or its hover/cluster state) stays + pinned to its own message instead of jumping to a neighbour when a read/write + completes and folds its result in. (T-285) - The status-bar footer marquee now **honours reduced motion**: when the OS reduce-motion setting (`MediaQuery.disableAnimations`) is on, a long status line no longer scrolls — it renders statically (clipped) — matching the turn diff --git a/lib/builtin/claude/src/conversation_view.dart b/lib/builtin/claude/src/conversation_view.dart index 6603d819..4c4fe560 100644 --- a/lib/builtin/claude/src/conversation_view.dart +++ b/lib/builtin/claude/src/conversation_view.dart @@ -260,8 +260,12 @@ class _ConversationViewState extends State { itemCount: groups.length, itemBuilder: (context, i) { final g = groups[i]; + // Stable keys pin each card's State (collapse/hover, cluster expand) + // to its logical item, so streaming result updates that reshape the + // visible list don't reattach State to the wrong card (T-285). return switch (g) { StickyItem(:final item) => _ConversationTurn( + key: ValueKey('turn.${item.uuid}'), item: item, tokens: tokens, toolUseOutcomes: widget.toolUseOutcomes, @@ -271,6 +275,7 @@ class _ConversationViewState extends State { runByToolUseId: fold.runByToolUseId, ), FoldedCluster(:final items) => _ActivityCard( + key: ValueKey('cluster.${items.first.uuid}'), items: items, tokens: tokens, toolUseOutcomes: widget.toolUseOutcomes, @@ -310,6 +315,7 @@ void _openRecord(BuildContext context, String id) { /// One conversation item, rendered by kind. class _ConversationTurn extends StatelessWidget { const _ConversationTurn({ + super.key, required this.item, required this.tokens, this.toolUseOutcomes = const {}, @@ -543,6 +549,7 @@ class _ConversationTurn extends StatelessWidget { children: [ for (final r in runItems) _ConversationTurn( + key: ValueKey('run.${r.uuid}'), item: r, tokens: tokens, toolUseOutcomes: toolUseOutcomes, @@ -650,6 +657,7 @@ class _ConversationTurn extends StatelessWidget { /// background toggles collapse. Stateless — the holder owns the expand state. class _ActivityCard extends StatelessWidget { const _ActivityCard({ + super.key, required this.items, required this.tokens, required this.toolUseOutcomes, @@ -676,6 +684,7 @@ class _ActivityCard extends StatelessWidget { children: [ for (final item in items) _ConversationTurn( + key: ValueKey('step.${item.uuid}'), item: item, tokens: tokens, toolUseOutcomes: toolUseOutcomes, diff --git a/test/builtin/claude/conversation_view_test.dart b/test/builtin/claude/conversation_view_test.dart index 465e6655..20f338d6 100644 --- a/test/builtin/claude/conversation_view_test.dart +++ b/test/builtin/claude/conversation_view_test.dart @@ -15,7 +15,7 @@ import 'package:clide/builtin/claude/src/transcript_reader.dart'; import 'package:clide/kernel/src/events/message_bus.dart'; import 'package:clide/widgets/widgets.dart'; import 'package:flutter/services.dart'; -import 'package:flutter/widgets.dart' show Image, FileImage; +import 'package:flutter/widgets.dart' show Image, FileImage, ValueKey; import 'package:flutter_test/flutter_test.dart'; import '../../helpers/kernel_fixture.dart'; @@ -172,6 +172,41 @@ void main() { expect(find.text('Waiting for Claude…'), findsOneWidget); }); + testWidgets('unfolded conversation cards carry stable per-item identity keys (T-285)', (tester) async { + tester.view.physicalSize = const Size(900, 800); + tester.view.devicePixelRatio = 1.0; + addTearDown(tester.view.resetPhysicalSize); + addTearDown(tester.view.resetDevicePixelRatio); + final stream = StreamController.broadcast(); + final c = ConversationController(stream: stream.stream); + addTearDown(c.dispose); + await tester.pumpWidget(harness(f, 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(); + // Each top-level card is keyed by its item uuid so a streaming reshape + // pins card State (collapse/hover) to its logical item, not its position. + expect(find.byKey(const ValueKey('turn.A')), findsOneWidget); + expect(find.byKey(const ValueKey('turn.B')), findsOneWidget); + }); + + testWidgets('a folded activity cluster carries a stable identity key (T-285)', (tester) async { + tester.view.physicalSize = const Size(900, 800); + tester.view.devicePixelRatio = 1.0; + addTearDown(tester.view.resetPhysicalSize); + addTearDown(tester.view.resetDevicePixelRatio); + final stream = StreamController.broadcast(); + final c = ConversationController(stream: stream.stream); + addTearDown(c.dispose); + await tester.pumpWidget(harness(f, ConversationView(controller: c, foldLevel: FoldLevel.tools))); + stream.add(AssistantToolUse(uuid: 'A', timestamp: _t, isSidechain: false, toolUseId: 'A', name: 'Bash', input: const {'command': 'echo a'})); + stream.add(AssistantToolUse(uuid: 'B', timestamp: _t, isSidechain: false, toolUseId: 'B', name: 'Read', input: const {'file_path': '/a'})); + await tester.pumpAndSettle(); + // The cluster is keyed by its first item's uuid, so re-folding never + // reattaches the holder's expand State to the wrong cluster by position. + expect(find.byKey(const ValueKey('cluster.A')), findsOneWidget); + }); + testWidgets('clicking a bare T-ref in a message opens the tickets reader (T-279)', (tester) async { Message? opened; final sub = f.services.messages.subscribe(publisher: 'builtin.tickets', channel: 'selection').listen((m) => opened = m);