diff --git a/src/__tests__/unit/components/EvalRunsBox.test.tsx b/src/__tests__/unit/components/EvalRunsBox.test.tsx index edee77a725..c23132345a 100644 --- a/src/__tests__/unit/components/EvalRunsBox.test.tsx +++ b/src/__tests__/unit/components/EvalRunsBox.test.tsx @@ -762,56 +762,56 @@ describe("EvalRunsBox — isSuperAdmin / StakworkRunLink", () => { ); }); - it("colSpan is 6 for skeleton rows when isSuperAdmin=false", () => { + it("colSpan is 7 for skeleton rows when isSuperAdmin=false", () => { renderBox({ isLoading: true, fixes: [], isSuperAdmin: false }); const skeletonCell = document.querySelector("td[colspan]"); - expect(skeletonCell?.getAttribute("colspan")).toBe("6"); + expect(skeletonCell?.getAttribute("colspan")).toBe("7"); }); - it("colSpan is 7 for skeleton rows when isSuperAdmin=true", () => { + it("colSpan is 8 for skeleton rows when isSuperAdmin=true", () => { renderBox({ isLoading: true, fixes: [], isSuperAdmin: true }); const skeletonCells = document.querySelectorAll("td[colspan]"); - expect(skeletonCells[0]?.getAttribute("colspan")).toBe("7"); + expect(skeletonCells[0]?.getAttribute("colspan")).toBe("8"); }); - it("colSpan is 6 for empty-state row when isSuperAdmin=false", () => { + it("colSpan is 7 for empty-state row when isSuperAdmin=false", () => { renderBox({ fixes: [], isLoading: false, isSuperAdmin: false }); const emptyCell = document.querySelector("td[colspan]"); - expect(emptyCell?.getAttribute("colspan")).toBe("6"); + expect(emptyCell?.getAttribute("colspan")).toBe("7"); }); - it("colSpan is 7 for empty-state row when isSuperAdmin=true", () => { + it("colSpan is 8 for empty-state row when isSuperAdmin=true", () => { renderBox({ fixes: [], isLoading: false, isSuperAdmin: true }); const emptyCell = document.querySelector("td[colspan]"); - expect(emptyCell?.getAttribute("colspan")).toBe("7"); + expect(emptyCell?.getAttribute("colspan")).toBe("8"); }); - it("colSpan is 7 for expanded-detail row when isSuperAdmin=true", async () => { + it("colSpan is 8 for expanded-detail row when isSuperAdmin=true", async () => { renderBox({ fixes: [makeFix()], isSuperAdmin: true }); const chevron = screen.getByRole("button", { name: "Expand" }); await act(async () => { fireEvent.click(chevron); }); - // Expanded row detail td should have colSpan=7 + // Expanded row detail td should have colSpan=8 const expandedCells = document.querySelectorAll("td[colspan]"); const expandedCell = Array.from(expandedCells).find( - (el) => el.getAttribute("colspan") === "7", + (el) => el.getAttribute("colspan") === "8", ); expect(expandedCell).toBeTruthy(); }); - it("colSpan is 6 for expanded-detail row when isSuperAdmin=false", async () => { + it("colSpan is 7 for expanded-detail row when isSuperAdmin=false", async () => { renderBox({ fixes: [makeFix()], isSuperAdmin: false }); const chevron = screen.getByRole("button", { name: "Expand" }); await act(async () => { fireEvent.click(chevron); }); const expandedCells = document.querySelectorAll("td[colspan]"); - // All colspan values should be 6 (no 7 present) - const hasSevenColspan = Array.from(expandedCells).some( - (el) => el.getAttribute("colspan") === "7", + // All colspan values should be 7 (no 8 present) + const hasEightColspan = Array.from(expandedCells).some( + (el) => el.getAttribute("colspan") === "8", ); - expect(hasSevenColspan).toBe(false); - expect(expandedCells[0]?.getAttribute("colspan")).toBe("6"); + expect(hasEightColspan).toBe(false); + expect(expandedCells[0]?.getAttribute("colspan")).toBe("7"); }); }); diff --git a/src/__tests__/unit/components/RecursionActivityRail.test.tsx b/src/__tests__/unit/components/RecursionActivityRail.test.tsx index d94007b3c4..c9e2076716 100644 --- a/src/__tests__/unit/components/RecursionActivityRail.test.tsx +++ b/src/__tests__/unit/components/RecursionActivityRail.test.tsx @@ -29,7 +29,8 @@ function makeRow(overrides: Partial = {}): AttemptRailRow { graphReportRef: null, reportPending: false, inFlight: false, - fixSnapshot: null, + fixSnapshots: [], + siblingCount: 0, ...overrides, }; } @@ -232,13 +233,16 @@ describe("RecursionActivityRail — fix snapshot diff control", () => { const rows = [ makeRow({ key: "with-snapshot", - fixSnapshot: { - ref_id: "fix-1", - target_type: "concept", - target_name: "Limitation of Liability", - old_value: '{"docs": "before"}', - new_value: '{"docs": "after"}', - }, + fixSnapshots: [ + { + ref_id: "fix-1", + target_type: "concept", + target_name: "Limitation of Liability", + old_value: '{"docs": "before"}', + new_value: '{"docs": "after"}', + }, + ], + siblingCount: 1, }), makeRow({ key: "without-snapshot" }), ]; diff --git a/src/__tests__/unit/components/RecursionBox.consolidated.test.tsx b/src/__tests__/unit/components/RecursionBox.consolidated.test.tsx index 742b30e772..f699e0cd5c 100644 --- a/src/__tests__/unit/components/RecursionBox.consolidated.test.tsx +++ b/src/__tests__/unit/components/RecursionBox.consolidated.test.tsx @@ -174,7 +174,8 @@ function makeAttemptRow(overrides: { graphReportRef: null, reportPending: false, inFlight: false, - fixSnapshot: null, + fixSnapshots: [], + siblingCount: 0, }; } diff --git a/src/__tests__/unit/components/RecursionBox.test.tsx b/src/__tests__/unit/components/RecursionBox.test.tsx index e8c884e15f..aef22a881c 100644 --- a/src/__tests__/unit/components/RecursionBox.test.tsx +++ b/src/__tests__/unit/components/RecursionBox.test.tsx @@ -822,6 +822,8 @@ describe("RecursionCard — activity rail", () => { graphReportRef: null, reportPending: false, inFlight: false, + fixSnapshots: [], + siblingCount: 0, }, ]; diff --git a/src/__tests__/unit/lib/harvey-lab/fix-sort.test.ts b/src/__tests__/unit/lib/harvey-lab/fix-sort.test.ts new file mode 100644 index 0000000000..901f7fc726 --- /dev/null +++ b/src/__tests__/unit/lib/harvey-lab/fix-sort.test.ts @@ -0,0 +1,59 @@ +import { describe, it, expect } from "vitest"; +import { compareFixRows } from "@/lib/harvey-lab/fix-sort"; + +describe("compareFixRows", () => { + it("orders by target_name ascending, nulls last", () => { + const a = { target_name: "Alpha", criterion_id: null, ref_id: "1" }; + const b = { target_name: "Beta", criterion_id: null, ref_id: "2" }; + expect(compareFixRows(a, b)).toBeLessThan(0); + expect(compareFixRows(b, a)).toBeGreaterThan(0); + }); + + it("falls through to criterion_id when target_name is equal (or null)", () => { + const a = { target_name: null, criterion_id: "AAA", ref_id: "1" }; + const b = { target_name: null, criterion_id: "BBB", ref_id: "2" }; + expect(compareFixRows(a, b)).toBeLessThan(0); + }); + + it("falls through to ref_id when target_name and criterion_id are both null", () => { + const a = { target_name: null, criterion_id: null, ref_id: "aaa" }; + const b = { target_name: null, criterion_id: null, ref_id: "bbb" }; + expect(compareFixRows(a, b)).toBeLessThan(0); + }); + + it("puts nulls last at every tier", () => { + type Row = { target_name: string | null; criterion_id: string | null; ref_id: string }; + const withName: Row = { target_name: "Alpha", criterion_id: null, ref_id: "1" }; + const noName: Row = { target_name: null, criterion_id: "ZZZ", ref_id: "2" }; + expect(compareFixRows(withName, noName)).toBeLessThan(0); // Alpha before null + expect(compareFixRows(noName, withName)).toBeGreaterThan(0); + }); + + it("handles prompt fixes (no target_name) ordering by criterion_id", () => { + const a = { target_name: null, criterion_id: "crit-001", ref_id: "1" }; + const b = { target_name: null, criterion_id: "crit-002", ref_id: "2" }; + expect(compareFixRows(a, b)).toBeLessThan(0); + }); + + it("produces identical ordering for an all-criterion_id-null concept group", () => { + const fixes = [ + { target_name: "Limitation", criterion_id: null, ref_id: "z3" }, + { target_name: "Arbitration", criterion_id: null, ref_id: "z1" }, + { target_name: "Indemnification", criterion_id: null, ref_id: "z2" }, + ]; + const sorted = [...fixes].sort(compareFixRows); + expect(sorted.map(f => f.target_name)).toEqual(["Arbitration", "Indemnification", "Limitation"]); + }); + + it("is stable on equal inputs", () => { + const a = { target_name: "Same", criterion_id: "same", ref_id: "same" }; + const b = { target_name: "Same", criterion_id: "same", ref_id: "same" }; + expect(compareFixRows(a, b)).toBe(0); + }); + + it("trims whitespace before comparing", () => { + const a = { target_name: " Alpha ", criterion_id: null, ref_id: "same" }; + const b = { target_name: "Alpha", criterion_id: null, ref_id: "same" }; + expect(compareFixRows(a, b)).toBe(0); + }); +}); diff --git a/src/app/api/mock/jarvis/graph/recursion-fixture.ts b/src/app/api/mock/jarvis/graph/recursion-fixture.ts index 05c7075b82..8e8661f20f 100644 --- a/src/app/api/mock/jarvis/graph/recursion-fixture.ts +++ b/src/app/api/mock/jarvis/graph/recursion-fixture.ts @@ -969,3 +969,232 @@ export const RECURSION_NODE_IDS = { FIX_REJECTED_UNSCORED_ID, FIX_REJECTED_ID, } as const; + +// ── Concept-sibling fixture (opt-in only) ───────────────────────────────────── +// +// A single eval run emitting 6 sibling concept ProposedFix nodes, all sharing +// one EvalTriggerOutput via PRODUCED_BY and all hanging off the same EvalTrigger +// via HAS_PROPOSED_FIX. Three variant groups cover the reconciliation and +// fallback-tier test cases. +// +// SECURITY NOTE: Do NOT fold these into the default buildRecursionNodes/Edges +// arrays. The mock Jarvis graph route (`api/mock/jarvis/graph/route.ts`) has +// no requireAuth/workspace guard of its own — it relies solely on the +// production-path block in `src/middleware.ts`. Anything folded into the +// default payload is therefore served unauthenticated on every non-production +// deployment. The `withConceptSiblings` composer is activated only when +// `NODE_ENV !== "production"` AND the request carries `?fixture=concept-siblings`. + +// ── Group A: 6 fully-materialized sibling concept fixes ────────────────────── +export const CONCEPT_SIBLING_EVALSET_ID = "mock-evalset-concept-siblings-001"; +const CONCEPT_SIBLING_TRIGGER_ID = "mock-evaltrigger-concept-siblings-001"; +const CONCEPT_SIBLING_OUTPUT_ID = "mock-evaltriggeroutput-concept-siblings-001"; + +// 6 ProposedFix nodes — all PRODUCED_BY the same EvalTriggerOutput, all +// HAS_PROPOSED_FIX from the same EvalTrigger, all target_type:"concept", +// eval_status:"accepted", distinct target_name, no criterion_id/prompt_id. +// The LAST sibling (index 5) deliberately has NO snapshot-bearing properties +// so tests can assert siblingCount===6 while fixSnapshots.length===5. +const CONCEPT_SIBLING_FIX_IDS = Array.from({ length: 6 }, (_, i) => + `mock-proposedfix-concept-sibling-${i + 1}-001`, +); +export const CONCEPT_SIBLING_FIX_NO_SNAPSHOT_ID = CONCEPT_SIBLING_FIX_IDS[5]; + +const CONCEPT_SIBLING_TARGET_NAMES = [ + "Limitation of Liability", + "Indemnification Scope", + "Force Majeure Triggers", + "Governing Law", + "Assignment Restrictions", + "Arbitration Venue", // ← this one has no snapshot properties +]; + +const SHARED_RUN_ID = "concept-siblings-run-001"; + +// ── Group B: 2 pending siblings (no output, shared run id) ─────────────────── +export const CONCEPT_PENDING_EVALSET_ID = "mock-evalset-concept-pending-001"; +const CONCEPT_PENDING_TRIGGER_ID = "mock-evaltrigger-concept-pending-001"; +const CONCEPT_PENDING_FIX_IDS = ["mock-proposedfix-concept-pending-1", "mock-proposedfix-concept-pending-2"]; +const PENDING_RUN_ID = "concept-pending-run-001"; + +// ── Group C: 3+3 mixed-materialization siblings ─────────────────────────────── +export const CONCEPT_MIXED_EVALSET_ID = "mock-evalset-concept-mixed-001"; +const CONCEPT_MIXED_TRIGGER_ID = "mock-evaltrigger-concept-mixed-001"; +const CONCEPT_MIXED_OUTPUT_ID = "mock-evaltriggeroutput-concept-mixed-001"; +const CONCEPT_MIXED_FIX_IDS = Array.from({ length: 6 }, (_, i) => + `mock-proposedfix-concept-mixed-${i + 1}-001`, +); +const MIXED_RUN_ID = "concept-mixed-run-001"; + +export const CONCEPT_SIBLING_NODE_IDS = { + CONCEPT_SIBLING_EVALSET_ID, + CONCEPT_SIBLING_TRIGGER_ID, + CONCEPT_SIBLING_OUTPUT_ID, + CONCEPT_SIBLING_FIX_IDS, + CONCEPT_SIBLING_FIX_NO_SNAPSHOT_ID, + CONCEPT_PENDING_EVALSET_ID, + CONCEPT_PENDING_TRIGGER_ID, + CONCEPT_PENDING_FIX_IDS, + CONCEPT_MIXED_EVALSET_ID, + CONCEPT_MIXED_TRIGGER_ID, + CONCEPT_MIXED_OUTPUT_ID, + CONCEPT_MIXED_FIX_IDS, +} as const; + +/** Concept-sibling nodes for Groups A, B, and C. */ +export const CONCEPT_SIBLING_NODES: JarvisNode[] = [ + // ── Group A EvalSet + trigger + shared output ───────────────────────────── + { + ref_id: CONCEPT_SIBLING_EVALSET_ID, + node_type: "EvalSet", + date_added_to_graph: "1760100000", + properties: { name: "Concept Sibling EvalSet", task_slug: "mock-concept-sibling-task" }, + }, + { + ref_id: CONCEPT_SIBLING_TRIGGER_ID, + node_type: "EvalTrigger", + date_added_to_graph: "1760100001", + properties: { agent: "concept-fix-agent", start_point: "start", end_point: "end" }, + }, + { + ref_id: CONCEPT_SIBLING_OUTPUT_ID, + node_type: "EvalTriggerOutput", + date_added_to_graph: "1760100002", + properties: { n_passed: 60, n_total: 74, result: "partial", score: 60 / 74 }, + }, + // 6 sibling ProposedFix nodes (last one has no snapshot fields) + ...CONCEPT_SIBLING_FIX_IDS.map((ref_id, i): JarvisNode => ({ + ref_id, + node_type: "ProposedFix", + date_added_to_graph: String(1760100010 + i), + properties: { + eval_status: "accepted", + target_type: "concept", + target_name: CONCEPT_SIBLING_TARGET_NAMES[i], + stakwork_run_id: SHARED_RUN_ID, + // All but the last carry a snapshot (the last one has none — by design) + ...(i < 5 ? { + ...FIX_SNAPSHOT_SHAPES.conceptEditDocs, + target_name: CONCEPT_SIBLING_TARGET_NAMES[i], + } : {}), + }, + })), + + // ── Group B: 2 pending siblings (no output, no after_score) ────────────── + { + ref_id: CONCEPT_PENDING_EVALSET_ID, + node_type: "EvalSet", + date_added_to_graph: "1760200000", + properties: { name: "Concept Pending EvalSet", task_slug: "mock-concept-pending-task" }, + }, + { + ref_id: CONCEPT_PENDING_TRIGGER_ID, + node_type: "EvalTrigger", + date_added_to_graph: "1760200001", + properties: { agent: "concept-fix-agent", start_point: "start", end_point: "end" }, + }, + ...CONCEPT_PENDING_FIX_IDS.map((ref_id, i): JarvisNode => ({ + ref_id, + node_type: "ProposedFix", + date_added_to_graph: String(1760200010 + i), + properties: { + eval_status: "pending", + target_type: "concept", + target_name: `Pending Fix ${i + 1}`, + stakwork_run_id: PENDING_RUN_ID, + // No after_score, no PRODUCED_BY output — exercises tier-3/4 grouping + }, + })), + + // ── Group C: 3 materialized + 3 non-materialized siblings (same run id) ── + { + ref_id: CONCEPT_MIXED_EVALSET_ID, + node_type: "EvalSet", + date_added_to_graph: "1760300000", + properties: { name: "Concept Mixed EvalSet", task_slug: "mock-concept-mixed-task" }, + }, + { + ref_id: CONCEPT_MIXED_TRIGGER_ID, + node_type: "EvalTrigger", + date_added_to_graph: "1760300001", + properties: { agent: "concept-fix-agent", start_point: "start", end_point: "end" }, + }, + { + ref_id: CONCEPT_MIXED_OUTPUT_ID, + node_type: "EvalTriggerOutput", + date_added_to_graph: "1760300002", + properties: { n_passed: 55, n_total: 74, result: "partial", score: 55 / 74 }, + }, + ...CONCEPT_MIXED_FIX_IDS.map((ref_id, i): JarvisNode => ({ + ref_id, + node_type: "ProposedFix", + date_added_to_graph: String(1760300010 + i), + properties: { + eval_status: "accepted", + target_type: "concept", + target_name: `Mixed Fix ${i + 1}`, + stakwork_run_id: MIXED_RUN_ID, + }, + })), +]; + +/** Concept-sibling edges for Groups A, B, and C. */ +export const CONCEPT_SIBLING_EDGES = [ + // ── Group A ────────────────────────────────────────────────────────────── + { source: CONCEPT_SIBLING_EVALSET_ID, target: CONCEPT_SIBLING_TRIGGER_ID, edge_type: "HAS_BASELINE_TRIGGER" }, + { source: CONCEPT_SIBLING_TRIGGER_ID, target: CONCEPT_SIBLING_OUTPUT_ID, edge_type: "HAS_OUTPUT" }, + // All 6 fixes hang off the same trigger + ...CONCEPT_SIBLING_FIX_IDS.map((fixId) => ({ + source: CONCEPT_SIBLING_TRIGGER_ID, + target: fixId, + edge_type: "HAS_PROPOSED_FIX", + })), + // All 6 fixes PRODUCED_BY the same output + ...CONCEPT_SIBLING_FIX_IDS.map((fixId) => ({ + source: fixId, + target: CONCEPT_SIBLING_OUTPUT_ID, + edge_type: "PRODUCED_BY", + })), + + // ── Group B ────────────────────────────────────────────────────────────── + { source: CONCEPT_PENDING_EVALSET_ID, target: CONCEPT_PENDING_TRIGGER_ID, edge_type: "HAS_BASELINE_TRIGGER" }, + ...CONCEPT_PENDING_FIX_IDS.map((fixId) => ({ + source: CONCEPT_PENDING_TRIGGER_ID, + target: fixId, + edge_type: "HAS_PROPOSED_FIX", + })), + // No PRODUCED_BY edges (pending — output not yet written) + + // ── Group C ────────────────────────────────────────────────────────────── + { source: CONCEPT_MIXED_EVALSET_ID, target: CONCEPT_MIXED_TRIGGER_ID, edge_type: "HAS_BASELINE_TRIGGER" }, + { source: CONCEPT_MIXED_TRIGGER_ID, target: CONCEPT_MIXED_OUTPUT_ID, edge_type: "HAS_OUTPUT" }, + ...CONCEPT_MIXED_FIX_IDS.map((fixId) => ({ + source: CONCEPT_MIXED_TRIGGER_ID, + target: fixId, + edge_type: "HAS_PROPOSED_FIX", + })), + // Only the first 3 fixes are PRODUCED_BY the output (mid-rerun state) + ...CONCEPT_MIXED_FIX_IDS.slice(0, 3).map((fixId) => ({ + source: fixId, + target: CONCEPT_MIXED_OUTPUT_ID, + edge_type: "PRODUCED_BY", + })), +]; + +/** + * Compose the concept-sibling fixture groups onto a base node/edge set. + * + * IMPORTANT: Activate this ONLY in non-production + `?fixture=concept-siblings` + * requests. See the security note above — the mock Jarvis graph route has no + * auth guard of its own, so anything folded into the default payload would be + * served unauthenticated on every non-production deployment. + */ +export function withConceptSiblings(base: { + nodes: JarvisNode[]; + edges: { source: string; target: string; edge_type: string }[]; +}): { nodes: JarvisNode[]; edges: { source: string; target: string; edge_type: string }[] } { + return { + nodes: [...base.nodes, ...CONCEPT_SIBLING_NODES], + edges: [...base.edges, ...CONCEPT_SIBLING_EDGES], + }; +} diff --git a/src/app/api/mock/jarvis/graph/route.ts b/src/app/api/mock/jarvis/graph/route.ts index c6108dadf0..fdf68b0e4b 100644 --- a/src/app/api/mock/jarvis/graph/route.ts +++ b/src/app/api/mock/jarvis/graph/route.ts @@ -2,7 +2,7 @@ import { NextRequest, NextResponse } from "next/server"; import { db } from "@/lib/db"; import type { JarvisNode, JarvisResponse } from "@/types/jarvis"; import { isRecursionSubgraphRequest } from "./fixture-constants"; -import { buildRecursionNodes, buildRecursionEdges } from "./recursion-fixture"; +import { buildRecursionNodes, buildRecursionEdges, withConceptSiblings } from "./recursion-fixture"; export const runtime = "nodejs"; @@ -170,10 +170,13 @@ export async function GET(request: NextRequest) { // Branch: return recursion fixture when the request targets the eval subgraph if (isRecursionSubgraphRequest({ nodeType, startNode })) { - const response: JarvisResponse = { - nodes: buildRecursionNodes(), - edges: buildRecursionEdges(), - }; + const base = { nodes: buildRecursionNodes(), edges: buildRecursionEdges() }; + // Opt-in concept-sibling fixture: only in non-production + ?fixture=concept-siblings + const fixtureParam = searchParams.get("fixture"); + const withSiblings = + process.env.NODE_ENV !== "production" && fixtureParam === "concept-siblings"; + const { nodes, edges } = withSiblings ? withConceptSiblings(base) : base; + const response: JarvisResponse = { nodes, edges }; return NextResponse.json({ success: true, status: 200, data: response }); } diff --git a/src/app/api/workspaces/[slug]/legal/benchmarks/fix-chain/route.ts b/src/app/api/workspaces/[slug]/legal/benchmarks/fix-chain/route.ts index 810f9af716..237b1eb496 100644 --- a/src/app/api/workspaces/[slug]/legal/benchmarks/fix-chain/route.ts +++ b/src/app/api/workspaces/[slug]/legal/benchmarks/fix-chain/route.ts @@ -19,6 +19,7 @@ import { buildPlateauCapNodes, buildRecursionEdges, buildRecursionNodes, + withConceptSiblings, } from "@/app/api/mock/jarvis/graph/recursion-fixture"; export const runtime = "nodejs"; @@ -158,11 +159,22 @@ export async function GET(request: NextRequest, { params }: RouteParams) { { evalSetRefId, scenario }, ); const build = MOCK_SCENARIO_BUILDERS[scenario]; + // Opt-in concept-sibling fixture: only when ?fixture=concept-siblings is present + const req = request as { url?: string }; + const reqUrl = req.url ?? ""; + const fixtureSiblings = + process.env.NODE_ENV !== "production" && + reqUrl.includes("fixture=concept-siblings"); + const rawNodes = build.nodes(); + const rawEdges = build.edges(); + const { nodes, edges } = fixtureSiblings + ? withConceptSiblings({ nodes: rawNodes, edges: rawEdges }) + : { nodes: rawNodes, edges: rawEdges }; return NextResponse.json({ success: true, data: { - nodes: build.nodes(), - edges: build.edges(), + nodes, + edges, partial: false, }, }); diff --git a/src/app/api/workspaces/[slug]/legal/benchmarks/proposed-fixes/route.ts b/src/app/api/workspaces/[slug]/legal/benchmarks/proposed-fixes/route.ts index c82a2474e3..64b0fb593f 100644 --- a/src/app/api/workspaces/[slug]/legal/benchmarks/proposed-fixes/route.ts +++ b/src/app/api/workspaces/[slug]/legal/benchmarks/proposed-fixes/route.ts @@ -9,6 +9,7 @@ import { db } from "@/lib/db"; import { StakworkRunType } from "@prisma/client"; import { parseBenchmarkRunResult } from "@/types/legal"; import type { ProposedFix } from "@/types/legal"; +import { compareFixRows } from "@/lib/harvey-lab/fix-sort"; type RouteParams = { params: Promise<{ slug: string }>; @@ -85,7 +86,7 @@ export async function GET(request: NextRequest, { params }: RouteParams) { // list is filtered the same way the real path is in Step 9. if (process.env.USE_MOCKS === "true" && process.env.NODE_ENV !== "production") { const mockFixes: ProposedFix[] = buildSnapshotMockFixes().filter( - (f) => f.status !== "rejected", + (f) => (f.eval_status ?? f.status) !== "rejected", ); return NextResponse.json({ fixes: mockFixes }); } @@ -137,12 +138,18 @@ export async function GET(request: NextRequest, { params }: RouteParams) { const fixes: ProposedFix[] = searchResult.nodes .map((node) => projectFix(node.ref_id, node.properties)) // Exclude only explicitly-rejected fixes; pending/accepted/untagged remain visible - .filter((f) => f.status !== "rejected") + .filter((f) => { + const effectiveStatus = f.eval_status ?? f.status; + return effectiveStatus !== "rejected"; + }) .sort((a, b) => { - // Entries with a rerun_run_id (more recent reruns) surface first + // Primary: entries with a rerun_run_id (more recent reruns) surface first const aHas = a.rerun_run_id != null ? 1 : 0; const bHas = b.rerun_run_id != null ? 1 : 0; - return bHas - aHas; + const byRerun = bHas - aHas; + if (byRerun !== 0) return byRerun; + // Secondary: deterministic tiebreak (target_name → criterion_id → ref_id) + return compareFixRows(a, b); }); return NextResponse.json({ fixes }); diff --git a/src/components/legal/EvalRunsBox.tsx b/src/components/legal/EvalRunsBox.tsx index 2a1de054d9..dcd26feb4c 100644 --- a/src/components/legal/EvalRunsBox.tsx +++ b/src/components/legal/EvalRunsBox.tsx @@ -10,6 +10,7 @@ import { useWorkspace } from "@/hooks/useWorkspace"; import { getPusherClient, getWorkspaceChannelName, PUSHER_EVENTS } from "@/lib/pusher"; import type { ProposedFix } from "@/types/legal"; import { StakworkRunLink } from "@/components/legal/StakworkRunLink"; +import { compareFixRows } from "@/lib/harvey-lab/fix-sort"; interface EvalRunsBoxProps { /** The task slug identifying which task's eval runs to show */ @@ -28,6 +29,12 @@ interface EvalRunsBoxProps { refetch?: () => void; /** Whether the current user is a super admin — gates the entire Stakwork column */ isSuperAdmin?: boolean; + /** Accept a proposed fix by ref_id */ + accept?: (refId: string) => Promise; + /** Reject a proposed fix by ref_id */ + reject?: (refId: string) => Promise; + /** Set of refIds currently being processed (guards double-submission) */ + pendingRefIds?: Set; } function StatusBadge({ status }: { status?: string | null }) { @@ -67,10 +74,17 @@ function scoreDisplay(fix: ProposedFix): string { /** Sort: unresolved (null resolved_at) first, then resolved newest-first */ function sortFixes(fixes: ProposedFix[]): ProposedFix[] { return [...fixes].sort((a, b) => { - if (a.resolved_at == null && b.resolved_at == null) return 0; - if (a.resolved_at == null) return -1; - if (b.resolved_at == null) return 1; - return new Date(b.resolved_at).getTime() - new Date(a.resolved_at).getTime(); + // Primary: unresolved (null resolved_at) first, then resolved newest-first + const aUnresolved = a.resolved_at == null; + const bUnresolved = b.resolved_at == null; + if (aUnresolved && !bUnresolved) return -1; + if (!aUnresolved && bUnresolved) return 1; + if (!aUnresolved && !bUnresolved) { + const byTime = new Date(b.resolved_at!).getTime() - new Date(a.resolved_at!).getTime(); + if (byTime !== 0) return byTime; + } + // Secondary: deterministic tiebreak (target_name → criterion_id → ref_id) + return compareFixRows(a, b); }); } @@ -83,6 +97,9 @@ export function EvalRunsBox({ isLoading = false, refetch = () => {}, isSuperAdmin = false, + accept, + reject, + pendingRefIds = new Set(), }: EvalRunsBoxProps) { const { workspace } = useWorkspace(); const { runEval, isSubmitting } = useLegalBenchmarkEval(); @@ -90,6 +107,10 @@ export function EvalRunsBox({ const [optimisticEntry, setOptimisticEntry] = useState(false); const [recursionPending, setRecursionPending] = useState(false); const [expandedKey, setExpandedKey] = useState(null); + const [confirmDialog, setConfirmDialog] = useState<{ + action: "accept" | "reject"; + fix: ProposedFix; + } | null>(null); const activeProjectIdRef = useRef(null); const fixesLengthAtLaunchRef = useRef(0); @@ -188,6 +209,53 @@ export function EvalRunsBox({ } }; + const handleAction = async (action: "accept" | "reject", fix: ProposedFix) => { + const effectiveStatus = fix.eval_status ?? fix.status; + const isConceptFix = (fix.target_type ?? fix.fix_type ?? "").trim().toLowerCase() === "concept"; + + if (action === "reject" && effectiveStatus === "accepted" && isConceptFix) { + // Show confirmation for rejecting auto-accepted concept fix + setConfirmDialog({ action, fix }); + return; + } + + if (!fix.ref_id) return; + try { + if (action === "accept") { + await accept?.(fix.ref_id); + } else { + await reject?.(fix.ref_id); + } + // Single refetch after action (not per-mutation) + refetch(); + } catch (err: unknown) { + const status = (err as { status?: number })?.status; + if (status === 429) { + const { toast } = await import("sonner"); + toast.error("Too many requests — please try again in a moment.", { description: "Rate limit reached." }); + } + } + }; + + const handleConfirmedAction = async () => { + if (!confirmDialog) return; + const { action, fix } = confirmDialog; + setConfirmDialog(null); + if (!fix.ref_id) return; + try { + if (action === "reject") { + await reject?.(fix.ref_id); + } + refetch(); + } catch (err: unknown) { + const status = (err as { status?: number })?.status; + if (status === 429) { + const { toast } = await import("sonner"); + toast.error("Too many requests — please try again in a moment."); + } + } + }; + const sorted = sortFixes(fixes); return ( @@ -252,6 +320,7 @@ export function EvalRunsBox({ Change Score Status + Actions {isSuperAdmin && ( Stakwork )} @@ -263,7 +332,7 @@ export function EvalRunsBox({ <> {[0, 1, 2].map((i) => ( - + @@ -271,7 +340,7 @@ export function EvalRunsBox({ ) : sorted.length === 0 && !optimisticEntry ? ( - + No eval results yet. @@ -289,6 +358,7 @@ export function EvalRunsBox({ — — — + {isSuperAdmin && ( )} - {sorted.map((fix) => { - const key = fix.ref_id ?? fix.criterion_id ?? String(Math.random()); + {sorted.map((fix, index) => { + const key = fix.ref_id ?? `${fix.target_name ?? ""}:${fix.criterion_id ?? ""}:${index}`; const isExpanded = expandedKey === key; return ( - {fix.criterion_title ?? fix.criterion_id ?? "—"} +
+ {fix.criterion_title ?? fix.criterion_id ?? fix.target_name ?? "—"} + {fix.target_type && ( + + {fix.target_type.toLowerCase()} + + )} +
{fix.prompt_name ?? "—"} @@ -321,6 +402,66 @@ export function EvalRunsBox({ + + {(() => { + const effectiveStatus = fix.eval_status ?? fix.status; + const isConceptFix = (fix.target_type ?? fix.fix_type ?? "").trim().toLowerCase() === "concept"; + const isPending = pendingRefIds.has(fix.ref_id ?? ""); + + if (effectiveStatus === "rejected" || !fix.ref_id) return null; + + if (effectiveStatus === "accepted") { + // Only concept fixes can be rejected after auto-accept + if (!isConceptFix) return null; + return ( + + ); + } + + // Pending fix: show both Accept and Reject + if (effectiveStatus === "pending" || effectiveStatus == null) { + return ( +
+ {accept && ( + + )} + {reject && ( + + )} +
+ ); + } + + return null; + })()} + {isSuperAdmin && ( @@ -342,7 +483,7 @@ export function EvalRunsBox({ {isExpanded && ( - + {fix.passing_value != null && (

@@ -383,6 +524,46 @@ export function EvalRunsBox({

+ + {/* Confirmation dialog for rejecting an auto-accepted concept fix */} + {confirmDialog && ( +
+
+

+ Reject this concept fix? +

+

+ This records a review decision and does{" "} + not undo the workflow's already-applied graph + write. The concept change remains in the knowledge graph. +

+
+ + +
+
+
+ )} ); } diff --git a/src/components/legal/LegalBenchmarkResults.tsx b/src/components/legal/LegalBenchmarkResults.tsx index acd384eb22..670a60b5f0 100644 --- a/src/components/legal/LegalBenchmarkResults.tsx +++ b/src/components/legal/LegalBenchmarkResults.tsx @@ -65,6 +65,9 @@ export function LegalBenchmarkResults({ runId, onReset, isSuperAdmin = false }: fixes, isLoading: fixesLoading, refetch: refetchFixes, + accept, + reject, + pendingRefIds, } = useProposedFixes(runId); const allPass = run?.runnerRun?.result?.all_pass; @@ -543,6 +546,9 @@ export function LegalBenchmarkResults({ runId, onReset, isSuperAdmin = false }: isLoading={fixesLoading} refetch={refetchFixes} isSuperAdmin={isSuperAdmin} + accept={accept} + reject={reject} + pendingRefIds={pendingRefIds} />
diff --git a/src/components/legal/RecursionActivityRail.tsx b/src/components/legal/RecursionActivityRail.tsx index adc8769b14..7e17b23612 100644 --- a/src/components/legal/RecursionActivityRail.tsx +++ b/src/components/legal/RecursionActivityRail.tsx @@ -255,17 +255,35 @@ export function RecursionActivityRail({ : "—"} - {/* The snapshot diff control renders only when the row's fix - actually carries a snapshot — eval-output and legacy rows - leave fixSnapshot unset, so the hide rule falls out of the - data rather than a series-kind check here. */} - {row.fixSnapshot && ( - - )} + {/* Sibling diffs: render one icon per sibling, cap at 3 then show +K */} + {row.fixSnapshots.length > 0 && (() => { + const MAX_VISIBLE = 3; + const visible = row.fixSnapshots.slice(0, MAX_VISIBLE); + const overflow = row.fixSnapshots.length - MAX_VISIBLE; + const siblingLabel = row.siblingCount > 1 ? `${row.siblingCount} fixes` : null; + return ( + <> + {siblingLabel && ( + + {siblingLabel} + + )} + {visible.map((snapshot, i) => ( + + ))} + {overflow > 0 && ( + + +{overflow} + + )} + + ); + })()} (a)-[r:${LOOP_EDGE_TYPES}]->(b) ` + - `RETURN DISTINCT a, r, b LIMIT 100` + `MATCH (s {ref_id: "${evalSetRefId}"})-[:${LOOP_EDGE_TYPES}*0..6]->(a)-[r:${LOOP_EDGE_TYPES}]->(b) ` + + `RETURN DISTINCT a, r, b LIMIT 300` ); } -/** Graph Explorer deep link that pre-runs the loop-subgraph Cypher. */ -export function loopSubgraphHref(workspaceSlug: string, evalSetRefId: string): string { - return `/w/${encodeURIComponent(workspaceSlug)}/context/graph?cypher=${encodeURIComponent(loopSubgraphCypher(evalSetRefId))}`; +/** Graph Explorer deep link that pre-runs the loop-subgraph Cypher. + * Returns null when evalSetRefId fails the allowlist — callers must not + * render a link in that case. */ +export function loopSubgraphHref(workspaceSlug: string, evalSetRefId: string): string | null { + const cypher = loopSubgraphCypher(evalSetRefId); + if (!cypher) return null; + return `/w/${encodeURIComponent(workspaceSlug)}/context/graph?cypher=${encodeURIComponent(cypher)}`; } // ─── ScoreBadge ────────────────────────────────────────────────────────────── @@ -222,19 +231,19 @@ interface ClimbTarget { function collectClimbTargets(rows: AttemptRailRow[]): ClimbTarget[] { const seen = new Map(); for (const row of rows) { - const fix = row.fixSnapshot; - if (!fix) continue; - const refId = fix.target_ref?.trim() || null; - const name = fix.target_name?.trim() || refId; - if (!name) continue; - const key = refId ?? `name:${name}`; - if (!seen.has(key)) { - seen.set(key, { - key, - kind: fix.target_type?.trim().toLowerCase() || null, - name, - refId, - }); + for (const fix of row.fixSnapshots) { + const refId = fix.target_ref?.trim() || null; + const name = fix.target_name?.trim() || refId; + if (!name) continue; + const key = refId ?? `name:${name}`; + if (!seen.has(key)) { + seen.set(key, { + key, + kind: fix.target_type?.trim().toLowerCase() || null, + name, + refId, + }); + } } } return [...seen.values()]; @@ -604,10 +613,10 @@ function RecursionCard({ entry, refetch, allRuns }: RecursionCardProps) { )} - {workspaceSlug && entry.refId && ( + {workspaceSlug && entry.refId && loopSubgraphHref(workspaceSlug, entry.refId) && ( <> (); - const hillClimbAttempts = buildHillClimbSeries(subgraph, { fixSnapshotsOut: fixSnapshots }); - const snapshotByPointRef = new Map(); + const siblingCountsOut = new Map(); + const hillClimbAttempts = buildHillClimbSeries(subgraph, { + fixSnapshotsOut: fixSnapshots, + siblingCountsOut, + partial: fixChain.partial ?? false, + }); + const snapshotByPointRef = new Map(); + let orphanSnapshotCount = 0; for (const snapshot of fixSnapshots.values()) { - if (snapshot.point_ref_id) snapshotByPointRef.set(snapshot.point_ref_id, snapshot); + if (snapshot.point_ref_id) { + const existing = snapshotByPointRef.get(snapshot.point_ref_id) ?? []; + existing.push(snapshot); + snapshotByPointRef.set(snapshot.point_ref_id, existing); + } else { + orphanSnapshotCount++; + } + } + if (orphanSnapshotCount > 0) { + logger.info( + `[legal/benchmarks/useEvalRunHistory] ${orphanSnapshotCount} snapshot(s) with null point_ref_id (orphans, not discarded)`, + "legal", + { evalSetRefId, orphanSnapshotCount }, + ); } // ── Step 4: Build history table (EvalRunsBox) ───────────────────── @@ -533,7 +554,8 @@ export function useEvalRunHistory(input: UseEvalRunHistoryInput): UseEvalRunHist extras: Pick & { graphTime?: string | null; graphReportRef?: string | null; - fixSnapshot?: FixSnapshotProps | null; + fixSnapshots?: FixSnapshotProps[]; + siblingCount?: number; }, ): AttemptRailRow => { const parsedStatusRun = statusRun ? parseBenchmarkRunResult(statusRun.result) : null; @@ -554,7 +576,8 @@ export function useEvalRunHistory(input: UseEvalRunHistoryInput): UseEvalRunHist parsedStatusRun?.generateRunReport === true && statusRun?.hasReport !== true, inFlight: NON_TERMINAL_STATUSES.has(statusRun?.status ?? ""), - fixSnapshot: extras.fixSnapshot ?? null, + fixSnapshots: extras.fixSnapshots ?? [], + siblingCount: extras.siblingCount ?? 0, }; }; @@ -576,10 +599,14 @@ export function useEvalRunHistory(input: UseEvalRunHistoryInput): UseEvalRunHist // concept-driven recursion writes no ProposedFix, so eval-output // rows legitimately carry none — the attempts-report path still // shows the task's fixes when they exist. - fixSnapshot: + fixSnapshots: + finalSeriesKind === "fix-chain" + ? (snapshotByPointRef.get(attempt.ref_id) ?? []) + : [], + siblingCount: finalSeriesKind === "fix-chain" - ? snapshotByPointRef.get(attempt.ref_id) ?? null - : null, + ? (siblingCountsOut.get(attempt.ref_id) ?? 0) + : 0, }); }); @@ -624,7 +651,8 @@ export function useEvalRunHistory(input: UseEvalRunHistoryInput): UseEvalRunHist graphReportRef: null, reportPending: false, inFlight: true, - fixSnapshot: null, + fixSnapshots: [], + siblingCount: 0, }; }); @@ -658,7 +686,8 @@ export function useEvalRunHistory(input: UseEvalRunHistoryInput): UseEvalRunHist hasReport: run.hasReport === true, graphReportRef: null, reportPending: false, - fixSnapshot: null, + fixSnapshots: [], + siblingCount: 0, score: passed != null && total != null ? { passed, total } : null, }; }); diff --git a/src/lib/harvey-lab/fix-sort.ts b/src/lib/harvey-lab/fix-sort.ts new file mode 100644 index 0000000000..6ec2cad385 --- /dev/null +++ b/src/lib/harvey-lab/fix-sort.ts @@ -0,0 +1,62 @@ +/** + * fix-sort.ts + * + * Shared comparator for ProposedFix rows used in: + * - `EvalRunsBox.tsx` (client-side `sortFixes`) + * - `GET /api/.../proposed-fixes/route.ts` (server-side sort) + * + * A single implementation is the only way to guarantee identical ordering + * across both surfaces. `localeCompare` is intentionally avoided: it resolves + * against differing ICU data in the browser vs Node, which is exactly the + * source of the nondeterministic-ordering bug described in T3. + * + * Sort order (tiebreakers applied left-to-right): + * 1. target_name — leads because concept fixes never set criterion_id; + * prompt fixes (which have criterion_id and lack target_name) + * therefore fall through correctly to tier 2. + * 2. criterion_id — demoted so prompt fixes keep their current relative ordering. + * 3. ref_id — final tiebreak for absolute stability. + * + * Nulls sort last at every tier. + */ + +/** + * Plain codepoint string comparison (< / >), stable across environments. + * Returns negative, zero, or positive — the same contract as Array.sort's + * comparator argument. + */ +function cmpStr(a: string | null | undefined, b: string | null | undefined): number { + const an = a == null ? null : a.trim(); + const bn = b == null ? null : b.trim(); + + if (an === bn) return 0; // identical (including both null/empty) + if (an === null || an === "") return 1; // nulls last + if (bn === null || bn === "") return -1; + if (an < bn) return -1; + if (an > bn) return 1; + return 0; +} + +/** + * Canonical comparator for ProposedFix rows — call this from both the + * client-side sort and the server-side sort so the ordering is provably + * identical regardless of environment. + * + * The fields are read as plain strings (trimmed, nulls-last) and compared + * via codepoint order — no locale, no collation. + */ +export function compareFixRows< + T extends { + target_name?: string | null; + criterion_id?: string | null; + ref_id?: string | null; + } +>(a: T, b: T): number { + const byTargetName = cmpStr(a.target_name, b.target_name); + if (byTargetName !== 0) return byTargetName; + + const byCriterionId = cmpStr(a.criterion_id, b.criterion_id); + if (byCriterionId !== 0) return byCriterionId; + + return cmpStr(a.ref_id, b.ref_id); +}