Milab-6720: prerun hint and run disabling

.changeset/prerun-feedback-and-run-gate.md

@@ -0,0 +1,51 @@
+---
+'@platforma-open/milaboratories.import-vdj.workflow': patch
+'@platforma-open/milaboratories.import-vdj.model': patch
+'@platforma-open/milaboratories.import-vdj.ui': patch
+'@platforma-open/milaboratories.import-vdj': patch
+---
+
+Say what the block is checking, and refuse to run until it has
+
+Loading a file left the panel silent while every column of it was profiled — a whole-file pass,
+minutes on remote storage — and the profile outputs are retentive, so the dropdowns went on
+answering with the *previous* file's headers as though nothing had happened. A mapping that had
+passed every check against that previous file also still counted as valid, so Run stayed live over
+a file nobody had read yet, against headers it might not even contain.
+
+- **The wait is announced.** Prerun now states which file a profile was taken from, and the model
+  pairs the two so the reported file cannot get ahead of the profile. The panel announces the scan
+  and withholds the mapping until the columns on offer are really this file's. Keyed to the file
+  rather than to "prerun is busy", because prerun also re-runs on every mapping edit to re-check
+  the id column. The import itself now shows the block's loader, which it never did.
+- **Loading a second file disables Run.** Picking a different file drops the parts of the mapping
+  that name columns, keeping the receptor declaration and the numbering scheme, which describe the
+  data rather than one file. Re-picking the same file is not a swap and keeps the mapping.
+- **A repeated id column now stops the run.** The record key is the identity's hash, so a value
+  repeated on rows that are not identical merges two records into one. Prerun always found these
+  and the panel always warned, but the warning was only a warning, and a run driven through the API
+  imported the merged set without complaint. Run is now refused both while the verdict is
+  outstanding and when it reports a repeat, and the platform enforces it as well as the interface.
+- **The warning names the mapping it is about.** It used to compare against the freshly picked
+  column while the verdict was still the previous mapping's, so changing an offending column flashed
+  the old accusation under the new selection. What a verdict covers is the id column *and* the
+  sequence columns, since a collision is a repeated id whose other mapped cells differ — keyed on
+  the id alone, a clean verdict outlived a remapped chain and the run gate accepted it. It also listed up to ten repeated values; it now lists three and a count,
+  printed whole, since the id column can hold sequences and trimming those hides what tells them
+  apart.
+- **Alert headings appear.** Four alerts passed their heading to a slot `PlAlert` does not have, so
+  the headings had never rendered — a warning about a non-unique id column read as an unlabelled
+  wall of values.
+- **The id column can be cleared.** Clearing it left the field reading "Value not available" in
+  red: "nothing chosen" is stored as an empty string, and a dropdown counts any value that is not
+  `undefined` as chosen. Relatedly, two places named the IG chain pair where they meant every
+  mapped chain, so a TCR mapping could never clear itself and was offered its own sequence columns
+  as record properties.
+
+Refusing the run on a prerun verdict needs that verdict inside the args projection, which sees only
+the block's own data, so the UI mirrors it in. That is a hairpin, and deliberate: unlike a column
+mapping there is no gesture at which the fact could be captured, because the scientist picks a
+column and only then does the check discover whether it is sound. The two rules that keep it safe —
+a verdict carries what it is about, and is dropped when the source changes — are stated on
+`BlockData.prerunChecks`, and the checks still to come should follow them. It can all go once
+`argsValid` can read prerun directly.

block/src/block-extra.ts

@@ -7,3 +7,9 @@
 // BlockPointer, platforma, or the block-named <PascalName>Block*
 // aliases — those names come from ./index and `export *` from this
 // file would shadow them.
+
+// The rule that decides whether a collision verdict is about the mapping now selected. Exposed
+// because the run gate depends on it and the block's own tests have to state the verdict the UI
+// would have mirrored in; a test that rebuilt the rule by hand would drift from the gate it means
+// to exercise.
+export { collisionCheckKey } from "@platforma-open/milaboratories.import-vdj.model";

model/src/index.ts

@@ -6,8 +6,14 @@ import {
   TreeNodeAccessor,
 } from "@platforma-sdk/model";
 import { blockDataModel } from "./data-model";
-import type { BlockArgs, BlockData, ColumnDescription, ColumnProfile } from "./types";
-import { bareSetValid } from "./types";
+import type {
+  BareSetMapping,
+  BlockArgs,
+  BlockData,
+  ColumnDescription,
+  ColumnProfile,
+} from "./types";
+import { bareSetValid, collisionCheckKey } from "./types";
 
 export * from "./types";
 export { upgradeLegacyData } from "./data-model";
@@ -38,6 +44,27 @@ function withoutDatasetDoorMapping(args: BlockArgs): BlockArgs {
   };
 }
 
+/**
+ * Refuse a bare set whose mapped columns prerun has not cleared.
+ *
+ * The checks themselves cannot live here — they read the whole file — so this reads the verdicts
+ * the UI mirrored into `data.prerunChecks`, and refuses both a verdict that reports a defect and
+ * the absence of one for the columns currently selected. Refusing the absence is the point: the
+ * record key is the identity's hash, so starting a run before the answer is in is exactly how two
+ * different records silently become one.
+ *
+ * Phrased for the checks rather than for the id column, because it is about to cover more of them.
+ */
+function requireCheckedColumns(data: BlockData): void {
+  const columns = collisionCheckKey(data.bareSet);
+  if (columns === undefined) return; // bareSetValid has already refused
+  const checks = data.prerunChecks;
+  if (checks?.columns !== columns) throw new Error("Validating the selected columns");
+  if (checks.identityCollides) {
+    throw new Error(`"${data.bareSet?.identity}" repeats on rows that are not identical`);
+  }
+}
+
 /**
  * The workflow's view of the block, and the only place validation lives.
  *
@@ -72,6 +99,7 @@ function projectArgs(data: BlockData): BlockArgs {
   // mapping unfurled under a format nobody had chosen.
   if (fileSource !== undefined) {
     if (!bareSetValid(data.bareSet)) throw new Error("Finish mapping the file's columns");
+    requireCheckedColumns(data);
     return withoutDatasetDoorMapping(args);
   }
 
@@ -85,6 +113,7 @@ function projectArgs(data: BlockData): BlockArgs {
     // identity column, because the key is the identity's hash and the label is its value.
     if (data.bareSet !== undefined) {
       if (!bareSetValid(data.bareSet)) throw new Error("Finish mapping the record's columns");
+      requireCheckedColumns(data);
       return withoutDatasetDoorMapping(args);
     }
 
@@ -177,7 +206,24 @@ export const platforma = BlockModelV3.create(blockDataModel)
     { isActive: true },
   )
 
+  /**
+   * Identity values the file repeats on rows that are not identical, and the mapping they were
+   * found under.
+   *
+   * Reported as one value because the two must never be read from different runs: the panel used
+   * to compare against `data.bareSet`, which updates the instant a column is picked, while the
+   * collisions were still the previous mapping's — so changing an offending column flashed the old
+   * verdict under the new selection.
+   *
+   * Not `retentive`: this reports a defect, so a briefly absent verdict beats a stale one.
+   */
   .output("identityCollisions", (ctx) => {
+    const mapping = ctx.prerun
+      ?.resolve({ field: "collisionsFor", allowPermanentAbsence: true })
+      ?.getDataAsJsonOrUndefined<Pick<BareSetMapping, "identity" | "sequences">>();
+    const key = collisionCheckKey(mapping);
+    if (key === undefined) return undefined;
+
     const raw = ctx.prerun
       ?.resolve({ field: "identityCollisions", allowPermanentAbsence: true })
       ?.getDataAsString();
@@ -188,7 +234,7 @@ export const platforma = BlockModelV3.create(blockDataModel)
       .split("\n")
       .map((l) => l.trim())
       .filter((l) => l.length > 0);
-    return lines.slice(1);
+    return { key, values: lines.slice(1) };
   })
 
   .retentiveOutput("datasetOptions", (ctx) => {
@@ -264,6 +310,30 @@ export const platforma = BlockModelV3.create(blockDataModel)
     }
   })
 
+  /**
+   * Which file the profile the UI currently sees was taken from, so a mismatch with
+   * `data.fileSource.sampleId` means "the panel is showing the last file's columns".
+   *
+   * Keyed to the file, not to whether prerun is busy: `prerunArgs` carries `bareSet`, so prerun
+   * re-runs on every mapping edit to re-check the identity column for collisions.
+   */
+  .retentiveOutput("profiledSampleId", (ctx) => {
+    const profile = ctx.prerun?.resolve({ field: "columnProfile", allowPermanentAbsence: true });
+    if (profile === undefined) return undefined;
+    // Marks the read unstable (pl-tree/src/accessors.ts:347), so `retentive` keeps reporting the
+    // previous file's id until the new profile lands — the id and the profile can never disagree.
+    if (!profile.getIsReadyOrError()) return undefined;
+    return ctx.prerun
+      ?.resolve({ field: "profiledSampleId", allowPermanentAbsence: true })
+      ?.getDataAsJsonOrUndefined<string>();
+  })
+
+  /**
+   * Drives the block's loader (`ui/src/app.ts`). Excludes prerun: the loader covers the whole
+   * block, and prerun re-runs while the settings panel is being edited.
+   */
+  .output("isRunning", (ctx) => ctx.outputs?.getIsReadyOrError() === false)
+
   /** Headers of the dataset selected from the pool. Absent on the file door. */
   .retentiveOutput("datasetColumns", (ctx) => {
     const headers = ctx.prerun

model/src/types.ts

@@ -110,6 +110,13 @@ export const CHAIN_SLOT_LABELS: Record<BareSetChain, string> = {
   TCRGamma: "TCR-ɣ",
 };
 
+/** What to call each numbering scheme in front of the scientist. */
+export const SCHEME_LABELS: Record<BareSetScheme, string> = {
+  imgt: "IMGT",
+  kabat: "Kabat",
+  chothia: "Chothia",
+};
+
 /** What a column can hold, decided by profiling every row of the file. */
 export type ColumnValueType = "Long" | "Double" | "String";
 
@@ -222,6 +229,31 @@ export type BlockData = {
   // --- bare set. Its presence is what selects the bare path in the workflow.
   bareSet?: BareSetMapping;
 
+  /**
+   * Verdicts prerun reached, mirrored here so `args` can refuse a run on them.
+   *
+   * `args` is a pure function of this data and cannot read prerun, and unlike the column mapping
+   * there is no gesture at which a verdict could be snapshotted: the scientist picks a column, and
+   * only then does the check discover whether it is sound. So the UI mirrors it in
+   * (`ui/src/app.ts`) — the hairpin the harness warns about, made safe by two rules every entry
+   * here must follow:
+   *
+   * - it carries what it is *about*, so a verdict for something nobody has selected any more is
+   *   ignored rather than applied;
+   * - it is cleared when the source changes, so a verdict cannot survive into a different file.
+   *
+   * Together those make the write idempotent — every client derives the same value from the same
+   * output, so concurrent writes agree instead of racing. Add further checks as sibling fields
+   * following the same two rules. The whole field goes away once args can read prerun directly
+   * (SDK request "Simplify prerun checks").
+   */
+  prerunChecks?: {
+    /** The mapping the verdicts below were reached for — see {@link collisionCheckKey}. */
+    columns: string;
+    /** The id column repeats on rows whose other mapped cells differ, so two records would merge. */
+    identityCollides: boolean;
+  };
+
   // --- view state. None of this is projected anywhere.
   tableState: PlDataTableStateV2;
   settingsOpen: boolean;
@@ -278,6 +310,47 @@ export function propertyCollisions(properties: ImportedProperty[]): Record<strin
   return Object.fromEntries(Object.entries(byToken).filter(([, hs]) => hs.length > 1));
 }
 
+/**
+ * What a collision verdict is about: the identity column *and* the sequence columns.
+ *
+ * The sequence columns are load-bearing, not incidental. A collision is not "the identity
+ * repeats" — it is an identity repeated on rows whose other mapped cells differ, so remapping a
+ * chain can turn a clean set into a colliding one and back (`bare-set-collisions.tpl.tengo`).
+ * Keying a verdict on the identity alone let a clean verdict outlive the mapping it was reached
+ * for, and the run gate would accept it: records merged.
+ *
+ * Sorted by slot so the key does not depend on the order the columns were picked in, and shared
+ * between the panel, the run gate and the mirror so all three agree on what "the same mapping"
+ * means.
+ */
+export function collisionCheckKey(
+  mapping: Pick<BareSetMapping, "identity" | "sequences"> | undefined,
+): string | undefined {
+  if (mapping === undefined || !mapping.identity) return undefined;
+  const mapped = Object.entries(mapping.sequences ?? {})
+    .filter(([, column]) => Boolean(column))
+    .sort(([a], [b]) => a.localeCompare(b))
+    .map(([slot, column]) => `${slot}=${column}`);
+  return [mapping.identity, ...mapped].join("\u0000");
+}
+
+/**
+ * The mapping with everything that names a column dropped.
+ *
+ * The receptor declaration and the numbering scheme describe the biology and outlive any one file;
+ * the identity column, the sequence columns and the accepted properties name headers, and mean
+ * nothing once the headers change.
+ */
+export function forgetMappedColumns(bare: BareSetMapping | undefined): BareSetMapping | undefined {
+  if (bare === undefined) return undefined;
+  return {
+    identity: "",
+    chainSelection: bare.chainSelection,
+    sequences: {},
+    scheme: bare.scheme,
+  };
+}
+
 /**
  * Whether a bare-set mapping is complete enough to run.
  *

test/src/wf.test.ts

@@ -19,7 +19,7 @@ import { blockSpec as sequencePropertiesSpec } from "@platforma-open/milaborator
 import type { PTableHandle } from "@platforma-sdk/model";
 import { createPlDataTableStateV2, uniquePlId } from "@platforma-sdk/model";
 import { awaitStableState, blockTest } from "@platforma-sdk/test";
-import { ImportVdjBlockPointer } from "this-block";
+import { collisionCheckKey, ImportVdjBlockPointer } from "this-block";
 
 /**
  * A complete `BlockData` from the fields a test actually cares about.
@@ -29,6 +29,9 @@ import { ImportVdjBlockPointer } from "this-block";
  * visibly `tableState`, which the stats table is built from.
  */
 function blockData(fields: Record<string, unknown>): Record<string, unknown> {
+  const bareSet = fields.bareSet as
+    | { identity: string; sequences: Record<string, string> }
+    | undefined;
   return {
     defaultBlockLabel: "",
     customBlockLabel: "",
@@ -40,6 +43,13 @@ function blockData(fields: Record<string, unknown>): Record<string, unknown> {
     mixcrColumnsPresent: false,
     crColumnsPresent: false,
     airrColumnsPresent: false,
+    // `args` refuses a bare set whose columns prerun has not cleared, and the verdict reaches data
+    // through a UI watcher these tests never run. So stand in for it — clean unless the test passes
+    // its own `prerunChecks`, which `...fields` below lets it do. Keyed with the block's own rule,
+    // so a change to what a verdict covers fails here rather than silently passing.
+    ...(collisionCheckKey(bareSet) !== undefined
+      ? { prerunChecks: { columns: collisionCheckKey(bareSet), identityCollides: false } }
+      : {}),
     ...fields,
   };
 }
@@ -307,33 +317,56 @@ blockTest(
           sequences: { IGHeavy: "VH", IGLight: "VL" },
           scheme: SCHEME,
         },
+        // What the UI mirrors in once prerun answers. Stated here because the mirror is a UI
+        // watcher and these tests drive the block directly.
+        prerunChecks: {
+          columns: collisionCheckKey({
+            identity: "mAb ID",
+            sequences: { IGHeavy: "VH", IGLight: "VL" },
+          }),
+          identityCollides: true,
+        },
       }),
     });
 
     const state = (await awaitStableState(project.getBlockState(blockId), 300000)) as {
       outputs?: Record<string, unknown>;
-      inputsValid?: boolean;
-      canRun?: boolean;
     };
 
-    const wrapped = state.outputs?.identityCollisions as
-      | { value?: string[] }
-      | string[]
-      | undefined;
-    const collisions = (Array.isArray(wrapped) ? wrapped : (wrapped?.value ?? [])) as string[];
+    // The refusal the block's name promises: a colliding id column makes args invalid, so the
+    // interface offers no Run. Read from the overview, not the block state — the block state
+    // carries outputs, and runnability lives on the overview.
+    const overview = (await project.overview.getValue())!;
+    const blockOverview = overview.blocks.find((b) => b.id === blockId)!;
+    expect(blockOverview.inputsValid).toBe(false);
+    expect(blockOverview.canRun).toBe(false);
+
+    type Verdict = { key: string; values: string[] };
+    const wrapped = state.outputs?.identityCollisions as { value?: Verdict } | Verdict | undefined;
+    const found = (wrapped && "key" in wrapped ? wrapped : wrapped?.value) as Verdict | undefined;
+
+    // The verdict names the mapping it is about — the id column AND the sequence columns, since a
+    // collision is a repeated id whose other mapped cells differ. Keyed on the id alone, a clean
+    // verdict outlived a remapped chain and the run gate accepted it, merging records.
+    expect(found?.key).toBe(
+      collisionCheckKey({ identity: "mAb ID", sequences: { IGHeavy: "VH", IGLight: "VL" } }),
+    );
+    // Remapping a chain is a different question, so the old verdict must not answer it.
+    expect(found?.key).not.toBe(
+      collisionCheckKey({ identity: "mAb ID", sequences: { IGHeavy: "VH", IGLight: "VL2" } }),
+    );
+    const collisions = found?.values ?? [];
 
     // The differing pair is reported, so the scientist is told which value to fix.
     expect(collisions).toContain("AB-001");
     // The identical pair is not: repeating a record verbatim discards nothing.
     expect(collisions).not.toContain("AB-002");
 
