Workflows CFG Extractor #455
Conversation
🦋 Changeset detectedLatest commit: d537846 The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 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 |
|
@karthikscale3 is attempting to deploy a commit to the Vercel Labs Team on Vercel. A member of the Team first needs to authorize it. |
|
|
||
| // Create a step function that captures closure variables | ||
| const calculate = async (x: number) => { | ||
| const computeValue = async (x: number) => { |
There was a problem hiding this comment.
this was causing a name clash, which is why I renamed it.
There was a problem hiding this comment.
we need to be resilient to name clashes
There was a problem hiding this comment.
what issue shows up because of this name conflict and how do we fix it?
| }: { | ||
| workflowBundlePath: string; | ||
| }): Promise<void> { | ||
| const workflowsManifestPath = this.resolvePath('.swc/workflows.json'); |
There was a problem hiding this comment.
I was considering incorporating the extraction into the steps and workflows manifest that gets generated. I think it's better to keep the generated graphs manifest separate for a couple of reasons:
- React flow is typically the UI component used for visualizing graphs and it's a nice DX to provide a
.jsonthat has the exact schema that react flow can understand natively. - The goal here is primarily visualizion in the o11y dashboard and potentially showing the runs(in realtime) progressing through the graph, keeping this file separate means
- We can render the graph even when no workflows are run.
- We can neatly apply runs on top of the graph even if the runs schema changes in the future.
There was a problem hiding this comment.
I actually don't think we should export a react flow json natively. Or atleast, we shouldn't export a manifest in the core builder that includes things like position: { x, y } etc. since that's beyond the scope of a workflow builder
it can be a simplified intermediate state that actually just represents the graph, from which the react flow graph can be rendered
I strongly think we should just have one manifest here. This split will cause more confusion and simply duplicate things we need to maintain. Let's simplify
| // Write workflows manifest to workflow data directory (post-bundle extraction) | ||
| const workflowDataDir = join( | ||
| this.config.workingDir, | ||
| '.next/workflow-data' |
There was a problem hiding this comment.
this data directory should come from an env variable and not be hardcoded
also we need to add the manifest extraction to all the other builders too besides next
we should also definitely add tests in the e2e dev and build test suite to ensure the manifest is generated and correct. you can check the existing e2e dev.test.ts file and local-build.test.ts files for inspiration
this would ensure we're buidling correctly across all builders
There was a problem hiding this comment.
also on further thought, the manifest should not go to workflow-data directory. That is meant to be used by the local world for its data storage
The manifest is a build artifact and should either just be output alongside the where the routes are stored imo (but not served publicly), or just in a new corresponding build output directory for each framework (so next -> .next/workflows/manifest.json, nitro -> .nitro/workflows/manifest.json, etc.) We already output a manifest.debug.json file there and this can just replace that
@ijjk @TooTallNate for the vercel build output API, we should output into the diagnostics folder yeah?
| const workflowBundlePath = join(workflowGeneratedDir, 'flow/route.js'); | ||
| await this.createWorkflowsManifest({ | ||
| workflowBundlePath, | ||
| outfile: join(workflowDataDir, 'workflows.json'), |
There was a problem hiding this comment.
manifest.json, but also see the other comments above
| @@ -0,0 +1,6 @@ | |||
| --- | |||
| "@workflow/builders": patch | |||
| "@workflow/next": patch | |||
There was a problem hiding this comment.
what about the other builders?
| */ | ||
| export abstract class BaseBuilder { | ||
| protected config: WorkflowConfig; | ||
| protected lastWorkflowManifest?: WorkflowManifest; |
There was a problem hiding this comment.
I'm worried about sticky issues by caching this here. do we need to store intermittent state?
also currently, both, the stepsBundle and the workflowsBundle generates this manifest (I believe) - which makes this variable name odd if you're specifically trying tor ely on the step bundle
There was a problem hiding this comment.
honestly the "partialStepManifest" logic and "partialWorkflowManifest" logic that's currently happening was hackily shipped - which is why we ended up with this split partial step and workflow manifest files that we store into the fs rather than doing it correctly
let's just take this opportunity to clean it up with some help from claude and do it right and clean :)
pranaygp
left a comment
pranaygp
left a comment
There was a problem hiding this comment.
Left a bunch of comments we need to address. Very excited about this!
Let's try and just get things aligned with how we want to do manifests according to the versioning spec. I'll be a nuisance to try and clean this up later
|
@pranaygp thanks for the review! Great and valid points! Let me do a bit of thinking to address these. |
2 participants
This PR adds a new control flow graph (CFG) extractor that analyzes bundled workflow files and generates graph manifests for workflow visualization.
Note: The corresponding UI updates for graph viewer are in #456
Changes
@workflow/builders: Addworkflows-extractor.tsthat parses workflow bundles using SWC to extract nodes and edges representing step calls, loops, conditionals, and parallel execution patterns@workflow/builders: AddcreateWorkflowsManifest()method toBaseBuilderfor generating workflow graph manifests post-bundle@workflow/next: Integrate manifest generation into Next.js builder (initial build and watch mode rebuilds)Output
Generates a
workflows.jsonmanifest containing React Flow-compatible graph data for each workflow, including: