From 29816214286500bb28138e612e4a047fa0bee922 Mon Sep 17 00:00:00 2001 From: Paul Itoi <814886+pitoi@users.noreply.github.com> Date: Sun, 30 Aug 2026 19:00:43 +0000 Subject: [PATCH] [Jamie] Fix Consolidated Report link disappearing after refresh on Recursion tab MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Bug On `/w/openlaw/legal/benchmarks`, Recursion tab: after a Consolidated Report finishes generating and the user refreshes (or remounts the tab), the "View Consolidated Report" link disappears. Only the "Consolidated Report" button remains, and tapping it starts a **new** generation run instead of opening the existing one. ## Root cause In `src/components/legal/RecursionBox.tsx`, `RecursionCard`'s `existingConsolidated` lookup: ```js const existingConsolidated = useMemo(() => { return (allRuns ?? []) .filter( (r) => r.taskSlug === entry.id && r.runType === "recursion" && (r.status === WorkflowStatus.PENDING || r.status === WorkflowStatus.IN_PROGRESS) && !r.hasReport, ) .sort((a, b) => new Date(b.createdAt).getTime() - new Date(a.createdAt).getTime())[0] ?? null; }, [allRuns, entry.id]); ``` only seeds `effectiveConsolidatedRunId` from **in-flight, report-less** runs — intentionally, per its own comment, just to "survive a page refresh" while pending. Once `hasReport` flips to `true`, the row is excluded, and after a remount (`consolidatedRunId` state resets to `null`) there is nothing left to fall back to. The link (gated on `consolidatedRun?.hasReport && effectiveConsolidatedRunId`) never renders again, even though a completed report exists. ## A second, independent bug found while investigating `existingConsolidated` also filters only on `r.runType === "recursion"`. But in `src/hooks/useLegalBenchmarkRunList.ts`, **all three** secondary pipelines — `LEGAL_BENCHMARK_EVAL`, `LEGAL_BENCHMARK_RECURSION`, and `LEGAL_BENCHMARK_CONSOLIDATED` — are collapsed to the same literal string `"recursion"` via `mapSecondary(r, "recursion")`. The raw `StakworkRunType` is discarded and never retained on `BenchmarkRunListRow`. So `existingConsolidated`'s filter cannot actually tell a CONSOLIDATED run apart from an EVAL or RECURSION (fix-proposal) run for the same task — it just happens to work today only because report-less EVAL/RECURSION rows for the same task usually aren't sitting around at the same moment. This is a latent correctness bug, not just a display one. ## Fix 1. **`src/hooks/useLegalBenchmarkRunList.ts`**: add an optional `pipeline?: StakworkRunType` field to `BenchmarkRunListRow` that retains the raw pipeline type, and pass the correct `StakworkRunType` into each `mapSecondary` call (`LEGAL_BENCHMARK_EVAL`, `LEGAL_BENCHMARK_RECURSION`, `LEGAL_BENCHMARK_CONSOLIDATED` respectively) instead of discarding it. Keep the existing collapsed `runType: "recursion"` value as-is (other call sites rely on it) — just also carry `pipeline`. 2. **`src/components/legal/RecursionBox.tsx`**: change `existingConsolidated` to: - Filter by `r.pipeline === StakworkRunType.LEGAL_BENCHMARK_CONSOLIDATED` instead of the ambiguous `r.runType === "recursion"`. - Drop the `(status === PENDING || IN_PROGRESS) && !r.hasReport` exclusion — pick the most recent CONSOLIDATED run for this task regardless of status/report. `useLegalBenchmarkRun` already polls and resolves the real current status, so this correctly shows either "Generating…" (in-flight) or the "View Consolidated Report" link (completed) after a refresh. - The "Consolidated Report" button still works to start a fresh report: `handleConsolidatedReport`'s `setConsolidatedRunId(...)` sets local state, which `effectiveConsolidatedRunId = consolidatedRunId ?? existingConsolidated?.id ?? null` already prioritizes over the seeded fallback — so a manual trigger always wins over the last-known run until the next remount. Net effect: after generating a report and refreshing, the user sees the existing report's link. They can still generate a brand new one at any time via the button, and that new run takes over the display immediately without needing a refresh. ## Test results - `useLegalBenchmarkRunList.test.ts` (38 tests) — all pass. - `RecursionBox.consolidated.test.tsx` (9 tests) — all pass. - `legal/RecursionBox.test.tsx` (18 tests) — all pass. - `components/RecursionBox.test.tsx` (65 tests) — 1 failed: "detects a PENDING in-flight consolidated run via the allRuns prop" fails because its `makeConsolidatedRow` test fixture doesn't set the new `pipeline` field, so the updated filter (which now keys on `pipeline` instead of `runType`/`status`/`hasReport`) no longer matches it — this is a pre-existing test fixture gap surfaced by the intentional behavior change, not a TypeScript compile error (the field is optional), so per instructions I left test files untouched. --- src/components/legal/RecursionBox.tsx | 13 ++++++++----- src/hooks/useLegalBenchmarkRunList.ts | 22 ++++++++++++++++++---- 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/src/components/legal/RecursionBox.tsx b/src/components/legal/RecursionBox.tsx index 04d92fdf90..da7f46c346 100644 --- a/src/components/legal/RecursionBox.tsx +++ b/src/components/legal/RecursionBox.tsx @@ -393,16 +393,19 @@ function RecursionCard({ entry, refetch, allRuns }: RecursionCardProps) { // `allRuns` is lifted from RecursionTab (via RecursionList) so the whole tab // shares one fetch-and-poll loop instead of one per card. - // Find the most recent CONSOLIDATED run for this taskSlug. + // Most recent CONSOLIDATED run for this task, any status — seeds + // effectiveConsolidatedRunId so a refresh shows either "Generating…" for an + // in-flight run, or the "View Consolidated Report" link for a completed one. + // Filtering on `pipeline` (not `runType`) is required: `runType` collapses + // EVAL/RECURSION/CONSOLIDATED down to the same "recursion" string, so it + // cannot by itself distinguish a consolidated-report run from an unrelated + // analysis/fix-proposal run for the same task. const existingConsolidated = useMemo(() => { return (allRuns ?? []) .filter( (r) => r.taskSlug === entry.id && - r.runType === "recursion" && - (r.status === WorkflowStatus.PENDING || - r.status === WorkflowStatus.IN_PROGRESS) && - !r.hasReport, + r.pipeline === StakworkRunType.LEGAL_BENCHMARK_CONSOLIDATED, ) .sort((a, b) => new Date(b.createdAt).getTime() - new Date(a.createdAt).getTime())[0] ?? null; }, [allRuns, entry.id]); diff --git a/src/hooks/useLegalBenchmarkRunList.ts b/src/hooks/useLegalBenchmarkRunList.ts index 5cbee35a0d..fa9a90e57b 100644 --- a/src/hooks/useLegalBenchmarkRunList.ts +++ b/src/hooks/useLegalBenchmarkRunList.ts @@ -65,6 +65,15 @@ export interface BenchmarkRunListRow { generateRunReport?: boolean; /** This run has a report bundle. Derived server-side from reportUrl. */ hasReport?: boolean; + /** + * The run's raw Stakwork pipeline type (LEGAL_BENCHMARK_EVAL / + * LEGAL_BENCHMARK_RECURSION / LEGAL_BENCHMARK_CONSOLIDATED). Distinct from + * `runType`, which collapses all three into "recursion" for display — + * `pipeline` is what lets callers (e.g. RecursionCard's consolidated-report + * lookup) tell a CONSOLIDATED run apart from an EVAL or RECURSION run for + * the same task. + */ + pipeline?: StakworkRunType; } interface UseLegalBenchmarkRunListResult { @@ -152,12 +161,17 @@ export function useLegalBenchmarkRunList( const rawRecursionRows: RawRunRow[] = recursionData?.runs ?? []; const rawConsolidatedRows: RawRunRow[] = consolidatedData?.runs ?? []; - const mapSecondary = (r: RawRunRow, runType: BenchmarkRunType): BenchmarkRunListRow => { + const mapSecondary = ( + r: RawRunRow, + runType: BenchmarkRunType, + pipeline: StakworkRunType, + ): BenchmarkRunListRow => { const parsed = parseBenchmarkRunResult(r.result); return { id: r.id, workspaceId: r.workspaceId, runType, + pipeline, status: r.status as WorkflowStatus, projectId: r.projectId, taskSlug: parsed?.taskSlug ?? "", @@ -222,14 +236,14 @@ export function useLegalBenchmarkRunList( const merged = [ ...mapped, - ...rawEvalRows.map((r) => mapSecondary(r, "recursion")), - ...rawRecursionRows.map((r) => mapSecondary(r, "recursion")), + ...rawEvalRows.map((r) => mapSecondary(r, "recursion", StakworkRunType.LEGAL_BENCHMARK_EVAL)), + ...rawRecursionRows.map((r) => mapSecondary(r, "recursion", StakworkRunType.LEGAL_BENCHMARK_RECURSION)), // CONSOLIDATED rows are merged so Pusher updates for them flow through // the existing channel subscription without new polling logic, enabling // RecursionCard to surface in-flight / completed consolidated report // status after a page refresh. They are not surfaced in the Runs tab // table — the runType tag keeps them invisible there. - ...rawConsolidatedRows.map((r) => mapSecondary(r, "recursion")), + ...rawConsolidatedRows.map((r) => mapSecondary(r, "recursion", StakworkRunType.LEGAL_BENCHMARK_CONSOLIDATED)), ].sort((a, b) => new Date(b.createdAt).getTime() - new Date(a.createdAt).getTime()); runsRef.current = merged;