Skip to content

Migrate to BlockModelV3#31

Merged
julenmendieta merged 3 commits into
mainfrom
chore/rarefaction-v3-migration
May 19, 2026
Merged

Migrate to BlockModelV3#31
julenmendieta merged 3 commits into
mainfrom
chore/rarefaction-v3-migration

Conversation

@erohinaelena

@erohinaelena erohinaelena commented May 19, 2026

Copy link
Copy Markdown
Contributor
  • 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)

Greptile Summary

This PR migrates the rarefaction block from BlockModel to BlockModelV3, consolidating the previously separate BlockArgs and UiState into a single BlockData shape. Legacy persisted state is preserved through DataModelBuilder.upgradeLegacy, and the SDK is bumped across the board to 1.77.0.

  • Unified data model: BlockData replaces the old BlockArgs/UiState split; the .args lambda now validates by throwing instead of using .argsValid, and defaultBlockLabel is dropped in favour of a derived .subtitle based on the snapshotted datasetLabel.
  • Legacy migration: upgradeLegacy faithfully maps V1 args and uiState to the new flat shape; datasetLabel is intentionally left undefined for old blocks and is reseeded on the next dataset-picker interaction.
  • SDK bump: All @platforma-sdk/* and @milaboratories/* packages are bumped; graph-maker@1.4.2 declares exact peer deps on SDK 1.73.3 while the workspace uses 1.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

Filename Overview
model/src/index.ts Core migration to BlockModelV3: unified BlockData, args-as-lambda validation, legacy upgrade via DataModelBuilder.upgradeLegacy; dead numRules removed. Two unreachable undefined guards exist for mem/cpu.
model/src/types.ts New file cleanly separates BlockData, BlockArgs, LegacyBlockArgs, and LegacyBlockUiState type definitions; well-documented.
ui/src/pages/GraphPage.vue Bindings moved to app.model.data; datasetRefModel setter snapshots dataset label. Parameter ref in the setter shadows Vue's imported ref function.
ui/src/pages/TablePage.vue Simple binding update from uiState.tableState to data.tableState; no issues.
ui/src/app.ts Clean swap from defineApp to defineAppV3 using the new platforma export.
pnpm-workspace.yaml SDK bumps are consistent; vue pinned down from ^3.5.24 to exact 3.5.24. graph-maker@1.4.2 declares exact peer deps on SDK 1.73.3 while the workspace uses 1.77.0.
.changeset/v3-migration.md Accurate and thorough changeset describing the migration, backward-compatibility strategy, and dropped fields.

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.*]
Loading
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
ui/src/pages/GraphPage.vue:39-45
The setter parameter `ref` shadows Vue's `ref` function imported on line 6. While this is syntactically valid (the setter body never needs to call `ref()`), it is a maintenance trap: any future code added inside the setter that tries to create a reactive ref will silently use the `PlRef | undefined` parameter instead of the Vue primitive.

```suggestion
  set: (selectedRef: PlRef | undefined) => {
    app.model.data.datasetRef = selectedRef;
    // Snapshot the chosen dataset's human label into `data` so `.subtitle`
    // can derive the default block label without re-querying the result pool.
    app.model.data.datasetLabel = selectedRef
      ? app.model.outputs.datasetOptions?.find((o) => plRefsEqual(o.ref, selectedRef))?.label
      : undefined;
```

### Issue 2 of 3
model/src/index.ts:75-76
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.
```

### Issue 3 of 3
pnpm-workspace.yaml:13
**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.

Reviews (1): Last reviewed commit: "Migrate to BlockModelV3" | Re-trigger Greptile

Greptile also left 3 inline comments on this PR.

- 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)

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread model/src/index.ts
Comment thread ui/src/pages/GraphPage.vue Outdated
Comment thread model/src/index.ts
Comment on lines +75 to +76
if (data.mem === undefined) throw new Error('Memory is required');
if (data.cpu === undefined) throw new Error('CPU is required');

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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.

Suggested change
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.

Comment thread pnpm-workspace.yaml Outdated
'@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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 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.

@julenmendieta julenmendieta merged commit 093500a into main May 19, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants