Skip to content

Migrate graph-maker block to the structurer and BlockModelV3 - #133

Merged
erohinaelena merged 18 commits into
mainfrom
chore/migrate-to-structurer
Sep 16, 2026
Merged

erohinaelena merged 18 commits into
mainfrom
chore/migrate-to-structurer

Conversation

@vadimpiven

@vadimpiven vadimpiven commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Migrates the graph-maker block onto the block-tools structurer, completes the SDK upgrade, and moves the model to BlockModelV3.

Changes

  • Structurer + SDK upgrade (1dd9d91): tool-managed layout (oxlint/oxfmt, managed tsconfig/turbo/catalog), dropping the vite/prettier toolchain. Full SDK upgrade — model/ui-vue 1.79, workflow-tengo 6, tengo-builder 4, block-tools 2.11.
  • Author-code + lib bump (109706b): *.svg ambient declaration for the new vue-tsc check (vite still bundles the assets); GraphPageState.settings narrowed to the chartType actually consumed (SDK 1.79 OutputWithStatus<PFrameHandle> no longer admits the placeholder pFrame: undefined); removed a stray console.log and an unused import; added the structurer block smoke test. Bumped @milaboratories/graph-maker 1.4.1 → 1.4.6.
  • BlockModelV3 (774be19): the multi-graph page list is unified UI-only state (BlockData.graphs) projected to empty args; a one-time upgradeLegacy carries the V1 uiState.graphs over. pFrame stays an outputWithStatus via createPFrameForGraphs; sections derive from ctx.data.graphs. UI moves to defineAppV3 and app.model.data.

Notes

  • The graph-maker plugin is intentionally not adopted. graphMakerPlugin is single-instance, but this block is multi-graph (unbounded user-created pages); the multi-graph plugin is not yet available. The V3 model is therefore hand-rolled, keeping the existing multi-graph behavior.
  • Legacy-upgrade verification (Phase 5) still pending — confirming upgradeLegacy fires on a V1-saved project needs a running backend; CI build is green.

Greptile Summary

This PR migrates the graph-maker block to the structurer toolchain (oxlint/oxfmt), upgrades the full SDK stack (model/ui-vue 1.79, workflow-tengo 6, tengo-builder 4, block-tools 2.11), and moves the block model from BlockModel (V1/V2) to BlockModelV3 with a DataModelBuilder. The multi-graph page list moves from split args+uiState into a single unified BlockData.graphs, with a one-time upgradeLegacy migration path for V1-saved projects.

  • BlockModelV3 + DataModelBuilder: blockDataModel defines the on-disk format as "v1" (BlockData { graphs: GraphPageState[] }); .upgradeLegacy extracts the old uiState.graphs list; .init seeds empty graphs for new blocks. platforma is built with .args<BlockArgs>(() => ({})) (no-op) and .outputWithStatus("pFrame", createPFrameForGraphs).
  • UI migration: defineAppdefineAppV3; all app.model.ui.* accesses replaced with app.model.data.*; stale null-guard on app.model.ui and a leftover console.log removed from MainPage.vue; GraphPageState.settings narrowed to Pick<GraphMakerProps, "chartType"> since pFrame is now sourced from the model output.
  • GraphPageState.settings type narrowing: The settings field previously held the full GraphMakerProps (including pFrame: undefined); it is now typed as Pick<GraphMakerProps, "chartType">. The state setter in GraphPage.vue assigns settings: graphProps.value inside an as GraphPageState cast, which hides the fact that graphProps.value can be undefined — runtime safety is preserved by the v-if guard but the cast is type-unsafe.

Key terms touched in this PR:

