Skip to content

Migrate table block to the structurer and BlockModelV3 - #57

Merged
mzueva merged 8 commits into
mainfrom
chore/migrate-to-structurer
Aug 3, 2026
Merged

Migrate table block to the structurer and BlockModelV3#57
mzueva merged 8 commits into
mainfrom
chore/migrate-to-structurer

Conversation

@vadimpiven

@vadimpiven vadimpiven commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Modernizes the table block in two stages, each as its own commit for review.

1. Structurer migration + SDK upgrade

  • block-tools structure refresh adopts the tool-managed layout (oxlint/oxfmt replacing eslint/prettier, managed tsconfig/turbo/catalog, block index, upgrade-sdk script).
  • Full SDK upgrade forced by the require-latest gate: @platforma-sdk/model & ui-vue 1.48 → 1.79.14, workflow-tengo 5 → 6.6.3, tengo-builder 2 → 4, block-tools → 2.11.0, vue pinned to 3.5.24.
  • Author-code fixes for the SDK bump: declare @milaboratories/helpers a direct model dep + align its catalog pin (fixes TS2742 on the inferred model type); drop the removed @platforma-sdk/ui-vue/styles import; remove an unused test param flagged by oxlint.

2. V1 → V3 model migration

  • Convert the legacy BlockModel (V1) to BlockModelV3 with a unified data model. A one-time .upgradeLegacy carries over the V1 uiState (group + tableStateV2) and drops obsolete keys the old UI-side upgrader used to delete.
  • All block state is UI-only (column selection + table grid state); .args projects to an empty object and never gates the block (the workflow has no inputs).
  • pTable is built with createPlDataTableV3, sourcing column snapshots via ColumnCollectionBuilder + collectCtxColumnSnapshotProviders (primary/secondary via the per-column isPrimary flag, replacing V2's coreColumnPredicate).
  • UI moves from defineApp/app.model.ui to defineAppV3/app.model.data.

Commits

  1. Automated structurer refresh output (+ .structure marker)
  2. SDK 1.79 author-code fixes
  3. Changeset
  4. BlockModelV3 + createPlDataTableV3 migration

Verification

  • pnpm build 8/8 green; pnpm check green (oxlint/oxfmt/type-check).
  • Integration test (test/src/wf.test.ts) passes against a local backend. Storage confirms the V3 shape: {"dataVersion":"v1","data":{group,tableStateV2}}.

Caveats

  • The integration test only exercises the empty case (no column selected). The populated findColumns → createPlDataTableV3 path uses canonical SDK helpers but isn't runtime-exercised by the trivial test.
  • .upgradeLegacy wasn't run against a real on-disk V1 project (no V1 fixture available) — worth a manual check by loading a pre-existing V1 project before relying on the upgrade in production.

Greptile Summary

This PR modernizes the table block in two stages: a structurer/SDK upgrade (oxlint/oxfmt, SDK 1.79, workflow-tengo 6, tengo-builder 4) and a full migration from the legacy BlockModel V1 API to BlockModelV3 with a unified DataModelBuilder data model.

  • BlockModelV3 migration: All block state (column selection + table grid state) moves into a single BlockData type via DataModelBuilder; a one-time .upgradeLegacy carries over V1 uiState fields (group + tableStateV2) and drops obsolete keys.
  • pTable output rewrite: createPlDataTableV2 + manual coreColumnPredicate is replaced by ColumnCollectionBuilder + collectCtxColumnSnapshotProviders + createPlDataTableV3, with primary/secondary distinction now expressed via the per-column isPrimary flag; output type changes from output to outputWithStatus.
  • UI bindings: defineApp/app.model.uidefineAppV3/app.model.data; the exported model identifier changes from model to platforma.

Confidence Score: 4/5

Safe to merge for new projects; the populated pTable path and the V1-to-V3 upgrade path are both untested and should be validated manually before this block is used with real data or upgraded V1 projects in production.

The structurer migration and all tooling changes are mechanical and low-risk. The UI wiring and data-model plumbing are straightforward. The one concern is in model/src/index.ts: collection.dispose() is called before the extracted snapshots are passed to createPlDataTableV3, and that entire code path is explicitly unexercised by the integration test. If snapshots carry live references to the collection's internal state, the table would receive stale or invalid data silently.

model/src/index.ts (pTable outputWithStatus — dispose ordering and untested snapshot path), model/src/dataModel.ts (upgradeLegacy — no V1 fixture test)

Important Files Changed

Filename Overview
model/src/index.ts Core model migrated from BlockModel V1 to BlockModelV3; pTable output switches to ColumnCollectionBuilder + createPlDataTableV3 with a potential use-after-dispose on the snapshot objects.
model/src/dataModel.ts New DataModelBuilder for V3 unified data, with a one-time legacy upgrader; logic is correct but the upgradeLegacy path has no test coverage against real V1 fixtures.
model/src/types.ts Clean extraction of BlockData, ColumnGroup, LegacyBlockArgs, and LegacyUiState type definitions into their own file.
ui/src/app.ts Straightforward migration from defineApp to defineAppV3, wiring the platforma model.
ui/src/pages/TableView.vue All app.model.ui references updated to app.model.data; bidirectional sync watches and reactive state pattern preserved correctly.
test/src/wf.test.ts Integration test only exercises the empty/no-column-selected path; the findColumns to createPlDataTableV3 and upgradeLegacy paths remain untested.
block/package.json Adds block index entry point and shx for cross-platform rm/mv in scripts.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[BlockModelV3.create blockDataModel] --> B[.args - empty BlockArgs]
    A --> C[.output pColumns]
    A --> D[.output pFrame]
    A --> E[.output sheets]
    A --> F[.outputWithStatus pTable]
    F --> G[ColumnCollectionBuilder + collectCtxColumnSnapshotProviders]
    G --> H[collection.findColumns]
    H --> I[collection.dispose]
    I --> J[byId Map lookup primary + secondary snapshots]
    J --> K[createPlDataTableV3 isPrimary flag per column]
    subgraph DataModelBuilder
        L[.from BlockData v1]
        L --> M[.upgradeLegacy V1 uiState to group + tableStateV2]
        M --> N[.init emptyGroup + createPlDataTableStateV2]
    end
    A -.uses.-> DataModelBuilder
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
    A[BlockModelV3.create blockDataModel] --> B[.args - empty BlockArgs]
    A --> C[.output pColumns]
    A --> D[.output pFrame]
    A --> E[.output sheets]
    A --> F[.outputWithStatus pTable]
    F --> G[ColumnCollectionBuilder + collectCtxColumnSnapshotProviders]
    G --> H[collection.findColumns]
    H --> I[collection.dispose]
    I --> J[byId Map lookup primary + secondary snapshots]
    J --> K[createPlDataTableV3 isPrimary flag per column]
    subgraph DataModelBuilder
        L[.from BlockData v1]
        L --> M[.upgradeLegacy V1 uiState to group + tableStateV2]
        M --> N[.init emptyGroup + createPlDataTableStateV2]
    end
    A -.uses.-> DataModelBuilder
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
model/src/index.ts:61-93
**`collection.dispose()` called before snapshots are consumed by `createPlDataTableV3`**

The collection is disposed on line 72, yet the snapshot objects extracted from it (`primaryColumns`, `secondaryColumns`) are passed into `createPlDataTableV3` on lines 87-93. The PR description explicitly notes this path is not runtime-exercised by the test. If `ColumnSnapshot` instances carry internal ties to the collection (e.g., lazy-resolved data-cell references, subscriptions, or live status handles), they will be stale by the time `createPlDataTableV3` reads them, potentially causing silent incorrect table state or crashes. The `dispose()` should occur after `createPlDataTableV3` returns, or snapshots must be verified to be pure value objects that survive collection teardown.

### Issue 2 of 2
model/src/dataModel.ts:15-18
**`.upgradeLegacy` path is not covered by any test**

The PR description explicitly notes this upgrader was not run against a real on-disk V1 project. If a V1 project with non-trivial `uiState` (e.g. non-empty `group.mainColumn`, persisted `tableStateV2`) loads for the first time after this deployment, the one-shot upgrade path executes and its result is permanently written. A regression here (e.g. if the V1 on-disk field path differs from what `LegacyUiState` expects) would silently reset user column selection. Adding a fixture-based test or at minimum a V1-shaped snapshot test before shipping would reduce this risk.

Reviews (1): Last reviewed commit: "Migrate model to BlockModelV3 and create..." | 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)

