Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 78 additions & 1 deletion desktop/src/apps/DecisionsApp.test.tsx
Original file line number Diff line number Diff line change
@@ -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";

Expand Down Expand Up @@ -161,4 +168,74 @@ 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(<DecisionsApp windowId="w1" />);
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 oldest first on demand", async () => {
const revision = {
...singleSelect,
id: "dec-2",
status: "answered",
question: "Revised tldraw replacement pick",
answer: { value: "excalidraw", answered_by: "jay" },
parent_decision_id: "dec-1",
};
const original = {
...singleSelect,
id: "dec-1",
status: "superseded",
question: "Original tldraw replacement pick",
};
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(<DecisionsApp windowId="w1" />);
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 only on expand.
const historyCall = fetchMock.mock.calls.find(
(c) => c[0] === "/api/decisions/dec-2/history",
);
expect(historyCall).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");
});
});
97 changes: 96 additions & 1 deletion desktop/src/apps/DecisionsApp.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useState, useEffect, useCallback } from "react";
import { useState, useEffect, useCallback, useRef } from "react";
import {
Inbox,
Sparkles,
Expand All @@ -8,6 +8,7 @@ import {
X,
CheckCircle2,
AlertCircle,
History,
} from "lucide-react";
import { Button, Textarea } from "@/components/ui";

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -320,6 +325,95 @@ 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<Decision[] | null>(null);
const [loading, setLoading] = useState(false);
const [error, setError] = useState<string | null>(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) {
setOpen(false);
return;
}
setOpen(true);
if (chain || loading) return;
setLoading(true);
setError(null);
try {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SUGGESTION: No abort handling on the in-flight history fetch. If the user collapses the panel, switches tabs, or the card unmounts (e.g. silent refresh re-renders the archive after an answer in the same view) before the request resolves, setLoading/setChain/setError will still run on an unmounted component. Consider an AbortController per fetch and check controller.signal.aborted (or a mounted ref) before calling the setters in the finally block.


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

const res = await fetch(`/api/decisions/${decisionId}/history`);
if (!res.ok) throw new Error("Could not load history.");
const items = asDecisionList(await res.json());
if (aliveRef.current) setChain(items);
} catch (e) {
if (aliveRef.current) {
setError(e instanceof Error ? e.message : "Could not load history.");
}
} finally {
if (aliveRef.current) setLoading(false);
}
}

return (
<div className="flex flex-col gap-1.5">
<button
type="button"
onClick={toggle}
aria-expanded={open}
className="flex items-center gap-1 self-start text-xs font-medium text-shell-text-secondary transition-colors hover:text-shell-text"
>
<History size={12} className="shrink-0" />
{open ? "Hide history" : "View history"}
</button>
{open && (
<div className="flex flex-col gap-1.5 border-l border-shell-border pl-3">
{loading && (
<p className="text-xs text-shell-text-tertiary">Loading history...</p>
)}
{error && (
<p className="text-xs text-red-400" role="alert">
{error}
</p>
)}
{chain && chain.length > 0 && (
<ol className="flex flex-col gap-1.5" data-testid="decision-history">
{chain.map((d, i) => (
<li key={d.id} className="flex flex-col gap-0.5 text-xs">
<span className="flex items-start gap-1.5 text-shell-text-secondary">
<span className="shrink-0 text-shell-text-tertiary">{i + 1}.</span>
{d.question}
</span>
<span className="pl-4 text-shell-text-tertiary">
{d.status === "superseded" ? "superseded" : answerLabel(d)}
{" · "}
{relativeTime(d.answer?.answered_at ?? d.created_at)}
</span>
</li>
))}
</ol>
)}
{chain && chain.length === 0 && !loading && (
<p className="text-xs text-shell-text-tertiary">No earlier versions.</p>
)}
</div>
)}
</div>
);
}

function AnsweredCard({ decision }: { decision: Decision }) {
return (
<li className="flex flex-col gap-2 rounded-xl border border-shell-border bg-shell-surface p-4">
Expand All @@ -343,6 +437,7 @@ function AnsweredCard({ decision }: { decision: Decision }) {
{decision.status === "superseded" ? "Superseded" : answerLabel(decision)}
</span>
</div>
{decision.parent_decision_id && <HistoryTrail decisionId={decision.id} />}
</li>
);
}
Expand Down
Loading