From 2d6e7f95338e4c8246172fe0aec5f59d23a0df41 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 29 Aug 2026 09:25:32 +0000 Subject: [PATCH 1/4] UVH-IN5: show the user-value chain on /evals when it has data MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The chain funnel was mounted on run detail but invisible on the runs where it was the only thing to show. Two gates decide whether the insight rail exists at all — `run-insight-rail.tsx`'s emptiness check and `run-detail-view.tsx`'s `hasInsightContent` — and both counted only the triage, goal-completion and groundedness cards. A run with a derived chain and no judge or triage output rendered no rail, so the funnel inside it was never drawn. Adding the card to those checks would have traded one bug for another, which is why its exclusion was deliberate and documented: the card is a truthy fragment whose two halves each suppress themselves from the inside, so counting the NODE keeps an otherwise-empty rail alive as a full-height column of dead space on every run with no insight content at all. The gates now read a fact about the DATA instead. A probe mounted above every layout branch asks the same rollup query the funnel uses — undefined while loading, null for a run with no rollup, which is exactly the panel's own render condition — and reports one boolean both gates consume. Convex de-duplicates identical subscriptions, so asking twice costs one query. The probe lives beside the panels and carries the same ErrorBoundary they do, for the same reason: `useQuery` throws when the query is not deployed (this is still dark-shipped) or when there is no ConvexProvider, and a probe that took a run-detail page down with it would be worse than the empty rail it exists to prevent. Undeployed reads as "no funnel", which is correct. The state starts false, so a run without one never flashes an empty rail on the way to finding out. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Y4jTtZJsDeaterEzKwpF2p --- .../evals-run-detail-funnel-visibility.md | 13 +++ .../evals/__tests__/run-insight-rail.test.tsx | 52 +++++++++- .../src/components/evals/run-detail-view.tsx | 45 ++++++++- .../src/components/evals/run-insight-rail.tsx | 95 +++++++++++-------- .../user-value-chain/StageFunnelPanels.tsx | 66 ++++++++++++- 5 files changed, 225 insertions(+), 46 deletions(-) create mode 100644 .changeset/evals-run-detail-funnel-visibility.md diff --git a/.changeset/evals-run-detail-funnel-visibility.md b/.changeset/evals-run-detail-funnel-visibility.md new file mode 100644 index 0000000000..303df22cb6 --- /dev/null +++ b/.changeset/evals-run-detail-funnel-visibility.md @@ -0,0 +1,13 @@ +--- +"@mcpjam/inspector": patch +--- + +The user-value chain is visible on `/evals` run detail when it has data + +The chain funnel was mounted on run detail but could not be seen on the runs where it was the only thing to show. Two gates decide whether the insight rail exists at all — `run-insight-rail.tsx`'s emptiness check and `run-detail-view.tsx`'s `hasInsightContent` — and both counted only the triage, goal-completion and groundedness cards. A run with a derived chain and no judge or triage output rendered no rail, so the funnel it contained was never drawn. + +Adding the chain card to those checks would have traded one bug for another, which is why the exclusion was deliberate and documented: the card is a truthy fragment whose two halves each suppress themselves from the inside, so counting the NODE would keep an otherwise-empty rail alive as a full-height column of dead space on every run with no insight content at all. + +So the gates now read a fact about the DATA instead. A probe mounted above every layout branch asks the same rollup query the funnel itself uses — `undefined` while loading, `null` for a run with no rollup, which is exactly the panel's own render condition — and reports one boolean that both gates consume. Convex de-duplicates identical subscriptions, so asking twice costs one query, and the probe carries the same `ErrorBoundary` the panels do: `useQuery` throws when the query is not deployed or when there is no `ConvexProvider`, and a probe that took the page down with it would be worse than the empty rail it exists to prevent. Undeployed reads as "no funnel", which is correct. + +The state starts `false`, so a run without a funnel never flashes an empty rail on the way to finding out. diff --git a/mcpjam-inspector/client/src/components/evals/__tests__/run-insight-rail.test.tsx b/mcpjam-inspector/client/src/components/evals/__tests__/run-insight-rail.test.tsx index a50a998e58..263ce25f69 100644 --- a/mcpjam-inspector/client/src/components/evals/__tests__/run-insight-rail.test.tsx +++ b/mcpjam-inspector/client/src/components/evals/__tests__/run-insight-rail.test.tsx @@ -129,10 +129,14 @@ describe("RunAccuracyHeroBand", () => { />, ); - expect(screen.getByRole("heading", { name: /Run run-2/i })).toBeInTheDocument(); + expect( + screen.getByRole("heading", { name: /Run run-2/i }), + ).toBeInTheDocument(); expect(screen.getByText("Failed")).toBeInTheDocument(); expect(screen.getByText(/7 passed · 3 failed ·/)).toBeInTheDocument(); - const band = screen.getByRole("heading", { name: /Run run-2/i }).closest("section"); + const band = screen + .getByRole("heading", { name: /Run run-2/i }) + .closest("section"); expect(band).toHaveTextContent("Accuracy"); expect(band).toHaveTextContent("70%"); }); @@ -174,6 +178,50 @@ describe("RunInsightRail", () => { screen.queryByRole("heading", { name: "Latency by test (p50 / p95)" }), ).not.toBeInTheDocument(); }); + + it("stays closed when the chain card is the ONLY thing passed and it has no data", () => { + // The reason the card was excluded from the emptiness check to begin + // with: the node is truthy even when both of its halves render nothing, + // so counting the node would leave a full-height column of dead space. + const { container } = render( + } + />, + ); + + expect(container).toBeEmptyDOMElement(); + expect(screen.queryByTestId("chain-slot")).not.toBeInTheDocument(); + }); + + it("opens for a run whose ONLY insight is its user-value chain", () => { + // UVH-IN5, and the bug this fixes: a run with a derived chain but no + // judge or triage output rendered no rail at all, so the chain was + // invisible on exactly the runs where it was the whole story. + render( + Funnel} + userValueChainHasContent + />, + ); + + expect(screen.getByTestId("chain-slot")).toBeInTheDocument(); + }); + + it("still opens for other insight content when the chain has none", () => { + render( + Insights} + userValueChainCard={
} + userValueChainHasContent={false} + />, + ); + + expect(screen.getByTestId("triage-slot")).toBeInTheDocument(); + // The card is still mounted — it suppresses itself from the inside. + expect(screen.getByTestId("chain-slot")).toBeInTheDocument(); + }); }); describe("RunDetailMetricsCharts", () => { diff --git a/mcpjam-inspector/client/src/components/evals/run-detail-view.tsx b/mcpjam-inspector/client/src/components/evals/run-detail-view.tsx index f48f7e83e8..7bd910789f 100644 --- a/mcpjam-inspector/client/src/components/evals/run-detail-view.tsx +++ b/mcpjam-inspector/client/src/components/evals/run-detail-view.tsx @@ -45,7 +45,10 @@ import { type JudgeCase, } from "./goal-completion-presentation"; import { RunInsightBand, type InsightSeverity } from "./run-insight-band"; -import { SuiteRunStageFunnelPanel } from "@/components/shared/user-value-chain/StageFunnelPanels"; +import { + SuiteRunStageFunnelAvailability, + SuiteRunStageFunnelPanel, +} from "@/components/shared/user-value-chain/StageFunnelPanels"; import { ExplanatoryFlowOptIn } from "@/components/shared/usage-insights/ExplanatoryFlowOptIn"; import type { InsightsScope } from "@/hooks/useUsageInsights"; import { useAvailableModels } from "@/hooks/use-available-models"; @@ -576,6 +579,14 @@ export function RunDetailView({ const embeddedInResultsSplit = hideKpiStrip; + /** + * Whether this run has a stage funnel to draw, reported by the probe below. + * + * Starts `false` so a run without one never flashes an empty rail on the way + * to finding out; the probe flips it once the rollup query resolves. + */ + const [hasStageFunnel, setHasStageFunnel] = useState(false); + const serverQualityTriage = selectedRunDetails.status === "completed" && !serverQualityUnavailable ? ( ); + /** + * Mounted unconditionally, and deliberately NOT inside the rail or the band + * it informs — it answers whether those should open, so it has to exist + * before they do. Renders nothing; costs one query, which Convex shares with + * the panel's own subscription. + */ + const stageFunnelProbe = ( + + ); + const insightRail = ( ); + /** + * The chain counts toward "is there anything to show" — but only when it + * actually has a funnel to draw. + * + * Both gates below read this ONE boolean, and it comes from a probe rather + * than from the panel, because the panel cannot answer: neither gate mounts + * it until they have already decided to open. Counting the card itself + * instead would keep a rail alive on every run with no insight content at + * all, since the node is truthy whether or not it renders anything — which + * is why it was excluded from these checks in the first place, and why the + * fix has to be data-driven rather than a matter of adding the node. + */ const hasInsightContent = Boolean( serverQualityTriage || goalCompletionPanel || groundednessPanel || - actionableFindingsPanel, + actionableFindingsPanel || + hasStageFunnel, ); const triageFixCount = useMemo( @@ -1041,6 +1079,9 @@ export function RunDetailView({ omitIterationList && "px-3 py-3", )} > + {/* Renders nothing. Sits above every layout branch below because all of + them gate on the answer it reports. */} + {stageFunnelProbe} {onExportTraces || pluginSubmissionVersions.length > 0 || onShare ? ( // Always-on run-level actions — placed here (not the accuracy hero) so // they survive the folded run-detail layout that hides the hero. diff --git a/mcpjam-inspector/client/src/components/evals/run-insight-rail.tsx b/mcpjam-inspector/client/src/components/evals/run-insight-rail.tsx index 904f7ccea5..969ef91bc6 100644 --- a/mcpjam-inspector/client/src/components/evals/run-insight-rail.tsx +++ b/mcpjam-inspector/client/src/components/evals/run-insight-rail.tsx @@ -180,8 +180,8 @@ export function RunAccuracyHeroBand({ stats.total > 0 ? normalizeRunPassRatePercent(stats.passRate) : run.summary - ? normalizeRunPassRatePercent(run.summary.passRate) - : null; + ? normalizeRunPassRatePercent(run.summary.passRate) + : null; const trendChips = useMemo(() => { if (runTrendData.length < 2) return { points: [], hiddenCount: 0 }; @@ -251,10 +251,7 @@ export function RunAccuracyHeroBand({ {runClient || runServers.length > 0 ? (
{runClient ? ( - + ) : null} {visibleServers.map((name) => ( ); - const recentRunsBlock = hasRecentRuns && !hideRecentRuns ? ( -
-
-

Recent runs

- {trendChips.hiddenCount > 0 ? ( -

- Last {RUN_TREND_CHIP_LIMIT} of {runTrendData.length} -

- ) : null} -
-
- {trendChips.points.map((point) => ( - - ))} + const recentRunsBlock = + hasRecentRuns && !hideRecentRuns ? ( +
+
+

Recent runs

+ {trendChips.hiddenCount > 0 ? ( +

+ Last {RUN_TREND_CHIP_LIMIT} of {runTrendData.length} +

+ ) : null} +
+
+ {trendChips.points.map((point) => ( + + ))} +
-
- ) : null; + ) : null; // With run identity: title/stats and recent runs share one row; accuracy on the right. if (includeRunIdentity) { @@ -380,8 +378,8 @@ export function shouldShowRunAccuracyHero({ stats.total > 0 ? normalizeRunPassRatePercent(stats.passRate) : run.summary - ? normalizeRunPassRatePercent(run.summary.passRate) - : null; + ? normalizeRunPassRatePercent(run.summary.passRate) + : null; return passRatePercent !== null; } @@ -417,6 +415,7 @@ export function RunInsightRail({ goalCompletionCard, groundednessCard, userValueChainCard, + userValueChainHasContent = false, className, embedded = false, }: { @@ -433,18 +432,38 @@ export function RunInsightRail({ * diagram beside it is bought per pass and waits for a click. Same traces, * different price, so different affordance. * - * Deliberately NOT part of the emptiness check below. Both halves render - * nothing of their own when there is no derived chain and no analyzable - * cohort, and the node itself is truthy either way — counting it would keep - * an otherwise-empty rail alive as a full-height column of dead space on - * every run with no insight content at all. + * The NODE is deliberately still not part of the emptiness check below. + * Both halves render nothing of their own when there is no derived chain and + * no analyzable cohort, yet the node itself is truthy either way — counting + * it would keep an otherwise-empty rail alive as a full-height column of + * dead space on every run with no insight content at all. + * + * What the check reads instead is `userValueChainHasContent`, a fact about + * the DATA rather than about the node. That is what lets a run whose only + * insight is its chain open the rail — the case this card was previously + * invisible on — without reintroducing the empty column. */ userValueChainCard?: ReactNode; + /** + * Whether the chain card will actually draw something. + * + * Supplied by the caller, which asks a probe mounted outside this rail: the + * rail cannot ask the card, because it does not mount the card until it has + * already decided to exist. + */ + userValueChainHasContent?: boolean; className?: string; /** Flush layout inside the run-detail split (shared dividers, no card gaps). */ embedded?: boolean; }) { - if (!triageCard && !goalCompletionCard && !groundednessCard) return null; + if ( + !triageCard && + !goalCompletionCard && + !groundednessCard && + !userValueChainHasContent + ) { + return null; + } return (
, ); expect(getByTestId("sibling").textContent).toBe("the rest of the page"); }); @@ -158,7 +160,7 @@ describe("SwarmRunStageFunnelPanels — the query answers", () => { + />, ); expect(container.innerHTML).toBe(""); }); @@ -168,7 +170,7 @@ describe("SwarmRunStageFunnelPanels — the query cannot answer", () => { it("renders nothing instead of throwing", () => { queryThrows(); const { container } = render( - + , ); expect(container.textContent).toBe(""); }); @@ -179,8 +181,103 @@ describe("SwarmRunStageFunnelPanels — the query cannot answer", () => {
the rest of the page -
+
, ); expect(getByTestId("sibling").textContent).toBe("the rest of the page"); }); }); + +describe("SuiteRunStageFunnelAvailability — the probe that opens the rail", () => { + it("reports true only when the rollup actually answered", () => { + convex.useQuery.mockReturnValue(SUMMARY); + const onChange = vi.fn(); + render( + , + ); + expect(onChange).toHaveBeenCalledWith("run-1", true); + }); + + it.each([ + ["still loading", undefined], + ["a run with no rollup", null], + ])("reports false while %s", (_label, value) => { + convex.useQuery.mockReturnValue(value); + const onChange = vi.fn(); + render( + , + ); + expect(onChange).toHaveBeenCalledWith("run-1", false); + }); + + it("renders nothing and never reports when the query throws", () => { + // The dark-ship state. Undeployed must read as "no funnel", and the probe + // must not take the run-detail page down with it. + queryThrows(); + const onChange = vi.fn(); + const { container } = render( +
+ the rest of the page + +
, + ); + expect(onChange).not.toHaveBeenCalled(); + expect(container.textContent).toBe("the rest of the page"); + }); + + it("names the run each answer is about, so a stale one is detectable", () => { + // The run selector reuses one view across runs. An answer that did not + // name its run could not be told apart from the previous run's, and a + // stale `true` would open an empty rail on the run you switched to. + convex.useQuery.mockReturnValue(SUMMARY); + const onChange = vi.fn(); + const { rerender } = render( + , + ); + expect(onChange).toHaveBeenLastCalledWith("run-1", true); + + convex.useQuery.mockReturnValue(null); + rerender( + , + ); + expect(onChange).toHaveBeenLastCalledWith("run-2", false); + }); + + it("re-arms after a failing run: a later run is still probed", () => { + // An ErrorBoundary that has caught stays in its fallback for the life of + // the element, so the boundary is keyed by run. Without the key, one + // transient failure would hide the chain on every run after it. + queryThrows(); + const onChange = vi.fn(); + const { rerender } = render( + , + ); + expect(onChange).not.toHaveBeenCalled(); + + convex.useQuery.mockReturnValue(SUMMARY); + rerender( + , + ); + expect(onChange).toHaveBeenCalledWith("run-2", true); + }); +}); From 257230a1e42cef3c1ae4c6ffe7ea37c6d0699694 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 19:26:39 +0000 Subject: [PATCH 3/4] UVH-IN5: report "no funnel" when the probe fails, not silence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the run-binding fix. Keying the ErrorBoundary by run re-arms the probe across a run CHANGE, but it does nothing for the other half: a query that throws AFTER answering for the run still on screen renders the fallback silently, so the caller keeps the last good `true` and holds the rail open over a funnel that is no longer there. `onError` now reports `onChange(suiteRunId, false)`, which makes a failure say exactly what the dark-ship case already says — no funnel. The boundary already accepted the hook; nothing new was needed to reach it. The dark-ship test's assertion moves from "never reports" to "reports false", and a success-to-error regression for the SAME run covers the case the key cannot. Mutation-checked: removing `onError` fails those two plus the re-arm test, and nothing else. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Y4jTtZJsDeaterEzKwpF2p --- .../user-value-chain/StageFunnelPanels.tsx | 13 +++++++- .../__tests__/StageFunnelPanels.test.tsx | 32 +++++++++++++++++-- 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/mcpjam-inspector/client/src/components/shared/user-value-chain/StageFunnelPanels.tsx b/mcpjam-inspector/client/src/components/shared/user-value-chain/StageFunnelPanels.tsx index 53e8a30ff2..0eb335b40d 100644 --- a/mcpjam-inspector/client/src/components/shared/user-value-chain/StageFunnelPanels.tsx +++ b/mcpjam-inspector/client/src/components/shared/user-value-chain/StageFunnelPanels.tsx @@ -186,7 +186,18 @@ export function SuiteRunStageFunnelAvailability({ // for the life of the element, so an unkeyed one would swallow the probe // for every LATER run too: one transient failure would hide the chain on // every run after it until the whole view remounted. - + // + // `onError` covers the other half of that: a query that throws AFTER + // answering for the run still on screen renders the fallback silently, so + // without this the caller would keep the last good `true` and hold the + // rail open over a funnel that is no longer there. Reporting `false` from + // here makes the failure say the same thing the dark-ship case does — + // no funnel. + onChange(suiteRunId, false)} + > ); diff --git a/mcpjam-inspector/client/src/components/shared/user-value-chain/__tests__/StageFunnelPanels.test.tsx b/mcpjam-inspector/client/src/components/shared/user-value-chain/__tests__/StageFunnelPanels.test.tsx index 63454ab1dd..b0b416d283 100644 --- a/mcpjam-inspector/client/src/components/shared/user-value-chain/__tests__/StageFunnelPanels.test.tsx +++ b/mcpjam-inspector/client/src/components/shared/user-value-chain/__tests__/StageFunnelPanels.test.tsx @@ -215,7 +215,7 @@ describe("SuiteRunStageFunnelAvailability — the probe that opens the rail", () expect(onChange).toHaveBeenCalledWith("run-1", false); }); - it("renders nothing and never reports when the query throws", () => { + it("reports false and renders nothing when the query throws", () => { // The dark-ship state. Undeployed must read as "no funnel", and the probe // must not take the run-detail page down with it. queryThrows(); @@ -229,10 +229,35 @@ describe("SuiteRunStageFunnelAvailability — the probe that opens the rail", () />
, ); - expect(onChange).not.toHaveBeenCalled(); + expect(onChange).toHaveBeenCalledWith("run-1", false); expect(container.textContent).toBe("the rest of the page"); }); + it("clears a previous answer when the SAME run's probe then fails", () => { + // The boundary key re-arms the probe across runs, but a query that throws + // after answering for the run still on screen renders the fallback + // silently. Without `onError` the caller would keep the last good `true` + // and hold the rail open over a funnel that is no longer there. + convex.useQuery.mockReturnValue(SUMMARY); + const onChange = vi.fn(); + const { rerender } = render( + , + ); + expect(onChange).toHaveBeenLastCalledWith("run-1", true); + + queryThrows(); + rerender( + , + ); + expect(onChange).toHaveBeenLastCalledWith("run-1", false); + }); + it("names the run each answer is about, so a stale one is detectable", () => { // The run selector reuses one view across runs. An answer that did not // name its run could not be told apart from the previous run's, and a @@ -269,7 +294,8 @@ describe("SuiteRunStageFunnelAvailability — the probe that opens the rail", () onChange={onChange} />, ); - expect(onChange).not.toHaveBeenCalled(); + // The failing run reports "no funnel" rather than staying silent. + expect(onChange).toHaveBeenLastCalledWith("run-1", false); convex.useQuery.mockReturnValue(SUMMARY); rerender( From 025334d3bb13062303f1292fa7d7e84d98eb4400 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 21:03:00 +0000 Subject: [PATCH 4/4] ci: run the test suites on this stack's PRs, not just its root `branches` filters on the PR's BASE, so a stacked PR based on the branch below it never matched `main` and never ran these jobs. Every PR above the root of this stack was green on previews and review bots alone. That is not hypothetical here. A test in this stack asserted a stage-reason label's wording verbatim; a later PR in the same stack changed that wording; the break sat unnoticed until the suite was run by hand. The workflow's own comment already anticipated this and carries a pattern for an earlier stack, so this follows that precedent rather than inventing one. Merged forward through the stack so every PR above this one picks it up: for `pull_request`, the workflow that runs is the one in the merge of head into base, so the pattern has to be present on each head branch. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Y4jTtZJsDeaterEzKwpF2p --- .github/workflows/test.yml | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index de3a377350..f78c869e35 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -5,10 +5,16 @@ on: # `branches` filters on the PR's BASE. A stacked PR is based on the branch # below it rather than on main, so with `main` alone every PR above the # root of a stack merges without these tests having run against it once. - # Drop the second pattern once the Connector Bench stack has landed. + # Drop each stack pattern once that stack has landed. + # + # The UVH pattern was added after this bit us: a test in the stack asserted + # a label's wording verbatim, a later PR in the same stack changed that + # wording, and the break sat unnoticed because every PR above the root was + # green on previews and review bots alone. branches: - main - "claude/mcp-benchmarks-v2-b0tw4b-**" + - "claude/uvc-mcp-eval-reporting-gyycwl**" push: branches: [main]