MILAB-6496: Run quality reads the distribution the run holds - #16
Merged
10 commits merged intoSep 3, 2026
Merged
Conversation
`count_bin_edges` built its edges with `np.geomspace`, which places them between whole numbers. A UMI count is a whole number, so a bin could fall strictly between two counts and stand empty at every weight a run could produce. `np.geomspace(1, 5155, 25)` puts one at [2.039, 2.911); on a 15-tag run it held nothing on all 15 panels and read as a missing bar. Edges are now whole and strictly increasing, so every bin holds at least one count. The step is `max(previous + 1, geometric)`, which draws one count per bar near a count of 1 and stays geometric above that. The last edge is one past the top count, making every bin half-open rather than closing the last one and giving it a count more than its width. A run now takes at most COUNT_BIN_COUNT bins instead of always that many. Nothing reads the count: `bin_values`, `per_tag_count_bins` and the chart all take it from the edge list.
Whole-number edges stop a bin standing empty, but they do not make bar heights comparable. Bin width in counts rises across the edge set -- one real set spans 1, 1, 2, 3, 4, 6, 9, 14, 21 -- so a bin covering 4 counts stood about four times a neighbour covering 1 at equal density. That step drew a second hump on tags whose counts hold one population, and the grid exists to answer whether two populations separated at all, so a hump the bins invented is the one error this surface cannot carry. `CountHistogram` takes a `density` flag, applied in the `log-bins` branch only, which divides each weight by the whole counts its bin spans. The y axis reads "Cells per count". The two linear callers, the score spread and the reference reading, are untouched. Atom 330 binds the x axis -- "the unit the gate is declared in follows from the axis" -- and only on the plots a scientist declares from. This grid informs nothing settable, so its y axis carries no such constraint. `PlChartHistogram` prints the number it is handed under a fixed `count:` label, so a hovered bar on this grid now reports the density. Each panel's caption carries the cell count instead.
VIEW_TABS is derived from the rung the run reports, and that rung is unreported until the run settles. A strip drawn mid-run therefore offered every plot and then dropped the ones the served rung cannot draw, so a reader could open a tab that stopped existing under them. `isRunning` is the block's own computing signal, the one already driving the block spinner. The open view's body is untouched and keeps drawing its own processing placeholder, so the section still shows progress while the strip is away.
The Status column read `readShare`, the sample's aggregate undeclared share, so it was one word repeated down every row of a sample. A sample carrying one heavy undeclared sequence among many light ones said nothing about which sequence to look at. It now reads `barcodeShare`, the row's own share of its sample's pre-refine reads, evaluated per row through the same scalar `status_for` every other status goes through rather than a polars expression rebuilding the thresholds. The frame is capped at UNDECLARED_BARCODES_KEPT rows per sample, so the loop cannot grow with the library. Warn above 0.01, alert above 0.05, carried in the three places `qcDefaults` pins together. Operator-set, not inherited: the field publishes 0.50/1.0 for a sample's AGGREGATE undeclared share and that line does not transfer to one sequence, since an aggregate reaches 0.50 while no single sequence comes near it. Admitting a per-barcode line needs an atom on 315, and so does saying what became of the aggregate one. The error comparison moved from `alerting-at` to `at-most`. It compared for equality, which fired only at exactly the error threshold and let every larger share read warn. `readShare` keeps its column, relabelled "Sample Undeclared (%)", and carries no status. Column order is unchanged: the order priorities already produced it. Every description in the table was rewritten to one instruction per sentence, active voice, and short sentences.
The undeclared-barcode status was evaluated by calling the scalar `status_for` once per row over a materialised column. That was justified on the row cap, and the justification was wrong: `keep` is `int | None` and None keeps every row, so the loop is a loop over every distinct pre-refine sequence -- 10.2M per sample and 240.7M over a run, by the figures recorded above the cap. Materialising that column also undoes the memory work this stage carries. `status_expr` is the same rule over a column. It reads the SAME `lines` dict and the SAME `_COMPARISON` table the scalar reads, and `_breaches_expr` mirrors `_breaches` branch for branch, so only the evaluator differs. Two evaluators can drift, which is what the loop was avoiding, so `test_status_expr_agrees_with_status_for` runs them against one another over every registered measurement: each line's own thresholds, 1e-9 and 0.01 either side of each, and null, NaN and both infinities. Four mutations were checked against it and each one fails there: at-most losing its strictness, alerting-at inverted, warn tested before alert, and the not-a-number guard removed.
PaulNewling
marked this pull request as ready for review
September 1, 2026 11:17
Collaborator
Author
Undeclared-barcode lines judge one sequence's own share, warning above 0.01 and alerting above 0.05. Several baseline tags combine by the highest count rather than stopping the run. Quality-line tooltips no longer claim no test asserts the default. Per-sample QC headers for reads parsed, counts removed, unique counts and sticky cells describe the one number each column holds. Baseline reading, cells called bound, seen-in and why-unsettled name the population or values they carry. Semicolons and em-dashes removed from every tooltip.
… can do The per-barcode grouping option reads "One identity per barcode". The combine-column alert no longer tells the reader to pick or clear a column the form does not offer. Uploading the panel file again is the one action that clears it, so the alert says that.
- Expose Expected binder fraction (ω), default 10% — the fit's starting split, settable per experiment - Expose Bound probability, default/floor 0.9 - Adopt the paper's initialisation (split at the ω quantile, was the median) - Bound-count marker per (sample, tag) on the distribution plots, plus boundAtCount in the fit output - Sample selector on Fitted background; barcodes in declared panel order via new tagOrder - New export antigenCellTagCounts [sampleId][cellId][tagId] → pre-floor UMI count, partitioned by sample Plots - Equal-width log1p bins (0.2) replacing integer geometric bins; drops the per-bar density division - Histograms include the zeros the fit was taken over - Bins computed inside the fit — 2.07M numbers held → 7.8k Fixes - Panel column hidden (axis + label) — it was a constant hash - bg 0 displayed instead of 0.000488 (formatter dropped its own significant digits) - Run quality page crashed on load (temporal dead zone) - Absent vs null boundAtCount reported as the same finding - Resize debounce; JSON indent dropped (~halves payload) Removals - count_bin_edges (no caller; reasoning folded into log1p_bin_edges) and the density prop
…teger-count-bin-edges
github-merge-queue
Bot
closed this pull request by merging all changes
into
main
in
5d73918
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
2 participants
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.
Five commits on the Run quality view. Four change what a scientist sees. The fifth fixes the third.
0f2fcb2Whole-number bin edgescount_bin_edgesusednp.geomspace, which places edges between whole numbers. A UMI count is a whole number, so a bin can fall between two counts and stand empty at every weight a run can produce.np.geomspace(1, 5155, 25)puts one at[2.039, 2.911). That bin held nothing on all 15 tags of a real run, and read as a missing bar.Edges are now whole and strictly increasing. The step is 1 near a count of 1, and geometric above that.
70f728bThe fitted background draws densityWhole edges stop an empty bin. They do not stop an invented hump. Bin width in counts rises across the edge set, so a bin covering 4 counts stood four times a neighbour covering 1 at equal density.
The raw series shows two humps. The density shows one mode that decays. Bars are now cells per count.
Atom
330-the-quality-readoutbinds the x axis, and only on the plots a scientist declares from. This grid informs nothing a scientist sets.Cost: the hover readout on this grid now reports the density.
PlChartHistogramprints the number it receives under a fixedcount:label. Each panel caption carries the cell count instead.b0818afRun quality hides its tab strip while the run computesThe tab set comes from the baseline rung, and the run does not report that rung until it settles. The strip therefore offered every plot mid-run, then dropped the ones the served rung cannot draw. The open view keeps drawing, and shows its own processing placeholder.
3af02c5The undeclared Status judges each barcodeStatus read the sample's aggregate share, so it was one word repeated down every row. It now reads each row's own share of its sample's pre-refine reads. It warns above 1%. It alerts above 5%.
The error comparison moved from
alerting-attoat-most. Equality fired alert only at exactly 5%, and every larger share read warn.Also here: the sample-level share is relabelled "Sample Undeclared (%)" and carries no status. Every column description in that table now uses one instruction per sentence and active voice. Column order is unchanged.
f02d7a4The per-row status is an expressionThe per-row status first called the scalar
status_foronce per row over a materialised column. The row cap does not bound that loop, becausekeepisint | NoneandNonekeeps every row.status_expris the same rule over a column, read from the samelinesdict and the same_COMPARISONtable.test_status_expr_agrees_with_status_forruns both over every registered measurement, at each line's thresholds, 1e-9 and 0.01 either side, and over null, NaN and both infinities. Four mutations each fail there.Verification
qcDefaults.test.ts19/19 pins the Python, Tengo and TypeScript line defaults together.uicheck andmodelcheck clean.build:dev-no-software10/10.Greptile Summary
The PR revises run-quality distributions, navigation readiness, and undeclared-barcode classification. It keeps quality defaults synchronized across the model, workflow, and Python implementation.
Confidence Score: 5/5
The PR appears safe to merge because no blocking failure remains within the eligible follow-up-review scope.
No blocking failure remains.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart LR Raw[Pre-refinement barcode counts] --> Tally[Per-sample undeclared tally] Tally --> BarcodeShare[Per-sequence barcode share] Tally --> SampleShare[Aggregate sample undeclared share] BarcodeShare --> StatusExpr[Vectorized status expression] Lines[1% warn / 5% alert lines] --> StatusExpr StatusExpr --> Table[Undeclared-barcode table] SampleShare --> Table Counts[Per-cell UMI counts] --> Edges[Whole-number shared bin edges] Edges --> Weights[Per-sample and tag bin weights] Weights --> Density[Divide by integer bin span] Density --> Grid[Fitted-background grid]Reviews (2): Last reviewed commit: "MILAB-6496: the per-row status is an exp..." | Re-trigger Greptile
Context used (6)