Skip to content

Commit f1fee40

Browse files
committed
refactor: address review feedback on the pinned PlanCard
Address code review feedback on cacdec3 (pin the latest ACP plan above the chat transcript): - SessionChatPane.svelte: key the PlanCard on session?.id so the expanded/collapsed toggle resets when the pane is reused for a different session, letting defaultExpanded apply per session instead of a collapse in one session leaking into the next. - acpTranscript.ts: drop the redundant Array.isArray(rawEntries) re-check in latestPlan since arrayProp already returns unknown[] | null; use a plain null check to match latestAvailableCommands. - acpTranscript.ts / acpTranscript.test.ts: remove the unused priority field from PlanEntry, its parsing, and the test assertions, since PlanCard never renders it. One raw input in the tests keeps a priority prop to cover that unparsed props are dropped. Verified with vitest (682 passing) and svelte-check (0 errors). Signed-off-by: Matt Toohey <contact@matttoohey.com>
1 parent cacdec3 commit f1fee40

3 files changed

Lines changed: 12 additions & 10 deletions

File tree

apps/staged/src/lib/features/sessions/SessionChatPane.svelte

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1737,9 +1737,13 @@
17371737
bind:this={modalElement}
17381738
class={`session-chat-pane ${compact ? 'compact' : ''} ${dragOver ? 'drag-over' : ''}`}
17391739
>
1740-
<!-- Latest plan, pinned above the scrollable transcript -->
1740+
<!-- Latest plan, pinned above the scrollable transcript. Keyed on the
1741+
session so the expanded/collapsed toggle resets when this pane is
1742+
reused for a different session. -->
17411743
{#if plan}
1742-
<PlanCard entries={plan} defaultExpanded={!viewport.isMobile && !compact} />
1744+
{#key session?.id}
1745+
<PlanCard entries={plan} defaultExpanded={!viewport.isMobile && !compact} />
1746+
{/key}
17431747
{/if}
17441748

17451749
<!-- Messages area -->

apps/staged/src/lib/features/sessions/acpTranscript.test.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -718,8 +718,8 @@ describe('latestPlan', () => {
718718
]);
719719

720720
expect(plan).toEqual([
721-
{ content: 'Check UI', status: 'completed', priority: 'high' },
722-
{ content: 'Fix bug', status: 'in_progress', priority: null },
721+
{ content: 'Check UI', status: 'completed' },
722+
{ content: 'Fix bug', status: 'in_progress' },
723723
]);
724724
});
725725

@@ -766,9 +766,9 @@ describe('latestPlan', () => {
766766
]);
767767

768768
expect(plan).toEqual([
769-
{ content: 'Mystery step', status: 'pending', priority: null },
770-
{ content: 'Statusless step', status: 'pending', priority: null },
771-
{ content: 'Failed step', status: 'failed', priority: null },
769+
{ content: 'Mystery step', status: 'pending' },
770+
{ content: 'Statusless step', status: 'pending' },
771+
{ content: 'Failed step', status: 'failed' },
772772
]);
773773
});
774774
});

apps/staged/src/lib/features/sessions/acpTranscript.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,6 @@ export type PlanEntryStatus = 'pending' | 'in_progress' | 'completed' | 'failed'
278278
export interface PlanEntry {
279279
content: string;
280280
status: PlanEntryStatus;
281-
priority: string | null;
282281
}
283282

284283
/**
@@ -293,7 +292,7 @@ export function latestPlan(metadataMessages: SessionMessage[]): PlanEntry[] | nu
293292
.find((message) => message.acpEventKind === 'plan_update');
294293
if (!latest) return null;
295294
const rawEntries = arrayProp(latest.acpContent, 'entries');
296-
if (!Array.isArray(rawEntries)) return null;
295+
if (!rawEntries) return null;
297296

298297
const entries = rawEntries
299298
.map((entry) => {
@@ -302,7 +301,6 @@ export function latestPlan(metadataMessages: SessionMessage[]): PlanEntry[] | nu
302301
return {
303302
content,
304303
status: normalizePlanEntryStatus(stringProp(entry, 'status')),
305-
priority: stringProp(entry, 'priority'),
306304
};
307305
})
308306
.filter((entry): entry is PlanEntry => entry !== null);

0 commit comments

Comments
 (0)