diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index 59f1627c0a..3ff2e2def9 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -124,6 +124,7 @@ jobs: - name: Upload Playwright Blob Report uses: actions/upload-artifact@v4 if: always() + continue-on-error: true with: name: playwright-blob-${{ matrix.shard }} path: blob-report/ diff --git a/src/__tests__/unit/components/BenchmarkRunsHistory.test.tsx b/src/__tests__/unit/components/BenchmarkRunsHistory.test.tsx index 4b7c3dc4c0..846392414f 100644 --- a/src/__tests__/unit/components/BenchmarkRunsHistory.test.tsx +++ b/src/__tests__/unit/components/BenchmarkRunsHistory.test.tsx @@ -29,6 +29,7 @@ const makeRun = (overrides: Partial<{ generateJamieChat: boolean; jamieChatStatus: string; jamieChatPath: string; + generateRunReport: boolean; hasReport: boolean; }> = {}) => ({ id: "runner-1", @@ -48,6 +49,7 @@ const makeRun = (overrides: Partial<{ generateJamieChat: undefined as boolean | undefined, jamieChatStatus: undefined as string | undefined, jamieChatPath: undefined as string | undefined, + generateRunReport: undefined as boolean | undefined, hasReport: undefined as boolean | undefined, ...overrides, }); @@ -262,13 +264,29 @@ describe("BenchmarkRunsHistory", () => { ); }); - it("renders Runner Status column header and Pass/Total column headers (no Fail header)", () => { + it("renders Runner Status column header and Pass/Fail/Total column headers", () => { render(React.createElement(BenchmarkRunsHistory)); expect(screen.getByText("Runner Status")).toBeInTheDocument(); expect(screen.getByText("Pass")).toBeInTheDocument(); + expect(screen.getByText("Fail")).toBeInTheDocument(); expect(screen.getByText("Total")).toBeInTheDocument(); expect(screen.queryByText("Score")).toBeNull(); - expect(screen.queryByText("Fail")).toBeNull(); + }); + + it("renders the score column headers in order Pass, Fail, Contested, Disputed, Total", () => { + render(React.createElement(BenchmarkRunsHistory)); + const headers = Array.from(document.querySelectorAll("thead th")).map( + (th) => th.textContent?.trim(), + ); + const scoreHeaders = headers.filter((h) => + ["Pass", "Fail", "Contested", "Disputed", "Total"].includes(h ?? ""), + ); + expect(scoreHeaders).toEqual(["Pass", "Fail", "Contested", "Disputed", "Total"]); + }); + + it("does NOT render a Chat column header", () => { + render(React.createElement(BenchmarkRunsHistory)); + expect(screen.queryByText("Chat")).toBeNull(); }); it("shows COMPLETED badge for a completed run", () => { @@ -466,7 +484,7 @@ describe("BenchmarkRunsHistory", () => { // ─── colSpan tests ───────────────────────────────────────────────────────── - it("expanded row colSpan is 9 for non-super-admin (Task + Type + Started + Runner Status + Score + Contested + Disputed + Chat + Report)", async () => { + it("expanded row colSpan is 10 for non-super-admin (Task + Type + Started + Runner Status + Pass + Fail + Contested + Disputed + Total + Report)", async () => { const user = userEvent.setup(); render(React.createElement(BenchmarkRunsHistory)); @@ -779,14 +797,11 @@ describe("BenchmarkRunsHistory", () => { expect(expandedCell.getAttribute("colspan")).toBe("10"); }); - // ─── Chat column tests ───────────────────────────────────────────────────── - - it("renders Chat column header", () => { - render(React.createElement(BenchmarkRunsHistory)); - expect(screen.getByText("Chat")).toBeInTheDocument(); - }); + // ─── Chat column removal ─────────────────────────────────────────────────── + // The Chat column/link were removed from this table (Jamie chat data and + // generation are unaffected — see useLegalBenchmarkRunList.ts). - it("renders icon-only 'View Chat' link when jamieChatPath is present", () => { + it("does NOT render the report-chat-link even when jamieChatPath is present", () => { mockUseList.mockReturnValue({ runs: [makeRun({ generateJamieChat: true, @@ -800,13 +815,7 @@ describe("BenchmarkRunsHistory", () => { setExpandedId: mockSetExpandedId, }); render(React.createElement(BenchmarkRunsHistory)); - const link = screen.getByTestId("report-chat-link"); - expect(link).toBeInTheDocument(); - expect(link.getAttribute("href")).toBe("/org/stakwork?chat=conv-123"); - expect(link.getAttribute("target")).toBe("_blank"); - expect(link.getAttribute("title")).toBe("View Chat"); - expect(link.getAttribute("aria-label")).toBe("View Chat"); - expect(link.textContent).not.toBe("View Chat"); + expect(screen.queryByTestId("report-chat-link")).toBeNull(); }); it("renders icon-only 'View Report' link with correct attributes when hasReport is true", () => { @@ -850,8 +859,10 @@ describe("BenchmarkRunsHistory", () => { }); it("shows Pending spinner when report requested but not yet written", () => { + // Keyed off generateRunReport (ReportCell's own field) — generateJamieChat + // no longer drives any visible UI in this table now that Chat is removed. mockUseList.mockReturnValue({ - runs: [makeRun({ status: "IN_PROGRESS", generateJamieChat: true })], + runs: [makeRun({ status: "IN_PROGRESS", generateRunReport: true })], total: 1, isLoading: false, error: null, @@ -863,23 +874,6 @@ describe("BenchmarkRunsHistory", () => { expect(screen.queryByTestId("report-chat-link")).toBeNull(); }); - it("shows 'Failed' when jamieChatStatus is failed", () => { - mockUseList.mockReturnValue({ - runs: [makeRun({ generateJamieChat: true, jamieChatStatus: "failed" })], - total: 1, - isLoading: false, - error: null, - refetch: mockRefetch, - setExpandedId: mockSetExpandedId, - }); - render(React.createElement(BenchmarkRunsHistory)); - const failedMatches = screen.getAllByText("Failed"); - expect(failedMatches.length).toBeGreaterThan(0); - expect( - failedMatches.some((el) => el.className.includes("text-destructive")), - ).toBe(true); - }); - it("shows dash (not Pending) for a FAILED run with generateJamieChat (report will never fire)", () => { mockUseList.mockReturnValue({ runs: [makeRun({ status: "FAILED", generateJamieChat: true })], @@ -1552,6 +1546,100 @@ describe("BenchmarkRunsHistory — graph-first score numerators", () => { expect(row.textContent).toContain("—"); expect(row.textContent).not.toContain("8/"); }); + + // ─── Fail column ─────────────────────────────────────────────────────────── + + it("renders the computed Fail count (fail-cell-count) when a rubric breakdown is computed", () => { + // roster has 10 entries, 2 contested → scorable 8; n_passed 8 → fail 0. + // Use 6 passed instead so fail is a clearly non-zero, non-clamped number. + mockUseList.mockReturnValue({ + runs: [makeRun({ id: "m-1", taskSlug: TASK, status: "COMPLETED", n_passed: 6, n_total: 10, all_pass: false })], + total: 1, + isLoading: false, + error: null, + refetch: mockRefetch, + setExpandedId: mockSetExpandedId, + }); + mockRubricsMapHook.mockImplementation(() => new Map([[TASK, roster]])); + + render(); + + const row = screen.getByTestId("run-row-m-1"); + const failCell = row.querySelector('[data-testid="fail-cell-count"]'); + expect(failCell).toBeInTheDocument(); + // scorable = 10 - 2 contested = 8; pass = 6; fail = 8 - 6 = 2. + expect(failCell?.textContent).toBe("2"); + }); + + it("renders n/a (fail-cell-unknown), never 0, for an output-ref pointer row (n_failed === null)", () => { + const manualWithPointer = { + ...makeRun({ id: "m-1", taskSlug: TASK, status: "COMPLETED", n_passed: 50, n_total: 74, all_pass: false }), + evalTriggerRef: "trig-1", + evalOutputRef: "out-9", + }; + mockUseList.mockReturnValue({ + runs: [manualWithPointer], + total: 1, + isLoading: false, + error: null, + refetch: mockRefetch, + setExpandedId: mockSetExpandedId, + }); + mockRubricsMapHook.mockImplementation(() => new Map([[TASK, roster]])); + mockGraphScoresMapHook.mockImplementation( + () => + new Map([ + [TASK, [graphOutput({ ref_id: "out-9", triggerRef: undefined, n_passed: 9, n_total: 10 })]], + ]), + ); + + render(); + + const row = screen.getByTestId("run-row-m-1"); + const unknownCell = row.querySelector('[data-testid="fail-cell-unknown"]'); + expect(unknownCell).toBeInTheDocument(); + expect(unknownCell?.textContent).toBe("n/a"); + expect(row.querySelector('[data-testid="fail-cell-count"]')).toBeNull(); + }); + + it("renders the muted dash for Fail when the row has no score data (in-progress run)", () => { + mockUseList.mockReturnValue({ + runs: [makeRun({ id: "m-1", taskSlug: TASK, status: "IN_PROGRESS" })], + total: 1, + isLoading: false, + error: null, + refetch: mockRefetch, + setExpandedId: mockSetExpandedId, + }); + mockRubricsMapHook.mockImplementation(() => new Map([[TASK, roster]])); + + render(); + + const row = screen.getByTestId("run-row-m-1"); + expect(row.querySelector('[data-testid="fail-cell-count"]')).toBeNull(); + expect(row.querySelector('[data-testid="fail-cell-unknown"]')).toBeNull(); + }); + + it("renders 0 (never a negative number) for Fail on a clamped row (pass exceeds scorable)", () => { + // n_passed (10) exceeds scorable (10 - 2 contested = 8) — rubricBreakdown + // clamps pass to 8, so fail = 8 - 8 = 0, never negative. + mockUseList.mockReturnValue({ + runs: [makeRun({ id: "m-1", taskSlug: TASK, status: "COMPLETED", n_passed: 10, n_total: 10, all_pass: true })], + total: 1, + isLoading: false, + error: null, + refetch: mockRefetch, + setExpandedId: mockSetExpandedId, + }); + mockRubricsMapHook.mockImplementation(() => new Map([[TASK, roster]])); + + render(); + + const row = screen.getByTestId("run-row-m-1"); + const failCell = row.querySelector('[data-testid="fail-cell-count"]'); + expect(failCell).toBeInTheDocument(); + expect(failCell?.textContent).toBe("0"); + }); }); // ─── Non-manual (recursion) row expand tests ────────────────────────────────── diff --git a/src/components/legal/BenchmarkRunsHistory.tsx b/src/components/legal/BenchmarkRunsHistory.tsx index 2fe623b679..62f0bf833d 100644 --- a/src/components/legal/BenchmarkRunsHistory.tsx +++ b/src/components/legal/BenchmarkRunsHistory.tsx @@ -530,8 +530,8 @@ export function BenchmarkRunsHistory({ ); } - // colSpan: Task + Type + Started + Runner Status + Pass + Total + Contested + - // Disputed + Chat + Report + (Stakwork if super admin) + // colSpan: Task + Type + Started + Runner Status + Pass + Fail + Contested + + // Disputed + Total + Report + (Stakwork if super admin) const colSpan = isSuperAdmin ? 11 : 10; return ( @@ -609,7 +609,14 @@ export function BenchmarkRunsHistory({ Started Runner Status Pass - Total + + + Fail + + - Chat + Total Report {isSuperAdmin && ( Stakwork @@ -702,7 +709,7 @@ export function BenchmarkRunsHistory({ - + @@ -710,12 +717,8 @@ export function BenchmarkRunsHistory({ - e.stopPropagation()}> - {run.runType === "manual" ? ( - - ) : ( - — - )} + + e.stopPropagation()}> {/* Report bundles land on recursion rows too (reportUrl @@ -809,9 +812,10 @@ function TaskProgressCard({ /** * The run report bundle — the nine-section report built from the Harvey - * runner's S3 output. A DIFFERENT artifact from the Jamie chat next door: this - * one is produced by the runner itself and rendered natively by Hive, while the - * chat is an org-canvas conversation written afterwards by the canvas agent. + * runner's S3 output. Produced by the runner itself and rendered natively by + * Hive. (The Jamie chat — an org-canvas conversation written afterwards by + * the canvas agent — is a separate artifact; its data is still generated and + * fetched, but this table no longer renders a column for it.) * * `hasReport` is derived server-side from the presence of the persisted * projection — never from the bundle URL, which does not reach this component. @@ -848,46 +852,6 @@ function ReportCell({ run, slug }: { run: BenchmarkRunListRow; slug?: string }) return —; } -/** - * The Jamie chat produced by `generateBenchmarkJamieChat` — an org-canvas - * conversation. Distinct from the run report bundle, which has its own column. - */ -function ChatCell({ run }: { run: BenchmarkRunListRow }) { - if (run.jamieChatPath) { - return ( - - - - ); - } - - if (run.jamieChatStatus === "failed") { - return Failed; - } - - // Requested but not yet started/written (run still executing, or the - // completion webhook is generating the chat right now). A FAILED run - // never triggers a chat, so fall through to the dash instead. - if (run.generateJamieChat && run.status !== WorkflowStatus.FAILED) { - return ( - - - Pending - - ); - } - - return —; -} - function hasScoreData(run: AdjustedRun): boolean { const isActive = run.status === WorkflowStatus.PENDING || run.status === WorkflowStatus.IN_PROGRESS; @@ -938,6 +902,46 @@ function PassCell({ run }: { run: AdjustedRun }) { ); } +/** + * Failed-criteria count, threaded straight from `AdjustedRun.n_failed` + * (computed once in `rubricBreakdown`, never recomputed here). Two accepted + * divergences from neighbouring cells, left as-is deliberately: + * (a) The visible columns need not sum to Total. `n_failed` is + * `scorable − pass` against a TRUE UNION of contested criteria, while + * `ContestedCountCell` renders `score.contested` = + * `Math.max(rosterContested, contestedInRun)` — a strictly smaller set + * on some rows (rubric-scoring.ts). Pass + Fail + Contested can land + * short of Total. + * (b) On rows where `rubricBreakdown` clamps `pass` to `scorable` + * (rubric-scoring.ts), Fail shows `0` while `PassCell` renders the + * *unclamped* `run.n_passed` — so Pass can visibly exceed + * `Total − Contested`. `PassCell` is intentionally left alone here; + * changing what Pass renders would alter displayed scores, which is out + * of scope. Reconciling the two contested sets is a follow-up in + * rubric-scoring.ts, not part of this cell. + */ +function FailCell({ run }: { run: AdjustedRun }) { + if (!hasScoreData(run)) { + return —; + } + if (typeof run.n_failed !== "number") { + return ( + + n/a + + ); + } + return ( + + {run.n_failed} + + ); +} + /** * The full graph rubric roster size. Independent of `hasScoreData`/`all_pass` * — an in-progress run with an already-loaded roster still shows its Total diff --git a/src/hooks/useLegalBenchmarkRunList.ts b/src/hooks/useLegalBenchmarkRunList.ts index 93b941afe7..67f7a2a695 100644 --- a/src/hooks/useLegalBenchmarkRunList.ts +++ b/src/hooks/useLegalBenchmarkRunList.ts @@ -61,6 +61,9 @@ export interface BenchmarkRunListRow { requestedModel?: string; /** Operator-chosen judge model (bare name). Absent on legacy runs. */ requestedJudgeModel?: string; + // The Legal Benchmark runs table no longer renders a Chat column — these + // fields are parsed and retained deliberately (chat data/generation still + // happen), not dead code. /** Operator checked "Jamie Chat" at run creation (legacy key name) */ generateJamieChat?: boolean; /** Jamie chat lifecycle: "generating" | "completed" | "failed" */