UVH-IN6: the run detail can show a run's canonical stage analytics - #4496
UVH-IN6: the run detail can show a run's canonical stage analytics#4496chelojimenez wants to merge 8 commits into
Conversation
Two readings of the same six stages exist, and the eval run detail could only
show the older one. The legacy rollup is a precomputed pass rate with no
reach/measured distinction and no slices; the canonical EvalStageAnalyticsV1
document carries three rates per stage, named exclusions, and the
intent/model/host marginals. The run detail showed the rollup and the suite
page showed the document, so the same run read differently depending on where
you looked at it.
New route GET /v1/projects/:projectId/eval-runs/:runId/stage-analytics, with
PlatformApiClient.getEvalRunStageAnalytics.
A separate read rather than a filter over the suite listing: that listing pages
newest-first, so reaching a known run through it costs work proportional to how
long ago the run finished and cannot answer once the run falls outside the
pages walked.
The route project-matches the run first, validates with the REFINED schema (the
structural one admits two overall slices, which is the invariant every rendered
number rests on), and requires the document's runId to be the run asked for —
runId is only string().min(1) to the schema, so another run's document parses
perfectly and would be served under this run's heading. A 404 covers both "no
document" and "not visible", deliberately not distinguished: both mean
unmeasured, and separating them would confirm a run exists in a project the
caller cannot see.
The UI choice is EXCLUSIVE — canonical or legacy, never both. The two disagree
by construction because their denominators differ, so both on screen leaves a
reader unable to tell which is the report card:
flag off legacy, unlabelled: today's page, and no request issued
in flight nothing; legacy-then-canonical would flash one set of
numbers and replace it with different ones
document read canonical only
no document legacy, labelled as the older rollup
route undeployed legacy, labelled, silently — the dark-ship window
a real failure legacy, labelled, plus a service note
notFound is its own `absent` status rather than an error: otherwise every run
that finished before the materializer shipped, which is most of them, would
carry a red service message about nothing being wrong.
The hook is called ONCE, in the run detail, and its result passed to the
renderer as props. The rail needs the same answer for its emptiness check, and
this wraps a plain fetch rather than a Convex subscription, so calling it in
both places would issue two HTTP requests per run with nothing de-duplicating
them. That emptiness check now counts a canonical document too: the existing
probe answers only for the legacy rollup and would have closed the rail over a
run that has a document and no rollup.
RunDocument is exported from the suite panel and reused as-is — pure props, no
hooks, no queries. Reusing the panel would have dragged a suite listing, its
paging and its run selector onto a page with exactly one run.
Every guard is mutation-tested: each of the route's three and the slot's six
decisions was broken in turn and caught by exactly its intended test.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y4jTtZJsDeaterEzKwpF2p
…vc-mcp-eval-reporting-gyycwl-in6
GitHub Actions
Inactive
dosubot
Bot
added
size:XL
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_c55a3bab-7564-4b7c-816f-eabe6bf2a738) |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Internal previewPreview URL: https://mcp-inspector-pr-4496.up.railway.app |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 657f07b530
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| })(); | ||
|
|
||
| return () => controller.abort(); | ||
| }, [active, projectId, runId, attempt]); |
There was a problem hiding this comment.
When a user opens a run while it is still running, this effect immediately fetches the not-yet-materialized document and settles on absent; because its dependencies contain only the IDs and manual attempt, the later Convex update that changes the same run to a terminal status never triggers another request. The page therefore remains on the legacy rollup until it is remounted or refreshed, even after canonical analytics have been materialized. Pass the run status into this hook and refetch when it transitions to a terminal state, or defer the initial request until then.
Useful? React with 👍 / 👎.
| const hasStageFunnel = | ||
| runChain.choice === "canonical" || | ||
| (runChain.choice === "legacy" && legacyStageFunnel); |
There was a problem hiding this comment.
If the canonical request fails with requestFailed or invalidContract and this run has no legacy funnel, runChain.choice is legacy while legacyStageFunnel is false, so this reports no content. On a failed/timed-out run with no other insight cards, both hasInsightContent and RunInsightRail then suppress the entire slot, including the service note created specifically for this failure. Count runChain.serviceNote as content (or render the failure outside the funnel gate) so the canonical failure is not silently hidden.
Useful? React with 👍 / 👎.
…vc-mcp-eval-reporting-gyycwl-in6
GitHub Actions
Inactive
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_56deebf5-973a-46df-968f-fda159d77a8d) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 01465ae88a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| * nothing de-duplicating them. | ||
| */ | ||
| const runChain = useRunUserValueChainChoice({ | ||
| projectId: selectedRunDetails.projectId ?? null, |
There was a problem hiding this comment.
EvalSuiteRun.projectId is optional (types.ts:635), and legacy/list projections can therefore supply a selected run without it even though the surrounding suite view already knows the project. With the feature flag enabled, passing null makes the fetch hook inactive and permanently leaves its status at idle; useRunUserValueChainChoice maps that to choice: "nothing", so the canonical request never runs and the existing legacy funnel is also suppressed. Pass the parent view's project ID, or treat missing identifiers as an unattempted legacy fallback rather than loading.
Useful? React with 👍 / 👎.
…vc-mcp-eval-reporting-gyycwl-in6
…visible Two review findings on the run-detail chain slot. A RUN OPENED MID-FLIGHT NEVER GOT ITS DOCUMENT. The document is materialized when a run terminalizes, so a page opened while the run is still going asks too early and gets a legitimate 404 — and the effect keyed only on the ids, so it never asked again. The run finished, the document appeared, and the page kept showing the older rollup until somebody reloaded it. The run's status is now part of the read's identity, collapsed to "is it over" rather than carried through raw: a run moving pending -> running says nothing about whether its document exists, and re-fetching on it would issue a request per status tick. Callers that track no status keep the old single-shot behaviour, so a status this hook never learns cannot become a read it never issues. A REAL READ FAILURE COULD BE HIDDEN BY THE GATE MEANT TO HIDE EMPTY ONES. With a requestFailed or invalidContract read, no legacy rollup and no other insight card, hasStageFunnel was false and both the rail and the insight band suppressed the whole slot — including the service note written specifically to report that failure. The one case where the message is the only thing to say was the one case it did not appear. A service note now counts as content. `absent` and the dark-ship window still report none, so a run that finished before the materializer shipped does not hold a rail open on a message about nothing being wrong. Mutation-checked: removing the terminal-status refetch, refetching on every status tick, and dropping either note case each fail exactly their intended tests. Client evals + hooks suites: 2541 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y4jTtZJsDeaterEzKwpF2p
GitHub Actions
Inactive
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_6fa32543-2623-495b-a516-0eff326e0ea1) |
|
Both Codex findings were right. Fixed in A run opened mid-flight never got its document. Confirmed: the document is materialized on terminalization, so a page opened while the run is still going asks too early and gets a legitimate 404 — and the effect keyed only on the ids, so it never asked again. The run finished, the document appeared, and the page kept showing the older rollup until somebody reloaded it. The run's status is now part of the read's identity, collapsed to "is it over" rather than carried through raw — a run moving A real read failure could be hidden by the gate meant to hide empty ones. Also right, and it's the sharper of the two: with a A service note now counts as content. Mutation-checked: removing the terminal-status refetch, refetching on every status tick, and dropping either note case each fail exactly their intended tests. Client evals + hooks suites: 2,541 passed. Generated by Claude Code |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bb89ab9f3a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| * Matches `use-run-group-quality.ts` rather than inventing a second list — | ||
| * "this run is over" is one fact and two definitions of it would drift. | ||
| */ | ||
| const TERMINAL_RUN_STATUSES = new Set(["completed", "failed", "cancelled"]); |
There was a problem hiding this comment.
When a page is opened while a run is active and the run later transitions to timed_out, runIsOver remains false, so the initial absent/provisional response is never refreshed and the canonical analytics remain missing or stale until reload. Fresh evidence beyond the earlier terminalization finding is that EvalSuiteRun.status explicitly includes timed_out, while the canonical server terminal-status set also classifies it as terminal; add it here and cover that transition alongside completed.
Useful? React with 👍 / 👎.
| two readings is on screen — the older one has no reach/measured | ||
| split and no slices, so its numbers are not comparable to the | ||
| canonical document's. */} | ||
| Older stage rollup — this run has no canonical stage analytics. |
There was a problem hiding this comment.
When the canonical request fails with requestFailed or invalidContract, this unconditional fallback label says the run has no canonical analytics even though the service note immediately below says the document merely could not be read or validated. A transient outage can therefore present an existing canonical document as nonexistent; reserve this absence wording for the absent state and use a neutral legacy-rollup label for error fallbacks.
Useful? React with 👍 / 👎.
…vc-mcp-eval-reporting-gyycwl-in6
…imed_out Two further findings on the chain slot. "THIS RUN HAS NO CANONICAL STAGE ANALYTICS" WAS SAID FOR EVERY FALLBACK, including a requestFailed or invalidContract read — reporting a transient outage as a permanent absence, and contradicting the service note printed directly under it. Only a 404 establishes that the run HAS no document; every error kind establishes only that we could not read one, which is a different and recoverable claim. The dark-ship window reads as unreadable too: the route not being deployed says nothing about whether a document exists. TIMED_OUT WAS MISSING FROM THE TERMINAL STATUSES. The runner types its own terminal transitions as "cancelled" | "timed_out", so a run that ran out of time is as over as one that was cancelled — but a page opened during such a run never re-asked and sat on the older rollup indefinitely. The set is now a superset of use-run-group-quality.ts's, which has the same gap; that one answers a different question and is worth reconciling separately rather than widening here. Mutation-checked: removing timed_out fails exactly the new terminal-status case, and the absence-vs-unreadable wording is pinned across all four read outcomes. Client evals + hooks: 2546 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y4jTtZJsDeaterEzKwpF2p
GitHub Actions
Inactive
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_27981d7b-b27c-4122-96f5-7ee64c6c08cd) |
|
Both right. Fixed in The fallback label claimed an absence it had not established. It said "this run has no canonical stage analytics" for every fallback, including a Only a 404 establishes that the run has no document; every error kind establishes only that we could not read one, which is a different and recoverable claim. The label now distinguishes them. I also put the dark-ship window on the "unreadable" side: the route not being deployed says nothing about whether a document exists, so claiming absence there would be the same over-reach in a quieter voice.
Worth flagging: Mutation-checked: removing Generated by Claude Code |
…vc-mcp-eval-reporting-gyycwl-in6
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_664e9ecb-8d42-49c1-8235-6287661cd2e0) |
2 participants
Stacked on #4494 (UVH-IN3). Review only the top commit; the base carries the rest of the stack.
The problem
Two readings of the same six stages exist, and the eval run detail could only show the older one.
evalStageRollups:getSuiteRunStageFunnelEvalStageAnalyticsV1The run detail showed the rollup. The suite page showed the document. The same run therefore read differently depending on where you looked at it.
New route
GET /v1/projects/:projectId/eval-runs/:runId/stage-analytics, withPlatformApiClient.getEvalRunStageAnalytics, over BE3'sgetEvalRunStageAnalyticsreader.A separate read, not a filter over the suite listing. That listing pages newest-first, so reaching a known run through it costs work proportional to how long ago the run finished, and cannot answer at all once the run falls outside the pages walked. A caller that already knows its run asks for its run.
Four promises, three of them mutation-tested (the fourth is the schema choice, proven separately below):
NOT_FOUNDrather than relying on the backend's fail-soft null — defense in depth is not the answer.overallslices; the refined one rejects it withexpected exactly one 'overall' slice, found 2. I checked this directly rather than assuming — that invariant is what every rendered rate rests on. A failure is a502, never a200carrying the bad row.runIdmust be the run asked for.runIdis onlystring().min(1)to the schema, so another run's document parses perfectly and would then be served under this run's heading.404is one answer for two facts — "no document" and "not visible to you" — deliberately not distinguished. Both mean unmeasured to a reader, and separating them would confirm that a run exists in a project the caller cannot see.Registered in both ratchets:
sdk-coverage(every route reaches the SDK) andopenapi-drift(every route is documented). The OpenAPI entry was inserted surgically — 48 lines added, none removed.Exactly one reading, never both
The two disagree by construction, because their denominators differ. Both on screen leaves a reader unable to tell which one is the report card, which is worse than either alone. So the choice is exclusive and made in one place:
notFoundis surfaced as its ownabsentstatus rather than an error. Otherwise every run that finished before the materializer shipped — most of them — would carry a red service message about nothing being wrong.requestFailedandinvalidContractstay errors and stay apart from each other: one is a service state, the other a bug report, and swapping silently to older numbers would hide either.An unresolved flag reads as off. PostHog answers
undefinedwhile loading, and reading that as on would fire a request per run for every user on every page load.Two things I got wrong and fixed
The hook was being called twice. I first had the slot call
useRunUserValueChainChoiceinternally and the run detail call it for the rail's emptiness check. Unlike Convex'suseQuery, afetchhook called twice issues two requests — nothing de-duplicates them. The hook is now called once in the run detail and its result passed to the renderer as props.The rail's emptiness check was wrong for canonical runs. The existing probe answers only for the legacy rollup, so a run with a canonical document and no rollup would have reported "no funnel" and closed the rail over content that is there. It now counts whichever reading the slot will actually draw.
Also in this PR: a fixup to IN4
SessionUserValueChain.test.tsxasserted the judge label's wording verbatim, and IN4 changed that wording ("no judge verdict was ever owed" → "no LLM judge verdict was ever owed"). CI here does not run the client suite on pull requests, so it went unnoticed on #4493. Fixed on the IN4 branch and merged forward, so #4493 is green on its own.The fix asserts against
STAGE_REASON_LABELS.judgeNotRequestedrather than a copy of its text — which is what the test's name actually claims, and which does not break the next time the copy is polished. Making the component render the raw wire enum still fails it.Verification
Full inspector suite: 10,969 passed, 29 skipped, 990 files — run on the IN4 branch to confirm nothing else in the stack was broken
New: 7 route tests, 6 hook tests, 13 slot tests
Client typecheck clean; the two
evals.tserrors under the whole-repo server tsconfig are pre-existing (identical atHEAD, only the line number shifted)Mutation-tested every guard. Each caught by exactly its intended test:
runIdidentity checkPrettier was not run across
evals.ts,client.tsorsdk-coverage.test.ts— they are not prettier-clean atHEAD, and formatting them would have churned ~1,400 lines of unrelated code. Their diffs are purely additive.Operator note
Dark until backend #1195 is deployed and the
eval-run-stage-analyticsflag is enabled. With the flag off this is a no-op on the run detail and issues no requests.Generated by Claude Code
Note
Medium Risk
New authenticated eval API and run-detail rendering path affect how stage metrics are shown; behavior is feature-flagged and heavily tested, but wrong validation or fallback wording could mislead users about measured vs missing analytics.
Overview
Adds
GET /v1/projects/:projectId/eval-runs/:runId/stage-analytics(OpenAPI, SDKgetEvalRunStageAnalytics, server route with project match, refined schema validation,runIdbinding, unified 404) so eval run detail can load the same canonicalEvalStageAnalyticsV1document the suite page uses, instead of only the legacy rollup.Behind
eval-run-stage-analytics, run detail picks canonical or legacy, never both: flag off stays legacy with no fetch; in-flight shows nothing; success uses exportedRunDocument; 404/absent and errors fall back to labelled legacy, with service notes only for real failures.useEvalRunStageAnalyticshandlesabsentvs errors, stale-run guards, and re-fetches when the run becomes terminal (includingtimed_out), keyed on “run is over” so pending→running does not spam requests.run-detail-viewcallsuseRunUserValueChainChoiceonce, passes the result toRunUserValueChainSlot, and widenshasStageFunnelso canonical docs and failure notes keep the insight rail open. Legacy fallback copy distinguishes no document (404) from could not read.Reviewed by Cursor Bugbot for commit 9f516a1. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Adds the run-scoped stage-analytics read to the eval run detail, behind the
eval-run-stage-analyticsflag, so the same run now shows the same canonical document the suite page does. Previously the run detail showed only the legacy rollup, which disagrees with the canonical document by construction, so reads differed by surface. Flag-off is today's page exactly and issues no request; the change is dark until the backend reader is deployed and the flag is enabled.New route
GET /v1/projects/:projectId/eval-runs/:runId/stage-analyticswithPlatformApiClient.getEvalRunStageAnalytics— a separate read, not a paged filter over the suite listing.runIdto be the run asked for.404deliberately covers both "no document" and "not visible"; a payload that fails validation is a502, never a200.Run detail UI
404— read errors say "could not be read", silently for the dark-ship window and with a service note for real failures.timed_outnow counts as terminal), and a real failure counts as content so the rail can't close over the note reporting it.failedrows that rest on an absence (missingToolCall,predicateFailed,judgePartial,judgeFailed) tonotMeasured / providerError, and the phase flag is read at the handover, not once the driver resolves.Written for commit 9f516a1. Summary will update on new commits.