diff --git a/.changeset/prerun-feedback-and-run-gate.md b/.changeset/prerun-feedback-and-run-gate.md new file mode 100644 index 0000000..b88624d --- /dev/null +++ b/.changeset/prerun-feedback-and-run-gate.md @@ -0,0 +1,92 @@ +--- +'@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, on both doors.** Prerun now states which file a profile was taken from, + and which dataset and format an inference was run for; the model pairs each with its result so the + two cannot get out of step. The panel announces the wait and withholds the mapping until the + columns on offer really belong to what is selected. Keyed to the input 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. The verdict about the selected columns + now sits below the controls it is about, rather than between them. +- **The column list waits until it has something to list.** "The following columns will be + imported:" appeared with nothing under it as soon as a dataset was picked, before any format was, + which read as an import that would emit nothing. It also outlived a dataset switch, listing the + previous dataset's columns. +- **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. +- **Run waits for the dataset to be judged.** For the seven non-custom formats Run was gated on + booleans the UI mirrored back from `validationResult`, keyed on the *format* alone and never on + the dataset. Switching between two datasets of the same format left the previous one's verdict + standing, so Run armed at once while the panel was still scanning. The five `*ColumnsPresent` + fields and their watcher are gone, replaced by a second case of `prerunCheck` — the same shape the + collision check already uses: the verdict carries the dataset *and format* it was reached for, + and one left over from an earlier selection reads as "not judged yet" rather than being applied. + Run is refused both while a verdict is outstanding and when it reports missing columns. + + `validationResult` now takes the format from prerun's stamp rather than from live block data, so + both halves of the question come from one staging context and a verdict can no longer judge one + dataset's headers under another's format. It states the dataset it judged, which is what lets the + UI mirror it in by copying rather than by matching the two outputs against the selection — there + is no comparison left in the hairpin to get wrong. +- **One provenance stamp, not two, and it names a dataset.** The file door emitted + `profiledSampleId` and the dataset door `inferredFor`; both answered the same question — what the + prerun results on screen were computed for — and only one door is ever live. They are now a + single `prerunDatasetValidationInfo`, tagged by door: `{ door: "file", datasetId }` or + `{ door: "dataset", datasetRef, format }`. Tagged rather than flattened because the two answers + are not interchangeable — the file door names a dataset that *is* the file, the dataset door + names one already in the pool — and side by side they otherwise read as one id in two formats. `FileSource.sampleId` is renamed `datasetId` to match: one file is + one dataset, and it is only *today* that the dataset is also one sample, which is why that value + also mints the `pl7.app/sampleId` key. A file carrying several samples would name those from its + own contents while this stayed the identity of the file they came from. The value mints an axis + key and v1 shipped in block 1.8.1, so the rename carries a `v1 -> v2` data migration rather than + relying on the old key being absent — without it a saved project would key its records on + `undefined` and lose every join a downstream block had made. + +- **Re-reading a file no longer changes its identity.** The id was minted on every pick, so + re-selecting the same file — which is also how a file gets re-read — handed it a new one. Since + that value mints the `pl7.app/sampleId` key, the re-read silently orphaned every join a + downstream block had made against the old key, with nothing on screen to say so. It is now minted + only when the handle changes, which is the same test that decides whether the mapping is dropped, + and which the field's own contract had claimed all along. Two different files that share a name + are still a swap: their handles differ. They do share a label, since that is the filename stem. +- **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. diff --git a/model/src/data-model.ts b/model/src/data-model.ts index ab96a0e..879ebf5 100644 --- a/model/src/data-model.ts +++ b/model/src/data-model.ts @@ -4,37 +4,12 @@ import type { BlockData, LegacyBlockArgs, LegacyUiState } from "./types"; /** The six chains a fresh block offers. Unchanged from V1's `withArgs` default. */ const DEFAULT_CHAINS = ["IGHeavy", "IGLight", "TCRAlpha", "TCRBeta", "TCRDelta", "TCRGamma"]; -/** - * View-state defaults, shared by the fresh-project initialiser and the legacy upgrade. - * - * The upgrade needs them because V1's `uiState` fields are all optional on disk: a project - * saved before a flag was introduced simply has no value for it, and an undefined boolean - * reaching `argsValid` used to read as false by accident. Making the defaults explicit here - * means the V3 data is complete whatever shape the project was saved in. - */ +/** View-state defaults, shared by the fresh-project initialiser and the legacy upgrade. */ const viewStateDefaults = () => ({ tableState: createPlDataTableStateV2(), settingsOpen: true, - qiagenColumnsPresent: false, - immunoSeqColumnsPresent: false, - mixcrColumnsPresent: false, - crColumnsPresent: false, - airrColumnsPresent: false, }); -/** - * V1 kept the scientist's edits in two buckets — `args` and `uiState`. V3 keeps them in one, - * and derives what the workflow sees. This upgrade runs once per project, the first time a - * project saved under V1 is opened. - * - * The mapping is field-for-field: nothing is reshaped, because nothing in V1's shape was bent - * to dodge the stale gate. What changes is where the fields *go afterwards* — the labels and - * the secondary count type stop being projected into args, which is the whole point of the - * migration for this block. See `index.ts`. - * - * V1's `loadFromFile` is dropped rather than carried: the panel now derives which door it is - * showing from whether a file is loaded, so a stored flag could only disagree with the data. - */ export function upgradeLegacyData({ args, uiState, @@ -59,9 +34,23 @@ export function upgradeLegacyData({ }; } +/** `FileSource.sampleId` became `datasetId`. The value mints an axis key, so it must carry over. */ +type FileSourceV1 = { sampleId?: string; datasetId?: string } & Record; + +function renameSampleIdToDatasetId(data: BlockData): BlockData { + const fileSource = data.fileSource as FileSourceV1 | undefined; + if (fileSource === undefined) return data; + const { sampleId, ...rest } = fileSource; + return { + ...data, + fileSource: { ...rest, datasetId: fileSource.datasetId ?? sampleId } as BlockData["fileSource"], + }; +} + export const blockDataModel = new DataModelBuilder() .from("v1") .upgradeLegacy(upgradeLegacyData) + .migrate("v2", renameSampleIdToDatasetId) .init(() => ({ ...viewStateDefaults(), defaultBlockLabel: "", diff --git a/model/src/index.ts b/model/src/index.ts index 1d1479a..ea8c583 100644 --- a/model/src/index.ts +++ b/model/src/index.ts @@ -1,4 +1,4 @@ -import type { InferOutputsType, PColumnKey, PColumnValue } from "@platforma-sdk/model"; +import type { InferOutputsType, PColumnKey, PColumnValue, PlRef } from "@platforma-sdk/model"; import { BlockModelV3, DataColumn, @@ -6,8 +6,15 @@ 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, + ImportFormat, +} from "./types"; +import { bareSetValid, collisionCheckKey, datasetCheckKey } from "./types"; export * from "./types"; export { upgradeLegacyData } from "./data-model"; @@ -39,14 +46,42 @@ function withoutDatasetDoorMapping(args: BlockArgs): BlockArgs { } /** - * The workflow's view of the block, and the only place validation lives. + * Refuse a bare set whose mapped columns prerun has not cleared — including when there is no + * verdict yet. The record key is the identity's hash, so running before the answer is in is how two + * different records silently become one. + */ +function requireCheckedColumns(data: BlockData): void { + const columns = collisionCheckKey(data.bareSet); + if (columns === undefined) return; // bareSetValid has already refused + const check = data.prerunCheck; + if (check?.check !== "columns" || check.subject !== columns) { + throw new Error("Validating the selected columns"); + } + if (check.identityCollides) { + throw new Error(`"${data.bareSet?.identity}" repeats on rows that are not identical`); + } +} + +/** + * Refuse the run until prerun has judged the dataset and format actually selected. * - * Three jobs, in order: refuse what cannot run (by throwing), drop the door that is not in - * use, and drop the fields the chosen path never reads. What it deliberately does NOT do is - * reorder `chains`: canonicalising a set the user picked in their own order would change the - * args of every project already on disk, and buys only the rare case of someone re-picking the - * same chains in a different order. + * The verdict carries the dataset it was reached for, so one left over from a previous selection + * reads as "not judged yet" rather than being applied to this one. That is the whole of the rule + * the old per-format flags broke: they were keyed on the format alone, so switching between two + * datasets of the same format left the previous verdict standing and Run armed on it. */ +function requireCheckedDataset(data: BlockData): void { + const dataset = datasetCheckKey(data); + if (dataset === undefined) return; // the file door, or nothing picked yet + const check = data.prerunCheck; + if (check?.check !== "dataset" || check.subject !== dataset) { + throw new Error("Validating the selected columns"); + } + if (!check.columnsPresent) { + throw new Error(`The dataset does not carry the columns a ${data.format} dataset needs`); + } +} + function projectArgs(data: BlockData): BlockArgs { const { datasetRef, format, chains, customMapping, primaryCountType, fileSource } = data; @@ -60,6 +95,8 @@ function projectArgs(data: BlockData): BlockArgs { datasetRef, fileSource, format, + // Not sorted: canonicalising an order the user chose would change the args of every project + // already on disk, to buy the rare re-pick of the same chains in a different order. chains, customMapping, primaryCountType, @@ -72,6 +109,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 +123,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); } @@ -100,21 +139,10 @@ function projectArgs(data: BlockData): BlockArgs { return args; } - // The per-format flags are written by the UI from the `validationResult` output. They are a - // mirror of a derivation, which is a hairpin, but replacing them is a separate change: the - // check they stand for needs the file's headers, and the args lambda cannot reach prerun. - const presentByFormat: Record = { - qiagen: data.qiagenColumnsPresent === true, - immunoSeq: data.immunoSeqColumnsPresent === true, - mixcr: data.mixcrColumnsPresent === true, - "mixcr-sc": data.mixcrColumnsPresent === true, - cellranger: data.crColumnsPresent === true, - airr: data.airrColumnsPresent === true, - "airr-sc": data.airrColumnsPresent === true, - }; - if (format in presentByFormat && !presentByFormat[format]) { - throw new Error(`The file does not carry the columns a ${format} dataset needs`); - } + // The check needs the dataset's headers, which live in prerun, and a V3 args lambda receives + // `data` only. So the verdict is mirrored in — the same hairpin the collision check uses, under + // the same contract, which is what keeps it from repeating the per-format flags' mistake. + requireCheckedDataset(data); return args; } @@ -143,15 +171,6 @@ export const platforma = BlockModelV3.create(blockDataModel) bareSet: data.bareSet, })) - /** - * Identity values that appear on rows which are not identical to each other. - * - * Not `retentive`: this gates the run, so a stale value is worse than a briefly absent one. - * - * Read straight from prerun here, and *not* mirrored into `uiState` from a UI watcher. That - * mirror is what the format-validity flags do, and it is a hairpin — an output written back - * into state that a derivation then reads. It survives on one client and races on two. - */ /** * Drives the upload for a directly-loaded file. * @@ -177,7 +196,19 @@ export const platforma = BlockModelV3.create(blockDataModel) { isActive: true }, ) + /** + * Identity values repeated on rows that are not identical, with the mapping they were found + * under. One value, so the two can never be read from different runs. + * + * 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>(); + const key = collisionCheckKey(mapping); + if (key === undefined) return undefined; + const raw = ctx.prerun ?.resolve({ field: "identityCollisions", allowPermanentAbsence: true }) ?.getDataAsString(); @@ -188,7 +219,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 +295,40 @@ export const platforma = BlockModelV3.create(blockDataModel) } }) + /** + * What the prerun results on screen were computed for. A mismatch with what is selected now + * means the panel is still showing the previous one's columns. + * + * Tagged by door because the two answers are not interchangeable: the file door names a dataset + * that IS the file, by a minted id; the dataset door names one already in the result pool, by a + * ref. Exactly one door is ever live — `projectArgs` refuses both at once. + * + * Keyed to the input rather than to "prerun is busy": prerun also re-runs on every mapping edit, + * to re-check the identity column. + */ + .retentiveOutput("prerunDatasetValidationInfo", (ctx) => { + // Gate on the result the stamp describes, whichever door produced it. The read is marked + // unstable, so `retentive` keeps reporting the previous input until the new result lands and + // the two can never disagree. + const backing = + ctx.prerun?.resolve({ field: "columnProfile", allowPermanentAbsence: true }) ?? + ctx.prerun?.resolve({ field: "headerColumns", allowPermanentAbsence: true }); + if (backing === undefined) return undefined; + if (!backing.getIsReadyOrError()) return undefined; + return ctx.prerun + ?.resolve({ field: "prerunDatasetValidationInfo", allowPermanentAbsence: true }) + ?.getDataAsJsonOrUndefined< + | { door: "file"; datasetId: string } + | { door: "dataset"; datasetRef?: PlRef; format?: ImportFormat } + >(); + }) + + /** + * 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 @@ -286,143 +351,156 @@ export const platforma = BlockModelV3.create(blockDataModel) }) ?.getDataAsJson(); - if (!headerColumns || !ctx.data.format) { + // The format comes from prerun's own stamp, not from `ctx.data`. Both halves of the question + // then come from the same staging context, so a verdict cannot pair one dataset's headers with + // another's format while a switch is in flight — which it could when the format was read live. + const stamp = ctx.prerun + ?.resolve({ field: "prerunDatasetValidationInfo", allowPermanentAbsence: true }) + ?.getDataAsJsonOrUndefined<{ door?: string; datasetRef?: PlRef; format?: ImportFormat }>(); + + if (!headerColumns || stamp?.door !== "dataset" || !stamp.format || !stamp.datasetRef) { return undefined; } - const format = ctx.data.format; + const format = stamp.format; const headers = headerColumns; + // The verdict carries what it is about, so the UI mirrors it in without matching anything. + const dataset = datasetCheckKey({ datasetRef: stamp.datasetRef, format }); + + const verdict = ((): { isValid: boolean; missingColumns: string[]; format: string } => { + if (format === "qiagen") { + const qiagenColumns = [ + "read set", + "chain", + "V-region", + "J-region", + "CDR3 nucleotide seq", + "CDR3 amino acid seq", + "frequency", + "rank", + "UMIs with analytical threshold", + "nucleotide length", + "amino acid length", + ]; + + const missingColumns = qiagenColumns.filter((col) => !headers.includes(col)); + + return { + isValid: missingColumns.length === 0, + missingColumns, + format: "qiagen", + }; + } - if (format === "qiagen") { - const qiagenColumns = [ - "read set", - "chain", - "V-region", - "J-region", - "CDR3 nucleotide seq", - "CDR3 amino acid seq", - "frequency", - "rank", - "UMIs with analytical threshold", - "nucleotide length", - "amino acid length", - ]; - - const missingColumns = qiagenColumns.filter((col) => !headers.includes(col)); - - return { - isValid: missingColumns.length === 0, - missingColumns, - format: "qiagen", - }; - } + if (format === "immunoSeq") { + const hasAny = (aliases: string[]) => aliases.some((alias) => headers.includes(alias)); + const missingColumns: string[] = []; + if (!hasAny(["rearrangement", "nucleotide"])) missingColumns.push("sequence"); + if (!hasAny(["amino_acid_sequence", "amino_acid", "aminoAcid"])) + missingColumns.push("cdr3-aa"); + if (!hasAny(["v_gene", "v-gene", "vGene", "vGeneName"])) missingColumns.push("v-gene"); + if (!hasAny(["d_gene", "d-gene", "dGene", "dGeneName"])) missingColumns.push("d-gene"); + if (!hasAny(["j_gene", "j-gene", "jGene", "jGeneName"])) missingColumns.push("j-gene"); + if (!hasAny(["v-index", "v_index", "vIndex"])) missingColumns.push("v-begin"); + if (!hasAny(["count (templates/reads)", "count (reads)", "seq_reads", "reads", "count"])) { + missingColumns.push("read-count"); + } - if (format === "immunoSeq") { - const hasAny = (aliases: string[]) => aliases.some((alias) => headers.includes(alias)); - const missingColumns: string[] = []; - if (!hasAny(["rearrangement", "nucleotide"])) missingColumns.push("sequence"); - if (!hasAny(["amino_acid_sequence", "amino_acid", "aminoAcid"])) - missingColumns.push("cdr3-aa"); - if (!hasAny(["v_gene", "v-gene", "vGene", "vGeneName"])) missingColumns.push("v-gene"); - if (!hasAny(["d_gene", "d-gene", "dGene", "dGeneName"])) missingColumns.push("d-gene"); - if (!hasAny(["j_gene", "j-gene", "jGene", "jGeneName"])) missingColumns.push("j-gene"); - if (!hasAny(["v-index", "v_index", "vIndex"])) missingColumns.push("v-begin"); - if (!hasAny(["count (templates/reads)", "count (reads)", "seq_reads", "reads", "count"])) { - missingColumns.push("read-count"); + return { + isValid: missingColumns.length === 0, + missingColumns, + format: "immunoSeq", + }; } - return { - isValid: missingColumns.length === 0, - missingColumns, - format: "immunoSeq", - }; - } - - if (format === "mixcr") { - // MiXCR minimal requirements aligned with infer-columns-mixcr.lib.tengo - const mixcrRequiredHeaders = ["readCount", "nSeqCDR3", "aaSeqCDR3"]; + if (format === "mixcr") { + // MiXCR minimal requirements aligned with infer-columns-mixcr.lib.tengo + const mixcrRequiredHeaders = ["readCount", "nSeqCDR3", "aaSeqCDR3"]; - const missingColumns = mixcrRequiredHeaders.filter((col) => !headers.includes(col)); + const missingColumns = mixcrRequiredHeaders.filter((col) => !headers.includes(col)); - return { - isValid: missingColumns.length === 0, - missingColumns, - format: "mixcr", - }; - } + return { + isValid: missingColumns.length === 0, + missingColumns, + format: "mixcr", + }; + } - if (format === "mixcr-sc") { - // Same as MiXCR plus at least one tagValueCELL* column - const mixcrRequiredHeaders = ["readCount", "nSeqCDR3", "aaSeqCDR3"]; - const missingBase = mixcrRequiredHeaders.filter((col) => !headers.includes(col)); - const hasTagValueCell = headers.some((h) => h.startsWith("tagValueCELL")); - const missingColumns = [ - ...missingBase, - ...(hasTagValueCell ? ([] as string[]) : ["tagValueCELL*"]), - ]; - return { - isValid: missingColumns.length === 0, - missingColumns, - format: "mixcr-sc", - }; - } + if (format === "mixcr-sc") { + // Same as MiXCR plus at least one tagValueCELL* column + const mixcrRequiredHeaders = ["readCount", "nSeqCDR3", "aaSeqCDR3"]; + const missingBase = mixcrRequiredHeaders.filter((col) => !headers.includes(col)); + const hasTagValueCell = headers.some((h) => h.startsWith("tagValueCELL")); + const missingColumns = [ + ...missingBase, + ...(hasTagValueCell ? ([] as string[]) : ["tagValueCELL*"]), + ]; + return { + isValid: missingColumns.length === 0, + missingColumns, + format: "mixcr-sc", + }; + } - if (format === "cellranger") { - // Cell Ranger VDJ clones per-chain table minimal required headers - const cellrangerRequired = ["cdr3_nt", "cdr3", "v_gene", "j_gene", "barcode"]; - const missingColumns = cellrangerRequired.filter((col) => !headers.includes(col)); - return { - isValid: missingColumns.length === 0, - missingColumns, - format: "cellranger", - }; - } + if (format === "cellranger") { + // Cell Ranger VDJ clones per-chain table minimal required headers + const cellrangerRequired = ["cdr3_nt", "cdr3", "v_gene", "j_gene", "barcode"]; + const missingColumns = cellrangerRequired.filter((col) => !headers.includes(col)); + return { + isValid: missingColumns.length === 0, + missingColumns, + format: "cellranger", + }; + } - if (format === "airr" || format === "airr-sc") { - // AIRR format uses case-insensitive column names - // Required: duplicate_count, junction (CDR3 nt), v_call, j_call - // For single-cell: also requires cell_id - // Handle case where headerColumns might be a single comma-separated string or array of strings - const flattenedHeaders: string[] = []; - for (const h of headers) { - const str = String(h).trim(); - // If the string contains commas, split it - if (str.includes(",")) { - flattenedHeaders.push( - ...str - .split(",") - .map((s) => s.trim()) - .filter((s) => s.length > 0), - ); - } else { - flattenedHeaders.push(str); + if (format === "airr" || format === "airr-sc") { + // AIRR format uses case-insensitive column names + // Required: duplicate_count, junction (CDR3 nt), v_call, j_call + // For single-cell: also requires cell_id + // Handle case where headerColumns might be a single comma-separated string or array of strings + const flattenedHeaders: string[] = []; + for (const h of headers) { + const str = String(h).trim(); + // If the string contains commas, split it + if (str.includes(",")) { + flattenedHeaders.push( + ...str + .split(",") + .map((s) => s.trim()) + .filter((s) => s.length > 0), + ); + } else { + flattenedHeaders.push(str); + } } - } - const headersLower = flattenedHeaders.map((h) => h.toLowerCase()); - const airrRequired = ["duplicate_count", "junction", "v_call", "j_call"]; - const missingColumns = airrRequired.filter((req) => !headersLower.includes(req)); - - // For single-cell AIRR, also require cell_id - if (format === "airr-sc") { - const hasCellId = headersLower.includes("cell_id"); - if (!hasCellId) { - missingColumns.push("cell_id"); + const headersLower = flattenedHeaders.map((h) => h.toLowerCase()); + const airrRequired = ["duplicate_count", "junction", "v_call", "j_call"]; + const missingColumns = airrRequired.filter((req) => !headersLower.includes(req)); + + // For single-cell AIRR, also require cell_id + if (format === "airr-sc") { + const hasCellId = headersLower.includes("cell_id"); + if (!hasCellId) { + missingColumns.push("cell_id"); + } } + + return { + isValid: missingColumns.length === 0, + missingColumns, + format: format, + }; } + // For other formats, validation is handled elsewhere or not needed return { - isValid: missingColumns.length === 0, - missingColumns, + isValid: true, + missingColumns: [], format: format, }; - } + })(); - // For other formats, validation is handled elsewhere or not needed - return { - isValid: true, - missingColumns: [], - format: format, - }; + return { ...verdict, dataset }; }) /** diff --git a/model/src/types.ts b/model/src/types.ts index 6ef71ee..06757f6 100644 --- a/model/src/types.ts +++ b/model/src/types.ts @@ -14,11 +14,8 @@ export type CountType = "read" | "umi"; export type FileSource = { handle: ImportFileHandle; - /** - * Minted in the UI at the moment the file is picked, not derived from the handle, so the - * sample keeps its identity across runs even if the same file is re-selected. - */ - sampleId: string; + /** Unique id for the dataset this file holds. Minted when the file is picked. */ + datasetId: string; /** The filename stem — exactly what samples-and-data would have labelled the sample. */ label: string; /** @@ -79,7 +76,7 @@ export const CHAIN_SLOTS: Record = { * IMGT is position-unified and chain-agnostic — ANARCI's `number_imgt` takes no chain type at * all. Kabat, Chothia, Martin and Wolfguy were defined on antibody structures and ANARCI * implements them for `H`/`K`/`L` only, raising "Unimplemented numbering scheme" for a TCR chain - * (anarci.py:558-592). So a TCR selection can only be numbered under IMGT, and offering the + * (anarci.py). So a TCR selection can only be numbered under IMGT, and offering the * others would hand the scientist a choice that fails the run. */ export const SCHEMES_FOR_SELECTION: Record = { @@ -110,6 +107,13 @@ export const CHAIN_SLOT_LABELS: Record = { TCRGamma: "TCR-ɣ", }; +/** What to call each numbering scheme in front of the scientist. */ +export const SCHEME_LABELS: Record = { + 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,14 +226,33 @@ export type BlockData = { // --- bare set. Its presence is what selects the bare path in the workflow. bareSet?: BareSetMapping; + /** + * What prerun found, kept here so the args projection — which sees only `data` — can gate the run + * on it. Written by the UI (`ui/src/app.ts`). + * + * The verdict carries what it is *about*, so one reached for something no longer selected is + * ignored rather than applied. Tagged by which check it is: the two paths through `projectArgs` + * that consult a verdict return before reaching each other, so only ever one is live. + */ + prerunCheck?: + | { + check: "columns"; + /** The mapping the verdict was reached for — see {@link collisionCheckKey}. */ + subject: string; + /** The id column repeats on rows whose other mapped cells differ, so two records merge. */ + identityCollides: boolean; + } + | { + check: "dataset"; + /** The dataset and format the verdict was reached for — see {@link datasetCheckKey}. */ + subject: string; + /** The dataset carries the columns its declared format needs. */ + columnsPresent: boolean; + }; + // --- view state. None of this is projected anywhere. tableState: PlDataTableStateV2; settingsOpen: boolean; - qiagenColumnsPresent: boolean; - immunoSeqColumnsPresent: boolean; - mixcrColumnsPresent: boolean; - crColumnsPresent: boolean; - airrColumnsPresent: boolean; }; /** The V1 `args` bucket, as it sits in projects saved before the V3 migration. */ @@ -250,17 +273,12 @@ export type LegacyBlockArgs = { export type LegacyUiState = { tableState?: PlDataTableStateV2; settingsOpen?: boolean; - qiagenColumnsPresent?: boolean; - immunoSeqColumnsPresent?: boolean; - mixcrColumnsPresent?: boolean; - crColumnsPresent?: boolean; - airrColumnsPresent?: boolean; }; /** * The SDK's `substituteSpecialCharacters` class, mirrored so the model can refuse a collision * without a round trip to the workflow. Kept in step with - * `sdk/workflow-tengo/src/strings.lib.tengo:4`. + * the SDK's `strings.lib.tengo`. */ const SPECIAL_CHARACTERS = /[-_,.:; +()!<>[\]}{"\\/:$%^#@*&]+/g; @@ -278,6 +296,51 @@ export function propertyCollisions(properties: ImportedProperty[]): Record hs.length > 1)); } +/** + * What a collision verdict is about. A collision is an identity repeated on rows whose *other + * mapped cells* differ, so the sequence columns are part of the question and remapping a chain + * invalidates the answer. Sorted, so the key does not depend on the order columns were picked in. + */ +export function collisionCheckKey( + mapping: Pick | 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"); +} + +/** + * What a dataset-door verdict is about: the dataset picked and the format it was declared to be. + * Both, because the same dataset answers differently under a different format. + * + * `undefined` when there is nothing to check yet — no dataset, or no format — which is also how + * the caller tells the dataset door from the file door. + */ +export function datasetCheckKey( + data: Pick, +): string | undefined { + const ref = data.datasetRef; + if (ref === undefined || data.format === undefined) return undefined; + return [ref.blockId, ref.name, data.format].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 column names do not. + */ +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. * diff --git a/test/src/messages.test.ts b/test/src/messages.test.ts new file mode 100644 index 0000000..f2f7401 --- /dev/null +++ b/test/src/messages.test.ts @@ -0,0 +1,76 @@ +import { collisionCheckKey } from "@platforma-open/milaboratories.import-vdj.model"; +import { describe, expect, test } from "vitest"; +import { + emptySamplesMessage, + formatLabel, + identityCollisionMessage, + missingColumnsMessage, + propertyCollisionMessage, +} from "../../ui/src/pages/messages"; + +describe("ui messages", () => { + test("empty samples: under the cap", () => { + expect(emptySamplesMessage({ emptySamples: ["S1", "S2"] })).toBe( + "After receptor chain filtering, no clonotypes found in sample(s) S1, S2", + ); + }); + test("empty samples: over the cap truncates at 5", () => { + expect(emptySamplesMessage({ emptySamples: ["a", "b", "c", "d", "e", "f", "g"] })).toBe( + "After receptor chain filtering, no clonotypes found in sample(s) a, b, c, d, e and 2 more", + ); + }); + test("empty samples: none is undefined, not an empty string", () => { + expect(emptySamplesMessage({ emptySamples: [] })).toBeUndefined(); + expect(emptySamplesMessage(undefined)).toBeUndefined(); + }); + test("format label falls back to the raw id", () => { + expect(formatLabel("mixcr")).toBe("MiXCR bulk"); + expect(formatLabel("MIXCR-SC")).toBe("MiXCR single cell"); + expect(formatLabel("nope")).toBe("nope"); + expect(formatLabel(undefined)).toBe(""); + }); + test("missing columns", () => { + expect( + missingColumnsMessage({ + isValid: false, + missingColumns: ["v_call", "j_call"], + format: "airr", + }), + ).toBe( + "The selected dataset is missing required AIRR bulk columns: v_call, j_call. Please verify the format selection or choose a different dataset.", + ); + expect(missingColumnsMessage({ isValid: true, missingColumns: [], format: "airr" })).toBe(""); + expect(missingColumnsMessage(undefined)).toBe(""); + }); + const mapping = { identity: "id", sequences: { IGHeavy: "VH" } } as never; + const keyFor = (m: never) => collisionCheckKey(m)!; + + test("identity collisions truncate at 3", () => { + expect( + identityCollisionMessage( + { key: keyFor(mapping), values: ["x", "y", "z", "w", "v"] }, + mapping, + ), + ).toBe( + "Repeated on rows that are not identical: x, y, z and 2 more. Two rows sharing an id become one record — pick a different column, or fix the file.", + ); + expect(identityCollisionMessage({ key: keyFor(mapping), values: [] }, mapping)).toBe(""); + expect(identityCollisionMessage(undefined, mapping)).toBe(""); + }); + + test("a verdict for a different mapping says nothing", () => { + const other = { identity: "other", sequences: { IGHeavy: "VH" } } as never; + expect(identityCollisionMessage({ key: keyFor(other), values: ["x"] }, mapping)).toBe(""); + }); + test("property collisions", () => { + const props = [ + { header: "A b", valueType: "String" }, + { header: "A/b", valueType: "String" }, + ] as never; + expect(propertyCollisionMessage(props)).toBe( + "These headers would become the same column: A b / A/b. Rename one in the file — importing both is not possible, and dropping one silently would lose a column you asked for.", + ); + expect(propertyCollisionMessage([])).toBe(""); + expect(propertyCollisionMessage(undefined)).toBe(""); + }); +}); diff --git a/test/src/model.test.ts b/test/src/model.test.ts index 8511cae..8d7957c 100644 --- a/test/src/model.test.ts +++ b/test/src/model.test.ts @@ -35,7 +35,7 @@ describe("upgradeLegacyData", () => { scheme: "kabat", }, }, - uiState: { settingsOpen: false, mixcrColumnsPresent: true }, + uiState: { settingsOpen: false }, }); expect(data.defaultBlockLabel).toBe("my import"); @@ -48,22 +48,15 @@ describe("upgradeLegacyData", () => { expect(data.secondaryCountType).toBe("read"); expect(data.bareSet?.scheme).toBe("kabat"); - // uiState survives too, including a flag the defaults set the other way. + // uiState survives too, set the other way from the defaults. expect(data.settingsOpen).toBe(false); - expect(data.mixcrColumnsPresent).toBe(true); }); - test("fills view state a project saved before a flag existed has no value for", () => { + test("fills view state and chains a project saved without them has no value for", () => { const data = upgradeLegacyData({ args: { format: "qiagen" }, uiState: {} }); - // Every flag present and false rather than undefined: an undefined boolean reaching the - // args projection would read as "columns not present" by accident rather than by rule. - expect(data.qiagenColumnsPresent).toBe(false); - expect(data.immunoSeqColumnsPresent).toBe(false); - expect(data.mixcrColumnsPresent).toBe(false); - expect(data.crColumnsPresent).toBe(false); - expect(data.airrColumnsPresent).toBe(false); expect(data.tableState).toBeDefined(); + expect(data.settingsOpen).toBe(true); expect(data.chains.length).toBe(6); }); @@ -74,7 +67,7 @@ describe("upgradeLegacyData", () => { args: { fileSource: { handle: "index://x" as never, - sampleId: "S1", + datasetId: "S1", label: "panel", extension: "tsv", }, diff --git a/test/src/wf.test.ts b/test/src/wf.test.ts index 61fa2b2..167cd63 100644 --- a/test/src/wf.test.ts +++ b/test/src/wf.test.ts @@ -14,6 +14,7 @@ real panel a sequence-derived key merges 119 of 1,243 antibodies. */ +import { collisionCheckKey } from "@platforma-open/milaboratories.import-vdj.model"; import { SamplesAndDataBlockPointer } from "@platforma-open/milaboratories.samples-and-data"; import { blockSpec as sequencePropertiesSpec } from "@platforma-open/milaboratories.sequence-properties"; import type { PTableHandle } from "@platforma-sdk/model"; @@ -29,17 +30,28 @@ import { ImportVdjBlockPointer } from "this-block"; * visibly `tableState`, which the stats table is built from. */ function blockData(fields: Record): Record { + const bareSet = fields.bareSet as + | { identity: string; sequences: Record } + | undefined; return { defaultBlockLabel: "", customBlockLabel: "", chains: [], tableState: createPlDataTableStateV2(), settingsOpen: true, - qiagenColumnsPresent: false, - immunoSeqColumnsPresent: false, - 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 `prerunCheck`, 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 + ? { + prerunCheck: { + check: "columns" as const, + subject: collisionCheckKey(bareSet)!, + identityCollides: false, + }, + } + : {}), ...fields, }; } @@ -307,33 +319,57 @@ 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. + prerunCheck: { + check: "columns" as const, + subject: collisionCheckKey({ + identity: "mAb ID", + sequences: { IGHeavy: "VH", IGLight: "VL" }, + })!, + identityCollides: true, + }, }), }); const state = (await awaitStableState(project.getBlockState(blockId), 300000)) as { outputs?: Record; - 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/); }, ); @@ -458,7 +494,7 @@ blockTest( chains: ["IGHeavy", "IGLight"], fileSource: { handle, - sampleId: "SDIRECT000000000000000001", + datasetId: "SDIRECT000000000000000001", label: "bare-paired-set", extension: "tsv", }, @@ -511,6 +547,16 @@ blockTest( "Affinity (nM)": "Double", }); + // The profile names the dataset 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?.prerunDatasetValidationInfo as + | { value?: { door?: string; datasetId?: string } } + | undefined + )?.value, + ).toEqual({ door: "file", datasetId: "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) { @@ -559,7 +605,7 @@ blockTest( }, fileSource: { handle, - sampleId: "SDIRECT000000000000000001", + datasetId: "SDIRECT000000000000000001", label: "bare-paired-set", extension: "tsv", }, @@ -614,7 +660,7 @@ blockTest( chains: ["IGHeavy", "IGLight"], fileSource: { handle, - sampleId: "SCHAIN0000000000000000001", + datasetId: "SCHAIN0000000000000000001", label: "bare-paired-set", extension: "tsv", }, @@ -677,7 +723,7 @@ blockTest( format: "custom", fileSource: { handle, - sampleId: "STCR00000000000000000001", + datasetId: "STCR00000000000000000001", label: "bare-tcrab-set", extension: "tsv", }, @@ -761,7 +807,7 @@ blockTest( chains: ["IGHeavy", "IGLight"], fileSource: { handle, - sampleId: "SXLSX00000000000000000001", + datasetId: "SXLSX00000000000000000001", label: "bare-paired-set", extension: "xlsx", }, diff --git a/ui/src/app.ts b/ui/src/app.ts index 064ee1d..ce4f56d 100644 --- a/ui/src/app.ts +++ b/ui/src/app.ts @@ -1,10 +1,15 @@ import { platforma } from "@platforma-open/milaboratories.import-vdj.model"; import { defineAppV3 } from "@platforma-sdk/ui-vue"; import MainPage from "./pages/MainPage.vue"; -import { watch } from "vue"; +import { watch, watchEffect } from "vue"; + +export const sdkPlugin = defineAppV3(platforma, (app) => { + syncPrerunChecks(app.model); -export const sdkPlugin = defineAppV3(platforma, () => { return { + // Main run only: the loader covers the whole block, and prerun re-runs while the settings + // panel is being edited. Prerun waits are announced inside the panel instead. + progress: () => app.model.outputs.isRunning, routes: { "/": () => MainPage, }, @@ -13,6 +18,54 @@ export const sdkPlugin = defineAppV3(platforma, () => { export const useApp = sdkPlugin.useApp; +type AppModel = ReturnType["model"]; + +/** + * Carry prerun's verdicts into `data` so the args projection can refuse the run on them. An output + * written back into state a derivation reads — the hairpin — kept safe by the contract on + * `BlockData.prerunCheck`: the verdict carries what it is about, so a stale one is ignored rather + * than applied. + */ +function syncPrerunChecks(model: AppModel) { + // Both writes are idempotent: every client derives the same verdict from the same output, and + // the guard means agreeing clients do not write at all. + const put = (next: NonNullable) => { + const current = model.data.prerunCheck; + if (JSON.stringify(current) === JSON.stringify(next)) return; + model.data.prerunCheck = next; + }; + + watchEffect(() => { + const found = model.outputs.identityCollisions; + if (found === undefined) return; + put({ check: "columns", subject: found.key, identityCollides: found.values.length > 0 }); + }); + + // A copy, not a comparison: `validationResult` states which dataset and format it judged, so + // there is nothing here to match up and so nothing to get wrong. The args projection does the + // comparing, against whatever is selected when it runs. + watchEffect(() => { + const result = model.outputs.validationResult; + if (result?.dataset === undefined) return; + put({ check: "dataset", subject: result.dataset, columnsPresent: result.isValid }); + }); + + // A verdict cannot outlive the file or dataset it was reached for. Watching primitives, not the + // refs, so a server patch swapping the data object does not clear it spuriously. Format is in + // here because the same dataset answers differently under a different format. + watch( + () => [ + model.data.fileSource?.datasetId, + model.data.datasetRef?.blockId, + model.data.datasetRef?.name, + model.data.format, + ], + () => { + model.data.prerunCheck = undefined; + }, + ); +} + // Make sure labels are initialized const unwatch = watch(sdkPlugin, ({ loaded }) => { if (!loaded) return; diff --git a/ui/src/pages/MainPage.vue b/ui/src/pages/MainPage.vue index 401fd9c..fc18f8c 100644 --- a/ui/src/pages/MainPage.vue +++ b/ui/src/pages/MainPage.vue @@ -1,17 +1,9 @@