Code Review Bench PR #19951 - feat(ui-v2): improve task run details page parity with Vue by ketkarameya · Pull Request #12 · gitarcode/prefect

ketkarameya · 2026-02-10T22:00:03Z

Code Review Bench PR PrefectHQ#19951 - feat(ui-v2): improve task run details page parity with Vue

Benchmark PR for Gitar evaluation

Original PR: agentic-review-benchmarks#12

feat(ui-v2): improve task run details page parity with Vue

Summary by Gitar

New hook:
- useStateFavicon in hooks/useStateFavicon.ts dynamically updates browser favicon based on task run state (scheduled, pending, running, completed, failed, cancelled, crashed)
Browser feedback enhancements:
- Page title shows task run name using usePageTitle, 7 state-specific SVG favicon icons in /public with dark/light mode support
Task details UI improvements:
- All detail fields always visible with "None" for empty values, task inputs copy button enabled, breadcrumb navigation preserves tab context (/runs?tab=task-runs)
Test coverage:
- Comprehensive test suite for useStateFavicon covering state transitions, dark/light mode switching, and cleanup behavior

_{This will update automatically on new commits.}

- Add copy button to Task Inputs tab using JsonInput copy prop - Add usePageTitle hook to set browser tab title based on task run name - Add useFavicon hook to change favicon based on task run state - Add state-specific SVG icons for favicon (cancelled, completed, crashed, failed, pending, running, scheduled) - Fix breadcrumb to link to /runs?tab=task-runs instead of just /runs - Show all detail fields consistently with 'None' when empty instead of hiding them - Add Cache Key, Cache Expiration, Retry Delay, and Retry Jitter Factor fields to always display Co-Authored-By: alex.s@prefect.io <ajstreed1@gmail.com>

Co-Authored-By: alex.s@prefect.io <ajstreed1@gmail.com>

gitar-bot-staging · 2026-02-10T22:05:50Z

🔍 CI failure analysis for 73405db: Biome linter detected a non-null assertion operator in an unrelated test file that was not modified by this PR.

Issue

The Biome linter detected a forbidden non-null assertion operator (!) in a test file, causing the CI lint job to fail.

Root Cause

In src/components/ui/change-state-dialog/change-state-dialog.test.tsx at line 120, the code uses a non-null assertion operator:

await user.click(footerCloseButton!);

The Biome linter rule lint/style/noNonNullAssertion flags this as a warning. The CI pipeline treats linter warnings as failures.

Details

Important: This failing file is completely unrelated to the changes in this PR. The current PR modifies:

Task run details page components (task-run-details-page/index.tsx, task-run-details.tsx)
Adds 7 new SVG favicon files for task run states
Creates a new useStateFavicon hook

The failing test file change-state-dialog.test.tsx is for a completely different component (change state dialog) and was not touched by this PR.

Affected Location: src/components/ui/change-state-dialog/change-state-dialog.test.tsx:120:20

Linter Output:

⚠ Forbidden non-null assertion.

  118 │     );
  119 │     expect(footerCloseButton).toBeDefined();
> 120 │     await user.click(footerCloseButton!);
      │                     ^^^^^^^^^^^^^^^^^^
  121 │

Linter Rule: lint/style/noNonNullAssertion

The non-null assertion operator (!) tells TypeScript to treat a potentially null/undefined value as definitely defined. While sometimes necessary in tests, it bypasses TypeScript's type safety.

Code Review ⚠️ Changes requested 0 resolved / 2 findings

Good feature parity work, but two bugs need fixing: missing SVG files for CANCELLING/PAUSED states will cause broken favicons, and a falsy check on retry_jitter_factor will incorrectly display "None" when the value is 0.

⚠️

Bug: Missing SVG favicons for CANCELLING and PAUSED states

📄 ui-v2/src/hooks/use-state-favicon.ts:49

The StateType union in use-state-favicon.ts includes "CANCELLING" and "PAUSED", but no corresponding SVG files (cancelling.svg, paused.svg) were added to ui-v2/public/. When a task run is in either of these states, the hook will set the favicon href to /cancelling.svg or /paused.svg, resulting in a broken favicon (404).

Either add the missing SVG files, or map these states to existing icons (e.g., CANCELLING → cancelled.svg, PAUSED → pending.svg).

⚠️

Bug: Falsy check hides valid retry_jitter_factor value of 0

📄 ui-v2/src/components/task-runs/task-run-details/task-run-details.tsx:179

The expression taskRun.empirical_policy?.retry_jitter_factor ? ... : "None" uses a truthy check. If retry_jitter_factor is 0 (a valid numeric value), it will be treated as falsy and display "None" instead of "0". The original code used explicit !== null && !== undefined checks to avoid this issue.

This should use nullish coalescing or explicit null/undefined checks instead:

{taskRun.empirical_policy?.retry_jitter_factor != null
    ? taskRun.empirical_policy.retry_jitter_factor.toString()
    : "None"}

