[Jamie] Fix Consolidated Report link disappearing after refresh on Recursion tab - #5200
Open
pitoi wants to merge 4 commits into
Open
[Jamie] Fix Consolidated Report link disappearing after refresh on Recursion tab#5200pitoi wants to merge 4 commits into
pitoi wants to merge 4 commits into
Conversation
…cursion tab
## 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.
pitoi
added a commit
that referenced
this pull request
Aug 30, 2026
…olidated-run test fixture (#5202) ## CI failure on PR #5200 `unit-tests` check fails: ``` FAIL src/__tests__/unit/components/RecursionBox.test.tsx > RecursionCard — allRuns prop (consolidated-run detection) > detects a PENDING in-flight consolidated run via the allRuns prop TestingLibraryElementError: Unable to find an element by: [data-testid="consolidated-generating"] ``` ## Cause This is the exact fixture gap flagged before merge. PR #5200 changed `RecursionCard`'s `existingConsolidated` lookup to filter on the new `pipeline === StakworkRunType.LEGAL_BENCHMARK_CONSOLIDATED` field instead of the ambiguous `runType === "recursion"`. The test's `makeConsolidatedRow` helper (in `src/__tests__/unit/components/RecursionBox.test.tsx`) builds a `BenchmarkRunListRow` fixture that never sets `pipeline`, so the new filter no longer matches it — `effectiveConsolidatedRunId` stays `null` and the "Generating…" indicator never renders. ## Fix Add `pipeline: StakworkRunType.LEGAL_BENCHMARK_CONSOLIDATED` to the `makeConsolidatedRow` helper's returned object (as a fixed value, not an override — every row this helper builds represents a consolidated run in this describe block). This is a test-only change; no production code is touched. Verified this does not change the meaning of the other two tests using this helper: - `"ignores runs with hasReport=true (already completed)"` only asserts the "Generating…" spinner (`consolidated-generating`) is absent for a completed run — still true after the fix, since a completed run should show the report link, not the spinner. Unaffected. - `"ignores consolidated runs for other task slugs"` asserts the same absence for an unrelated taskSlug — unaffected by adding `pipeline`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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'sexistingConsolidatedlookup:only seeds
effectiveConsolidatedRunIdfrom in-flight, report-less runs — intentionally, per its own comment, just to "survive a page refresh" while pending. OncehasReportflips totrue, the row is excluded, and after a remount (consolidatedRunIdstate resets tonull) there is nothing left to fall back to. The link (gated onconsolidatedRun?.hasReport && effectiveConsolidatedRunId) never renders again, even though a completed report exists.A second, independent bug found while investigating
existingConsolidatedalso filters only onr.runType === "recursion". But insrc/hooks/useLegalBenchmarkRunList.ts, all three secondary pipelines —LEGAL_BENCHMARK_EVAL,LEGAL_BENCHMARK_RECURSION, andLEGAL_BENCHMARK_CONSOLIDATED— are collapsed to the same literal string"recursion"viamapSecondary(r, "recursion"). The rawStakworkRunTypeis discarded and never retained onBenchmarkRunListRow. SoexistingConsolidated'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
src/hooks/useLegalBenchmarkRunList.ts: add an optionalpipeline?: StakworkRunTypefield toBenchmarkRunListRowthat retains the raw pipeline type, and pass the correctStakworkRunTypeinto eachmapSecondarycall (LEGAL_BENCHMARK_EVAL,LEGAL_BENCHMARK_RECURSION,LEGAL_BENCHMARK_CONSOLIDATEDrespectively) instead of discarding it. Keep the existing collapsedrunType: "recursion"value as-is (other call sites rely on it) — just also carrypipeline.src/components/legal/RecursionBox.tsx: changeexistingConsolidatedto:r.pipeline === StakworkRunType.LEGAL_BENCHMARK_CONSOLIDATEDinstead of the ambiguousr.runType === "recursion".(status === PENDING || IN_PROGRESS) && !r.hasReportexclusion — pick the most recent CONSOLIDATED run for this task regardless of status/report.useLegalBenchmarkRunalready 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.handleConsolidatedReport'ssetConsolidatedRunId(...)sets local state, whicheffectiveConsolidatedRunId = consolidatedRunId ?? existingConsolidated?.id ?? nullalready 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 itsmakeConsolidatedRowtest fixture doesn't set the newpipelinefield, so the updated filter (which now keys onpipelineinstead ofrunType/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.