KIL-537 Replace repair UI with feedback UI#1271
Merged
Conversation
Remove all repair UI code (repair form, repair edit form, repair review/accept/delete flows) and replace with a new feedback UI that uses the Feedback data model from KIL-534. - Rename "Output Rating" to "Rating and Feedback" - Add inline feedback list (up to 3, truncated) with "Add Feedback" link - Add "All Feedback" modal with sortable table - Add "Add Feedback" modal using FormContainer - Delete output_repair_edit_form.svelte - Remove model_name/provider/focus_repair_on_appear props from Run Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRemoved repair-output editing and related bindings; added feedback feature with client-side state, sorting, dialogs, and GET/POST /feedback interactions; added two exported types to shared types; added a form element prop to conditionally hide labels. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as "Run UI (client)"
participant API as "Web API"
participant DB as "Database"
rect rgba(200,200,255,0.5)
UI->>API: GET /feedback?task_id=...&run_id=...
API->>DB: query feedback for task/run
DB-->>API: feedback list
API-->>UI: return feedback list
end
rect rgba(200,255,200,0.5)
UI->>API: POST /feedback (new feedback payload)
API->>DB: insert feedback record
DB-->>API: created feedback
API-->>UI: return created feedback
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request removes the output repair functionality and introduces a new feedback system for task runs, allowing users to add, view, and sort feedback entries. The review identifies a bug where the submission loading state is not correctly set and suggests improvements to the feedback loading logic to prevent race conditions and redundant network requests.
📊 Coverage ReportOverall Coverage: 91% Diff: origin/main...HEAD
Summary
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/web_ui/src/routes/(app)/run/run.svelte (1)
602-609: Duplicate CSS classes.Line 603 has
flex items-centerrepeated twice. While this doesn't affect functionality, it should be cleaned up.♻️ Proposed fix
- <div - class="flex items-center flex items-center text-nowrap 2xl:min-w-32" - > + <div class="flex items-center text-nowrap 2xl:min-w-32">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/web_ui/src/routes/`(app)/run/run.svelte around lines 602 - 609, The container div that renders the Overall Rating (the <div class="flex items-center flex items-center text-nowrap 2xl:min-w-32"> wrapping the "Overall Rating:" label and InfoTooltip) has duplicated CSS classes; edit that element in run.svelte to remove the duplicate "flex items-center" so the class list is deduplicated (e.g., "flex items-center text-nowrap 2xl:min-w-32").
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/web_ui/src/routes/`(app)/run/run.svelte:
- Around line 444-473: The submit_feedback function never sets
add_feedback_submitting = true at the start so the UI never shows a
loading/disabled state; inside the submit_feedback function (before the try
block or immediately after the early-return check) set add_feedback_submitting =
true, ensuring the finally block still sets it back to false; update the
submit_feedback function reference and ensure no other control flow paths skip
resetting add_feedback_submitting (submit_feedback, add_feedback_submitting,
add_feedback_error, add_feedback_dialog are the identifiers to locate).
---
Nitpick comments:
In `@app/web_ui/src/routes/`(app)/run/run.svelte:
- Around line 602-609: The container div that renders the Overall Rating (the
<div class="flex items-center flex items-center text-nowrap 2xl:min-w-32">
wrapping the "Overall Rating:" label and InfoTooltip) has duplicated CSS
classes; edit that element in run.svelte to remove the duplicate "flex
items-center" so the class list is deduplicated (e.g., "flex items-center
text-nowrap 2xl:min-w-32").
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f20df88a-a11e-43f8-b6a3-8fc7c91c4ed4
📒 Files selected for processing (4)
app/web_ui/src/lib/types.tsapp/web_ui/src/routes/(app)/run/+page.svelteapp/web_ui/src/routes/(app)/run/output_repair_edit_form.svelteapp/web_ui/src/routes/(app)/run/run.svelte
💤 Files with no reviewable changes (2)
- app/web_ui/src/routes/(app)/run/+page.svelte
- app/web_ui/src/routes/(app)/run/output_repair_edit_form.svelte
- Add request ID tracking and run ID dedup to load_feedback to prevent race conditions and redundant requests when switching runs - Set add_feedback_submitting = true at start of submit_feedback Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/web_ui/src/routes/`(app)/run/run.svelte:
- Around line 619-621: The class attribute on the highlighted <div> contains the
duplicate token "flex items-center" twice; edit the <div> element's class
attribute (the one containing "text-nowrap 2xl:min-w-32") to remove the repeated
"flex items-center" so each utility appears only once and keep spacing
consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e9d9654c-8be0-4537-b960-c8b693e44d7e
📒 Files selected for processing (1)
app/web_ui/src/routes/(app)/run/run.svelte
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/web_ui/src/routes/`(app)/run/run.svelte:
- Around line 756-782: The "Created At" and other sortable <th> headers use
on:click but lack keyboard accessibility; update the headers (the ones calling
handle_feedback_sort and referencing feedback_sort_column and
feedback_sort_direction) to be focusable and operable via keyboard by adding
tabindex="0", role="button", and an on:keydown handler that invokes
handle_feedback_sort when Enter or Space is pressed (mirror the pattern used on
the feedback preview div for keyboard activation), and also ensure the visual
sort indicator logic remains unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7fe5bca6-e9be-42e2-ad84-22b7c12ea5ab
📒 Files selected for processing (1)
app/web_ui/src/routes/(app)/run/run.svelte
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/web_ui/src/routes/`(app)/run/run.svelte:
- Around line 428-446: The issue is that an in-flight load_feedback() can
overwrite newer locally-submitted feedback; to fix, ensure submit_feedback()
either (A) increments the shared feedback_request_id (so any pending requests
return early because request_id !== feedback_request_id) before/after pushing
the new feedback, or (B) upon successful submit merge the returned/created
feedback into the existing feedbacks array instead of replacing feedbacks
wholesale; update code paths that set feedbacks (the async GET handler that uses
request_id/feedback_request_id and the submit_feedback flow) to use one of these
strategies so a slower load cannot replace a newer local entry.
- Around line 419-455: The function load_feedback currently sets
last_loaded_run_id before the GET completes, preventing retries on failure; move
the assignment so last_loaded_run_id = run.id is only set after the fetch
succeeds (i.e., after the request_id check and after confirming fetch_error is
null and data assigned). Keep the initial guard (if run.id ===
last_loaded_run_id) at the top and keep request_id checks in catch/finally; do
NOT set last_loaded_run_id on error so subsequent calls for the same run can
retry. Reference: load_feedback, last_loaded_run_id, feedback_request_id,
feedbacks, feedback_error.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d245b4ff-7d98-4488-8d42-660789d629f3
📒 Files selected for processing (2)
app/web_ui/src/lib/utils/form_element.svelteapp/web_ui/src/routes/(app)/run/run.svelte
sfierro
requested review from
chiang-daniel,
leonardmq,
scosman and
tawnymanticore
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (4)
app/web_ui/src/routes/(app)/run/run.svelte (4)
619-621:⚠️ Potential issue | 🟡 MinorDuplicate CSS class.
The class attribute contains
flex items-centertwice.🐛 Proposed fix
- <div - class="flex items-center flex items-center text-nowrap 2xl:min-w-32" - > + <div class="flex items-center text-nowrap 2xl:min-w-32">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/web_ui/src/routes/`(app)/run/run.svelte around lines 619 - 621, The class attribute on the div in run.svelte contains a duplicated "flex items-center"; remove the duplicate so the class string only includes "flex items-center" once (e.g., update the div with class on the element that currently reads 'class="flex items-center flex items-center text-nowrap 2xl:min-w-32"'), ensuring spacing and other classes like "text-nowrap" and "2xl:min-w-32" remain unchanged.
460-490:⚠️ Potential issue | 🟠 MajorPotential race condition: in-flight
load_feedbackcan overwrite submitted feedback.If
submit_feedbackcompletes while a slowload_feedbackis still pending, line 446 may replace the locally-appended entry with stale server data. Consider incrementingfeedback_request_idinsubmit_feedbackto invalidate pending loads, or trigger a fresh reload after successful submit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/web_ui/src/routes/`(app)/run/run.svelte around lines 460 - 490, submit_feedback can race with an in-flight load_feedback and have the later-resolving load overwrite the newly appended feedback; to fix, in submit_feedback (after validating and before awaiting the POST) increment the shared feedback_request_id (or set a local requestId token) and pass it to load_feedback so pending loads check and ignore stale requestIds, or alternatively after a successful POST call load_feedback() immediately to fetch a fresh list; update references to feedbacks, submit_feedback, load_feedback and feedback_request_id to implement this invalidation/reload logic.
419-456:⚠️ Potential issue | 🟠 MajorMark run as loaded only after successful fetch.
Setting
last_loaded_run_id = run.idat line 426 before the request completes will prevent retries if the initial load fails, since line 425 will return early on subsequent calls. Move the assignment to after successful data fetch.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/web_ui/src/routes/`(app)/run/run.svelte around lines 419 - 456, In load_feedback, don't set last_loaded_run_id before the network request completes; move the assignment so it's only set after a successful fetch (i.e., after you've checked request_id matches and fetch_error is null and assigned feedbacks). Specifically, remove or relocate the early assignment to last_loaded_run_id = run.id that currently occurs before the client.GET call and instead set last_loaded_run_id = run.id right after feedbacks = data and feedback_error = null inside the try block (keeping the existing request_id checks intact).
757-783:⚠️ Potential issue | 🟡 MinorSortable table headers lack keyboard accessibility.
The
<th>elements haveon:clickhandlers for sorting but cannot be focused or activated via keyboard, unlike the feedback preview div which correctly implementstabindex,role, andon:keydown.♿ Proposed fix: Add keyboard support to sortable headers
<th - class="w-[15%] hover:bg-base-200 cursor-pointer" + class="w-[15%] hover:bg-base-200 cursor-pointer select-none" + tabindex="0" + role="button" on:click={() => handle_feedback_sort("created_by")} + on:keydown={(e) => { + if (e.key === "Enter" || e.key === " ") { + e.preventDefault() + handle_feedback_sort("created_by") + } + }} >Apply the same pattern to the "Created At" header.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/web_ui/src/routes/`(app)/run/run.svelte around lines 757 - 783, The sortable table headers ("Created By" and "Created At") use on:click with handle_feedback_sort but lack keyboard accessibility; update those <th> elements to be focusable and operable via keyboard by adding tabindex="0", role="button", and an on:keydown handler that invokes handle_feedback_sort when Enter or Space is pressed (same pattern used by the feedback preview div), keeping the existing feedback_sort_column and feedback_sort_direction logic for the sort indicator.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@app/web_ui/src/routes/`(app)/run/run.svelte:
- Around line 619-621: The class attribute on the div in run.svelte contains a
duplicated "flex items-center"; remove the duplicate so the class string only
includes "flex items-center" once (e.g., update the div with class on the
element that currently reads 'class="flex items-center flex items-center
text-nowrap 2xl:min-w-32"'), ensuring spacing and other classes like
"text-nowrap" and "2xl:min-w-32" remain unchanged.
- Around line 460-490: submit_feedback can race with an in-flight load_feedback
and have the later-resolving load overwrite the newly appended feedback; to fix,
in submit_feedback (after validating and before awaiting the POST) increment the
shared feedback_request_id (or set a local requestId token) and pass it to
load_feedback so pending loads check and ignore stale requestIds, or
alternatively after a successful POST call load_feedback() immediately to fetch
a fresh list; update references to feedbacks, submit_feedback, load_feedback and
feedback_request_id to implement this invalidation/reload logic.
- Around line 419-456: In load_feedback, don't set last_loaded_run_id before the
network request completes; move the assignment so it's only set after a
successful fetch (i.e., after you've checked request_id matches and fetch_error
is null and assigned feedbacks). Specifically, remove or relocate the early
assignment to last_loaded_run_id = run.id that currently occurs before the
client.GET call and instead set last_loaded_run_id = run.id right after
feedbacks = data and feedback_error = null inside the try block (keeping the
existing request_id checks intact).
- Around line 757-783: The sortable table headers ("Created By" and "Created
At") use on:click with handle_feedback_sort but lack keyboard accessibility;
update those <th> elements to be focusable and operable via keyboard by adding
tabindex="0", role="button", and an on:keydown handler that invokes
handle_feedback_sort when Enter or Space is pressed (same pattern used by the
feedback preview div), keeping the existing feedback_sort_column and
feedback_sort_direction logic for the sort indicator.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e05686af-46aa-4047-97c8-ed538034d007
📒 Files selected for processing (1)
app/web_ui/src/routes/(app)/run/run.svelte
chiang-daniel
approved these changes
Apr 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Linear ticket: https://linear.app/kiln_ai/issue/KIL-537/replace-repair-ui-with-feedback-ui
Description
Replace the repair UI with a new feedback UI that uses the Feedback data model added in KIL-534.
output_repair_edit_form.svelteTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Changes
Chores