From b3bbc7850b70215637c5285c155198e71a2be56e Mon Sep 17 00:00:00 2001 From: jaylfc Date: Tue, 23 Jun 2026 19:18:36 +0100 Subject: [PATCH 1/2] feat(decisions): surface the L1 supersession lineage in the archive The supersede backend (#1339) records a parent_decision_id chain and serves it at GET /api/decisions/{id}/history, but the app only showed a flat 'Superseded' label with no way to see what replaced what. Add an expandable history trail on answered cards that have a parent (a revision): it lazily fetches the chain on first expand (oldest first) and renders each step's question + chosen answer + when. Originals with no parent show no affordance. Read-only, reuses the shell tokens already in the file; vitest covers the expand/fetch/render path and the no-parent case. Visual check against the live app deferred to a live session (same as the canvas read-only slices). --- desktop/src/apps/DecisionsApp.test.tsx | 65 ++++++++++++++++++++ desktop/src/apps/DecisionsApp.tsx | 82 ++++++++++++++++++++++++++ 2 files changed, 147 insertions(+) diff --git a/desktop/src/apps/DecisionsApp.test.tsx b/desktop/src/apps/DecisionsApp.test.tsx index 3394a6fb..24c949b8 100644 --- a/desktop/src/apps/DecisionsApp.test.tsx +++ b/desktop/src/apps/DecisionsApp.test.tsx @@ -161,4 +161,69 @@ describe("DecisionsApp", () => { await waitFor(() => expect(screen.getByText("Excalidraw")).toBeTruthy()); }); + + it("offers no history affordance for an original (no parent) decision", async () => { + const answered = { + ...singleSelect, + status: "answered", + answer: { value: "excalidraw", answered_by: "jay" }, + }; + vi.stubGlobal( + "fetch", + mockFetch({ + "GET /api/decisions?status=pending": { ok: true, body: [] }, + "GET /api/decisions?status=answered": { ok: true, body: [answered] }, + }), + ); + render(); + await flush(); + fireEvent.click(screen.getByRole("button", { name: /archive/i })); + await flush(); + + await waitFor(() => expect(screen.getByText("Excalidraw")).toBeTruthy()); + expect(screen.queryByRole("button", { name: /view history/i })).toBeNull(); + }); + + it("loads and renders the supersession lineage on demand for a revision", async () => { + const revision = { + ...singleSelect, + id: "dec-2", + status: "answered", + question: "Which canvas engine should replace tldraw? (revised)", + answer: { value: "excalidraw", answered_by: "jay" }, + parent_decision_id: "dec-1", + }; + const original = { + ...singleSelect, + id: "dec-1", + status: "superseded", + question: "Which canvas engine should replace tldraw?", + }; + const fetchMock = mockFetch({ + "GET /api/decisions?status=pending": { ok: true, body: [] }, + "GET /api/decisions?status=answered": { ok: true, body: [revision] }, + "GET /api/decisions/dec-2/history": { + ok: true, + body: { items: [original, revision] }, + }, + }); + vi.stubGlobal("fetch", fetchMock); + render(); + await flush(); + fireEvent.click(screen.getByRole("button", { name: /archive/i })); + await flush(); + + const historyBtn = await waitFor(() => + screen.getByRole("button", { name: /view history/i }), + ); + fireEvent.click(historyBtn); + await flush(); + + // The chain is fetched lazily on expand and rendered oldest first. + const historyCall = fetchMock.mock.calls.find( + (c) => c[0] === "/api/decisions/dec-2/history", + ); + expect(historyCall).toBeTruthy(); + await waitFor(() => expect(screen.getByText(/superseded/i)).toBeTruthy()); + }); }); diff --git a/desktop/src/apps/DecisionsApp.tsx b/desktop/src/apps/DecisionsApp.tsx index 126ce390..2f3b575f 100644 --- a/desktop/src/apps/DecisionsApp.tsx +++ b/desktop/src/apps/DecisionsApp.tsx @@ -8,6 +8,7 @@ import { X, CheckCircle2, AlertCircle, + History, } from "lucide-react"; import { Button, Textarea } from "@/components/ui"; @@ -45,6 +46,10 @@ interface Decision { answer?: DecisionAnswer | null; created_at: number | string; deadline?: number | null; + // The decision this one supersedes (L1). Present on a revision; absent on an + // original. When set, the supersession lineage can be walked via the history + // endpoint. + parent_decision_id?: string | null; } // created_at is stored as an epoch-seconds REAL on the backend, but the API @@ -320,6 +325,82 @@ function answerLabel(decision: Decision): string { return labels.join(", "); } +/** Expandable supersession lineage for a revised decision. Fetches the chain + * lazily on first expand so the archive list does not issue a request per card. + * The endpoint returns the chain oldest first. */ +function HistoryTrail({ decisionId }: { decisionId: string }) { + const [open, setOpen] = useState(false); + const [chain, setChain] = useState(null); + const [loading, setLoading] = useState(false); + const [error, setError] = useState(null); + + async function toggle() { + if (open) { + setOpen(false); + return; + } + setOpen(true); + if (chain || loading) return; + setLoading(true); + setError(null); + try { + const res = await fetch(`/api/decisions/${decisionId}/history`); + if (!res.ok) throw new Error("Could not load history."); + setChain(asDecisionList(await res.json())); + } catch (e) { + setError(e instanceof Error ? e.message : "Could not load history."); + } finally { + setLoading(false); + } + } + + return ( +
+ + {open && ( +
+ {loading && ( +

Loading history...

+ )} + {error && ( +

+ {error} +

+ )} + {chain && chain.length > 0 && ( +
    + {chain.map((d, i) => ( +
  1. + + {i + 1}. + {d.question} + + + {d.status === "superseded" ? "superseded" : answerLabel(d)} + {" ยท "} + {relativeTime(d.answer?.answered_at ?? d.created_at)} + +
    2. + ))} +
+ )} + {chain && chain.length === 0 && !loading && ( +

No earlier versions.

+ )} +
+ )} +
+ ); +} + function AnsweredCard({ decision }: { decision: Decision }) { return (
  • @@ -343,6 +424,7 @@ function AnsweredCard({ decision }: { decision: Decision }) { {decision.status === "superseded" ? "Superseded" : answerLabel(decision)} + {decision.parent_decision_id && }
    • ); } From 9b37a7979607522697f1cbeaf90b34ec941bd166 Mon Sep 17 00:00:00 2001 From: jaylfc Date: Tue, 23 Jun 2026 19:22:56 +0100 Subject: [PATCH 2/2] fix(decisions): guard history-trail state updates after unmount Fold #1416 review: an archive card can unmount mid-fetch (tab switch or list refresh) while /history is in flight, so the post-await setState would warn and leak. Track liveness with a ref cleared on unmount and gate every post-await setState on it (gitar Bug; kilo concurs). Also assert the oldest-first ordering the description claims, via a testid on the history list (kilo). --- desktop/src/apps/DecisionsApp.test.tsx | 24 ++++++++++++++++++------ desktop/src/apps/DecisionsApp.tsx | 23 ++++++++++++++++++----- 2 files changed, 36 insertions(+), 11 deletions(-) diff --git a/desktop/src/apps/DecisionsApp.test.tsx b/desktop/src/apps/DecisionsApp.test.tsx index 24c949b8..5e6a82fc 100644 --- a/desktop/src/apps/DecisionsApp.test.tsx +++ b/desktop/src/apps/DecisionsApp.test.tsx @@ -1,4 +1,11 @@ -import { render, screen, fireEvent, act, waitFor } from "@testing-library/react"; +import { + render, + screen, + fireEvent, + act, + waitFor, + within, +} from "@testing-library/react"; import { describe, it, expect, vi, beforeEach } from "vitest"; import { DecisionsApp } from "./DecisionsApp"; @@ -184,12 +191,12 @@ describe("DecisionsApp", () => { expect(screen.queryByRole("button", { name: /view history/i })).toBeNull(); }); - it("loads and renders the supersession lineage on demand for a revision", async () => { + it("loads and renders the supersession lineage oldest first on demand", async () => { const revision = { ...singleSelect, id: "dec-2", status: "answered", - question: "Which canvas engine should replace tldraw? (revised)", + question: "Revised tldraw replacement pick", answer: { value: "excalidraw", answered_by: "jay" }, parent_decision_id: "dec-1", }; @@ -197,7 +204,7 @@ describe("DecisionsApp", () => { ...singleSelect, id: "dec-1", status: "superseded", - question: "Which canvas engine should replace tldraw?", + question: "Original tldraw replacement pick", }; const fetchMock = mockFetch({ "GET /api/decisions?status=pending": { ok: true, body: [] }, @@ -219,11 +226,16 @@ describe("DecisionsApp", () => { fireEvent.click(historyBtn); await flush(); - // The chain is fetched lazily on expand and rendered oldest first. + // The chain is fetched lazily only on expand. const historyCall = fetchMock.mock.calls.find( (c) => c[0] === "/api/decisions/dec-2/history", ); expect(historyCall).toBeTruthy(); - await waitFor(() => expect(screen.getByText(/superseded/i)).toBeTruthy()); + + // It must render oldest first: the original (superseded) before the revision. + const trail = await waitFor(() => screen.getByTestId("decision-history")); + const steps = within(trail).getAllByRole("listitem"); + expect(steps[0].textContent).toContain("Original tldraw replacement pick"); + expect(steps[1].textContent).toContain("Revised tldraw replacement pick"); }); }); diff --git a/desktop/src/apps/DecisionsApp.tsx b/desktop/src/apps/DecisionsApp.tsx index 2f3b575f..f709673b 100644 --- a/desktop/src/apps/DecisionsApp.tsx +++ b/desktop/src/apps/DecisionsApp.tsx @@ -1,4 +1,4 @@ -import { useState, useEffect, useCallback } from "react"; +import { useState, useEffect, useCallback, useRef } from "react"; import { Inbox, Sparkles, @@ -333,6 +333,16 @@ function HistoryTrail({ decisionId }: { decisionId: string }) { const [chain, setChain] = useState(null); const [loading, setLoading] = useState(false); const [error, setError] = useState(null); + // Guard against a state update after the card unmounts mid-fetch (an archive + // card can unmount on a tab switch or list refresh while /history is still in + // flight). + const aliveRef = useRef(true); + useEffect(() => { + aliveRef.current = true; + return () => { + aliveRef.current = false; + }; + }, []); async function toggle() { if (open) { @@ -346,11 +356,14 @@ function HistoryTrail({ decisionId }: { decisionId: string }) { try { const res = await fetch(`/api/decisions/${decisionId}/history`); if (!res.ok) throw new Error("Could not load history."); - setChain(asDecisionList(await res.json())); + const items = asDecisionList(await res.json()); + if (aliveRef.current) setChain(items); } catch (e) { - setError(e instanceof Error ? e.message : "Could not load history."); + if (aliveRef.current) { + setError(e instanceof Error ? e.message : "Could not load history."); + } } finally { - setLoading(false); + if (aliveRef.current) setLoading(false); } } @@ -376,7 +389,7 @@ function HistoryTrail({ decisionId }: { decisionId: string }) {

    )} {chain && chain.length > 0 && ( -
      +
        {chain.map((d, i) => (