-    // GAP, verified here rather than assumed: `argsValid` disables Run in the interface, but
-    // the platform does not enforce it — `project.runBlock` resolves happily on an invalid
-    // block. So "the run does not start" holds for a scientist clicking Run and not for an API
-    // caller, and a colliding set driven through the API would still import and merge records.
-    // Closing that needs a workflow-side refusal, which is data-dependent and therefore a
-    // separate awaiting template.
-    await expect(project.runBlock(blockId)).resolves.toBeUndefined();
+    // Enforced by the platform, not only by the interface: invalid args means no args to render a
+    // production from, so an API caller cannot drive a colliding set through either. This was a
+    // documented gap while the collision verdict sat outside the gate — args stayed valid, and
+    // `runBlock` imported a set that merged records without complaint.
+    await expect(project.runBlock(blockId)).rejects.toThrow(/currentArgs not set/);
   },
 );
 
@@ -511,6 +544,12 @@ blockTest(
       "Affinity (nM)": "Double",
     });
 
+    // The profile names the file it came from. The panel reads this to tell this file's columns
+    // from the previous file's, still retained while the new one is scanned.
+    expect((state.outputs?.profiledSampleId as { value?: string } | undefined)?.value).toBe(
+      "SDIRECT000000000000000001",
+    );
+
     // Indistinguishable from the pool door: same axes, same key, same columns — abundance
     // alone on [sampleId, variantKey], every property of the record on the record axis.
     for (const c of columns) {