FE-717: Prepare Petrinaut for AI assistant#8721
Conversation
… prepare actions for AI assistance
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
PR SummaryMedium Risk Overview Adds a comprehensive set of Zod schemas (with descriptions) for every mutation in Updates the editor/UI to call mutations with single-object inputs (replacing callback/positional mutation APIs), introduces granular actions like Reviewed by Cursor Bugbot for commit a2406d2. Bugbot is set up for automated code reviews on this repo. Configure here. |
🤖 Augment PR SummarySummary: Preparatory refactor to make Petrinaut’s SDCPN mutations callable as validated “actions”, enabling future AI-assistant tool calling. Changes:
Technical Notes: Action inputs use 🤖 Was this summary useful? React with 👍 or 👎 |
| } | ||
| for (const equation of sdcpn.differentialEquations) { | ||
| if (equation.colorId === parsedTypeId) { | ||
| equation.colorId = ""; |
There was a problem hiding this comment.
libs/@hashintel/petrinaut/src/core/actions.ts:~320 — removeType assigns equation.colorId = "", but differentialEquationSchema requires a non-empty idSchema for colorId, so this can leave the document in an invalid state where later equation updates/validations will throw and the equation becomes effectively uneditable.
Severity: high
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| }); | ||
|
|
||
| if (typeof window !== "undefined") { | ||
| window.addEventListener("beforeunload", () => { |
There was a problem hiding this comment.
libs/@hashintel/petrinaut/src/core/instance.ts:66 — This beforeunload cleanup doesn’t cover instance.dispose() (and the handler itself isn’t removed), so doc subscriptions/listeners can still accumulate in long-lived pages or if multiple Petrinaut instances are created/destroyed.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| (item) => item.elementId === input.elementId, | ||
| ); | ||
| if (element) { | ||
| Object.assign(element, clone(input.update)); |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a2406d2. Configure here.
| type: z.enum(["real", "integer", "boolean", "ratio"]), | ||
| ...currentScenarioParameterSchema.shape, | ||
| identifier: z.string(), | ||
| default: z.number(), |
There was a problem hiding this comment.
File format rejects equations with cleared colorId
High Severity
The differentialEquationSchema in both file-format and clipboard types now inherits colorId: idSchema (which enforces min(1)) from currentDifferentialEquationSchema.shape, but only overrides id and name to be more permissive. The old schema used plain z.string() for colorId. Since removeType and deleteItemsByIds actively set equation.colorId = "" when a referenced type is removed, any file exported after that operation cannot be re-imported, and clipboard paste of such equations fails.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit a2406d2. Configure here.
| existingParameter.name = event.target.value; | ||
| updateParameter({ | ||
| parameterId: parameter.id, | ||
| update: { name: event.target.value }, |
There was a problem hiding this comment.
Validated mutations throw on intermediate onChange keystrokes
Medium Severity
The onChange handlers for parameter name, type name, and differential equation name call mutation actions directly on every keystroke. The new actions validate through displayNameSchema, which rejects empty strings. When a user clears or selects-all-and-retypes the field, the validation throws an uncaught error, preventing the update and causing an input desync. The old callback-based mutations had no such validation. Other properties (e.g., place name) correctly use local state with onBlur validation.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit a2406d2. Configure here.
| }); | ||
|
|
||
| if (typeof window !== "undefined") { | ||
| window.addEventListener("beforeunload", () => { |
There was a problem hiding this comment.
We should not add references to DOM in Core.
And it is (at the moment) the responsibility of the consumer to do the cleanup.
We could enhance that later (there are multiple ways of doing it, but attaching an event listener on window is a bad one)


🌟 What is the purpose of this PR?
This is preparatory work for adding an AI assistant, which will need access to tools to mutate the Petri net.
The PR:
actionsproperty on the instance (e.g.instance.actions.addType).updateXfunctions which take a callback to instead take an update objectupdateX(id, update)becomesupdateX({ id, update }). This has caused changes to core. While I was touching these functions in core I just took their typing from the mutation functions in core rather than having them all repeat the interface, but can revert it if there was a reason for how it was before.Pre-Merge Checklist 🚀
🚢 Has this modified a publishable library?
This PR:
📜 Does this require a change to the docs?
The changes in this PR:
🕸️ Does this require a change to the Turbo Graph?
The changes in this PR:
🐾 Next steps
Add the AI assistant UI and demo website server endpoint.
🛡 What tests cover this?
❓ How to test this?