FE-1262: Unbreak array methods in scenario code, add a range() helper, and surface scenario compile errors - #9092
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
1cec691 to
d0d473d
Compare
PR SummaryMedium Risk Overview Adds a Python-style Simulation Settings now shows a red callout when the selected scenario fails to compile ( Reviewed by Cursor Bugbot for commit dcc9a5d. Bugbot is set up for automated code reviews on this repo. Configure here. |
There was a problem hiding this comment.
Pull request overview
Adds safer scenario authoring, a typed range() helper, visible compilation errors, and a bundled constellation example.
Changes:
- Restores built-in array methods and adds validated
range()support. - Exposes scenario compilation failures in Simulation Settings.
- Updates LSP behavior, tests, documentation, and examples.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
simulation-settings.tsx |
Displays scenario compilation errors. |
playback-settings-menu.stories.tsx |
Updates mocked simulation context. |
simulation/provider.tsx |
Exposes compilation errors. |
simulation/context.ts |
Adds compilation errors to context. |
playback/provider.test.tsx |
Updates test context fixture. |
use-petrinaut-mutations.test.tsx |
Updates test context fixture. |
use-petrinaut-commands.test.tsx |
Updates test context fixture. |
docs/scenarios.md |
Documents helpers and errors. |
docs/examples.md |
Documents constellation example. |
scenario/helpers.ts |
Implements range(). |
scenario/helpers.test.ts |
Tests helper behavior. |
compile-scenario.ts |
Injects scenario helpers. |
compile-scenario.test.ts |
Tests arrays, helpers, and errors. |
sandbox.ts |
Makes constructor masking array-compatible. |
scenario-session.test.ts |
Tests LSP scenario diagnostics. |
generate-virtual-files.ts |
Declares helpers and skips empty code. |
satellites-launcher.ts |
Adds constellation scenario. |
satellites-launcher.test.ts |
Tests constellation compilation. |
satellites-pre-deployed-constellation.md |
Adds example changeset. |
fe-1262-scenario-range-helper.md |
Adds feature changeset. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit d0d473d. Configure here.
|
@kube the copilot comments look sensible. Would you like to address them before me or Ciaran review? |
…surface compile errors The scenario sandbox masked `.constructor` on built-in prototypes with a throwing getter, which broke `Array.prototype.map`/`filter`/`slice`/ `concat`/`flatMap` in user-authored scenario code (ArraySpeciesCreate reads the array's constructor). The getter now returns `undefined` instead — the spec falls back to the default Array while the constructor-walk escape still dead-ends. Adds a Python-style `range(end)` / `range(start, end, step?)` helper to scenario expressions and initial-state code, typed in the Monaco editors via the scenario session virtual files, with a length cap so oversized ranges fail fast instead of freezing render-time compilation. SimulationProvider previously discarded scenario compilation errors, making failures invisible; they are now exposed on SimulationContext and rendered as an error callout in Simulation Settings.
An empty "Define as code" editor produced a cryptic TS2355 ("A function
whose declared type is neither 'undefined', 'void', nor 'any' must
return a value.") the moment the mode was toggled on, because the empty
body was wrapped in `function __check(): InitialState {}`. The runtime
compiler explicitly ignores empty code, so the virtual file is now
skipped for blank content — the same treatment empty parameter-override
and per-place expressions already get.
Also adds language-service regression coverage asserting that the
satellites-style initial-state code (range + scenario + parameters)
type-checks cleanly end-to-end.
…ample A fifth bundled scenario for the Probabilistic Satellite Launcher whose initial state is authored in code mode: range(...).map(...) builds an evenly-spaced ring of satellites from two scenario parameters (number_of_satellites, initial_altitude), on top of the net's planet_radius. Doubles as an in-product showcase of the range() helper and code-mode initial state introduced in this branch. Covered by an example-level test compiling the scenario end-to-end with default and user-supplied parameter values.
d0d473d to
27eb9b1
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated no new comments.
Suppressed comments (4)
libs/@hashintel/petrinaut-core/src/examples/satellites-launcher.ts:562
Satellite Orbit Dynamicsdivides the direction derivative byvelocityatlibs/@hashintel/petrinaut-core/src/examples/satellites-launcher.ts:321-324. Every token created here has zero velocity, so its first ODE evaluation produces a non-finite direction and the showcased scenario cannot simulate correctly beyond frame 0. Initialize a tangential direction (angle + Math.PI / 2) and circular speed (Math.sqrt(parameters.gravitational_constant / distanceToCenter)), then extend the example test to advance the simulation rather than only checking the compiled marking.
direction: angle,
velocity: 0,
libs/@hashintel/petrinaut-core/src/simulation/authoring/scenario/helpers.ts:50
- The
Math.ceilcount can violate the documented end-exclusive behavior for supported fractional steps. In JavaScript,0.07 / 0.01is7.000000000000001, sorange(0, 0.07, 0.01)currently iterates eight times and includes the endpoint0.07. Check each generated candidate againsttoin the step's direction before pushing it.
const length = Math.max(0, Math.ceil((to - from) / by));
libs/@hashintel/petrinaut-core/src/simulation/authoring/scenario/helpers.ts:71
Object.freeze(range)is shallow: the ordinary function'srange.prototypeobject remains mutable. User code can assignrange.prototype.someState, and that value is visible to later scenario compilations through this module singleton, violating the stated no-cross-evaluation-state guarantee. Freeze the prototype object before exposing the helper.
range: Object.freeze(range),
libs/@hashintel/petrinaut/docs/scenarios.md:85
- Empty code compiles to an empty initial-state object, and
SimulationProvidermerges that object over the manual marking (react/simulation/provider.tsx:584-588). It therefore retains manually configured tokens rather than making every place empty, and it also differs from an explicit empty per-place expression, which compiles to zero. Document the runtime no-op behavior instead.
The TypeScript editor type-checks against the current net's place names and types as you write, so unrecognised names show up as compile errors before save. Leaving the code editor empty is not an error: empty code simply defines no initial state (every place starts without tokens), matching how empty per-place expressions behave.
Deriving the element count from `Math.ceil((to - from) / by)` trusted a floating-point quotient: `0.28 / 0.01` is `28.000000000000004`, so `range(0, 0.28, 0.01)` produced 29 elements and included the excluded endpoint. The quotient is now treated as an upper bound (still used to reject oversized ranges before allocating), and each value is compared against `to` before being emitted.
…empty Skipping the virtual file for empty "Define as code" input silenced TS2355 but also removed the source file completions resolve against, so TypeScript threw "Could not find source file" and the worker turned that into a failed completion request. Ctrl+Space in a freshly opened empty editor — exactly where `range`, `scenario`, and `parameters` most need discovering — offered nothing. The file is now always generated while the mode is active; the wrapper's return type widens to `InitialState | void` only when the body is blank, which suppresses TS2355 without weakening the check for real code.
The scenario seeded every satellite with a radially outward heading and `velocity: 0`. The example's orbit dynamics divide by velocity, and at `direction === angle` the numerator cancels exactly, so the direction derivative evaluated to `0 / 0` and the whole ring went NaN on the first frame — the showcase constellation fell apart the moment you pressed play. Satellites now start tangentially (`angle + PI / 2`) at circular-orbit speed `sqrt(mu / r)`, which holds the ring's radius. The example test now runs real simulation frames instead of only asserting compilation.
4f305e9 to
56dd4dc
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated no new comments.
Suppressed comments (1)
libs/@hashintel/petrinaut/docs/scenarios.md:85
- Empty code does not make every place token-free:
compileScenarioproduces an empty scenario marking, whichSimulationProvideroverlays onto the manual marking (src/react/simulation/provider.tsx:584-588), leaving manual tokens in place. The user guide currently promises different runtime behavior, so describe this as “no scenario-specific initial state” instead.
The TypeScript editor type-checks against the current net's place names and types as you write, so unrecognised names show up as compile errors before save. Leaving the code editor empty is not an error: empty code simply defines no initial state (every place starts without tokens), matching how empty per-place expressions behave.
`Object.freeze` is shallow, so freezing the `range` function declaration left `range.prototype` writable. Helpers are module singletons handed to every evaluation by reference, so user code could stash a closure there during one evaluation and read it back in the next — exactly the cross-evaluation state leak the freeze was meant to prevent. Arrow functions have no `prototype`, which closes the channel outright rather than requiring a second freeze to remember.
The guide claimed empty code left "every place without tokens". It does not: `compileScenario` returns an empty marking, and the provider spreads the scenario marking over the manual one, so an empty scenario overrides nothing and manual markings survive. Also drops the claim that this matches per-place mode — clearing a per-place expression writes an explicit `0` for that place, so the two modes genuinely differ. The in-app assistant reads these pages at runtime, so a wrong promise here becomes wrong advice in the product.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated no new comments.
Suppressed comments (1)
libs/@hashintel/petrinaut-core/src/simulation/authoring/scenario/compile-scenario.ts:309
rangeis injected as a formal parameter, so previously valid code such asconst range = [1, 2]; return { Queue: range.length };now fails while constructing the function withIdentifier 'range' has already been declared. The LSP declaresrangeambiently and wraps the user's body in its own function (generate-virtual-files.ts:471,557), where this local shadowing is valid, so the editor will report no error. Run the user body in a nested scope while retaining the helper parameters in the outer function; this preserves helper lookup without reserving helper names in user code.
...SCENARIO_HELPER_NAMES,

🌟 What is the purpose of this PR?
Scenario "Initial State as code" silently did nothing for everyday code: the evaluation sandbox masked
.constructoron built-in prototypes with a throwing getter, which brokeArray.prototype.map/filter/slice/concat/flatMap(they read the array'sconstructorvia the spec'sArraySpeciesCreate) — and the resulting compile errors were then discarded bySimulationProvider, so nothing appeared in the UI.This PR unbreaks those array methods, adds a Python-style
range()helper for scenario code, and makes scenario compile errors visible in Simulation Settings.What scenario authors can now write:
And when a scenario fails to compile, a red callout below the Scenario dropdown now lists each error instead of silently falling back to the manual marking.
🔗 Related links
🔍 What does this change?
runSandboxed(petrinaut-core): the.constructormask on built-in prototypes returnsundefinedinstead of throwing. Spec-internal species lookups fall back to the defaultArray, so array methods work; the({}).constructor.constructor(...)escape still dead-ends (undefined.constructoris a TypeError). All pre-existing escape tests pass unchanged.range(end)/range(start, end, step?)helper injected into scenario parameter-override expressions, per-place expressions, and initial-state code. Frozen, validated (finite args, non-zero step), and capped at 1,000,000 elements so oversized ranges fail fast instead of freezing render-time compilation.SimulationProviderno longer discardscompileScenarioerrors: they are exposed asscenarioCompilationErrorsonSimulationContextand rendered as an error callout in Simulation Settings.function __check(): InitialState {}, surfacing a cryptic TS2355 ("A function whose declared type is neither 'undefined', 'void', nor 'any' must return a value.") the moment the mode was toggled on. Empty code is a runtime no-op, so the virtual file is now skipped — same treatment as empty parameter-override and per-place expressions. Regression-tested end-to-end through the language service, including the satellites-stylerange(...).map(...)code checking clean.range(scenario.number_of_satellites).map(...)atplanet_radius + scenario.initial_altitude. Serves as an in-product showcase of the new helper; covered by an example-level compile test and verified live (8 satellites at frame 0, scaling with the scenario parameters).docs/scenarios.md) updated:rangedocumented in both authoring modes, error callout documented under "Running a scenario".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:
SimulationProvider); behaviour is covered by the corecompileScenariotests plus the rendering being a simple conditional.🛡 What tests cover this?
helpers.test.ts:rangesemantics (arities, negative/non-integer steps, empty ranges, validation errors, length cap, works underrunSandboxed).compile-scenario.test.ts: regression test for the exactArray.from({ length }).map(...)shape that used to fail;rangein code mode, override expressions, and per-place expressions; cap/zero-step error reporting; helper escape-safety (range.constructor === undefined); all 8 pre-existing sandbox escape tests unchanged and passing.❓ How to test this?
yarn devinlibs/@hashintel/petrinaut).nand "Define as code" initial state returning{ SomePlace: range(n).map((i) => ({ ... })) }.n— the compiled initial state updates and the simulation uses it.range(0, 5, 0)) and confirm a red callout appears below the Scenario dropdown listing the error.🤖 Generated with Claude Code