Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: cannot repair CSV-imported runs #254

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

leonardmq
Copy link
Contributor

What does this PR do?

Add fallback to use simple prompt builder during repair if the TaskRun does not have a prompt_id or prompt_builder_name - as in the case of a CSV-imported TaskRun. This fixes part of the issue that makes CSV-imported TaskRun not repairable.

Related Issues

#248

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
Copy link
Contributor Author

@scosman - There now is another problem that might require some UI. The Repair API requires a model_name and provider. The UI for a synthetic or human runs uses the original run's model / provider; but CSV-imported (as well as future manual runs) ones do not have such properties.

image

Probably need an extra piece of UI to let the user select the model they want to use for the repair. It may make sense also in general because some repairs might be better done with a different model than the one used to generate the original (broken) output.

I was thinking a similar form similar to that one:
image

Maybe in the sidebar, and call it Repair Options - and save the selection in local storage to avoid requiring the user to pick a model on each repair.

@scosman
Copy link
Collaborator

scosman commented Mar 18, 2025

Oh right, good catch.

Hmmm. I'm torn on just disabling repair for imports or a UI like you suggest. Simplicity is good....

@leonardmq
Copy link
Contributor Author

leonardmq commented Mar 18, 2025

Oh right, good catch.

Hmmm. I'm torn on just disabling repair for imports or a UI like you suggest. Simplicity is good....

I was thinking a UI like this could work reasonably well - default it to the run's model / provider if there is one, but let the user pick another model if they want to or need to:

image

But also agree possibly confusing. Maybe disable the repair for imports for now, then allow manual repairing for imports when manual repair is added; and then reassess?


I opened a separate PR for disabling the repair UI. Does not hurt to have it disabled regardless of what the permanent solution eventually is.

@leonardmq leonardmq mentioned this pull request Mar 19, 2025
2 tasks
@leonardmq leonardmq force-pushed the leonard/bug-csv-cannot-repair branch from dc845ae to a4d2c6b Compare March 20, 2025 02:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants