Migrate to BlockModelV3#31
Conversation
- Unified BlockData (UI-shaped); args lambda projects workflow shape and validates by throw (replaces argsValid) - defaultBlockLabel no longer stored: .subtitle derives label from data.datasetLabel (snapshotted by UI on dataset-picker gesture) plus data.numPoints and data.extrapolation - Drop dead `numRules` field (unserializable function array, unread) - Persisted V1 state preserved via DataModelBuilder.upgradeLegacy - UI bindings move to app.model.data; defineApp -> defineAppV3 - Bump SDK to 1.77.0 (model, ui-vue, workflow-tengo, test, graph-maker)
There was a problem hiding this comment.
Code Review
This pull request migrates the rarefaction block to BlockModelV3, unifying the data model for UI-shaped persistence and updating the model definition. It includes a transition to DataModelBuilder for legacy state upgrades, updates to the UI bindings, and a dependency version bump across the project. I have reviewed the changes and identified an inconsistency between the validation logic in isValidNum and the user-facing error messages regarding integer constraints and numeric bounds.
| if (data.mem === undefined) throw new Error('Memory is required'); | ||
| if (data.cpu === undefined) throw new Error('CPU is required'); |
There was a problem hiding this comment.
The
mem and cpu fields are typed as number (non-optional) in BlockData, and both .init() and .upgradeLegacy() always assign concrete values. These === undefined guards can never be true at runtime; TypeScript will report them as dead code under strictNullChecks. They add noise without adding safety.
| if (data.mem === undefined) throw new Error('Memory is required'); | |
| if (data.cpu === undefined) throw new Error('CPU is required'); | |
| // mem and cpu are guaranteed non-optional by BlockData; no runtime guard needed. |
Prompt To Fix With AI
This is a comment left during a code review.
Path: model/src/index.ts
Line: 75-76
Comment:
The `mem` and `cpu` fields are typed as `number` (non-optional) in `BlockData`, and both `.init()` and `.upgradeLegacy()` always assign concrete values. These `=== undefined` guards can never be true at runtime; TypeScript will report them as dead code under `strictNullChecks`. They add noise without adding safety.
```suggestion
// mem and cpu are guaranteed non-optional by BlockData; no runtime guard needed.
```
How can I resolve this? If you propose a fix, please make it concise.| '@platforma-sdk/model': 1.71.0 | ||
| '@platforma-sdk/ui-vue': 1.71.0 | ||
| '@platforma-sdk/tengo-builder': 2.5.20 | ||
| '@platforma-sdk/workflow-tengo': 5.24.0 |
There was a problem hiding this comment.
graph-maker peer dependency version mismatch
@milaboratories/graph-maker@1.4.2 declares exact peer dependencies on @platforma-sdk/model: 1.73.3 and @platforma-sdk/ui-vue: 1.73.3, but the workspace catalog pins both at 1.77.0. The lock file resolves the package with 1.77.0, silently ignoring the exact constraint. If strict peer dependency checking is enforced in CI or in a future pnpm install, the install will fail; and if any internal API consumed by graph-maker changed between 1.73.3 and 1.77.0, there is a latent runtime incompatibility. Consider aligning the graph-maker version to one whose declared peer ranges actually cover 1.77.0, or adding an explicit pnpm.peerDependencyRules override documenting the skipped mismatch.
Prompt To Fix With AI
This is a comment left during a code review.
Path: pnpm-workspace.yaml
Line: 13
Comment:
**graph-maker peer dependency version mismatch**
`@milaboratories/graph-maker@1.4.2` declares exact peer dependencies on `@platforma-sdk/model: 1.73.3` and `@platforma-sdk/ui-vue: 1.73.3`, but the workspace catalog pins both at `1.77.0`. The lock file resolves the package with `1.77.0`, silently ignoring the exact constraint. If strict peer dependency checking is enforced in CI or in a future `pnpm install`, the install will fail; and if any internal API consumed by graph-maker changed between 1.73.3 and 1.77.0, there is a latent runtime incompatibility. Consider aligning the graph-maker version to one whose declared peer ranges actually cover 1.77.0, or adding an explicit `pnpm.peerDependencyRules` override documenting the skipped mismatch.
How can I resolve this? If you propose a fix, please make it concise.
numRulesfield (unserializable function array, unread)Greptile Summary
This PR migrates the rarefaction block from
BlockModeltoBlockModelV3, consolidating the previously separateBlockArgsandUiStateinto a singleBlockDatashape. Legacy persisted state is preserved throughDataModelBuilder.upgradeLegacy, and the SDK is bumped across the board to1.77.0.BlockDatareplaces the oldBlockArgs/UiStatesplit; the.argslambda now validates by throwing instead of using.argsValid, anddefaultBlockLabelis dropped in favour of a derived.subtitlebased on the snapshotteddatasetLabel.upgradeLegacyfaithfully maps V1argsanduiStateto the new flat shape;datasetLabelis intentionally leftundefinedfor old blocks and is reseeded on the next dataset-picker interaction.@platforma-sdk/*and@milaboratories/*packages are bumped;graph-maker@1.4.2declares exact peer deps on SDK1.73.3while the workspace uses1.77.0, which pnpm silently accepts but could break with strict peer dependency enforcement.Confidence Score: 3/5
The model migration is well-structured and backward-compatible, but the graph-maker package declares an exact peer dependency on SDK 1.73.3 while the workspace installs 1.77.0 — pnpm resolves this silently, leaving a gap that strict peer dep checking or a runtime API difference could expose.
The core V3 migration logic is clean and the legacy upgrade path is correctly handled. The main risk is the graph-maker/SDK version mismatch: the lock file resolves graph-maker@1.4.2 against SDK 1.77.0 despite the package declaring an exact 1.73.3 requirement. If that peer dep constraint exists because of a breaking API change, the graph component could misbehave silently at runtime.
pnpm-workspace.yaml and pnpm-lock.yaml — the graph-maker peer dependency resolution should be verified or the mismatch explicitly acknowledged via pnpm peer dependency rules.
Important Files Changed
refin the setter shadows Vue's importedreffunction.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Block loads] --> B{Has V3 data 'V20260518'?} B -- Yes --> C[Use BlockData directly] B -- No / Legacy --> D[upgradeLegacy: args + uiState to BlockData] D --> C C --> E{args lambda validates data} E -- throws --> F[Block not runnable] E -- returns BlockArgs --> G[Workflow receives datasetRef, numPoints, numIterations, extrapolation, mem, cpu] C --> H[subtitle from customBlockLabel OR getDefaultBlockLabel] C --> I[UI binds to app.model.data.*]Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "Migrate to BlockModelV3" | Re-trigger Greptile