quantified-uncertainty / squiggle

An estimation language
https://squiggle-language.com
MIT License
156 stars 23 forks source link

Serializable workflows and other heavy refactorings #3413

Closed berekuk closed 1 month ago

berekuk commented 1 month ago

This PR implements the full serialization/deserialization of workflows with our new @quri/serializer package.

So we now have three different types:

ClientWorkflow can't be reverted back to the Workflow, but SerializedWorkflow can, assuming that the code hasn't changed that much, i.e. that we still have implementations of all templates for the workflow and its steps in the codebase.

The old format where we stored ClientWorkflow is still supported, through AiWorkflow.format DB column. This will require DB migration when this PR is merged.

Other things this PR includes:

TODO:

changeset-bot[bot] commented 1 month ago

⚠️ No Changeset found

Latest commit: a1824b65d1fe137260b3d8e659644aa3b1db2cec

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

vercel[bot] commented 1 month ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
quri-hub ✅ Ready (Inspect) Visit Preview Oct 12, 2024 5:57pm
3 Skipped Deployments | Name | Status | Preview | Updated (UTC) | | :--- | :----- | :------ | :------ | | **quri-ui** | ⬜️ Ignored ([Inspect](https://vercel.com/quantified-uncertainty/quri-ui/3xc6uQWxaCWBtX31nWXvTjrQjsje)) | [Visit Preview](https://quri-ui-git-resumable-workflows-quantified-uncertainty.vercel.app) | Oct 12, 2024 5:57pm | | **squiggle-components** | ⬜️ Ignored ([Inspect](https://vercel.com/quantified-uncertainty/squiggle-components/HjMmqy1UtPtcVWbtyaCSHzLKiVCu)) | [Visit Preview](https://squiggle-components-git-resumable-967014-quantified-uncertainty.vercel.app) | Oct 12, 2024 5:57pm | | **squiggle-website** | ⬜️ Ignored ([Inspect](https://vercel.com/quantified-uncertainty/squiggle-website/Fhg9zojEuHf4Pyg1G4gmTYe4dFn1)) | [Visit Preview](https://squiggle-website-git-resumable-workflows-quantified-uncertainty.vercel.app) | Oct 12, 2024 5:57pm |
berekuk commented 1 month ago

Ok, I kind of cleaned up the logger stuff. I had to shift the complexity to other areas though; here are my raw brainstorm notes while I looked for solutions to the problem. To be clear, the problem is:

step -> workflow reference solutions

  1. avoid; pass workflow instance everywhere to pass it to logger
    • ugly
    • conceptually wrong - steps are stateful, state was produced by a workflow, we don't juggle steps between workflows, that reference should be present
  2. inject workflow reference somehow by improving serialization framework, e.g. by having "context" of deserialize visitor call
    • conceptually wrong because of deduplication
  3. serialize/deserialize "workflow-less" steps, instantiate to workflow-aware steps
    • naive approach is clunky, too many data types
    • deserialize to InstanceParams, then inject workflow ref? maybe not that bad
    • so we serialize/deserialize StepState, instead of Step objects, ok
  4. give up on serialization
  5. implement serialization for circular references
    • how to handle deserializations? how to wire things together?
    • lazily? pass getWorkflow callback instead of workflow
    • or pass workflow as a proxy object (whoa)

tbh deduplication gets in the way a little, most workflow/step references are not circular (but some are, e.g. artifacts)

I ended up doing (3).

berekuk commented 1 month ago

implement a route for resuming a workflow and expose it in the frontend (this can be done separately, but it's an important proof-of-concept for other changes)

This part from the top post I'm not going to do now, better to get this in first, and get back to it later.

berekuk commented 1 month ago

I've updated the top post with more explanations about what's included in this PR.

OAGr commented 1 month ago

I suggest documenting a lot of this stuff in the Squiggle AI README. I'd like it to not be too terrible for us/others to understand the repo, with that as a starting point.

OAGr commented 1 month ago

Also, I assume we really could use some tests for the serialization stuff. Seems pretty messy, easy to break later on.