feat: allow manual repair edit #261

leonardmq · 2025-03-20T02:29:15Z

What does this PR do?

This PR adds support for manually editing a task run repair. After Attempt repair, the user has a new button (Edit) that turns the repair output display into a text area that allows users editing the repair attempt.

This PR also fixes the missing validation of the repair output in the repair API. There was a bug that allowed repair outputs that do not match the structured output schema, which might have caused downstream problems due to corrupt shapes.

This is an acceptable solution but it has a major limitation due to requiring the user to first Attempt (automatic) repair. In the case of a TaskRun accessed from the Dataset page, the user does not have the ability to select the model used during the repair - it is by default the same as the one that generated the original run output. If a run used a small model (e.g. the small Llama models), the user may be stuck as the small model may fail to even pass validation during Attempt repair, which means the user then never gets access to the Edit.

New Edit button:

Edit mode:

Error state (if JSON does not match schema):

Related Issues

Manual edit requested: #115

Contributor License Agreement

I, @leonardmq, confirm that I have read and agree to the Contributors License Agreement.

Checklists

Tests have been run locally and passed
New tests have been added to any work in /lib

leonardmq · 2025-03-20T02:29:56Z

WIP because I want to add a couple of tests in the repair API

leonardmq · 2025-03-20T02:31:14Z

app/desktop/studio_server/repair_api.py

@@ -73,6 +76,12 @@ async def post_repair_run(
        project_id: str, task_id: str, run_id: str, input: RepairRunPost
    ) -> TaskRun:
        task, run = task_and_run_from_id(project_id, task_id, run_id)
+
+        # the repair must match the task's output schema if structured output is enabled
+        if task.output_json_schema:


I have not found a centralized utility that could be used to do the validation for this. The validation in the adapter does not seem to be usable outside the context of a generation run.

Is there a better existing method to check the object against this task's output schema?

leonardmq · 2025-03-20T02:33:23Z

app/web_ui/src/routes/(app)/run/run.svelte

+    if (!repair_run) {
+      return
+    }
+    repair_run.output.output = repair_output_edited


The way it currently works is that the repair run would still be attributed to whichever model generated the repair output that was repaired. Would you rather want the repair run to be different?

yeah, we should attribute it to the person making the edit. A standard Human datasource should work!

scosman · 2025-03-20T18:32:37Z

ohhh, can't wait!

Let's pretty print for the text-area if it's JSON? Makes it easier to edit. Basically: JSON.stringify(obj, null, 2)

leonardmq · 2025-03-22T12:42:56Z

app/web_ui/src/routes/(app)/run/output_repair_edit_form.svelte

+  function validate_output(text: string) {
+    if (task.output_json_schema) {
+      try {
+        // TODO: validate against the JSON schema instead of just checking if it is valid JSON


@scosman - I'd want to validate the schema on the client-side here, but the webapp does not have a library for JSON schema validation.

Is there any particular library you would prefer to use for JSON schema validation on the client side?

We can leave validation to server! We just shouldn't save it without validation, and should make a decent error message there.

I added validation on the server side, but doing it on the client too would be nice as it allows validating in realtime and doing things like disabling the submit button while invalid.

The current UI for editing the repair requires that you first save to apply the edit to the unsaved repair, and then you submit the repair. If we do server-only, the validation feedback would not be on this same form.

I could change it to make the edit form directly submit to the API (instead of applying to the run without submitting) if you think that makes more sense - then server validation errors could come through on this same form

leonardmq · 2025-03-22T12:44:22Z

app/web_ui/src/routes/(app)/run/output_repair_edit_form.svelte

+      output: {
+        ...repair_run.output,
+        output: repair_output_edited,
+        source: {


I tried source: null and source: undefined instead of prefilling like this, but the API validation does not allow that

leonardmq · 2025-03-22T12:44:50Z

app/web_ui/src/routes/(app)/run/output_repair_edit_form.svelte

+      <div
+        class="text-xs font-medium text-error flex flex-row mb-2 items-center"
+      >
+        <svg


This SVG comes from the Warning component

leonardmq · 2025-03-22T12:45:38Z

app/web_ui/src/routes/(app)/run/output_repair_edit_form.svelte

+  {#if task.output_json_schema}
+    {#if repair_output_edited_valid}
+      <div class="text-xs font-medium text-gray-500 flex flex-row mb-2">
+        <svg


This SVG is the same used elsewhere in the page to show Structure Valid.

Maybe worth wrapping each SVG into components and giving them names

totally. I try to do that when I duplicate it.... but as you can tell I'm not religious about it

…common form

leonardmq · 2025-03-22T13:05:58Z

@scosman

Added the following changes:

add a disabled state on the FormContainer to prevent submitting
prevent submitting form if the submit button is not visible
add server-side validation of the repair output if the task has structured output
refactor repair edit form:
- use the FormContainer
- prettify initial value
- add client-side validation if the task has structured output
- sets the human source (prefilled on frontend, and user id is set on the server side)
test for manual repair source

Kapture.2025-03-22.at.13.51.51.mp4

scosman · 2025-03-22T14:59:25Z

This looks great. Few quick comments, but I'll be able to look in more depth next week!

leonardmq marked this pull request as draft March 20, 2025 02:29

leonardmq commented Mar 20, 2025

View reviewed changes

leonardmq commented Mar 22, 2025

View reviewed changes

leonardmq force-pushed the leonard/repair-edit branch from 4737e61 to 364849c Compare March 22, 2025 12:47

leonardmq added 6 commits March 22, 2025 13:48

feat: allow users to edit a repair output manually

929700c

fix: add validation of repair output

417cdd7

Add disabled form btn and ignore keyboard when not submittable

085749b

Set userid on repair from human source

7d9e93a

refactor: extract output repair edit form into own component and use …

3a55c23

…common form

refactor: use standalone repair edit form

0fcefea

leonardmq force-pushed the leonard/repair-edit branch from 364849c to 3c45a75 Compare March 22, 2025 12:48

Add test for manual edit source

7e0ba5f

leonardmq force-pushed the leonard/repair-edit branch from 3c45a75 to 7e0ba5f Compare March 22, 2025 12:58

leonardmq marked this pull request as ready for review March 22, 2025 13:06

leonardmq requested a review from scosman March 22, 2025 13:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: allow manual repair edit #261

feat: allow manual repair edit #261

leonardmq commented Mar 20, 2025 •

edited

Loading

leonardmq commented Mar 20, 2025

leonardmq Mar 20, 2025 •

edited

Loading

leonardmq Mar 20, 2025

scosman Mar 20, 2025

scosman commented Mar 20, 2025

leonardmq Mar 22, 2025

scosman Mar 22, 2025

leonardmq Mar 22, 2025

leonardmq Mar 22, 2025

leonardmq Mar 22, 2025

leonardmq Mar 22, 2025

scosman Mar 22, 2025

leonardmq commented Mar 22, 2025

scosman commented Mar 22, 2025

feat: allow manual repair edit #261

Are you sure you want to change the base?

feat: allow manual repair edit #261

Conversation

leonardmq commented Mar 20, 2025 • edited Loading

What does this PR do?

Related Issues

Contributor License Agreement

Checklists

leonardmq commented Mar 20, 2025

leonardmq Mar 20, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

scosman commented Mar 20, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leonardmq commented Mar 22, 2025

scosman commented Mar 22, 2025

leonardmq commented Mar 20, 2025 •

edited

Loading

leonardmq Mar 20, 2025 •

edited

Loading