Term Definition Change
BlockModelV3 New SDK block-model builder that accepts a DataModelBuilder-produced data model rather than separate args/uiState Replaces BlockModel.create('Heavy') throughout
BlockData Unified on-disk data type ({ graphs: GraphPageState[] }) that replaces the former split BlockArgs + UiState New type; declared in model/src/types.ts
DataModelBuilder SDK builder for declaring on-disk format version, upgrade paths, and default initializer Newly introduced via blockDataModel in model/src/dataModel.ts
upgradeLegacy DataModelBuilder method that transforms legacy (args, uiState) pairs to BlockData; fires once on first load of a V1 project Added to carry uiState.graphs forward; V1 args were always empty
GraphPageState Per-page view state: { id, label, state: GraphMakerState, settings } settings narrowed from GraphMakerProps to Pick<GraphMakerProps, "chartType">pFrame removed since it now comes from the model output
LegacyUiState Type for the old V1 uiState shape consumed by upgradeLegacy New type alias; references the new GraphPageState (extra pFrame field in legacy JSON is harmless at runtime)
defineAppV3 SDK Vue plugin factory for BlockModelV3-based apps Replaces defineApp; accessed via app.model.data instead of app.model.ui
OutputWithStatus<PFrameHandle> Reactive wrapper around a PFrameHandle output that carries load/error status Wired via .outputWithStatus("pFrame", (ctx) => createPFrameForGraphs(ctx)); no longer admits pFrame: undefined placeholder

Confidence Score: 4/5

The migration is structurally sound; the only concern is a type-unsafe cast in GraphPage.vue that a v-if guard makes safe at runtime, and a console.dir without assertion in the new smoke test.

The core V3 model wiring, upgradeLegacy path, and UI migration all look correct. The state setter in GraphPage.vue uses as GraphPageState to hide a potential undefined in the settings field — the v-if guard prevents this from firing at runtime but the cast makes the invariant invisible to future editors. The smoke test logs the block state but makes no assertion, so it can never fail even on a broken block.

ui/src/GraphPage.vue (type-unsafe cast in state setter) and test/src/wf.test.ts (assertion-free smoke test)

Important Files Changed

Filename Overview
model/src/index.ts Migrated from BlockModel to BlockModelV3; sections and pFrame output wiring look correct; BlockArgs projected to empty object as intended
model/src/dataModel.ts New DataModelBuilder wiring with upgradeLegacy preserving uiState.graphs; init to empty array is correct; logic is straightforward and safe
model/src/types.ts New types file: BlockData, GraphPageState (settings narrowed to Pick<GraphMakerProps,"chartType">), legacy shapes; LegacyUiState reuses new GraphPageState type which is acceptable since the extra legacy fields (pFrame) are harmless at runtime
ui/src/app.ts Switched from defineApp to defineAppV3; routes unchanged; clean migration
ui/src/GraphPage.vue Migrated from app.model.ui to app.model.data; state setter uses type-unsafe cast (as GraphPageState) to hide potential undefined settings — safe in practice due to v-if guard but worth addressing
ui/src/MainPage.vue Migrated to app.model.data, removed stale null-guard on app.model.ui, removed console.log; logic unchanged and correct
test/src/wf.test.ts New smoke test covers block instantiation and run; ends with console.dir rather than an assertion, making it more of an exploratory stub than a passing/failing test

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    subgraph Legacy["Legacy V1 on-disk format"]
        A["args: {}"]
        B["uiState: { graphs: GraphPageState[] }"]
    end

    subgraph DataModel["DataModelBuilder (dataModel.ts)"]
        C[".upgradeLegacy()\nextract uiState.graphs"]
        D[".init()\ngraphs: []"]
        E[".from<BlockData>('v1')"]
    end

    subgraph V3["BlockModelV3 (index.ts)"]
        F[".args<BlockArgs>(() => ({}))"]
        G[".sections(ctx => ctx.data.graphs.map(...))"]
        H[".outputWithStatus('pFrame', createPFrameForGraphs)"]
        I[".done()"]
    end

    subgraph UI["Vue UI"]
        J["defineAppV3(platforma)"]
        K["app.model.data.graphs"]
        L["GraphPage.vue\nstate + settings"]
        M["MainPage.vue\nnewId + addSection"]
    end

    B -->|"first load"| C
    C --> E
    D -->|"new block"| E
    E --> F
    F --> G
    G --> H
    H --> I
    I --> J
    J --> K
    K --> L
    K --> M
    H -->|"pFrame output"| L
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    subgraph Legacy["Legacy V1 on-disk format"]
        A["args: {}"]
        B["uiState: { graphs: GraphPageState[] }"]
    end

    subgraph DataModel["DataModelBuilder (dataModel.ts)"]
        C[".upgradeLegacy()\nextract uiState.graphs"]
        D[".init()\ngraphs: []"]
        E[".from<BlockData>('v1')"]
    end

    subgraph V3["BlockModelV3 (index.ts)"]
        F[".args<BlockArgs>(() => ({}))"]
        G[".sections(ctx => ctx.data.graphs.map(...))"]
        H[".outputWithStatus('pFrame', createPFrameForGraphs)"]
        I[".done()"]
    end

    subgraph UI["Vue UI"]
        J["defineAppV3(platforma)"]
        K["app.model.data.graphs"]
        L["GraphPage.vue\nstate + settings"]
        M["MainPage.vue\nnewId + addSection"]
    end

    B -->|"first load"| C
    C --> E
    D -->|"new block"| E
    E --> F
    F --> G
    G --> H
    H --> I
    I --> J
    J --> K
    K --> L
    K --> M
    H -->|"pFrame output"| L
