Tell a fully-loaded feature apart from one the memory cap only sampled - #153
Conversation
`resident` means a feature has AT LEAST ONE point inside the memory cap. On a truncated element that is true of nearly every feature, so the panel greyed nothing, printed each feature's full dataset count beside it, and presented a sample as the whole answer. On Xenium pancreas transcripts (8,073,840 points, 4M cap) all 541 features read as resident while half the data was absent. `describeFeatureRowState` takes optional resident/dataset point counts and returns a new `partial` tone: drawn, so not greyed, but labelled and explained with both numbers and the share. A completed feature-index scan vetoes it — that supplies the feature whole, so the resident shortfall is no longer what is on screen. With counts unknown the classification is exactly what it was. The panel prints `resident / dataset` on those rows and a summary line above the list. Percentages are rounded so they can never state the opposite of the truth: a feature with three points in a million reads `<1%`, not `0%`, and one missing a handful reads `>99%`, not `100%`. This exposed a real bug underneath. `getResidentFeatureCounts` answered from the preload result's own `featureCodeCounts`, which is frozen in the resident preview's code space; `reconcileRowCodes` re-expresses the row codes when the full catalog supersedes that preview but does not touch the counts map, so it kept answering in the old space. Counts now derive from the reconciled row codes, memoised on the same identity as the resident-codes set and invalidated with it. Measured on the real dataset, against the settled 541-feature catalog: the old map gave 206 features MORE resident points than the dataset contains — impossible, and the signature of a scrambled code space. The derived map gives zero such violations, sums to exactly 4,000,000 (the cap) against a dataset total of 8,073,840, and no feature exceeds its own total. The unit test pins the mechanism on a two-gene case, and fails without the fix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 33 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1 participant
The remaining half of #149 — the part I deliberately left out of #152 because it needed more than a reporting change.
The problem
residentmeans a feature has at least one point inside the memory cap. On a truncated element that is true of nearly every feature, so the panel greyed nothing, printed each feature's full dataset count beside it, and presented a sample as the whole answer.On Xenium pancreas transcripts — 8,073,840 points at the 4M default — all 541 features are resident while half the dataset is absent.
AMY2Ashowed1,182,402next to a row drawing 113,154 of them.The change
describeFeatureRowStatetakes optionalresidentPointCount/datasetPointCountand returns a newpartialtone: drawn (so not greyed), but labelled and explained with both numbers and the share. The panel printsresident / dataseton those rows, plus a summary line above the list.Three judgements worth flagging for review:
partial. The matched batch supplies the feature whole, so the resident shortfall is no longer what's on screen. Without the veto a fully-loaded feature would still be labelled partial.<1%, not0%; one missing a handful reads>99%, not100%. Both naive roundings state the opposite of the truth, which is the whole failure mode being fixed.The bug underneath
Building this surfaced a real defect.
getResidentFeatureCountsanswered from the preload result's ownfeatureCodeCounts, which is frozen in the resident preview's code space.reconcileRowCodesre-expresses the row codes when the full catalog supersedes that preview, but never touches the counts map — so it kept answering in the old space, attributing one gene's count to another.It was largely invisible before because the panel only used those counts as a stopgap until dataset totals arrived, and then dropped them. Displaying them permanently is exactly what makes it matter.
Counts now derive from the reconciled row codes, memoised on the same identity as the resident-codes set and invalidated alongside it.
Verification
Measured in the running app against the settled 541-feature catalog, comparing the old map with the new one directly:
206 impossible rows out of 541 is the signature of a scrambled code space. The derived map has none, sums to exactly 4,000,000 against a dataset total of 8,073,840 (ratio 0.4954 = the cap), and no feature exceeds its own total.
Parsed straight from the rendered panel afterwards, all 541 rows show a
resident / datasetpair, 0 violations, sums exact.The unit test in
pointsDataEngine.spec.tspins the mechanism on a minimal two-gene dict-only case; I confirmed it fails without the fix. Six newfeatureRowStatetests cover the tone, the veto, the fallback and the rounding.core396 / 52,layers236 / 19,vis155 / 16 — all passing. Biome gate clean. Changeset: patch forcore, minor forvis.Note
partialis a new member of the exportedFeatureRowToneunion, so a consumer exhaustively switching on it will need a case. Everything else is additive and optional.