Stage B/C: OpenMS-Insight viewers for FLASHDeconv & FLASHTnT#91
Open
t0mdavid-m wants to merge 10 commits into
Open
Stage B/C: OpenMS-Insight viewers for FLASHDeconv & FLASHTnT#91t0mdavid-m wants to merge 10 commits into
t0mdavid-m wants to merge 10 commits into
Conversation
Stage B (FLASHDeconv) + Stage D (FLASHQuant) of the migration off the bespoke flash_viewer_grid onto the reusable OpenMS-Insight library. Parse layer (src/parse/deconv.py): add long-format producers (deconv_spectrum_long, anno_spectrum_long, combined_spectrum_long with is_signal, mass_table_long with index+mass_id) alongside the existing array-format frames (additive, legacy path untouched), so components can filter by column value instead of row index. FLASHDeconv: new FLASHDeconvViewerOI builds the scan/mass tables, deconv/ annotated/combined LinePlots, Scatter3D precursor-signal plot, 4 heatmaps, DensityPlot and SequenceView, wired through one StateManager per experiment panel (distinct session_key => side-by-side selection isolation) with full [experiment][row][col] layout parity. Routed behind settings flag use_openms_insight_viewer (default true); legacy render_grid retained. FLASHQuant: render the feature-group view via FeatureView (standalone), no data transformation required. requirements: add openms-insight>=0.1.11.
Stage C of the migration off flash_viewer_grid. New FLASHTnTViewerOI mirrors the FLASHDeconv viewer: protein/tag Tables, SequenceView (coverage + fixed mods), the combined-spectrum LinePlot with tagger overlay (2nd series + signal-peak markers + sequence-tag highlight), TnT-mode DensityPlot, and heatmaps — wired through one StateManager per experiment panel (distinct session_key => side-by-side isolation) with full [experiment][row][col] layout parity. Routed behind the same use_openms_insight_viewer flag; legacy render_grid retained. Proteoform->scan resolution: add build_proteoform_scan_frame (additive, legacy build_proteoform_scan_map untouched) so proteoform selections resolve to deconv scans and components value-filter by proteoform_index, reproducing the old PyArrow pushdown. Tag selection resolves tag m/z values into tagMasses for the spectrum overlay. InternalFragmentMap remains disabled (as in the legacy path); enabling it needs the per-proteoform sequence_data store extended with internal- fragment arrays.
deconv.py (_explode_long_by_position): the long-format explode kept the full per-scan value lists on every exploded row and then gathered the scalar — O(rows x max_len) memory, which OOMs on real spectra (an 865k-row annotated frame with multi-thousand-length lists reached tens of GB). Pad each value list to the per-scan max length with nulls and zip-explode the id column and all value lists together, staying O(output rows) with identical output (parity tests unchanged). FLASHTnTViewerOI (_build_sequence_frame): sequence_data is a pickle-backed dict keyed by proteoform_index, not a tabular frame. Loading it as a LazyFrame raised AttributeError and left SequenceView blank for every TnT dataset. Load the raw dict and iterate it, slicing the displayed proteoform substring and its coverage together so they stay aligned.
The new FLASHApp viewers render via OpenMS-Insight, whose components are not yet in the published PyPI release. Add an oi-build node stage (x86 + arm) that clones the OpenMS-Insight branch, builds its Vue bundle, and stages it at openms_insight/js-component/dist; then install the package from source in the run-app stage so the bundle is baked into site-packages where the runtime bridge loads it. openms-insight is stripped from requirements.txt so the PyPI release is not pulled instead. Repo/branch are overridable via the OPENMS_INSIGHT_REPO/OPENMS_INSIGHT_BRANCH build args (default: the migration branch). CI builds the image unchanged via the build-arg defaults.
Contributor
|
Warning Review limit reached
More reviews will be available in 7 minutes and 36 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis PR integrates OpenMS-Insight into FLASHApp: adds Docker oi-build stages, produces long-format per-peak/mass frames, adds viewer routing flags, implements OpenMS-Insight viewers for FLASHDeconv and FLASHTnT (and switches FLASHQuant to FeatureView), and adds tests and config updates. ChangesOpenMS-Insight Viewer Integration
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
content/FLASHTnT/FLASHTnTViewerOI.py (1)
195-230: ⚡ Quick winAvoid collecting the full tag table just to resolve one selected tag.
_resolve_tag_masses()only needs the currently selectedTagIndex, but_tag_mass_lookup()rebuilds a full lookup dict on every rerun. On largetag_dfs, every panel interaction pays that full collect again.♻️ Proposed refactor
-def _tag_mass_lookup(file_manager, experiment_id: str) -> dict: - """Map ``TagIndex`` -> list[float] of the tag's masses (parsed from the - comma-joined ``mzs`` string with its trailing comma). Used to resolve a tag - selection into the mass list the combined-spectrum tagger overlay matches.""" - tags = _lazy(file_manager, experiment_id, "tag_dfs") - if tags is None: - return {} - df = ( - tags.select(["TagIndex", "mzs"]) - .with_columns( - pl.col("mzs") - .str.strip_chars(",") - .str.split(",") - .list.eval(pl.element().cast(pl.Float64, strict=False)) - .alias("tag_masses") - ) - .collect() - ) - return { - int(ti): [m for m in masses if m is not None] - for ti, masses in zip(df["TagIndex"], df["tag_masses"].to_list()) - } - - def _resolve_tag_masses(file_manager, experiment_id: str, state_manager) -> None: """Resolve the selected ``tagData`` (a ``TagIndex``) to its list of masses and publish under ``tagMasses`` so the combined-spectrum LinePlot tagger overlay can read it. Clears ``tagMasses`` when no tag is selected.""" tag_index = state_manager.get_selection(TAG_KEY) if tag_index is None: state_manager.clear_selection(TAG_MASSES_KEY) return - lookup = _tag_mass_lookup(file_manager, experiment_id) - masses = lookup.get(int(tag_index)) + + tags = _lazy(file_manager, experiment_id, "tag_dfs") + if tags is None: + state_manager.clear_selection(TAG_MASSES_KEY) + return + + selected = ( + tags.filter(pl.col("TagIndex") == int(tag_index)) + .select( + pl.col("mzs") + .str.strip_chars(",") + .str.split(",") + .list.eval(pl.element().cast(pl.Float64, strict=False)) + .alias("tag_masses") + ) + .collect() + ) + masses = selected["tag_masses"][0] if selected.height else None + if masses: state_manager.set_selection(TAG_MASSES_KEY, list(masses)) else: state_manager.clear_selection(TAG_MASSES_KEY)
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@content/FLASHDeconv/FLASHDeconvViewer.py`:
- Around line 14-34: The page introduces viewer-selection helpers
(_use_oi_viewer and render_panel) before calling page_setup(), violating the
content/**/*.py contract; to fix, ensure page_setup() is invoked at the very top
of this module before any session-state dependent helpers or move the helper
functions (_use_oi_viewer and render_panel) into a non-page module so they no
longer execute prior to page initialization; update the file so page_setup()
runs first (above _use_oi_viewer) or relocate the functions accordingly and
adjust imports where render_panel or _use_oi_viewer are used.
In `@content/FLASHDeconv/FLASHDeconvViewerOI.py`:
- Around line 343-345: The code currently logs a warning whenever a builder
returns None for a component (the block checking "if component is None:
st.warning(f\"No data for '{comp_name}'.\")"), which produces noisy warnings for
optional-but-absent frames; change this to silently skip by removing the
st.warning call and just continue when component is None, and instead add a
single early validation where the layout is parsed (using the layout/component
registry you already use) to warn only if the layout references an
unknown/unsupported comp_name (not when a builder returns None for an optional
frame). Ensure you update FLASHDeconvViewerOI.py to reference the same comp_name
and component-building registry so the silent-skip behavior is applied in the
current loop and the explicit warning happens only during layout validation.
In `@content/FLASHTnT/FLASHTnTViewer.py`:
- Around line 9-11: The module currently imports render_experiment_panel at
import time which can break the whole page; modify FLASHTnTViewer.py to
lazy-load it inside the _use_oi_viewer() branch: remove the top-level import of
render_experiment_panel and, within the _use_oi_viewer() conditional, attempt to
import render_experiment_panel from content.FLASHTnT.FLASHTnTViewerOI inside a
try/except, falling back to calling render_grid() if the import (or subsequent
use) raises an exception; ensure you reference render_experiment_panel,
_use_oi_viewer, and render_grid so callers remain unchanged and log or handle
the exception appropriately in the except block.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c13bd654-0f00-4559-be76-ec387cf19082
📒 Files selected for processing (12)
DockerfileDockerfile.armcontent/FLASHDeconv/FLASHDeconvViewer.pycontent/FLASHDeconv/FLASHDeconvViewerOI.pycontent/FLASHQuant/FLASHQuantViewer.pycontent/FLASHTnT/FLASHTnTViewer.pycontent/FLASHTnT/FLASHTnTViewerOI.pyrequirements.txtsettings.jsonsrc/parse/deconv.pysrc/render/scan_resolution.pytests/test_deconv_long_format.py
From the CodeRabbit review on #91: - Lazy-import the OpenMS-Insight viewer inside render_panel (FLASHDeconv and FLASHTnT) with a try/except fallback to the legacy grid, so an import-time failure of the OI stack no longer breaks the whole page before the use_openms_insight_viewer fallback can engage. - FLASHDeconvViewerOI: skip absent optional components silently instead of emitting a warning on every rerun (a builder returns None for legitimately absent optional frames). - FLASHTnTViewerOI: resolve the selected tag's masses by filtering tag_dfs on the chosen TagIndex instead of collecting the whole table into a lookup dict on every rerun (preserves trailing-comma parsing and null-dropping). Not applied: the suggestion to move page_setup() above the new helper definitions - those are plain function defs that do not execute at import, and page_setup()'s call site is unchanged from the file's existing structure (which already defines DEFAULT_LAYOUT and other helpers before page_setup()).
Address feature-parity gaps in the OpenMS-Insight viewers vs the legacy grid:
- Tables (protein/tag in TnT, scan/mass in Deconv): pass curated
column_definitions mirroring the legacy Tabulator*.vue configs (human-
readable titles, only the relevant fields, 2-decimal formatting), instead
of dumping every raw frame column with auto-generated names.
- Sequence View "No data": _build_sequence_frame expected the pickled-dict
sequence_data format (stale example cache), but the live parseTnT writes
it as parquet (build_table -> parquet_sink). Handle BOTH: parquet frame
(LazyFrame/DataFrame) and legacy dict, building the same coverage/fixed-mod
frame either way.
- Augmented Deconvolved Spectrum: _resolve_tag_masses now publishes the tag
walk {masses, residues} (residues = reversed TagSequence aligned to the
stored-order mass gaps, matching the legacy reversed-index rule) so the OI
LinePlot renders the residue walk + auto-zoom, not just peak highlights.
Clicking a covered amino acid in the Sequence View now filters the Tag
Table to the tags covering that residue, matching the legacy cross-link.
- _build_sequence_frame emits a sequence_offset column so the SequenceView
can report protein-absolute residue positions even when the displayed
sequence is sliced to the proteoform substring (offset is 0 / unchanged
for the bundled example proteoforms).
- SequenceView interactivity gains {selectedAApos: "residue_position"} (keeps
the peak_id mapping); the Tag Table gains
range_filters={selectedAApos: ("StartPos", "EndPos")} alongside the
existing proteoform filter, so it shows tags with StartPos <= pos <= EndPos.
…sets Parse (src/parse/deconv.py): preserve per-deconv-peak signal arrays (signal_mzs/charges/intensities on combined_spectrum_long + deconv_spectrum_long) for the charge-state drill-down, and add Scan/PrecursorScan/PrecursorMass/ MonoMass to threedim_SN_plot for the 3D precursor-signal lookup. FLASHTnT viewer: protein/tag tables use dashNegativeOne (-1 -> "-"); protein table reproduces the default-ON "Best per spectrum" collapse (max Score per Scan) with a toggle; SequenceView now renders the FULL protein with truncation (proteoform_start/end + -2 sentinels), ambiguous mod_ranges, Precursor/Proteoform mass header, and tag-span highlight; _resolve_tag_masses publishes nTerminal + the tag span; combined spectrum gets the signal drill-down columns; dependent selections (tag/tagMasses/AApos/tagSpan/mass) clear on proteoform change. FLASHDeconv viewer: scan/mass tables use the plain 4-decimal "fixed" formatter (no thousands grouping); Scatter3D wired for the precursor-signal lookup; deconv spectrum gets the drill-down columns; submitted-sequence SequenceView bakes the selected fixed C/M mods into the sequence (correct fragment masses), enables the variable-mod menu, consumes the custom sequence (sequence_out), and emits the mass header; massIndex resets on scan change.
…ecursor Re-audit follow-ups: - Tables: drop precision from the dashNegativeOne columns (ProteoformMass, ProteoformLevelQvalue, Nmass, Cmass) so they show the RAW value like legacy (precision-4 was destroying tiny Q-values); keep the -1 -> "-" sentinel. - TnT tag walk: publish selectedAA (= selectedAApos - StartPos when the selected residue is within the tag) so the selected-residue gold highlight renders; drop zero-valued tag masses (legacy filters != 0); silently skip absent components instead of st.warning (match Deconv). - Deconv Scatter3D: pass the precursor-lookup columns only when all four are present in the frame, so stale 4-column threedim_SN_plot caches no longer crash the panel (ValueError) and degrade to the legacy per-scan view. - Deconv SequenceView: stop emitting computed_mass (it forced the TnT branch, mislabeled the header "Proteoform", and disabled the variable-mod menu); emit the observed precursor_mass from the selected scan so the "Precursor" header renders and variable mods stay enabled. - Remove dead code (_data_path, unused imports) and drop the inert internal_fragment_map from the Deconv selectable layout (parity with TnT).
The LinePlot tag walk receives residues already REVERSED to align with the stored mass order, so the tag-relative selectedAA (an N->C index) always maps to the mirrored walk gap. Publish nTerminal=False on the tagMasses walk so the LinePlot mirrors it -- geometrically correct for both N- and C-anchored tags and matching the legacy behavior (whose nTerminal was effectively always false). Driving it off Nmass==-1 misplaced the gold highlight for C-anchored tags. The SequenceView tag-span still carries the real nTerminal (Nmass==-1) for its own orientation.
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.
Summary
Adds new OpenMS-Insight-based viewers for FLASHDeconv (Stage B) and FLASHTnT (Stage C) workflows, replacing the legacy bespoke
flash_viewer_gridVue component. Both viewers use reusable OpenMS-Insight components (Table, LinePlot, Heatmap, SequenceView, DensityPlot) with shared StateManager per experiment panel to enable cross-component interactivity while preventing state leakage between side-by-side panels.Key Changes
New Viewers
content/FLASHDeconv/FLASHDeconvViewerOI.py(Stage B): Renders FLASHDeconv results using OpenMS-Insight components. Wires scan/mass selection across Scan Table → Mass Table → Deconvolved/Combined Spectrum → SequenceView → 3D S/N Plot via sharedStateManagerkeyed bysvc_state_deconv_<experiment_id>.content/FLASHTnT/FLASHTnTViewerOI.py(Stage C): Renders FLASHTnT (top-down identification) results. Adds proteoform→scan resolution viabuild_proteoform_scan_frame()so OpenMS-Insight value-filters work correctly. Implements tagger overlay: Tag Table selection resolves to mass list, LinePlot highlights matching peaks within ±1e-5 Da tolerance.Long-Format Frame Producers
src/parse/deconv.py: Added_explode_long_by_position()helper and producers:deconv_spectrum_long()— one row per deconvolved peakanno_spectrum_long()— one row per annotated/raw peakcombined_spectrum_long()— deconv peaks with signal-membership flagmass_table_long()— one row per mass with charge/score columnsThese replace array-per-scan frames for OpenMS-Insight components, which filter by column VALUE (e.g.,
filters={'scanIndex': 'index'}) rather than row INDEX. Handles ragged scans (spectrum longer than per-mass arrays) by padding with nulls, matching legacy Vue expansion semantics.Routing & Settings
content/FLASHDeconv/FLASHDeconvViewer.py&content/FLASHTnT/FLASHTnTViewer.py: Addedrender_panel()dispatcher that routes to OpenMS-Insight viewer whensettings.use_openms_insight_vieweris True (default), else legacy grid.settings.json: Added"use_openms_insight_viewer": trueflag to enable new viewers by default.Scan Resolution (TnT-specific)
src/render/scan_resolution.py: Addedbuild_proteoform_scan_frame()(additive, reproduces legacybuild_proteoform_scan_mapas a Polars frame). Surfacesproteoform_index → (scan, deconv_index)as columns so spectrum/sequence frames can be joined and value-filtered by proteoform selection.Testing
tests/test_deconv_long_format.py: Unit tests for long-format producers validating row-count fidelity, index-filter parity, ragged-scan padding, and id-column alignment without requiring Streamlit or pyopenms.Docker & Dependencies
Dockerfile&Dockerfile.arm: Updated to build OpenMS-Insight Vue bundle from source (migration branch) in a separateoi-buildstage, since the pinned PyPI release does not yet include new FLASHApp components.requirements.txt: Addedopenms-insight>=0.1.11dependency.Implementation Details
https://claude.ai/code/session_01VM8hWcakFabPeAAx1UjGUE
Summary by CodeRabbit
New Features
Tests