Run `block-tools structure refresh` + `pnpm fmt` (block-tools 2.11.0): adopt
the canonical tool-managed layout — tsconfig/turbo/oxlint/oxfmt configs, block
index, managed package.json and catalog across all packages, eslint/prettier
removed in favour of oxlint/oxfmt.

Full SDK upgrade forced by the require-latest gate: @platforma-sdk/model and
ui-vue 1.48 -> 1.79.14, workflow-tengo 5.7 -> 6.6.3, tengo-builder 2 -> 4,
block-tools -> 2.11.0, vue pinned to 3.5.24.

Author-code fixes required by the SDK bump follow in a separate commit.
- model: emit `pTable` via `.outputWithStatus` so it satisfies the new
  `usePlDataTableSettingsV2({ model })` signature, which now expects
  `OutputWithStatus<PlDataTableModel | undefined>` instead of the bare value.
- ui: drop `import '@platforma-sdk/ui-vue/styles'` — the `./styles` subpath
  export was removed; styles now ship via the package entry.
- model: declare `@milaboratories/helpers` a direct dependency to resolve
  TS2742 on the inferred `model` type (SDK pframe-builder return types leak a
  helpers type that must be nameable from a direct dep).
- catalog: align `@milaboratories/helpers` to ^1.14.2 (the SDK's version) so
  the model dedupes to a single copy.
Patch bump for block, model, ui, workflow (test is private).
Convert the table block from the legacy V1 model API to BlockModelV3:

- Unified `data` model (model/src/types.ts, model/src/dataModel.ts). A one-time
  `.upgradeLegacy` carries over the V1 `uiState` (group + tableStateV2) and drops
  the obsolete keys the old UI-side upgrader used to delete.
- All block state is UI-only (column selection + table grid state); `.args`
  projects to an empty object and never gates the block (the workflow has no
  inputs).
- pTable is built with createPlDataTableV3, sourcing column snapshots via
  ColumnCollectionBuilder + collectCtxColumnSnapshotProviders (primary/secondary
  via the per-column isPrimary flag, replacing V2's coreColumnPredicate).
- UI moves from defineApp/app.model.ui to defineAppV3/app.model.data; the legacy
  UI-state upgrader IIFE is removed.

@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 table block to the block-tools structurer and upgrades the SDK to BlockModelV3. Key changes include updating package configurations, adopting tool-managed layouts (oxlint/oxfmt), migrating the data model from legacy V1 to BlockModelV3 with a unified data model, and moving UI bindings from app.model.ui to app.model.data. A critical issue was identified in model/src/index.ts where disposing of the ColumnCollection immediately will invalidate the returned snapshots; it is recommended to use ctx.use() to manage its lifetime instead.

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 model/src/index.ts Outdated
Comment thread model/src/index.ts
Comment thread model/src/dataModel.ts
@vadimpiven

Copy link
Copy Markdown
Contributor Author

Reviewed the automated comments. Assessment below — no code changes warranted; reasoning with evidence:

collection.dispose() before createPlDataTableV3 (gemini "critical", greptile P1) — not a defect

This mirrors the canonical pattern in the shipped V3 block clonotype-browser (model/src/index.ts): it calls collection.findColumnVariants(...), then collection.dispose(), then feeds those variants into createPlDataTableV3(...) after disposal. dispose() releases the spec-frame discovery WASM resource (used for axis/selector matching); the returned ColumnSnapshots are value objects holding an already-extracted spec, id, dataStatus, and a data accessor tied to column-data resources — not to the discovery frame. createPlDataTableV3 consumes them synchronously within the lambda to build the table model; it does not re-run discovery.

gemini's suggested fix (ctx.use(...)) references an API that does not exist on the V3 RenderCtxBase (no block uses ctx.use(); it would not compile. Leaving the code as-is to match the proven pattern.

.upgradeLegacy field-path risk (greptile P2) — verified against the actual V1 shape

The concern is "if the V1 on-disk field path differs from what LegacyUiState expects." The pre-migration UiState (commit d5a0e46) is exactly:

type UiState = { group: { mainColumn?: PColumnIdAndSpec; additionalColumns: PColumnIdAndSpec[]; enrichmentColumns: PColumnIdAndSpec[] }; tableStateV2: PlDataTableStateV2 };

LegacyUiState mirrors this exactly (fields made optional for defensive defaulting), and the upgrade picks only group + tableStateV2, intentionally dropping the obsolete keys (filtersOpen, filterModel, tableState, settingsOpened) the old UI-side upgrader used to delete. A synthetic unit test would only re-assert this hand-written type; the meaningful validation is loading a real pre-existing V1 project, which remains the documented pre-merge check.

…tart

Respect the structurer's test: true instead of disabling the lane. 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 and a per-test timeout to the smoke test. No pl-docker-tag
pin (default image). Verified green on the graph-maker block.
@mzueva
mzueva added this pull request to the merge queue Aug 3, 2026
Merged via the queue into main with commit 788f027 Aug 3, 2026
11 checks passed
@mzueva
mzueva deleted the chore/migrate-to-structurer branch August 3, 2026 14:25
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.

4 participants