Loading

Fix All in Claude Code

Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
ui/src/GraphPage.vue:23-34
**Type-unsafe `settings` assignment in state setter**

`graphProps.value` is typed as `Pick<GraphMakerProps, "chartType"> | undefined` because `Array.find()` can return `undefined`. The `as GraphPageState` cast on line 31 silently coerces the potential `undefined` into the non-optional `settings` field. In practice the `v-if="state && graphProps"` guard on line 55 ensures `<graph-maker>` only renders (and therefore only emits state updates) when `graphProps` is truthy, so the setter should never run with `graphProps.value === undefined`. However, the cast hides the type mismatch and future callers may not notice this invariant. A non-null assertion (`graphProps.value!`) would make the assumption explicit and traceable.

### Issue 2 of 2
test/src/wf.test.ts:14
**Debug `console.dir` left in smoke test**

The smoke test ends with `console.dir(await blockState.awaitStableValue(), { depth: 5 })` but has no assertion. For ongoing CI this will print the full block state to test output on every run without verifying anything. Consider adding at least a basic assertion (e.g. that the stable value is non-null or that the block reached the expected state), and removing or replacing the `console.dir` with a structured `expect`.

Reviews (1): Last reviewed commit: "Add changeset for structurer + V3 migrat..." | Re-trigger Greptile

Greptile also left 2 inline comments on this PR.

Context used:

  • Context used - Terms is a types in codebase. Provide the list of ... (source)

- Apply oxfmt formatting across ui/test sources (oxfmt could not run in the
  refresh commit until the lint errors below were fixed).
- Add a *.svg ambient module declaration so vue-tsc resolves icon imports
  (vite still handles the actual asset bundling).
- Narrow GraphPageState.settings to the chartType actually consumed; in SDK
  1.79 OutputWithStatus<PFrameHandle> no longer admits the placeholder
  `pFrame: undefined` set at graph creation (pFrame comes live from outputs).
- Remove a stray console.log and an unused RenderCtx import.
- Add the structurer block smoke test (replacing the empty placeholder).
- Bump @milaboratories/graph-maker 1.4.1 -> 1.4.6.
Hand-rolled V3 migration that keeps the multi-graph model (the planned
multi-graph plugin is not ready, so the graphMakerPlugin is intentionally
not adopted here).

- Model: BlockModelV3.create(blockDataModel); the graph-page list is unified
  UI-only state (BlockData.graphs) projected to empty args. A one-time
  upgradeLegacy carries the V1 uiState.graphs over; fresh projects init to [].
- pFrame stays an outputWithStatus via createPFrameForGraphs(ctx); sections
  are derived from ctx.data.graphs.
- UI: defineApp -> defineAppV3; app.model.ui.graphs -> app.model.data.graphs;
  drop the now-unnecessary uiState init guard (V3 data is always initialized).

@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 graph-maker block to the block-tools structurer and BlockModelV3, which includes upgrading the SDK dependencies, transitioning the model from the legacy V1 API, and updating UI bindings to use app.model.data with defineAppV3. The feedback recommends improving robustness by avoiding non-null assertions in tests, filtering out non-numeric IDs when calculating the next graph ID to prevent NaN propagation, and directly using mapped item settings in GraphPage.vue instead of relying on an external computed property.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread ui/src/GraphPage.vue
Comment thread ui/src/MainPage.vue
Comment thread test/src/wf.test.ts
Comment thread ui/src/GraphPage.vue
Comment thread test/src/wf.test.ts
vadimpiven and others added 7 commits June 19, 2026 16:15
- wf.test.ts: add { timeout: 30000 } to the block smoke test; the default
  5s vitest timeout is too short for the addBlock + runBlock + awaitBlockDone
  backend round-trip (CI failure). Matches the 20-30s convention used by other
  block tests.
