Milab-6720: prerun hint and run disabling - #49
Open
erohinaelena wants to merge 8 commits into
Open
Conversation
Picking a file left the panel silent while every column of it was profiled, and the profile outputs are retentive, so the dropdowns kept answering with the previous file's headers as though nothing was happening. A mapping that had passed against that previous file also still satisfied bareSetValid, so Run stayed live over a file nobody had looked at and over headers it might not have. Prerun now states which file the profile came from, and the model pairs the two under retentive: getIsReadyOrError marks the read unstable, so the reported id stays the old file's until the new scan lands. The panel compares it with the loaded file, announces the wait, 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 identity column for collisions. Picking a different file drops the parts of the mapping that name columns, keeping the receptor declaration and the numbering scheme. That is what disables Run, and it is done on the gesture: args is a pure function of data and cannot consult prerun, and mirroring prerun's verdict back into data is the pattern this block is shedding. Re-picking the same file is not a swap. The import itself now shows the block's loader, which it never did, scoped to the main run so it cannot cover the settings panel during prerun.
PlAlert declares only a default slot and takes its heading from the `label` prop, so the four `<template #title>` headings in the panel had never rendered. The bodies showed with nothing above them.
The panel quoted bareSet.identity, which updates on the pick, against collisions produced for the previous column, so changing an offending column flashed the old verdict under the new column's name. Prerun now states which column it checked and the model reports both as one value, so the panel can tell a current verdict from a stale one. Also cuts the listed values from ten to three: the list buried the sentence saying what to do about them.
The record key is the identity's hash, so a value repeated on rows that are not identical merges two records into one. Prerun found these and the panel warned, but Run stayed live and the API imported the merged set without complaint. Run is now refused while the verdict for the selected column is outstanding and when it says the column repeats, and the panel says which. The platform enforces it too: invalid args leave no production to render. The args projection sees only data, and no gesture can capture this fact -- the scientist picks a column and only then does the check discover whether it is sound -- so the UI mirrors the verdict in. The rules that keep that safe are stated on BlockData.prerunChecks for the checks still to come.
Clearing it left the field reading "Value not available" in red. `identity` is a required string, so "nothing chosen" is stored as "", and a dropdown treats any non-undefined value as chosen -- "" is a chosen value that is not in the options. The field now reports nothing chosen. Also: clearing a mapping only reset it when the mapped chains were the IG pair, so a TCR mapping could never clear itself. The message shown while the checks are outstanding is now phrased for the checks in general rather than naming the id column, since more of them are coming.
Five separate entries for one PR's worth of related work read as five independent releases in the changelog. One entry, same package list.
MainPage had grown to 1037 lines, over half of it one subject: mapping a file of bare sequences onto records. That form, the checks that gate it and the numbering that follows from it now live in pages/components/BareSetForm.vue, leaving MainPage with the two doors, the dataset-door format mapping and the table (720 lines). The parent still decides whether the shape is being mapped at all -- a file is loaded and its columns are known -- and passes the header list it already computes for the other door. SCHEME_LABELS and forgetMappedColumns move to the model: both are shared with MainPage, and both are data-shape facts that sit naturally beside CHAIN_SLOT_LABELS and bareSetValid. Fixes a latent bug carried along in the move: propertyCandidates excluded only IGHeavy and IGLight from the property candidates, so a TCR mapping was offered its own sequence columns as record properties.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // the equality guard means agreeing clients do not write at all. | ||
| watchEffect(() => { | ||
| const found = model.outputs.identityCollisions; | ||
| if (found === undefined) return; |
There was a problem hiding this comment.
When a mapped sequence column changes without changing the identity column, this early return preserves the previous collision verdict while prerun recomputes it, and the run gate accepts that verdict based only on the identity name. A previously clean verdict can therefore enable an unchecked mapping and merge distinct same-identity rows into one record; a stale collision verdict can also keep a valid mapping blocked.
Knowledge Base Used:
Prompt To Fix With AI
This is a comment left during a code review.
Path: ui/src/app.ts
Line: 36
Comment:
**Sequence changes retain stale verdicts**
When a mapped sequence column changes without changing the identity column, this early return preserves the previous collision verdict while prerun recomputes it, and the run gate accepts that verdict based only on the identity name. A previously clean verdict can therefore enable an unchecked mapping and merge distinct same-identity rows into one record; a stale collision verdict can also keep a valid mapping blocked.
**Knowledge Base Used:**
- [Bare repertoire set processing](https://app.greptile.com/milaboratories/-/custom-context/knowledge-base/platforma-open/import-vdj-data/-/docs/bare-set-processing.md)
- [Workflow orchestration and preparation](https://app.greptile.com/milaboratories/-/custom-context/knowledge-base/platforma-open/import-vdj-data/-/docs/workflow-orchestration.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
A collision is not "the id column repeats" -- it is an id repeated on rows whose other mapped cells differ, so remapping a chain can turn a clean set into a colliding one and back. Keying the verdict on the id alone meant a chain remap left the previous verdict standing: a clean one enabled a mapping nothing had checked and merged distinct rows into one record, and a stale colliding one kept a valid mapping blocked until prerun caught up. Prerun now states the identity and the mapped sequence columns it checked, and one shared rule (collisionCheckKey) decides whether a verdict is about the current mapping. The panel, the run gate and the mirror all use it, so they cannot disagree about what "the same mapping" means. Exposed on the facade so the tests state the verdict with the same rule the gate applies rather than rebuilding it.
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
1 participant
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.
Greptile Summary
This PR adds prerun progress feedback and gates bare-set imports on identity-collision checks, while extracting the bare-set mapping UI into its own component.
BlockDatafor argument validation.Confidence Score: 4/5
The stale collision-verdict path should be fixed before merging because it can allow an unchecked sequence mapping to import and merge distinct records.
Collision detection depends on the mapped sequence columns, but the persisted verdict survives sequence changes and the run gate validates it using only the unchanged identity-column name.
Files Needing Attention: ui/src/app.ts, model/src/index.ts
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Select file] --> B[Profile columns] B --> C[Profile tagged with sample ID] C --> D[Map identity and sequence columns] D --> E[Prerun collision check] E --> F[Mirror verdict into BlockData] F --> G{Verdict matches current mapping?} G -->|Pending or collision| H[Disable Run] G -->|Clear| I[Project workflow arguments] I --> J[Import bare-set records]Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "MILAB-6720: split the bare-set form out ..." | Re-trigger Greptile
Context used (5)