Skip to content

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

Open
vadimpiven wants to merge 7 commits into
mainfrom
chore/migrate-to-structurer
Open

Migrate graph-maker block to the structurer and BlockModelV3#133
vadimpiven wants to merge 7 commits into
mainfrom
chore/migrate-to-structurer

Conversation

@vadimpiven

@vadimpiven vadimpiven commented Jun 19, 2026

Copy link
Copy Markdown

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

1 participant