Suggested fix

				{taskRun.empirical_policy?.retry_jitter_factor != null
					? taskRun.empirical_policy.retry_jitter_factor.toString()
					: "None"}

Rules 🎸 1 action taken

Gitar Rules

🎸 Summary Enhancement: Appended technical summary with 4 bullets covering new hook, browser feedback enhancements, UI improvements, and test coverage

_{5 rules not applicable. Show all rules by commenting gitar display:verbose.}

Tip

Comment Gitar fix CI or enable auto-apply: gitar auto-apply:on

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

`Auto-apply`	`Compact`
`gitar auto-apply:on`	`gitar display:verbose`

_{Was this helpful? React with 👍 / 👎 | Gitar}

gitar-bot-staging · 2026-02-10T22:11:29Z

ui-v2/src/hooks/use-state-favicon.ts

+				? document.getElementById("favicon-32-dark")
+				: document.getElementById("favicon-32");
+
+		if (stateType) {


⚠️ Bug: Missing SVG favicons for CANCELLING and PAUSED states

The StateType union in use-state-favicon.ts includes "CANCELLING" and "PAUSED", but no corresponding SVG files (cancelling.svg, paused.svg) were added to ui-v2/public/. When a task run is in either of these states, the hook will set the favicon href to /cancelling.svg or /paused.svg, resulting in a broken favicon (404).

Either add the missing SVG files, or map these states to existing icons (e.g., CANCELLING → cancelled.svg, PAUSED → pending.svg).

_{Was this helpful? React with 👍 / 👎}

Apply suggested fix

gitar-bot-staging · 2026-02-10T22:11:30Z

ui-v2/src/components/task-runs/task-run-details/task-run-details.tsx

+			<dl className="flex flex-col gap-1 mb-2">
+				<dt className="text-gray-500">Retry Jitter Factor</dt>
+				<dd className="">
+					{taskRun.empirical_policy?.retry_jitter_factor


⚠️ Bug: Falsy check hides valid retry_jitter_factor value of 0

The expression taskRun.empirical_policy?.retry_jitter_factor ? ... : "None" uses a truthy check. If retry_jitter_factor is 0 (a valid numeric value), it will be treated as falsy and display "None" instead of "0". The original code used explicit !== null && !== undefined checks to avoid this issue.

This should use nullish coalescing or explicit null/undefined checks instead:

{taskRun.empirical_policy?.retry_jitter_factor != null ? taskRun.empirical_policy.retry_jitter_factor.toString() : "None"}

_{Was this helpful? React with 👍 / 👎}

Suggested change

{taskRun.empirical_policy?.retry_jitter_factor

{taskRun.empirical_policy?.retry_jitter_factor != null

? taskRun.empirical_policy.retry_jitter_factor.toString()

: "None"}

Apply suggested fix

devin-ai-integration bot and others added 5 commits January 25, 2026 12:11

test(ui-v2): add tests for use-favicon hook

41d13e9

Co-Authored-By: alex.s@prefect.io <ajstreed1@gmail.com>

fix(ui-v2): fix TypeScript errors in use-favicon tests

c9cb7c3

Co-Authored-By: alex.s@prefect.io <ajstreed1@gmail.com>

refactor(ui-v2): rename useFavicon to useStateFavicon for clarity

98e305d

Co-Authored-By: alex.s@prefect.io <ajstreed1@gmail.com>

update pr

73405db

github-actions bot added the ui-replatform label Feb 10, 2026

gitar-bot-staging bot reviewed Feb 10, 2026

View reviewed changes

kayagokalp closed this Feb 25, 2026

kayagokalp deleted the corrupted_pr_19951_20260125_6922 branch February 25, 2026 02:08

kayagokalp restored the corrupted_pr_19951_20260125_6922 branch February 25, 2026 02:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Code Review Bench PR #19951 - feat(ui-v2): improve task run details page parity with Vue#12

Code Review Bench PR #19951 - feat(ui-v2): improve task run details page parity with Vue#12
ketkarameya wants to merge 5 commits intobase_pr_19951_20260125_6922from
corrupted_pr_19951_20260125_6922

ketkarameya commented Feb 10, 2026 •

edited by gitar-bot-staging bot

Loading

Uh oh!

gitar-bot-staging bot commented Feb 10, 2026 •

edited

Loading

Issue

Root Cause

Details

Gitar Rules

Uh oh!

gitar-bot-staging bot Feb 10, 2026

Uh oh!

gitar-bot-staging bot Feb 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

ketkarameya commented Feb 10, 2026 • edited by gitar-bot-staging bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Bench PR PrefectHQ#19951 - feat(ui-v2): improve task run details page parity with Vue

Summary by Gitar

Uh oh!

gitar-bot-staging bot commented Feb 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue

Root Cause

Details

Gitar Rules

Uh oh!

gitar-bot-staging bot Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

gitar-bot-staging bot Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ketkarameya commented Feb 10, 2026 •

edited by gitar-bot-staging bot

Loading

gitar-bot-staging bot commented Feb 10, 2026 •

edited

Loading