Skip to content

MILAB-6648: failing test — export and import disagree about a block id inside a column id - #1781

Open
dbolotin wants to merge 1 commit into
MILAB-6648_block-kind-prototypefrom
fix/template-export-import-guard-asymmetry
Open

MILAB-6648: failing test — export and import disagree about a block id inside a column id#1781
dbolotin wants to merge 1 commit into
MILAB-6648_block-kind-prototypefrom
fix/template-export-import-guard-asymmetry

Conversation

@dbolotin

@dbolotin dbolotin commented Aug 11, 2026

Copy link
Copy Markdown
Member

Targets MILAB-6648_block-kind-prototype rather than main: this is a repro for #1767, not a fix.

Two of the four tests fail on purpose. They demonstrate that export and import answer the same question opposite ways.

  • Export tests membership. unrewrittenBlockIds (template_export.ts:176-182) treats a block id found in already-template-form params as a fault only if the document does not describe that block. Its own comment: "a hit naming an exported entry is a wire", and template_export.test.ts:217 asserts exactly that case is legal.
  • Import tests presence, subtracting nothing. foreignBlockIds (template_validate.ts:78-85) reports every id it finds and rejects the entry.

So a project whose params carry a PColumn id exports cleanly and is then refused on import, with the id named as coming "from another project" — while it is an entry of the same document. The advice in that message is also unfollowable: removing the value removes the block's configuration.

This is the case bf60d7bc3 ("remap the block ids buried inside a column id") was written for — "clonotype-browser's annotation filters reference result-pool columns. This is not hypothetical." Because the validator runs before the codec (middle_layer.ts:419), that commit's import-side remap is unreachable through the driver.

The tests

Test Result
CONTROL — a plain template-local reference survives export → import passes, so the harness is sound
BUG — at walk level: the real exporter accepts params the real validator rejects fails
BUG, at file level — the whole pipeline: assemble → stringify → parse → validate fails
DIAGNOSTIC — prints the refusal so the failures read on their own passes

No mocks of either side. The file-level test rules out "a later export stage would have caught it" and shows the artifact a user actually holds. What export writes:

schema: template-v1
blocks:
  - id: a
    kind: "@platforma-open/milaboratories.demo.kind@1.0.0"
    params: {}
  - id: b
    kind: "@platforma-open/milaboratories.demo.kind@1.0.0"
    params:
      anchor: '{"__isRef":true,"blockId":"a","name":"clonotypes"}'

That file parses cleanly and is then rejected.

Why CI is green today

Each side tests its own half; neither covers the crossing. template_validate.test.ts only uses foreign ids, and its one "not mistaken for a foreign one" case uses the plain {block, output} form. template-round-trip.test.ts passes against a live backend because both its blocks carry flat primitive params with no column ids.

Suggested fix

Give the import guard the document's own entry ids and apply the same membership subtraction export already uses — inferAllReferencedBlocks already takes an optional allowed set (args.ts:30). Then flip the two failing assertions into regression tests and add the crossing case to both suites.

Verified locally: pnpm check passes for pl-middle-layer (types, lint, format); the surrounding suites are unaffected — 230 of 232 tests pass across src/model/, src/mutator/template_construct.test.ts and src/block_registry/, the two failures being the ones above.

Greptile Summary

This PR adds a focused reproduction demonstrating that template export accepts an in-document block reference embedded in a PColumn ID while template import rejects the resulting document. Two failing assertions are intentionally retained to expose the export/import asymmetry at both the traversal and serialized-YAML levels.

  • Adds a passing control for ordinary template-local references.
  • Adds walk-level and file-level reproductions using the real exporter, serializer, parser, and validator.
  • Adds a diagnostic test that verifies the misleading “from another project” rejection.
  • Important touched terms:
    • PColumn ID — A canonicalized JSON string identifying a data column and embedding its producing block ID; the new tests demonstrate its treatment during template export and import.
    • Template-local reference — A structured reference from one template block to another block in the same document; the control test confirms this form still completes the round trip.
    • Project template v1 — The serialized document containing block definitions, kinds, and parameters; the new file-level test constructs, serializes, parses, and validates this artifact.
    • Template export walk — The traversal that gathers export entries and reports unresolved references; the tests show it permits a PColumn ID naming an exported entry.
    • Apply-time template validation — The import guard run before applying a parsed template; the tests show it currently classifies the same in-document PColumn reference as foreign.
    • Block kind selector/reference — The package-and-version identifiers attached to template blocks; the reproduction supplies selector references during validation and concrete references during assembly.

Confidence Score: 5/5

The PR appears safe to merge as a deliberate test-only reproduction whose intentionally failing cases are clearly documented.

The change modifies no production path and accurately captures the acknowledged exporter/importer disagreement through both direct and serialized-template tests.

Important Files Changed

Filename Overview
lib/node/pl-middle-layer/src/model/template_export_import_asymmetry.bug.test.ts Adds an intentional regression reproduction, a passing control, full YAML-pipeline coverage, and diagnostic assertions without changing production behavior.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Project params contain PColumn ID naming block a] --> B[Export walk accepts in-document membership]
  B --> C[Assemble template-v1]
  C --> D[Serialize to YAML]
  D --> E[Parse YAML successfully]
  E --> F[Apply-time validator reports block a as foreign]
  F --> G[Template import is refused]
Loading

Reviews (1): Last reviewed commit: "MILAB-6648: failing test for the export/..." | Re-trigger Greptile

Context used:

  • Context used - Terms is a types in codebase. Provide the list of ... (source)
  • Knowledge Base — pl-middle-layer

Export and import disagree about a block id found inside an already
template-form params value. Export tests membership: a hit naming a block the
document describes is a wire, not a fault (unrewrittenBlockIds). Import tests
presence and subtracts nothing (foreignBlockIds), so it rejects the entry.

A project whose params carry a PColumn id therefore exports cleanly and is
refused on import, naming an entry of the same document as coming "from another
project". The advice in that message is also unfollowable: removing the value
removes the block's configuration.

This is the case the column-id remap was written for, so the remap on the import
side is unreachable through the driver, which runs validation before the codec.

Four tests, two of which fail:

- CONTROL passes, so the harness is sound: a plain template-local reference
  survives export and import.
- BUG fails at walk level: the real exporter accepts params the real validator
  then rejects.
- BUG fails at file level, through the whole shipped pipeline: assemble,
  stringify to YAML, parse back, validate. Export writes a well-formed portable
  file and import refuses it.
- DIAGNOSTIC passes and prints the refusal, so the failures read on their own.

The existing suites stay green because each side tests its own half: neither
covers a column id naming an in-document entry.
@notion-workspace

Copy link
Copy Markdown

@changeset-bot

changeset-bot Bot commented Aug 11, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: d22afc9

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

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