Repository navigation
Review saved execution traces in Release reviews - #2219
sidneyswift wants to merge 1 commit into
Conversation
Reviewers could see evidence versions, gaps and review receipts on /releases but nothing about the runs that produced them: no entry, retry, failure, claim or node state was reviewable in the product UI, even though list_executions and read_execution are merged and deployed. Add an "Execution traces" panel under the current case that lists the request's saved executions and opens one as a node table joining the immutable plan with recorded outcomes and claims. The review state is the outcome status, else "unknown (claimed, needs reconciliation)" for a claim without an outcome, else not_started, so uncertainty is never shown as success or failure. Plan states map to template, implemented and blocked by policy. Cost is reported as unknown, never zero, and the rows name what was not recorded (prompts, cost, outcome). The hook only reads through the shared operations and never retries or dispatches. Refs #2146 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to The trace panel is mergeable after normal checks. No concrete issue remains that would prevent reviewers from loading or reading saved traces. Pre-merge checks |
  |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Solid & Clean Code |  |
The pull request introduces several oversized, multi-purpose functions. ReleaseExecutionNodes spans lines 28–126 and renders the node table, state legend, and trace disclaimer. `ReleaseExecutionTrac… |
Split the UI into focused components for the execution list, node table, state legend, and trace notice. Extract row-state, missing-data, and row-projection helpers from buildExecutionNodeRows. Separate list and open operations from the h… |
✅ Passed checks (2 passed)
-
Check name Status Explanation Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Full details: Solid & Clean Code
-
Explanation
The pull request introduces several oversized, multi-purpose functions.
ReleaseExecutionNodesspans lines 28–126 and renders the node table, state legend, and trace disclaimer.ReleaseExecutionTracespans lines 16–90 and handles loading, execution-list rendering, trace rendering, and notices.useExecutionTracespans lines 16–77 and combines lifecycle invalidation, request orchestration, state updates, and error mapping.buildExecutionNodeRowsspans lines 20–67 and combines joins, review-state calculation, presentation labels, and missing-data messages. The repeated review label also appears in bothReleaseExecutionNodes.tsx:21andbuildExecutionNodeRows.ts:52. These new functions exceed the stated 20-line guideline and mix responsibilities.Resolution
Split the UI into focused components for the execution list, node table, state legend, and trace notice. Extract row-state, missing-data, and row-projection helpers from
buildExecutionNodeRows. Separate list and open operations from the hook while retaining one small shared request/error wrapper. Define shared constants for repeated review labels and missing-data messages.
- Fix all pre-merge checks with AI
✨ Finishing Touches
-
📝 Generate docstrings
-
-
- Commit to this branch
- Create a new PR
-
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
A saved trace unfolds in rows
Plan and outcome find their place
Claims mark time, or leave it blank
Notices speak when requests fail
Release cases show the trail
Quietly, each state comes into view
Comment @coderabbitai help to get the list of available commands.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
6 issues found across 9 files
Confidence score: 4/5
ReleaseCasesPage.tsxandReleaseExecutionNodes.tsxnow coordinate several distinct UI responsibilities, which makes these components harder to maintain. Split out focused components where that simplifies ownership.ReleaseExecutionTrace.tsxdoes not announce loaded results or an opened node table to screen-reader users. Add a persistent polite status message for those actions.ReleaseExecutionNodes.tsxcan showsaved,reused,failed, andblockedreview states, but the legend does not explain them. Add those states to the legend so users can interpret the column.ReleaseExecutionTrace.test.tsxdoes not assert that an unclaimed, unrecorded node renders asnot_started. Add that assertion to catch regressions in this state.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="components/Releases/ReleaseCasesPage.tsx">
<violation number="1" location="components/Releases/ReleaseCasesPage.tsx:98">
P2: Custom agent: **Code Structure and Size Limits for Readability and Single Responsibility**
`ReleaseCasesPage.tsx` is now 109 lines and coordinates another panel alongside the list, intake, details, and assessment. Extract the current-case panel composition into a focused component so this page stays under the rule’s 100-line limit.</violation>
</file>
<file name="components/Releases/ReleaseExecutionNodes.tsx">
<violation number="1" location="components/Releases/ReleaseExecutionNodes.tsx:10">
P3: The legend explains only `template`, `implemented`, `blocked by policy`, `unknown (claimed, needs reconciliation)` and `not_started`, but the Review state column can also render `saved`, `reused`, `failed` and `blocked`, which are neither in the legend nor qualified in the prose below it. Add legend entries for the four outcome statuses so reviewers can tell recorded outcomes apart from policy states.</violation>
<violation number="2" location="components/Releases/ReleaseExecutionNodes.tsx:28">
P2: Custom agent: **Code Structure and Size Limits for Readability and Single Responsibility**
`ReleaseExecutionNodes.tsx` is 126 lines and combines the node table, state legend, and trace disclaimer, exceeding Rule 1’s 100-line limit. Split the legend and/or footer into focused components so this file stays under the limit.</violation>
</file>
<file name="lib/releases/__tests__/buildExecutionNodeRows.test.ts">
<violation number="1" location="lib/releases/__tests__/buildExecutionNodeRows.test.ts:1">
P2: Custom agent: **Enforce Clear Code Style and Maintainability Practices**
This new test file is 174 lines, over the 100-line limit the style rules set for all files. The bulk is the inline `trace` fixture and `node`/`row` helpers, not assertions. Extract the fixture into a sibling fixture module (the repo already does this with `streamHistoryFixture.ts`/`streamSeriesFixture.ts`) and import it here, keeping the test file under 100 lines.</violation>
</file>
<file name="components/Releases/__tests__/ReleaseExecutionTrace.test.tsx">
<violation number="1" location="components/Releases/__tests__/ReleaseExecutionTrace.test.tsx:125">
P3: The fixture includes an unclaimed, unrecorded node, but this panel test never asserts its rendered `not_started` state. Assert that label so the UI test catches regressions that stop distinguishing unstarted nodes from other states.</violation>
</file>
<file name="components/Releases/ReleaseExecutionTrace.tsx">
<violation number="1" location="components/Releases/ReleaseExecutionTrace.tsx:45">
P2: The non-empty list and opened node table appear without a live announcement, so screen-reader users receive no result announcement after either action. Add a persistent polite status message for loaded results.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| organizationId={selectedOrgId} | ||
| getAccessToken={getAccessToken} | ||
| /> | ||
| <ReleaseExecutionTrace |
There was a problem hiding this comment.
P2: Custom agent: Code Structure and Size Limits for Readability and Single Responsibility
ReleaseCasesPage.tsx is now 109 lines and coordinates another panel alongside the list, intake, details, and assessment. Extract the current-case panel composition into a focused component so this page stays under the rule’s 100-line limit.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At components/Releases/ReleaseCasesPage.tsx, line 98:
<comment>`ReleaseCasesPage.tsx` is now 109 lines and coordinates another panel alongside the list, intake, details, and assessment. Extract the current-case panel composition into a focused component so this page stays under the rule’s 100-line limit.</comment>
<file context>
@@ -94,6 +95,12 @@ export default function ReleaseCasesPage() {
organizationId={selectedOrgId}
getAccessToken={getAccessToken}
/>
+ <ReleaseExecutionTrace
+ key={"trace" + currentKey}
+ requestId={current.request_id}
</file context>
| @@ -0,0 +1,126 @@ | |||
| import { | |||
There was a problem hiding this comment.
P2: Custom agent: Code Structure and Size Limits for Readability and Single Responsibility
ReleaseExecutionNodes.tsx is 126 lines and combines the node table, state legend, and trace disclaimer, exceeding Rule 1’s 100-line limit. Split the legend and/or footer into focused components so this file stays under the limit.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At components/Releases/ReleaseExecutionNodes.tsx, line 28:
<comment>`ReleaseExecutionNodes.tsx` is 126 lines and combines the node table, state legend, and trace disclaimer, exceeding Rule 1’s 100-line limit. Split the legend and/or footer into focused components so this file stays under the limit.</comment>
<file context>
@@ -0,0 +1,126 @@
+];
+const formatTime = (value?: string) =>
+ value ? new Date(value).toLocaleString() : "not recorded";
+export default function ReleaseExecutionNodes({
+ rows,
+}: {
</file context>
| @@ -0,0 +1,174 @@ | |||
| import { expect, it } from "vitest"; | |||
There was a problem hiding this comment.
P2: Custom agent: Enforce Clear Code Style and Maintainability Practices
This new test file is 174 lines, over the 100-line limit the style rules set for all files. The bulk is the inline trace fixture and node/row helpers, not assertions. Extract the fixture into a sibling fixture module (the repo already does this with streamHistoryFixture.ts/streamSeriesFixture.ts) and import it here, keeping the test file under 100 lines.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At lib/releases/__tests__/buildExecutionNodeRows.test.ts, line 1:
<comment>This new test file is 174 lines, over the 100-line limit the style rules set for all files. The bulk is the inline `trace` fixture and `node`/`row` helpers, not assertions. Extract the fixture into a sibling fixture module (the repo already does this with `streamHistoryFixture.ts`/`streamSeriesFixture.ts`) and import it here, keeping the test file under 100 lines.</comment>
<file context>
@@ -0,0 +1,174 @@
+import { expect, it } from "vitest";
+import { buildExecutionNodeRows } from "../buildExecutionNodeRows";
+import type { ExecutionTrace, PlanNode } from "../executionTypes";
</file context>
| No recorded executions for this request. | ||
| </p> | ||
| )} | ||
| {!!traces.executions?.length && ( |
There was a problem hiding this comment.
P2: The non-empty list and opened node table appear without a live announcement, so screen-reader users receive no result announcement after either action. Add a persistent polite status message for loaded results.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At components/Releases/ReleaseExecutionTrace.tsx, line 45:
<comment>The non-empty list and opened node table appear without a live announcement, so screen-reader users receive no result announcement after either action. Add a persistent polite status message for loaded results.</comment>
<file context>
@@ -0,0 +1,90 @@
+ No recorded executions for this request.
+ </p>
+ )}
+ {!!traces.executions?.length && (
+ <ul className="space-y-2" aria-label="Recorded executions">
+ {traces.executions.map((execution) => (
</file context>
| expect(table.getByText("collection_not_permitted")).toBeTruthy(); | ||
| expect(table.getByText(resultId)).toBeTruthy(); | ||
| expect(table.getByText("template")).toBeTruthy(); | ||
| expect(table.getByText("blocked by policy")).toBeTruthy(); |
There was a problem hiding this comment.
P3: The fixture includes an unclaimed, unrecorded node, but this panel test never asserts its rendered not_started state. Assert that label so the UI test catches regressions that stop distinguishing unstarted nodes from other states.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At components/Releases/__tests__/ReleaseExecutionTrace.test.tsx, line 125:
<comment>The fixture includes an unclaimed, unrecorded node, but this panel test never asserts its rendered `not_started` state. Assert that label so the UI test catches regressions that stop distinguishing unstarted nodes from other states.</comment>
<file context>
@@ -0,0 +1,176 @@
+ expect(table.getByText("collection_not_permitted")).toBeTruthy();
+ expect(table.getByText(resultId)).toBeTruthy();
+ expect(table.getByText("template")).toBeTruthy();
+ expect(table.getByText("blocked by policy")).toBeTruthy();
+ expect(table.getAllByText("implemented")).toHaveLength(2);
+ expect(table.getAllByText("unknown")).toHaveLength(4);
</file context>
| TableRow, | ||
| } from "@/components/ui/table"; | ||
| import type { ExecutionNodeRow } from "@/lib/releases/executionTypes"; | ||
| const LEGEND: ReadonlyArray<readonly [string, string]> = [ |
There was a problem hiding this comment.
P3: The legend explains only template, implemented, blocked by policy, unknown (claimed, needs reconciliation) and not_started, but the Review state column can also render saved, reused, failed and blocked, which are neither in the legend nor qualified in the prose below it. Add legend entries for the four outcome statuses so reviewers can tell recorded outcomes apart from policy states.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At components/Releases/ReleaseExecutionNodes.tsx, line 10:
<comment>The legend explains only `template`, `implemented`, `blocked by policy`, `unknown (claimed, needs reconciliation)` and `not_started`, but the Review state column can also render `saved`, `reused`, `failed` and `blocked`, which are neither in the legend nor qualified in the prose below it. Add legend entries for the four outcome statuses so reviewers can tell recorded outcomes apart from policy states.</comment>
<file context>
@@ -0,0 +1,126 @@
+ TableRow,
+} from "@/components/ui/table";
+import type { ExecutionNodeRow } from "@/lib/releases/executionTypes";
+const LEGEND: ReadonlyArray<readonly [string, string]> = [
+ [
+ "template",
</file context>
This branch was successfully deployed
1 participant
Summary
Makes saved execution traces reviewable from the shipped
/releasesreviewer UI using only the already-deployed, read-onlylist_executionsandread_executionoperations (no api or database change). A new "Execution traces" panel under the current case lists the request's saved executions (policy version, created time, recorded/planned node counts) and opens one as a node table that joins the immutable plan with recorded outcomes and claims. Review state is the outcome status (saved/reused/failed/blocked), elseunknown (claimed, needs reconciliation)for a claim without an outcome, elsenot_started, so uncertainty is never rendered as success or failure. Plan states are separated intotemplate(not_implemented),implemented(ready_for_dispatch / reuse_candidate) andblocked by policy; dependencies, block reasons, failure stage, reasons, evidence result IDs and claim/outcome timestamps are shown; cost is alwaysunknown(never zero) and each row names what was not recorded (prompts, cost, outcome). The hook mirrorsuseCompanyAssessment(generation counter, shareduseReleaseCaseRequestwith workspaceorganization_idon every call, 401 → sign-in message) and never retries or dispatches.Refs #2146 (child of epic #2116).
Merge sequencing
docs → database → api → app:
list_executions/read_executionincontext.json) — merge first.read_context_executionandlist_context_request_executionsshipped in database PR77 and are deployed.main(lib/context/processContextOperation.ts). No dependency on open api#981/fix: update font-family from "Inter Medium" to "Inter Regular" in glo… #983/Sweetmantech/myc 2703 remove unused chat title generation libs #985 (this PR does not touch api).<CompanyAssessment …/>inReleaseCasesPage.tsxplus two typed actions inlib/releases/responseTypes.ts; everything else is new files, to stay conflict-light with sibling tickets mounting panels on the same page.Verification
pnpm exec vitest run lib/releases/__tests__/buildExecutionNodeRows.test.ts components/Releases/__tests__/ReleaseExecutionTrace.test.tsx→ 2 files, 7 tests passed, 0 failed, 0 skipped.pnpm exec vitest run components/Releases hooks lib/releases→ 59 files, 187 tests passed, 0 failed (baseline on origin/main before this change: 57 files, 180 passed).pnpm exec tsc --noEmit→ no errors in changed files. The only reported error is pre-existing and unrelated:components/VercelChat/tools/SpotifyDeepResearchResult.tsx(5,25)imports a.pngand needs the gitignored, generatednext-env.d.ts(absent in a fresh worktree); with that file generated, tsc reports zero errors.pnpm exec eslint <9 changed files>→ clean;pnpm exec prettier --check <9 changed files>→ all files use Prettier code style.grep -c 'read_execution' components/Releases/ReleaseExecutionTrace.tsx hooks/useExecutionTrace.ts→ 1 and 1.Cannot find module '../buildExecutionNodeRows'and'../ReleaseExecutionTrace'); after the first implementation 6/7 passed andReleaseExecutionTrace.test.tsxfailed onfindByText("unknown (claimed, needs reconciliation)")matching both the table cell and the legend; the test was scoped to the node table viawithin(table)and all 7 pass./api/contextproxy), MCP transport. State reached: implemented + fixture-tested.Out of scope / known gaps
components/Releases/ReleaseExecutionNodes.tsx(table + legend + footer) beyond the plan's file list to keep one component per file.🤖 Generated with Claude Code
Summary by cubic
Makes saved execution runs reviewable in Release reviews for the first time. A new "Execution traces" panel under the current case lists the request's saved executions and opens one as a node table joining the immutable plan with recorded outcomes and claims, using only the already-deployed read-only
list_executionsandread_executionoperations. Opening a trace never retries work, dispatches a provider, or spends credits.saved/reused/failed/blocked), elseunknown (claimed, needs reconciliation)for a claim with no outcome, elsenot_started, so uncertainty is never rendered as success or failure.template(not_implemented),implemented(ready_for_dispatch / reuse_candidate), andblocked by policy; dependencies, block reasons, failure stage, reasons, evidence result IDs, and claim/outcome timestamps are shown.unknown, never zero, and each row names what was not recorded: prompts, cost, and outcome.useCompanyAssessmentbehavior and shows a sign-in message on 401.Written for commit cab0135. Summary will update on new commits.
Summary by CodeRabbit