Add built-in AI flow launch inputs#24595
Conversation
|
@codex review |
Validation ReportAll 21 validations passed. Show details
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 70b9fb063d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| names = [flow_input.name for flow_input in inputs] | ||
| if len(names) != len(set(names)): | ||
| raise ValueError("Input names must be unique") | ||
| reserved_names = {flow_input.name for flow_input in BUILT_IN_FLOW_INPUTS} |
There was a problem hiding this comment.
When a flow declares a valid custom input named max_timeout, this validator lets it through because only BUILT_IN_FLOW_INPUTS (prd) are reserved. The launch modal renders declared inputs with id="input-{name}" and then always adds the built-in timeout field with id="input-max_timeout", so that flow ends up with duplicate widget IDs / an ambiguous #input-max_timeout lookup and cannot reliably launch. Please also reserve max_timeout or give the built-in field an ID that cannot collide with flow inputs.
Useful? React with 👍 / 👎.
AAraKKe
left a comment
AAraKKe
left a comment
There was a problem hiding this comment.
Thanks Luis! I think we are mixing reposibilities and that is why so much code needs to change. Take a look at the comment on the models.
We want to inject them where inputs live, always. Every integration needs a prd and the timeout to run, so we just inject them at validation (is intended to modify values as well, not only raise errors) and everything else will adapt to it. Define the max timeout as a number.
| reserved_names = {flow_input.name for flow_input in BUILT_IN_FLOW_INPUTS} | ||
| if conflicts := sorted(set(names) & reserved_names): | ||
| raise ValueError(f"Input names are reserved for built-in launch inputs: {conflicts}") |
There was a problem hiding this comment.
request: instead of simply validating them, inject them if they are not there. Both pr and max timeout. The validator can be used to modify the variables. The reason is that at the moment you are defining the built in inputs here but then in variables you need to handle them as well, generating a coupling between specific inputs and variables, which did not exist at the moment.
If in validation, when a prd and max_timeout input are not defined, you inject them here, the rest of the code stays unchanged, because the model is the only one responsible for having its inputs.
You don't even need to modify the TUI layer. The reason we are is because we are bypassing the owner of the inputs being the flow and we are defining a set of inputs outside of their natural ownership layer.
| yield Label(f"{PRD_INPUT.label.upper()} ({PRD_INPUT.input_type.value})", classes="eyebrow") | ||
| yield from self._widget_for(PRD_INPUT) | ||
| yield Label("MAX TIMEOUT (SECONDS)", classes="eyebrow") | ||
| yield Input(placeholder="Leave empty for unbounded", id=MAX_TIMEOUT_INPUT_ID) |
There was a problem hiding this comment.
note: this would not exist if the 2 mandatory inputs already came from inputs.
| file_access_policy=FileAccessPolicy(write_root=write_root), | ||
| callbacks=callbacks, | ||
| resume=self.resume, | ||
| max_timeout=self.max_timeout, |
There was a problem hiding this comment.
note: this would be just get the timeout from the run_time variables because it will always be here, and if not, pass None.
2 participants
What does this PR do?
Adds built-in launch settings to every
ddev meta aiflow:prdruntime variable.UI screenshot
Motivation
Flows need consistent product requirements and configurable run duration without repeating framework-owned inputs in every flow definition. Custom placeholders also let flow authors provide examples without accidentally submitting those examples as default values.
Review checklist (to be filled by reviewers)
qa/requiredif this PR needs QA validation, orqa/skip-qaif it does not. Exactly one of the two is required.backport/<branch-name>label to the PR and it will automatically open a backport PR once this one is merged