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
Open
Conversation
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.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Targets
MILAB-6648_block-kind-prototyperather thanmain: 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.
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", andtemplate_export.test.ts:217asserts exactly that case is legal.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
CONTROL— a plain template-local reference survives export → importBUG— at walk level: the real exporter accepts params the real validator rejectsBUG, at file level— the whole pipeline: assemble → stringify → parse → validateDIAGNOSTIC— prints the refusal so the failures read on their ownNo 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:
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.tsonly uses foreign ids, and its one "not mistaken for a foreign one" case uses the plain{block, output}form.template-round-trip.test.tspasses 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 —
inferAllReferencedBlocksalready takes an optionalallowedset (args.ts:30). Then flip the two failing assertions into regression tests and add the crossing case to both suites.Verified locally:
pnpm checkpasses forpl-middle-layer(types, lint, format); the surrounding suites are unaffected — 230 of 232 tests pass acrosssrc/model/,src/mutator/template_construct.test.tsandsrc/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.
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
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]Reviews (1): Last reviewed commit: "MILAB-6648: failing test for the export/..." | Re-trigger Greptile
Context used: