-
Notifications
You must be signed in to change notification settings - Fork 23
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
AI cleanups #3443
AI cleanups #3443
Conversation
berekuk
commented
Nov 19, 2024
- simplify accumulated cruft around controllers
- optional outputs and a more straightforward/type-safe API for steps
- fix logs for finished workflows (show from stored MDX instead of rebuilding from a deserialized workflow)
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Skipped Deployments
|
} | ||
|
||
export function fixAdjustRetryLoop<Shape extends IOShape>( | ||
export function getDefaultTransitionRule<Shape extends IOShape>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No more "addLinearRule", this function now returns the rule itself (expressed as another function), and every specific workflow has a single rule.
I think this is as declarative as we can get, on this outer layer of configuring workflows, and this approach will hold as long as workflows are sequential.
workflow.addStep(generateCodeStep, { prompt: inputs.prompt }); | ||
}, | ||
getInitialStep: (workflow) => | ||
generateCodeStep.prepare({ prompt: workflow.inputs.prompt }), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This API is also new: instead of imperative "configure" = "do anything, inject even handlers", now we return the step itself.
I had to implement PreparedStep
types, separate from both LLMStepTemplate
and LLMStepInstance
, and I'm not happy about that, but I think it was unavoidable (see the long comment on PreparedStep
type in LLMStepTemplate
).
outputIds: Record<string, number>; | ||
// Legacy - in the modern format we store outputs on SerializedState, but we still support this field for old workflows. | ||
// Can be removed if we deserialize and serialize again all workflows in the existing database. | ||
outputIds?: Record<string, number>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hope I haven't broken old workflows with this change (moving outputs to step.state
, and changing serialization code accordingly); as far as I can tell it's fine, i.e. old workflows still load in my dev hub instance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy to generally see cleanups like this