refactor: workflow API #1264
refactor: workflow API #1264
Conversation
🦋 Changeset detectedLatest commit: c021076 The changes in this PR will be included in the next version bump. This PR includes changesets to release 17 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
| ); | ||
| }); | ||
| // fixme: design a better API for validation | ||
| // test("workflow validation", async () => { |
There was a problem hiding this comment.
This is seems like a helper function for in/out Event relationship.
But should we check this in runtime, instead of top of the calling run
himself65
changed the title
@llamaindex/autotool
@llamaindex/community
@llamaindex/cloud
@llamaindex/core
@llamaindex/env
@llamaindex/experimental
llamaindex
@llamaindex/wasm-tools
commit: |
| } | ||
|
|
||
| set(key: string, value: any): void { |
There was a problem hiding this comment.
I removed the globals here. because I think it's not really used in the code. Im thinking wheter to support this by adding extend syntax
There was a problem hiding this comment.
I don't see globals removed? But in any case, the get/set api (which stores stuff into globals) is a pretty useful api for sharing data between steps
himself65
changed the title
…' into himself65/20240925/signal
| msg: `Writing app using this specification: ${truncate(spec)}`, | ||
| }), | ||
| ); | ||
| console.log(`Writing app using this specification: ${truncate(spec)}`); |
There was a problem hiding this comment.
@himself65 how would you do context.writeEventToStream now?
There was a problem hiding this comment.
I can expose sendEvent to context api, but I'm not sure if it's oky for seriazlize
| return new StopEvent({ result: code }); | ||
| } | ||
|
|
||
| return new ReviewEvent({ review, code }); | ||
| }; | ||
|
|
||
| const codeAgent = new Workflow({ validate: true }); | ||
| const codeAgent = new Workflow().with(kvContext); |
There was a problem hiding this comment.
storing a map seems to be the default use case for me using workflow, how about using SimpleKVStore per default as Context? Then it would more behave like the old code
| return new StopEvent({ result: code }); | ||
| } | ||
|
|
||
| return new ReviewEvent({ review, code }); | ||
| }; | ||
|
|
||
| const codeAgent = new Workflow({ validate: true }); | ||
| const codeAgent = new Workflow().with(kvContext); | ||
| codeAgent.addStep(StartEvent, architect, { outputs: CodeEvent }); |
There was a problem hiding this comment.
@logan-markewich @himself65 if we change the API, we might also allow this instead:
| codeAgent.addStep(StartEvent, architect, { outputs: CodeEvent }); | |
| codeAgent.addStep(architect, { inputs: StartEvent, outputs: CodeEvent }); |
| context: Context, | ||
| ev: AnalysisEvent | CritiqueEvent, | ||
| _: unknown, | ||
| ...events: [AnalysisEvent, CritiqueEvent] |
There was a problem hiding this comment.
this always awaits now the events in the order given in the events array?
@logan-markewich @himself65 is this something we want?
There was a problem hiding this comment.
e.g. how do we trigger to the user now that we haven't received all events yet? we might want to do that
| @@ -15,32 +14,28 @@ export class JokeEvent extends WorkflowEvent<{ joke: string }> {} | |||
| export class CritiqueEvent extends WorkflowEvent<{ critique: string }> {} | |||
| export class AnalysisEvent extends WorkflowEvent<{ analysis: string }> {} | |||
|
|
|||
| const generateJoke = async (_context: Context, ev: StartEvent) => { | |||
| const generateJoke = async (_: unknown, ev: StartEvent) => { | |||
There was a problem hiding this comment.
don't like this unknown a lot, i would prefer having a Context even if it's not used (see above that we might want to use a kv store as default)
There was a problem hiding this comment.
how do we do this example?
There was a problem hiding this comment.
how do we do now the validation?
| yield* context.streamEvents(); | ||
| // This method is used to create a new instance of Workflow with the same steps | ||
| // though this API we follow functional programming principles | ||
| private static from<Start, Stop, ContextData extends object, Checked>( |
| * if you want stop immediately once reach a StopEvent, you should handle it in the other side. | ||
| * @private | ||
| */ | ||
| #createStreamEvents(): AsyncIterableIterator<WorkflowEvent> { |
There was a problem hiding this comment.
- can we reduce the complexity of this function?
- how can we implement to limit the number of promises per step - in python, that's:
@step(num_workers=4)
See discussion: #1261
checklist for Oct 1 - 5