From 1ed1727b849d093af1f6de7b06ad72570f981d19 Mon Sep 17 00:00:00 2001 From: Paul Newling Date: Wed, 29 Jul 2026 10:11:36 +0200 Subject: [PATCH 1/3] MILAB-6651: pin cellLinker side assignment in the TS linker engine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Characterization tests for LinkerMap, mirroring the Rust LinkerIndex tests. They assert today's behaviour, which is wrong, so they must flip when sides become explicit. The real cellLinker is authored [sampleId, cellId <- sampleId, scClonotypeKey]. getAxesGroups returns groups ordered by smallest contained index and fromColumns destructures [left, right], so the sample/cell component becomes the one-side. Consequences, both asserted: - a clonotype-keyed source reaches cell-level axes (edges are stored many-side -> one-side), which is how cell-level columns end up in a clonotype-keyed p-frame; - a cell-keyed source cannot reach the clonotype at all. Authoring the same linker with the clonotype first reverses both. Also records the cross-engine parity expectation. Rust takes its component ordering from disjoint::DisjointSet::sets(); this file's getAxesGroups derives it from its own index scan. Two independent implementations that agree by coincidence rather than by contract, and they have already drifted twice. Keeping the two sides in sync is manual — a real shared fixture would need a generated contract. Note the trunk in these tests carries the whole parent tree: linker map keys come from getArrayFromAxisTree, so a cell trunk keyed on cellId alone misses for an unrelated reason. --- .../src/drivers/pframe/linker_columns.test.ts | 97 +++++++++++++++++++ 1 file changed, 97 insertions(+) diff --git a/lib/model/common/src/drivers/pframe/linker_columns.test.ts b/lib/model/common/src/drivers/pframe/linker_columns.test.ts index f272885e24..496d9ffd32 100644 --- a/lib/model/common/src/drivers/pframe/linker_columns.test.ts +++ b/lib/model/common/src/drivers/pframe/linker_columns.test.ts @@ -314,3 +314,100 @@ describe("Linker columns", () => { expect(linkerMap.getReachableByLinkersAxesFromAxes([axisA])).toEqual([]); }); }); + +/** + * MILAB-6651 — side assignment for the real `pl7.app/sc/cellLinker`. + * + * In the data, many cells map to ONE clonotype, so the clonotype is the linker's + * one-side and the sample/cell pair is its many-side. `mixcr-clonotyping` authors + * the axes as `[sampleId, cellId <- sampleId, scClonotypeKey]`, and side assignment + * is purely positional — `getAxesGroups` returns groups ordered by their smallest + * contained index and `fromColumns` destructures `const [left, right] = groups` + * (`linker_columns.ts:53`). So the engine reads the sides backwards. + * + * These are characterization tests: they assert the WRONG behaviour that exists + * today. When explicit sides land, they must flip — and that flip is the proof. + */ +describe("MILAB-6651 cellLinker side assignment", () => { + const axisSampleId = makeTestAxis({ name: "sampleId" }); + const axisCellId = makeTestAxis({ name: "cellId", parents: [axisSampleId] }); + const axisClonotype = makeTestAxis({ name: "scClonotypeKey" }); + + /** `[sampleId, cellId <- sampleId, scClonotypeKey]` — the layout as authored. */ + const asAuthored = makeLinkerColumn({ + name: "cellLinker", + from: [axisSampleId, axisCellId], + to: [axisClonotype], + }); + + /** The same linker with the clonotype component authored first. */ + const corrected = makeLinkerColumn({ + name: "cellLinker", + from: [axisClonotype], + to: [axisSampleId, axisCellId], + }); + + test("grouping order puts the sample/cell component first, so it becomes the one-side", () => { + const groups = LinkerMap.getAxesGroups( + getNormalizedAxesList([axisSampleId, axisCellId, axisClonotype]), + ); + + expect(groups.length).toBe(2); + // groups[0] is destructured as `left` — the one-side. This is inverted: + // {sampleId, cellId} is the many-side in reality. + expect(groups[0].map((a) => a.name)).toEqual(["sampleId", "cellId"]); + expect(groups[1].map((a) => a.name)).toEqual(["scClonotypeKey"]); + }); + + test("a clonotype-keyed source reaches cell-level axes today (the leakage)", () => { + const linkerMap = LinkerMap.fromColumns([asAuthored]); + + // Edges are stored many-side -> one-side (`linker_columns.ts:91`), so with the + // sides inverted a clonotype-keyed table can traverse down to per-cell axes. + // This is what puts cell-level columns into a clonotype-keyed p-frame. + expect( + new Set(linkerMap.getReachableByLinkersAxesFromAxes([axisClonotype]).map((a) => a.name)), + ).toEqual(new Set(["cellId", "sampleId"])); + }); + + test("a cell-keyed source cannot reach the clonotype today (the lost capability)", () => { + // The trunk must carry the whole parent tree. Linker map keys are built from + // `getArrayFromAxisTree`, so the cell trunk's key is [cellId, sampleId]; passing + // cellId alone yields the key [cellId] and misses for an unrelated reason. + const cellTrunk = [axisSampleId, axisCellId]; + + const asAuthoredMap = LinkerMap.fromColumns([asAuthored]); + expect(asAuthoredMap.getReachableByLinkersAxesFromAxes(cellTrunk)).toEqual([]); + + // Authoring the same linker with correct sides makes the intended cell -> + // clonotype enrichment appear, and removes the reverse traversal. + const correctedMap = LinkerMap.fromColumns([corrected]); + expect(correctedMap.getReachableByLinkersAxesFromAxes(cellTrunk).map((a) => a.name)).toEqual([ + "scClonotypeKey", + ]); + expect(correctedMap.getReachableByLinkersAxesFromAxes([axisClonotype])).toEqual([]); + }); + + /** + * H3 (partial) — cross-engine parity. + * + * The Rust `LinkerIndex` and this TS `LinkerMap` are independent implementations + * of the same convention. Rust gets its component ordering from + * `disjoint::DisjointSet::sets()`; TS derives it from its own index scan + * (`getAxesGroups`, `linker_columns.ts:304,330`). Nothing links the two, and they + * have already drifted twice (malformed-linker handling, `excludeColumns`). + * + * This asserts the TS half of the shared expectation. The Rust half is + * `cell_linker_sides_are_inverted_today` in `linker_index.rs`. Keeping them in + * sync is manual — a genuine shared fixture would need a generated contract. + */ + test("parity: TS groups the real cellLinker the same way Rust splits it", () => { + const groups = LinkerMap.getAxesGroups( + getNormalizedAxesList([axisSampleId, axisCellId, axisClonotype]), + ); + + // Mirrors Rust: one_side_axes_ids().len() == 2, many_side_axes() == [scClonotypeKey]. + expect(groups[0].length).toBe(2); + expect(groups[1].map((a) => a.name)).toEqual(["scClonotypeKey"]); + }); +}); From 5f5f16f4b60cfd5782f57f3a897e3a26e6759701 Mon Sep 17 00:00:00 2001 From: Paul Newling Date: Wed, 29 Jul 2026 10:55:52 +0200 Subject: [PATCH 2/3] MILAB-6651: strengthen the TS linker tests after a behavioural review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two problems with the tests added in 1ed1727b8. A test whose name overstated it. "parity: TS groups the real cellLinker the same way Rust splits it" asserted a strict subset of the test above it and performed no cross-engine check of any kind — the two repos share no fixture. Replaced with an exhaustive test that earns its place, and the parity rationale is kept as prose where it belongs. Assertions pinning order the source disclaims. getAxesGroups documents "There are no order inside every group" (linker_columns.ts:271), but the tests asserted group contents as ordered arrays. Now compared as sorted sets, so only the order *between* groups is pinned — which is the part side assignment actually reads. The replacement walks all 6 authoring orders and separates the two things MILAB-6651 turns on: membership is authoring-order independent, position is not. That is why no structural rule can recover the intended direction — it has to be declared or read from data. Mirrors split_component_order_follows_authoring_order_for_every_permutation on the pframes-rs side. --- .../src/drivers/pframe/linker_columns.test.ts | 64 +++++++++++++------ 1 file changed, 44 insertions(+), 20 deletions(-) diff --git a/lib/model/common/src/drivers/pframe/linker_columns.test.ts b/lib/model/common/src/drivers/pframe/linker_columns.test.ts index 496d9ffd32..36118f4fb6 100644 --- a/lib/model/common/src/drivers/pframe/linker_columns.test.ts +++ b/lib/model/common/src/drivers/pframe/linker_columns.test.ts @@ -347,16 +347,25 @@ describe("MILAB-6651 cellLinker side assignment", () => { to: [axisSampleId, axisCellId], }); + /** + * Group membership as a sorted set. `getAxesGroups` documents "There are no order + * inside every group" (`linker_columns.ts:271`), so asserting a particular order + * *within* a group would pin an implementation detail the source explicitly + * disclaims. Which group comes *first* is what side assignment reads, and that is + * asserted positionally. + */ + const names = (axes: AxisSpecNormalized[]) => axes.map((a) => a.name).sort(); + test("grouping order puts the sample/cell component first, so it becomes the one-side", () => { const groups = LinkerMap.getAxesGroups( getNormalizedAxesList([axisSampleId, axisCellId, axisClonotype]), ); expect(groups.length).toBe(2); - // groups[0] is destructured as `left` — the one-side. This is inverted: + // groups[0] is destructured as `left` — the one-side. That is inverted: // {sampleId, cellId} is the many-side in reality. - expect(groups[0].map((a) => a.name)).toEqual(["sampleId", "cellId"]); - expect(groups[1].map((a) => a.name)).toEqual(["scClonotypeKey"]); + expect(names(groups[0])).toEqual(["cellId", "sampleId"]); + expect(names(groups[1])).toEqual(["scClonotypeKey"]); }); test("a clonotype-keyed source reaches cell-level axes today (the leakage)", () => { @@ -389,25 +398,40 @@ describe("MILAB-6651 cellLinker side assignment", () => { }); /** - * H3 (partial) — cross-engine parity. + * Exhaustive companion to the ordering test above, and the TS mirror of + * `split_component_order_follows_authoring_order_for_every_permutation` in + * pframes-rs `axes_spec.rs`. * - * The Rust `LinkerIndex` and this TS `LinkerMap` are independent implementations - * of the same convention. Rust gets its component ordering from - * `disjoint::DisjointSet::sets()`; TS derives it from its own index scan - * (`getAxesGroups`, `linker_columns.ts:304,330`). Nothing links the two, and they - * have already drifted twice (malformed-linker handling, `excludeColumns`). + * Group *membership* must not depend on authoring order; which group comes *first* + * must. That distinction is the whole of MILAB-6651 — the cellLinker's axes group + * correctly however they are written down, and it is purely their position that + * decides which component gets treated as the one-side. So no structural rule can + * recover the intended direction; it has to be declared or read from data. * - * This asserts the TS half of the shared expectation. The Rust half is - * `cell_linker_sides_are_inverted_today` in `linker_index.rs`. Keeping them in - * sync is manual — a genuine shared fixture would need a generated contract. + * Note the two engines reach this ordering independently: Rust via + * `disjoint::DisjointSet::sets()`, TS via its own index scan (`linker_columns.ts:304,330`). + * Nothing couples them, and they have already drifted twice (malformed-linker + * handling, `excludeColumns` support), so both sides assert it separately. A real + * shared fixture would need a generated cross-repo contract. */ - test("parity: TS groups the real cellLinker the same way Rust splits it", () => { - const groups = LinkerMap.getAxesGroups( - getNormalizedAxesList([axisSampleId, axisCellId, axisClonotype]), - ); - - // Mirrors Rust: one_side_axes_ids().len() == 2, many_side_axes() == [scClonotypeKey]. - expect(groups[0].length).toBe(2); - expect(groups[1].map((a) => a.name)).toEqual(["scClonotypeKey"]); + test("grouping is authoring-order independent, but group position is not", () => { + const normalized = getNormalizedAxesList([axisSampleId, axisCellId, axisClonotype]); + + for (const order of allPermutations(normalized)) { + const groups = LinkerMap.getAxesGroups(order); + expect(groups.length).toBe(2); + + // Membership is invariant across all 6 orderings. + expect(groups.map(names).sort((a, b) => a[0].localeCompare(b[0]))).toEqual([ + ["cellId", "sampleId"], + ["scClonotypeKey"], + ]); + + // Position tracks the earliest-written member of each group — which is exactly + // why a mis-authored linker inverts its own sides. + const earliest = (group: AxisSpecNormalized[]) => + Math.min(...group.map((a) => order.findIndex((o) => o.name === a.name))); + expect(earliest(groups[0])).toBeLessThan(earliest(groups[1])); + } }); }); From c990c47bc052fd2d2da955315835246d2843f891 Mon Sep 17 00:00:00 2001 From: Paul Newling Date: Wed, 29 Jul 2026 13:24:22 +0200 Subject: [PATCH 3/3] MILAB-6651: add an empty changeset for the test-only change MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit check-changesets failed: the change touches a file under lib/model/common/src, so changesets counts the package as modified and requires a changeset. An empty one declares that no release is needed, which is accurate — the tests add no API, behaviour, or type change. Note the changeset has to be tracked by git. `changeset status --since` reads changesets from git rather than the working tree, so an untracked file is invisible to it and the check keeps failing. --- .changeset/milab-6651-linker-side-assignment-tests.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/milab-6651-linker-side-assignment-tests.md diff --git a/.changeset/milab-6651-linker-side-assignment-tests.md b/.changeset/milab-6651-linker-side-assignment-tests.md new file mode 100644 index 0000000000..4b3531fbae --- /dev/null +++ b/.changeset/milab-6651-linker-side-assignment-tests.md @@ -0,0 +1,6 @@ +--- +--- + +Test-only change. Adds characterisation tests for `LinkerMap` linker side +assignment (MILAB-6651). No API, behaviour, or type changes, so no release is +needed.