- GraphPage.vue: write `item.settings` directly in the state setter instead of
  the external `graphProps.value` computed (per review) — local, non-undefined.
The structurer's refresh flipped build.yaml `test` to true; the original
graph-maker config (and the table block) use `test: false`. With a real
backend-driven smoke test now present, `test: true` made block CI try to run
it without a pl backend -> RpcError ECONNREFUSED 127.0.0.1:6345. Restore
`test: false`; the smoke test stays in-repo for local/manual backend runs.
…d start

Revert the earlier test:false workaround — respect the structurer's test:true.
The block smoke test runs against a CI-provisioned pl backend whose cold start
has no readiness gate (pl-compose uses a bare `sleep 1`), so a single attempt
loses the startup race -> RpcError ECONNREFUSED 127.0.0.1:6345. Match the green
test:true blocks (clonotype-browser, mixcr-clonotyping): add retry: 2 +
testTimeout to test/vitest.config.mts. No pl-docker-tag pin (default image).
…cturer

# Conflicts:
#	block/package.json
#	pnpm-lock.yaml
#	pnpm-workspace.yaml
#	ui/package.json
Output of `pnpm run upgrade-sdk` with block-tools 2.15.1 — no hand edits.

- Catalog to SDK 1.83 (model/ui-vue/test), tengo-builder 4.0.26,
  workflow-tengo 6.10.0, ts-builder 1.7.2, plus @platforma-sdk/block-kind 1.1.0.
- `.structure` v1 -> v2: the block package becomes a slim facade
  (block/src/index.ts, from-pack-v2 pointer) replacing block/index.js, and the
  mandatory kind/ package is scaffolded.
- Managed turbo/root scripts move to the PL_BUILD_* build matrix; CI switches to
  node 22 on hz-ubuntu-dind and gains the changeset bump gate.
Resolves the structurer's two kind sentinels and adapts the author code the
structure-v2 facade changed.

- kind: BlockParams is the graph pages a project template seeds the block with —
  id, label, chartType and template. A live GraphMakerState cannot travel:
  optionsState keys columns of the project it was built in, and
  usedDefaultOptions is the flag that suppresses re-application of defaults, so a
  carried-over page would bind to columns that do not exist and never receive the
  defaults that would fix it. chartType is kept alongside template because it is
  not derivable from it — the UMAP scatterplot shares template `dots` with the
  ordinary one. The chart-type and layer-template lists the parser checks against
  are held to the graph-maker types by the compiler, so a library release that
  adds or drops one breaks this build rather than admitting a chart the editor
  cannot open.
- model: DataModelBuilder({ kind }) + BlockModelV3.create({ dataModel, kind }),
  the mandatory templateParams projection, and init expanding a seed into exactly
  what MainPage.addSection writes for a new page — so a seeded page and a
  hand-created one are indistinguishable.
- ui: declare "*.scss" for the side-effect style imports; the newer vue-tsc
  rejects them otherwise (TS2882).
- test: the facade exports GraphMakerBlockPointer instead of blockSpec.
GraphSeed gains `optionsState` — which column or axis feeds each chart input,
and the filters on them — so a template seeds pages already bound rather than
only on the right chart. The parser reads it as an envelope (component
discriminant, selector shape, filter type/range/values) and leaves a component
with nothing chosen yet alone, so a half-bound page exports and re-applies
unchanged. Verified lossless against a real configured heatmap.

KNOWN LIMIT — the mapping does not yet relocate across projects. graph-maker
stores a selected source as `{kind:"column",name:"<PlRef JSON>",type}`, and the
SDK's relocateBlockIds only descends into values `isColumnUniversalKey`
recognizes; that outer shape is not one, so the source block's id survives the
apply verbatim (a bare PlRef, or a PlRef as a canonical string, both relocate
correctly — it is only the pf-plots wrapper that hides it). Axis sources are
harder still: their block id sits in a `domain` entry
(`pl7.app/vdj/clonotypingRunId`), which the relocator declines to rewrite by
design. So the mapping restores only where the applied project reuses the same
block ids.
A column's identity contains the PlRef naming the block that produced it, and on
apply the SDK repoints those refs at the blocks of the project being built. Its
relocator descends through objects and rewrites any reference it recognizes, but
graph-maker holds a selected source as one canonical string, and a ref sealed two
JSON layers inside it is not something the relocator can reach — `relocateBlockIds`
left the whole mapping untouched, so a seeded page named blocks of the project it
was exported from.

