Fix navigation to repeated stage occurrences

This commit is contained in:
Scott Werner 2026-10-20 13:02:46 -04:00
parent 59342f7e99
commit 636fff0599
11 changed files with 401 additions and 19 deletions

View file

@ -3,6 +3,7 @@ import { Link } from "react-router";
import { StageState, type RunStage } from "@qltysh/fabro-api-client";
import { HoverCard, PopoverHeader, PopoverRow, PopoverRows } from "./ui";
import { stagePath } from "../lib/stage-routes";
import { isVisibleStage } from "../data/runs";
import { formatAbsoluteTs, formatDurationMs } from "../lib/format";
import {
@ -150,7 +151,7 @@ function stageRow(runId: string, stage: RunStage, nowMs: number): Row | null {
endMs,
durationMs,
barClass: stageBarClass(stage.status),
href: `/runs/${runId}/stages/${encodeURIComponent(stage.id)}`,
href: stagePath(runId, stage.id),
popover: stagePopover(stage, durationMs, inFlight),
};
}

View file

@ -1,6 +1,7 @@
import { Link } from "react-router";
import type { StageState } from "@qltysh/fabro-api-client";
import { stagePath } from "../lib/stage-routes";
import { formatTokenCount } from "../lib/format";
import {
formatStageLabel,
@ -106,7 +107,7 @@ export function StagePopover({ runId, stage, duration }: StagePopoverProps) {
{stage.resumedFromStageId && (
<PopoverRow label="Resumed from">
<Link
to={`/runs/${runId}/stages/${encodeURIComponent(stage.resumedFromStageId)}`}
to={stagePath(runId, stage.resumedFromStageId)}
className="font-mono text-teal-500 hover:underline"
>
{stage.resumedFromStageId}

View file

@ -5,6 +5,7 @@ import {
ArrowsRightLeftIcon,
} from "@heroicons/react/20/solid";
import { stagePath } from "../../lib/stage-routes";
import type { Stage } from "../stage-sidebar";
import { StageMetaBar } from "./meta-bar";
import type { EdgeSelection } from "./helpers";
@ -67,7 +68,7 @@ export function ConditionalDecision({
{edge ? (
targetStage ? (
<Link
to={`/runs/${runId}/stages/${targetStage.id}`}
to={stagePath(runId, targetStage.id)}
className="group rounded-md bg-teal-500/10 px-3 py-2 text-center transition-colors hover:bg-teal-500/20 focus-visible:outline-2 focus-visible:-outline-offset-2 focus-visible:outline-teal-500"
>
<div className="text-[10px] font-medium uppercase tracking-wider text-teal-500">

View file

@ -119,8 +119,8 @@ describe("ParallelChildren", () => {
expect(rows[1]).toContain("Running");
expect(rows[1]).toContain("review_opus");
expect(hrefs(renderer)).toEqual([
"/runs/run-1/stages/review_glm@1",
"/runs/run-1/stages/review_opus@1",
"/runs/run-1/stages/review_glm%401",
"/runs/run-1/stages/review_opus%401",
]);
expect(statValue(renderer, "Succeeded")).toBe("1");
expect(statValue(renderer, "Failed")).toBe("0");
@ -154,7 +154,7 @@ describe("ParallelChildren", () => {
],
);
expect(hrefs(renderer)).toEqual(["/runs/run-1/stages/review_glm@1"]);
expect(hrefs(renderer)).toEqual(["/runs/run-1/stages/review_glm%401"]);
});
test("shows a late-starting branch when lower indexes have no stage yet", () => {
@ -168,7 +168,7 @@ describe("ParallelChildren", () => {
"PendingBranch 2",
"Runningreview_opus",
]);
expect(hrefs(renderer)).toEqual(["/runs/run-1/stages/review_opus@1"]);
expect(hrefs(renderer)).toEqual(["/runs/run-1/stages/review_opus%401"]);
expect(statValue(renderer, "Branches")).toBe("3");
});
@ -192,7 +192,7 @@ describe("ParallelChildren", () => {
);
expect(branchRowText(renderer)).toEqual(["Runningreview_glm@2"]);
expect(hrefs(renderer)).toEqual(["/runs/run-1/stages/review_glm@2"]);
expect(hrefs(renderer)).toEqual(["/runs/run-1/stages/review_glm%402"]);
});
test("keeps duplicate branch targets in index order and only links recorded stages", () => {
@ -208,7 +208,7 @@ describe("ParallelChildren", () => {
expect(rows).toHaveLength(2);
expect(rows[0]).toContain("Succeeded");
expect(rows[1]).toContain("Failed");
expect(hrefs(renderer)).toEqual(["/runs/run-1/stages/review@1"]);
expect(hrefs(renderer)).toEqual(["/runs/run-1/stages/review%401"]);
});
test("renders a completed result without a matching stage as an unlinked row", () => {

View file

@ -3,6 +3,7 @@ import { Link } from "react-router";
import { ArrowTopRightOnSquareIcon } from "@heroicons/react/20/solid";
import { StageState } from "@qltysh/fabro-api-client";
import { stagePath } from "../../lib/stage-routes";
import type { Stage } from "../stage-sidebar";
import { formatStageLabel, stageStatusLabel, stageStatusTone } from "../../lib/stage-sidebar";
import { StageMetaBar } from "./meta-bar";
@ -82,7 +83,7 @@ function ChildRow({
<li className="flex items-center gap-3 px-4 py-2.5">
{row.stageId ? (
<Link
to={`/runs/${runId}/stages/${row.stageId}`}
to={stagePath(runId, row.stageId)}
className="group flex flex-1 items-center gap-3 rounded -m-1 p-1 transition-colors hover:bg-overlay focus-visible:bg-overlay focus-visible:outline-2 focus-visible:-outline-offset-2 focus-visible:outline-teal-500"
>
{inner}

View file

@ -18,6 +18,7 @@ import {
MapIcon,
PaperClipIcon,
} from "@heroicons/react/24/outline";
import { stagePath } from "../lib/stage-routes";
import { formatDurationSecs } from "../lib/format";
import { ACTIVE_STAGE_STATES, formatStageLabel, type Stage } from "../lib/stage-sidebar";
import { elapsedSecsSince, useTickingNow } from "../lib/time";
@ -207,7 +208,7 @@ export function StageSidebar({ stages, runId, selectedStageId, activeLink }: Sta
return (
<SidebarRow
key={stage.id}
to={`/runs/${runId}/stages/${stage.id}`}
to={stagePath(runId, stage.id)}
icon={config.icon}
iconClass={config.color}
label={formatStageLabel(stage)}

View file

@ -0,0 +1,19 @@
/** Build a stage link from the original occurrence ID, never an encoded ID. */
export function stagePath(runId: string, stageId: string): string {
return `/runs/${encodeURIComponent(runId)}/stages/${encodeURIComponent(stageId)}`;
}
/**
* Read the original URL segment once. React Router's params replace literal
* `%2F` with `/` after decoding, so they cannot distinguish those stage IDs.
* Pass useLocation().pathname, which excludes the query and fragment.
*/
export function stageIdFromPathname(pathname: string): string | undefined {
const segment = pathname.match(/\/stages\/([^/]+)\/*$/i)?.[1];
if (segment === undefined) return undefined;
try {
return decodeURIComponent(segment);
} catch {
return undefined;
}
}

View file

@ -1,5 +1,6 @@
import { useCallback, useEffect, useMemo, useRef, useState } from "react";
import { useNavigate, useParams } from "react-router";
import { stagePath } from "../lib/stage-routes";
import { ApiError } from "../lib/api-client";
import { useRun, useRunGraph, useRunGraphSource, useRunStages } from "../lib/queries";
import { FloatingTooltip } from "../components/floating-tooltip";
@ -76,7 +77,9 @@ export default function RunOverview() {
const [hoveredNode, setHoveredNode] = useState<RunGraphNodeHover | null>(null);
const openStage = useCallback(
(stageId: string) => navigate(`/runs/${id}/stages/${stageId}`),
(stageId: string) => {
if (id) navigate(stagePath(id, stageId));
},
[id, navigate],
);
useAnnotatedRunGraphSvg({

View file

@ -177,8 +177,8 @@ describe("a two-branch parallel run", () => {
/>,
);
expect(html).toContain("Branches");
expect(html).toContain("/runs/run-1/stages/a@1");
expect(html).toContain("/runs/run-1/stages/b@1");
expect(html).toContain("/runs/run-1/stages/a%401");
expect(html).toContain("/runs/run-1/stages/b%401");
expect((html.match(/Succeeded/g) ?? []).length).toBeGreaterThanOrEqual(2);
});
@ -274,7 +274,7 @@ describe("a human gate answered through the API", () => {
runId="run-1"
/>,
);
expect(html).toContain("/runs/run-1/stages/no@1");
expect(html).toContain("/runs/run-1/stages/no%401");
expect(html).not.toContain("No target");
});

View file

@ -0,0 +1,331 @@
import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test";
import type { ReactNode } from "react";
import TestRenderer, { act } from "react-test-renderer";
import { createMemoryRouter, Outlet, RouterProvider } from "react-router";
import { loadPetriFixture } from "../lib/petri-fixtures";
import { makeStage, setupReactTestEnv, textContent } from "../lib/test-utils";
import { ParallelChildren } from "../components/stage-renderers/parallel-children";
import { ConditionalDecision } from "../components/stage-renderers/conditional-decision";
import { StagePopover } from "../components/stage-popover";
import { RunWaterfall } from "../components/run-waterfall";
const ids = [
"reviewer@1",
"reviewer/e2@1",
"reviewer/50%@1",
"reviewer%20x@1",
"reviewer%2Fe2@1",
"reviewer%252Fe2@1",
"reviewer?x#y@1",
"réview/日本@1",
];
const fixture = loadPetriFixture("command");
const stages = ids.map((id) => ({
id,
name: id.slice(0, -2),
node_id: id.slice(0, -2),
visit: 1,
handler: "command",
status: "succeeded",
usage: fixture.projection.stages["say@1"].usage,
}));
const projection = {
...fixture.projection,
stages: Object.fromEntries(
ids.map((id) => [id, fixture.projection.stages["say@1"]]),
),
};
// Keep real Petri records and give each occurrence a visibly distinct command.
const stream = ids.flatMap((id) =>
fixture.stream
.filter(
(entry) =>
entry.kind === "petri" && entry.item.subject?.node?.name === "say",
)
.map((entry) => {
const copy = structuredClone(entry);
copy.item.subject.node.name = id.slice(0, -2);
copy.item.subject.node.meta.script = `echo selected:${id}`;
return copy;
}),
);
let loading = false;
let available = stages;
const logRequests: string[] = [];
mock.module("../lib/queries", () => ({
useRunGraph: () => ({ data: undefined }),
useRunGraphSource: () => ({ data: undefined }),
useRun: () => ({ data: undefined }),
useRunStages: () => ({
data: loading ? undefined : { data: available },
isLoading: loading,
}),
useRunState: () => ({ data: projection }),
useRunStream: () => ({ data: stream }),
useRunStageContextWindow: () => ({ data: undefined }),
useRunStageLog: (_runId: string, stageId: string) => {
logRequests.push(stageId);
const output = Buffer.from(`output:${stageId}`, "utf8");
return {
data: {
bytes_base64: output.toString("base64"),
next_offset: output.length,
total_bytes: output.length,
},
};
},
}));
mock.module("../components/run-summary-panel", () => ({
RunSummaryPanel: () => null,
}));
let graphClick: (id: string) => void;
mock.module("../hooks/use-annotated-run-graph-svg", () => ({
useAnnotatedRunGraphSvg: ({
onStageClick,
}: {
onStageClick: (id: string) => void;
}) => {
graphClick = onStageClick;
},
}));
const { default: RunOverview } = await import("./run-overview");
const { default: RunStages } = await import("./run-stages");
let renderer: TestRenderer.ReactTestRenderer;
let teardown: () => void;
beforeEach(() => {
teardown = setupReactTestEnv();
loading = false;
available = stages;
logRequests.length = 0;
});
afterEach(() => {
if (renderer) act(() => renderer.unmount());
teardown();
});
function render(entry = "/runs/run-1/stages", source?: ReactNode) {
const router = createMemoryRouter(
[
{
path: "/runs/:id",
element: <Outlet />,
children: [
{ index: true, element: source ?? <RunOverview /> },
{ path: "stages", element: <RunStages /> },
{ path: "stages/:stageId", element: <RunStages /> },
],
},
],
{ initialEntries: [entry] },
);
act(() => {
renderer = TestRenderer.create(<RouterProvider router={router} />);
});
return router;
}
async function clickStage(id: string) {
const link = renderer.root
.findAllByType("a")
.find(
(node) =>
node.props.href.includes("/stages/") &&
node
.findAllByType("span")
.some((span) => textContent(span) === id.slice(0, -2)),
);
expect(link).toBeDefined();
await act(async () =>
link!.props.onClick({
button: 0,
defaultPrevented: false,
preventDefault() {
this.defaultPrevented = true;
},
}),
);
}
function expectSelected(id: string) {
const text = textContent(renderer.root);
expect(text).toContain(`echo selected:${id}`);
expect(text).toContain(`output:${id}`);
expect(logRequests.at(-1)).toBe(id);
const active = renderer.root
.findAllByType("a")
.filter((node) => node.props.className?.includes("bg-overlay text-fg"));
expect(active).toHaveLength(1);
expect(active[0].props.href).toBe(
`/runs/run-1/stages/${encodeURIComponent(id)}`,
);
for (const other of ids.filter((value) => value !== id)) {
expect(text).not.toContain(`echo selected:${other}`);
expect(text).not.toContain(`output:${other}`);
}
}
describe("stage occurrence navigation", () => {
test.each(ids)("clicking the sidebar opens exactly %s", async (id) => {
const router = render();
await clickStage(id);
expect(router.state.location.pathname).toBe(
`/runs/run-1/stages/${encodeURIComponent(id)}`,
);
expectSelected(id);
});
test.each(ids)("direct links preserve %s through the nested route", (id) => {
render(`/runs/run-1/stages/${encodeURIComponent(id)}/?view=detail#output`);
expectSelected(id);
});
test("back and forward restore the exact occurrence", async () => {
const router = render();
await clickStage(ids[1]);
await clickStage(ids[4]);
await act(async () => {
await router.navigate(-1);
});
expectSelected(ids[1]);
await act(async () => {
await router.navigate(1);
});
expectSelected(ids[4]);
});
test("the stage index still selects the first stage", () => {
render();
expectSelected(ids[0]);
});
test.each(["missing%401", "bad%ZZ"])(
"an explicit unavailable ID %s never selects another stage",
(segment) => {
render(`/runs/run-1/stages/${segment}`);
expect(textContent(renderer.root)).toContain("Stage not found");
expect(logRequests).toEqual([]);
},
);
test("an explicit ID waits for the stage list then selects it", async () => {
loading = true;
const router = render(`/runs/run-1/stages/${encodeURIComponent(ids[1])}`);
expect(textContent(renderer.root)).toContain("Loading stages");
expect(textContent(renderer.root)).not.toContain("Stage not found");
loading = false;
await act(async () => {
await router.navigate(router.state.location.pathname, { replace: true });
});
expectSelected(ids[1]);
});
});
describe("other stage navigation surfaces", () => {
const target = makeStage({
id: ids[1],
name: "reviewer/e2",
nodeId: "reviewer/e2",
handler: "command",
status: "succeeded",
parallelGroupId: "fork@1",
parallelBranchIndex: 0,
});
const source = makeStage({
id: "fork@1",
nodeId: "fork",
status: "succeeded",
});
const producers = [
[
"parallel child",
<ParallelChildren
stage={source}
runId="run-1"
allStages={[target]}
overview={{ branchCount: 1, results: [] }}
/>,
],
[
"conditional target",
<ConditionalDecision
stage={source}
runId="run-1"
allStages={[target]}
edge={{
fromNode: "fork",
toNode: target.nodeId,
reason: "unconditional",
condition: null,
}}
/>,
],
[
"resumed-stage popover",
<StagePopover
stage={{ ...source, resumedFromStageId: target.id }}
runId="run-1"
duration="1s"
/>,
],
[
"waterfall",
<RunWaterfall
runId="run-1"
phases={[]}
stages={[
{
...stages[1],
started_at: "2026-10-20T12:00:00Z",
wall_time_ms: 1000,
},
]}
createdAtIso="2026-10-20T12:00:00Z"
completedAtIso="2026-10-20T12:00:02Z"
/>,
],
] as const;
test.each(producers)(
"%s opens the offered occurrence",
async (_name, element) => {
const router = render("/runs/run-1", element);
const link = renderer.root
.findAllByType("a")
.find((node) => node.props.href.includes("/stages/"));
expect(link).toBeDefined();
await act(async () =>
link!.props.onClick({
button: 0,
defaultPrevented: false,
preventDefault() {
this.defaultPrevented = true;
},
}),
);
expect(router.state.location.pathname).toBe(
`/runs/run-1/stages/${encodeURIComponent(target.id)}`,
);
expectSelected(target.id);
},
);
test("the resumed-stage banner opens the source occurrence", async () => {
available = stages.map((stage, index) =>
index === 0 ? { ...stage, resumed_from_stage_id: ids[1] } : stage,
);
render();
const link = renderer.root
.findAllByType("a")
.find((node) => textContent(node) === ids[1]);
await act(async () =>
link!.props.onClick({
button: 0,
defaultPrevented: false,
preventDefault() {
this.defaultPrevented = true;
},
}),
);
expectSelected(ids[1]);
});
test("the graph stage-click handler navigates to the offered occurrence", async () => {
const router = render("/runs/run-1");
await act(async () => graphClick(ids[1]));
expect(router.state.location.pathname).toBe(
`/runs/run-1/stages/${encodeURIComponent(ids[1])}`,
);
expectSelected(ids[1]);
});
});

View file

@ -5,7 +5,7 @@ import {
useState,
type ReactNode,
} from "react";
import { Link, useParams } from "react-router";
import { Link, useLocation, useParams } from "react-router";
import {
ArrowDownTrayIcon,
ChevronDownIcon,
@ -38,6 +38,7 @@ import type {
ThreadDnaItem,
ThreadDnaSelection,
} from "../components/event-debug";
import { stageIdFromPathname, stagePath } from "../lib/stage-routes";
import { StageContext } from "../components/stage-context";
import { StageInsightsSidebar } from "../components/stage-insights-sidebar";
import { StageSidebar } from "../components/stage-sidebar";
@ -2374,7 +2375,7 @@ function RunStageActivityStage({
<p className="pb-2 text-xs text-fg-muted">
Resumed from{" "}
<Link
to={`/runs/${runId}/stages/${encodeURIComponent(selectedStage.resumedFromStageId)}`}
to={stagePath(runId, selectedStage.resumedFromStageId)}
className="font-mono text-teal-500 hover:underline"
>
{selectedStage.resumedFromStageId}
@ -2526,7 +2527,9 @@ function RunStageActivity({
}
export default function RunStages() {
const { id, stageId } = useParams();
const { id, stageId: stageParam } = useParams();
const { pathname } = useLocation();
const stageId = stageIdFromPathname(pathname);
const runQuery = useRun(id);
const stagesQuery = useRunStages(id);
const stages = useMemo(
@ -2535,7 +2538,9 @@ export default function RunStages() {
);
const selectedStage =
stages.find((s: Stage) => s.id === stageId) ?? stages[0];
stageParam === undefined
? stages[0]
: stages.find((s: Stage) => s.id === stageId);
const selectedStageId = selectedStage?.id;
const runStart =
selectedStage?.startedAt ??
@ -2563,6 +2568,25 @@ export default function RunStages() {
? runStateQuery.data?.stages[selectedStageId]
: undefined;
if (stagesQuery.isLoading && !stagesQuery.data) {
return (
<div className="py-12">
<EmptyState title="Loading stages" description="Fetching the run’s stages." />
</div>
);
}
if (id && stageParam !== undefined && !selectedStage) {
return (
<div className="py-12">
<EmptyState
title="Stage not found"
description="This stage occurrence is not available in this run."
/>
</div>
);
}
if (!id || !selectedStage) {
return (
<div className="py-12">