feat(preview): FCS multi-panel gating grid with biological marker labels#5109
feat(preview): FCS multi-panel gating grid with biological marker labels#5109Austin-s-h wants to merge 2 commits into
Conversation
Replace the single-scatter FCS preview with a canonical flow-cytometry gating grid: Cells (FSC-A×SSC-A), Singlets (FSC-H×FSC-A), then the fluorescence channel pairs present in the file, capped at 6 panels. Axis titles prefer the $PnS biological marker (e.g. "CD3 (FL1-A)"), falling back to the detector name and skipping redundant marker==channel labels. A single-channel file keeps the original responsive brush-select panel; multi-panel grids get explicit cell sizing (Vega-Lite ignores `container` width inside a concat). Built on master's existing fcsparser-based extract_fcs (no dependency swap): the grid is derived from the parsed pandas DataFrame's columns, and markers are read from fcsparser's reformat_meta `_channels_` DataFrame ($PnN/$PnS columns), with fallbacks to flat $P<idx>N / lower-cased p<idx>n keys for other parsers. Catalog side renders the emitted vegaLite spec via the existing Vega renderer and adds responsive layout CSS (overflow/word-wrap) to the FCS preview. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5109 +/- ##
==========================================
+ Coverage 49.46% 49.62% +0.16%
==========================================
Files 843 843
Lines 34411 34518 +107
Branches 5826 5829 +3
==========================================
+ Hits 17020 17130 +110
+ Misses 15502 15499 -3
Partials 1889 1889
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| FCS_SCATTER_LIMIT = 50_000 | ||
| FCS_SCATTER_RANDOM_SEED = 0 |
There was a problem hiding this comment.
FCS_SCATTER_LIMIT = 50_000 is applied per panel, not across all panels combined. With six panels active (FCS_MAX_PANELS = 6), the data.values arrays embedded inline in the Vega-Lite spec sum to up to 300,000 {x, y} dicts. At roughly 30 bytes of JSON per point that is ~9 MB of payload, well above the LAMBDA_MAX_OUT = 6_000_000 constant defined in the preview Lambda handler. make_json_response calls json.dumps directly without any size guard on info['vegaLite'], so a request for a large clinical FCS file (≥50k events, with FSC/SSC and two or more FL pairs present) would produce a response the Lambda infrastructure silently truncates or rejects.
A simple mitigation is to budget a total event count across all panels — e.g., per_panel_limit = max(1, total_limit // len(panels)) — or reduce FCS_SCATTER_LIMIT to a value that keeps the total payload inside 2–3 MB (≈ 5 000–10 000 events per panel is also sufficient to see gating structure). Flow cytometry tools commonly subsample to 5 000–20 000 events for scatter displays.
There was a problem hiding this comment.
Fixed in b03b6e5. Replaced the per-panel FCS_SCATTER_LIMIT with FCS_SCATTER_TOTAL_LIMIT = 60_000 budgeted across all selected panels (per_panel_limit = max(1, total // len(panels))), so a full 6-panel grid stays ~2-3 MB — well under the 6 MB LAMBDA_MAX_OUT. Added a test asserting a large synthetic FCS spec serializes under the cap.
| markers = {} | ||
|
|
||
| channels = meta.get('_channels_') | ||
| if ( | ||
| channels is not None | ||
| and hasattr(channels, 'columns') | ||
| and '$PnN' in channels.columns | ||
| and '$PnS' in channels.columns | ||
| ): | ||
| for name, marker in zip(channels['$PnN'], channels['$PnS']): | ||
| if name is None: | ||
| continue | ||
| marker = _keep_marker(marker) | ||
| if marker: | ||
| markers[str(name)] = marker | ||
| if markers: | ||
| return markers | ||
|
|
||
| idx = 1 | ||
| while True: | ||
| name = meta.get(f'$P{idx}N') or meta.get(f'p{idx}n') |
There was a problem hiding this comment.
_fcs_channel_markers flat-key loop silently stops at the first index gap
The while True loop increments idx and breaks when meta.get(f'$P{idx}N') or meta.get(f'p{idx}n') evaluates to None. If a non-standard FCS file omits a channel parameter in the middle (e.g., has $P1N, $P2N, $P4N but not $P3N), the loop breaks at index 3 and $P4N's marker is silently dropped. The result is that axis labels for those channels fall back to raw channel names even when a $PnS marker was present in the file. This edge case is unlikely in well-formed FCS files but worth noting for robustness.
There was a problem hiding this comment.
Fixed in b03b6e5. The flat-key fallback now iterates a bounded range ($PAR count when present, else 512) and skips missing indices instead of breaking on the first gap. Added a test covering a $P3 gap with $P4S still picked up. The primary _channels_ path is unchanged.
| def _build_fcs_panel(data, x_axis, y_axis, label, channel_markers, *, limit): | ||
| import numpy | ||
| import pandas |
There was a problem hiding this comment.
import pandas inside _build_fcs_panel
pandas is already imported at module level (line 12). The inline import pandas here is a no-op after the first call but is mildly confusing. numpy is legitimately absent from the module-level imports, so that inline import is fine to keep; the pandas one can be dropped.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Fixed in b03b6e5 — dropped the redundant inline import pandas in _build_fcs_panel; kept the inline import numpy as you noted.
…arse
Address Greptile review on the FCS gating-grid PR:
- P1: replace per-panel FCS_SCATTER_LIMIT with FCS_SCATTER_TOTAL_LIMIT
(60k) budgeted ACROSS all selected panels. With up to 6 panels the old
50k-per-panel cap could emit ~300k inline {x,y} dicts (~9 MB), over the
6 MB LAMBDA_MAX_OUT response cap. Now _build_fcs_scatter_spec derives a
per-panel cap (total // len(panels)) so the whole payload stays ~2-3 MB.
- P2: make the flat-key marker fallback in _fcs_channel_markers gap-tolerant
-- iterate a bounded range ($PAR count, else 512) and skip missing indices
instead of breaking on the first gap. Primary _channels_ path unchanged.
- P2 nit: drop the redundant inline `import pandas` in _build_fcs_panel
(pandas is module-level; numpy stays inline since it is not).
Tests: rename to FCS_SCATTER_TOTAL_LIMIT, add a gap-tolerance test and a
test asserting a large synthetic FCS spec stays under the 6 MB cap.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
What
Replaces the single-scatter FCS preview with a canonical flow-cytometry gating grid:
CD3 (FL1-A)), fall back to the detector name, and skip redundantmarker == channellabels.width: containerinside aconcat).vegaLitespec via the existingVegarenderer, plus responsive layout CSS (overflow / word-wrap) on the FCS preview.Why / approach
This is built on top of
master's existingfcsparser-basedextract_fcs— no parser/dependency swap. The gating grid is derived entirely from the parsed pandasDataFrame's columns, and thevegaLitespec is emitted fromextract_fcsalongside the existing HTML/metadata.The one subtlety:
mastercallsfcsparser.parse(..., reformat_meta=True), which collapses the per-channel$PnN/$PnSkeywords into a_channels_DataFrame and drops the flat$P<idx>Nkeys. So_fcs_channel_markersreads the marker map from that_channels_DataFrame first, with fallbacks to flat$P<idx>N(raw fcsparser) and lower-casedp<idx>n(other parsers) — the marker labels work whichever metadata shape is handed to it.No new deps and no new test fixtures: the existing
lambdas/shared/tests/data/fcs/normal.fcs(14 channels, 191 events) already exercises the full multi-panel grid.Tests
lambdas/shared/tests/test_preview.py— augmentedtest_fcsto assert the gating grid onnormal.fcs, plus new unit tests for panel selection, marker parsing (flat +_channels_), axis-label dedup, downsampling/seeding, single-vs-multi panel structure.uv run pytest tests/test_preview.py -k fcs→ 14 passed. Full file: 20 passed, 1 unrelated failure (test_excelhits the external W3C validator; SSL/network only).loaders/Fcs.spec.tsxverifiesvegaLiteflows from preview info intoPreviewData.Fcs.ruff checkclean on both Python files.🤖 Generated with Claude Code
Greptile Summary
This PR replaces the single-scatter FCS preview with a canonical flow-cytometry gating grid using Vega-Lite, built entirely on top of the existing
fcsparser-basedextract_fcsinfrastructure. The new grid selects up to six biologically meaningful channel pairs (Cells, Singlets, fluorescence), labels axes with$PnSbiological marker names, and falls back to a responsive single-panel view for non-standard files.preview.py): Adds_fcs_channel_markers,_build_fcs_panel,_build_fcs_scatter_spec, and_select_fcs_panelsto build a complete Vega-Lite spec (with inline event data) stored ininfo['vegaLite']; includesFCS_SCATTER_LIMIT = 50_000events per panel and a cap of six panels.Fcs.js,Fcs.jsx,types.js): Threads the newvegaLitefield through the loader and renders it via the existingVega(vega-embed) renderer, with updated CSS for responsive layout.Confidence Score: 3/5
Safe to merge for small FCS files, but large clinical samples with canonical channels could trigger a Lambda response failure due to the unbounded inline data payload.
The frontend wiring and test coverage are solid. The main risk is in preview.py: with 50,000 events per panel and up to 6 panels, the Vega-Lite spec embeds up to 300,000 x,y dicts directly in the JSON response. make_json_response calls json.dumps with no size guard on the vegaLite key, while the Lambda handler defines LAMBDA_MAX_OUT = 6,000,000 bytes as its documented ceiling. A clinical FCS file with 50k+ events and a full set of FSC/SSC/FL channels will produce a response that exceeds that limit, causing the preview to fail for exactly the files where the gating grid would be most useful.
lambdas/shared/src/t4_lambda_shared/preview.py — specifically the FCS_SCATTER_LIMIT constant and the absence of a cross-panel total-event budget.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[FCS file via S3] --> B[extract_fcs] B --> C[fcsparser.parse reformat_meta=True] C --> D{data is not None?} D -- Yes --> E[_fcs_channel_markers meta] E --> F[_build_fcs_scatter_spec data] F --> G[_select_fcs_panels columns] G --> H{Canonical panels match?} H -- Yes --> I[Up to 6 canonical pairs\nCells / Singlets / FL pairs] H -- No --> J[Fallback: first 2 columns] I --> K[_build_fcs_panel per pair\nfilter NaN/inf, downsample ≤50k] J --> K K --> L{1 sub-spec?} L -- Single --> M[Single responsive panel\nwidth=container, brush params] L -- Multi --> N[Gating grid concat\nexplicit 300px cells] M --> O[info.vegaLite] N --> O O --> P[json.dumps via make_json_response\nno size guard applied to vegaLite] P --> Q[Catalog FCS Loader\ninfo.vegaLite] Q --> R[Vega renderer\nvega-embed] D -- No --> S[metadata only]%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% flowchart TD A[FCS file via S3] --> B[extract_fcs] B --> C[fcsparser.parse reformat_meta=True] C --> D{data is not None?} D -- Yes --> E[_fcs_channel_markers meta] E --> F[_build_fcs_scatter_spec data] F --> G[_select_fcs_panels columns] G --> H{Canonical panels match?} H -- Yes --> I[Up to 6 canonical pairs\nCells / Singlets / FL pairs] H -- No --> J[Fallback: first 2 columns] I --> K[_build_fcs_panel per pair\nfilter NaN/inf, downsample ≤50k] J --> K K --> L{1 sub-spec?} L -- Single --> M[Single responsive panel\nwidth=container, brush params] L -- Multi --> N[Gating grid concat\nexplicit 300px cells] M --> O[info.vegaLite] N --> O O --> P[json.dumps via make_json_response\nno size guard applied to vegaLite] P --> Q[Catalog FCS Loader\ninfo.vegaLite] Q --> R[Vega renderer\nvega-embed] D -- No --> S[metadata only]Reviews (1): Last reviewed commit: "feat(preview): FCS multi-panel gating gr..." | Re-trigger Greptile