A seed now carries each source id as the object it is (`SourceId`), and the model
translates: `templateParams` splits the canonical string, `init` rebuilds it with
the same `canonicalizeJson` pf-plots builds these ids with, so the string handed
back is the one graph-maker would have computed. An id is carried whole — a field
this parser does not know about is preserved, because dropping one would rebuild
an id naming a different column.

Verified end to end against a real configured heatmap: through templateParams →
JSON → expandTemplateRefs → relocateBlockIds → the kind's parser → init, all three
column sources repoint at the new block id, the filters and the rest of the mapping
come back unchanged, and with an empty id map the round-trip is lossless.

REMAINING GAP — axis sources still do not relocate. An axis carries its block id
inside `domain` (`pl7.app/vdj/clonotypingRunId`), and the relocator leaves domain
entries alone by design: that is what stops a value which merely looks like an id
from being rewritten. Nothing a block can do reaches this — relocation runs before
both the kind's parser and `init`, and neither is handed the block-id map. A page
bound to such an axis still needs re-binding in the new project.
`@milaboratories/pl-model-common` 1.49.0 carries the domain relocation the axis
sources need, and `@platforma-sdk/model` 1.83.17 depends on it, so the block no
longer links a locally packed build. Catalog bumped by `pnpm run update-sdk`.
A seeded page arrived on the right chart and bound to its data, but plain: axes,
layers, statistics and palettes were left behind, so every cosmetic choice had to
be made again. The seed now carries all four, and `init` puts them back.

Whole, not as a difference from the chart type's defaults. The smaller thing to
carry would be the difference, and graph-maker produces those defaults itself —
`getInitialAxesSettings`, `getInitialStatisticsState`, `getDefaultLayersSettings` —
but they live in a module that imports the editor's icon components, so bundling it
into the model fails outright (rolldown: "Bundling CSS is no longer supported"). The
cost is stated where the fields are declared: a seeded page keeps the defaults of the
day it was exported, and a later change to them does not reach it.

The kind reads none of the fields inside a settings group. What a setting means is
settled where it is applied, and a parser that knew their shape would need teaching
again every time the editor gains a knob — so each group is checked as an object and
carried whole, like a source id.

Still left behind, deliberately: the open tab, the zoom, the lasso and the tooltip
hint, which are per-view; and `usedDefaultOptions`, which records the defaults already
applied and would stop the new project's from being applied at all.
A custom colour mapping was lost in a project created from a template. graph-maker
holds `dataBindAes` as a map keyed BY the source id, and a key is a string: the
reference inside it — the `PlRef` in a column id, the block id in an axis domain — is
past the reach of relocation, so every mapping arrived under the exported-from
project's key and matched nothing. Both kinds of key were affected; neither moved.

The seed now carries a list of `{ source, mapping }` with the source taken apart, the
same shape that made the data mapping relocate, and rebuilds the key canonically on
the way in so it is the string graph-maker looks the mapping up by.

Verified against the real ids: both a column-keyed and an axis-keyed mapping repoint,
the colours are unchanged, and with an empty id map the keys come back byte-identical
— which is what says the rebuilt key still matches.
Comment thread model/src/seedOptions.ts Outdated
Comment on lines +26 to +28
function splitSourceId(id: string): SourceId {
return JSON.parse(id) as SourceId;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what? loos like its parse, not split

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

replaced with sdk function

Comment thread model/src/seedOptions.ts Outdated
Comment on lines +30 to +32
function joinSourceId(id: SourceId): string {
return canonicalizeJson(id);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

again, naming looks strange

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

removed the function, use just canonicalizeJson

Comment thread model/src/seedOptions.ts Outdated
return canonicalizeJson(id);
}

export function optionsStateToSeed(live: LiveOptionsState): SeedOptionsState {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

pls start functions from verb

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

renamed

@erohinaelena
erohinaelena added this pull request to the merge queue Sep 16, 2026
Merged via the queue into main with commit 14fb797 Sep 16, 2026
10 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.

3 participants