Rework LFQ/TMT workflow integration and sync core framework with streamlit-template - #30
Merged
Merged
Conversation
This reverts commit 83646ec.
Allows input_TOPP() to designate parameters as flags (present/absent) instead of key-value pairs, persisted to params.json and session_state so run_topp() can correctly build the command line.
Split WorkflowTest configure() into separate LFQ and TMT tool tabs, and add new preprocessing/analysis pages (filtering, normalization, imputation, statistical testing, GO enrichment, clustered heatmap, pathway analysis) built on openms_insight engine functions.
Split WorkflowTest configure() into separate LFQ and TMT tool tabs, and add new preprocessing/analysis pages (filtering, normalization, imputation, statistical testing, GO enrichment, clustered heatmap, pathway analysis) built on openms_insight engine functions.
Split WorkflowTest configure() into separate LFQ and TMT tool tabs, and add new preprocessing/analysis pages (filtering, normalization, imputation, statistical testing, GO enrichment, clustered heatmap, pathway analysis) built on openms_insight engine functions.
…oint.sh, app.py, src/Workflow.py Wholesale swap-in of streamlit-template's current files for every path except the quantms-web-specific ones (content pages, k8s manifests, the container entrypoint, app.py's navigation, and the template's example Workflow.py). Files that exist only in quantms-web (e.g. src/WorkflowTest.py, src/workflow/QueueManager.py, default-parameters.json values) have no template counterpart to replace them with, so they are left untouched. requirements.txt is the one file merged rather than replaced outright: template's dependency list is the base, with quantms-web-only packages (openms-insight, easypqp, pyprophet, mygene, polars, scipy, scikit-learn, streamlit_plotly_events, statsmodels, cython) added back so the app still runs. default-parameters.json, presets.json, and settings.json were restored to their quantms-web values after the initial replace overwrote quantms-specific defaults. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…eamlit-template values Per follow-up instruction: these three config files now also take the template's values (app-name reverts to "OpenMS WebApp Template", library generation defaults are gone, presets/settings match template's set). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 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 |
JKVISION1101
force-pushed
the
sync-template-262d451
branch
from
August 20, 2026 02:27
2ce5aff to
fdc5880
Compare
The streamlit-template sync in the prior two commits replaced these along with everything else, but they should stay quantms-web-specific rather than take the template's generic versions: - .claude, .github, .streamlit (CI workflows, Claude skills, Streamlit config) - README.md (project-specific docs) - presets.json, default-parameters.json (quantms-web workflow defaults) Also pin openms-insight to the exact version (0.2.0) this app was verified against, instead of the open-ended >=0.1.13 range. Drop pr-397.patch (stray local artifact, not meant to be tracked) and add it to .gitignore. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
JKVISION1101
force-pushed
the
sync-template-262d451
branch
from
August 20, 2026 03:14
fdc5880 to
c4b4ca2
Compare
Syncs k8s/base/traefik-ingressroute.yaml with streamlit-template 76a793e ("fix treafik ingress"), which quantms-web never picked up. Without `websecure` in entryPoints, Traefik never matches the host on 443, so ACME issues no certificate and the default self-signed cert is served: opendda.webapps.openms.de:443 -> issuer=CN=TRAEFIK DEFAULT CERT template.webapps.openms.de:443 -> issuer=Let's Encrypt CN=YR2 Also sets `secure: true` on the sticky-session cookie so the affinity cookie is only sent over HTTPS. The file is now byte-identical to the template. The other k8s/ deltas (3Ti PVC, 8Gi memory-tier-high limit, quantms-ddalfq overlay identity) are deliberate and left untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Qn3mLqr7zBr7rgCokx6ku
The workflow was ~15 template commits behind (289 -> 799 lines), still on
the single-arch, full-variant-only shape. This PR already pulls in
Dockerfile.arm, Dockerfile_simple and Dockerfile_simple.arm from the
template, so those three Dockerfiles were being shipped with nothing that
builds or tests them.
Brings in:
- build-amd64 / build-arm64 split + create-manifest stitching, so
:main-full and :latest resolve to multi-arch manifest lists (the k8s
overlay ref is unchanged and keeps working)
- simple variant across all build and test jobs
- test-apptainer / publish-apptainer
- k8s test hardening in test-nginx/test-traefik: free-disk-space step,
`kind load image-archive` (drops the host docker copy, ~5-8 GB),
imagePullPolicy sed extended to `Always`, and an on-failure cluster
state dump
One deliberate deviation from the template: the template hardcodes
`ghcr.io/openms/streamlit-template:main-full` as the tag it loads into
kind. Copied verbatim that would leave the rendered quantms-web manifests
pointing at an image the cluster does not have (ErrImageNeverPull), so the
ref is derived from the prod overlay's `images:` entry instead. Same
rationale as the existing "Discover overlay identity" step; worth pushing
back upstream, since template 92f1d2c set out to keep forks green.
Verified locally: kubeconform passes on k8s/base and on the rendered prod
overlay, the derived ref (ghcr.io/openms/quantms-web:main-full) matches all
four image refs in the rendered manifests, all four Dockerfiles referenced
by the build matrices exist, and kind-config.yaml already labels a node
for the memory-tier-high nodeSelector this overlay selects.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016Qn3mLqr7zBr7rgCokx6ku
The template sync took settings.json wholesale in order to pick up the new `legal_links` block (template PR #396, "configurable legal links"), which also overwrote this app's own identity with the template's: app-name: "quantms-web (DDA-LFQ)" -> "OpenMS WebApp Template" version: "1.0" -> "1.1.1" Nothing restores it at deploy time -- k8s/base/configmap.yaml only overrides `online_deployment` and `demo_workspaces` -- so the deployed app would have shown the template's name in the browser tab (common.py:367), the footer version box (common.py:793), and the generated methods/citation text (StreamlitUI.py:1735). Restores the app name and bumps quantms-web to 1.1. settings.json now differs from the template in exactly these two lines; `legal_links` and everything else from the sync are kept as-is. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Qn3mLqr7zBr7rgCokx6ku
The CI sync in 7ac86ae enabled the arm64 + simple build path for the first time, and it fails: error: [Errno 2] No such file or directory: 'gcc' ERROR: Failed building wheel for pyprophet requirements.txt:152 pins pyprophet>=2.2.0, a quantms-web dependency that is not in the template. PyPI ships x86_64 wheels only -- there is no aarch64 wheel -- so on arm64 pip builds it from sdist, and pyprophet has a C extension (pyprophet.scoring._optimized). Dockerfile_simple.arm installed no compiler. Checked all 11 quantms-web-only requirements: pyprophet is the only one without an aarch64 wheel or a pure-python build, so this is a single narrow gap rather than the first of many. The full images are unaffected -- Dockerfile and Dockerfile.arm already install gcc/g++ -- and build-amd64 (simple) passes because the x86_64 wheel needs no compiler. Verified by reproducing both directions in a container with --no-binary pyprophet: without a compiler the CI error appears verbatim; with build-essential the extension compiles and imports. This also unblocks test-nginx and test-traefik, which declare `needs: [build-amd64, build-arm64]` and were being skipped entirely because one matrix leg failed -- so the k8s CI hardening from 7ac86ae has not actually been exercised yet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Qn3mLqr7zBr7rgCokx6ku
`build-arm64 (full)` failed in `make -j4 pyopenms`:
AttributeError: module 'Cython.Compiler.Nodes' has no attribute
'CConstOrVolatileTypeNode'
Line 65 of both Dockerfiles installs `cython` unpinned alongside a pinned
`autowrap<=0.24`. Cython 3.3.0 (released 2026-08-22) removed the AST node
classes autowrap 0.24 relies on, so pyOpenMS code generation dies.
Verified the boundary empirically: CConstOrVolatileTypeNode is present in
cython 3.0.12, 3.1.4 and 3.2.9, and gone in 3.3.0. With the pin, pip resolves
3.2.9 and autowrap's PXDParser/CodeGenerator import cleanly; without it,
3.3.0 reproduces the CI ImportError exactly.
Not caused by the CI sync in 7ac86ae -- that only exposed it. The arm64
registry cache was cold (`cache:full-arm64: not found`, first ever arm64
build), so pip resolved cython fresh. The amd64 build has been passing purely
because its warm cache reuses layer 65 with an older cython, so it would have
broken silently at the next cache eviction. Pinning both Dockerfiles defuses
that rather than just unblocking arm64.
Note this diverges from streamlit-template, whose Dockerfiles are otherwise
byte-identical and carry the same latent defect. Keep the pin across future
template syncs until the autowrap pin moves past 0.24.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016Qn3mLqr7zBr7rgCokx6ku
CI installs only pylint, so `import streamlit` is unresolvable there (E0401 is disabled). pylint therefore cannot see that `st.stop()` is annotated `-> NoReturn`, and reads branches ending in it as falling through -- hence the E0606 reports. Locally, with streamlit installed, four of the six do not reproduce at all. - enrichment.py, statistical.py: invert to a guard clause so the `st.stop()` path exits first and the assignments become unconditional. Behaviour is unchanged; `group_count` is a `len()`, so `< 2` is the exact complement of the `== 2` / `>= 3` pair. - filtering.py, imputation.py: turn the final `elif` into `else`, making the chain exhaustive. These two were genuine, not artefacts -- the value comes from a fixed `st.selectbox`, but nothing enforced that, and an unmatched value would have raised NameError at `.collect()`. - docs/toppframework.py: removed. It imports `src.Workflow`, which this repo renamed to `src/WorkflowTest.py`, and its only consumer `content/documentation.py` was deleted in f43b0ad / e40fe71 as a vestigial template page. b022de5 had already deleted this file; the template sync resurrected it as an orphan. Verified against a venv holding only pylint==4.0.7 / astroid==4.0.4, to match CI exactly: 6 errors -> 0, exit 2 -> 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Qn3mLqr7zBr7rgCokx6ku
t0mdavid-m
added a commit
that referenced
this pull request
Aug 31, 2026
The template sync in #30 restored the template's own `test_gui.py` and two tests aimed at example pages quantms-web deleted long ago, so the `continuous-integration` job has been red on main since 2026-08-29: 34 failed, 89 passed, 4 errors 28 of those failures and all 4 errors were FileNotFoundError for pages that do not exist here - content/documentation.py, content/simple_workflow.py, content/run_subprocess.py, content/topp_workflow_*.py, content/file_upload.py, content/raw_data_viewer.py, content/run_example_workflow.py and content/download_section.py. Rewrites test_gui.py to smoke-test the pages app.py actually registers, the same approach f43b0ad took the last time a sync reintroduced these files, and deletes tests/test_run_subprocess.py and tests/test_simple_workflow.py. Only workflow_fileupload, workflow_configure and workflow_run load standalone; the other 14 call st.page_link and need the navigation context that exists only when app.py runs (loading them directly raises KeyError: 'url_pathname'), so they stay covered indirectly by test_app_loads. Adds test_registered_pages_exist, which parses app.py and asserts every registered page exists on disk - a direct guard against the next sync reintroducing this class of breakage. pytest test_gui.py tests/ before: 34 failed, 89 passed, 4 errors after: 100 passed Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Qn3mLqr7zBr7rgCokx6ku
t0mdavid-m
added a commit
that referenced
this pull request
Aug 31, 2026
* fix(workflow): stop losing widget input on Streamlit < 1.50
`_input_widget_impl` fed the persisted parameter back into every widget as
its initial-value argument (`value=` / `default=` / `index=`). On Streamlit
< 1.50 that argument is hashed into the widget's element id, so the id
changed on every interaction; the following interaction then arrived under
the now-stale id and was silently discarded.
The visible effect was in the mzML file selector: selecting six files kept
only three, because every second click was dropped.
Verified against the real `StreamlitUI._input_widget_impl` and
`ParameterManager` on streamlit 1.49.1, the pinned version:
before: 6 clicked -> 3 stick ['A.mzML', 'C.mzML', 'E.mzML']
after: 6 clicked -> 6 stick
Streamlit ignores the initial-value argument once a keyed widget already
has an entry in session state, so seeding on first render only is
behaviour-preserving: `apply_preset()` and `clear_parameter_session_state()`
already delete the session keys when they want params.json to take effect
again. Regression-tested for text, number (int and float), checkbox,
selectbox, slider and multiselect - first-render seeding, reload from
params.json and value type are all unchanged, on both 1.49.1 and 1.53.1.
Also sorts the `select_input_file` options: `path.iterdir()` is unordered,
so both the displayed order and (on < 1.50) the element id varied between
restarts.
Streamlit >= 1.50 stopped hashing `default` into the element id and was
already unaffected; this fix is version-independent.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016Qn3mLqr7zBr7rgCokx6ku
* fix(ci): bump pinned cmake to 3.31.12, the pin no longer resolves
`build-openms` started failing on every branch with:
cmake - cmake not installed. The package was not found with the source(s) listed.
Version was specified as '3.31.1'.
Chocolatey delisted that exact patch: its package listing feed now returns
3.31.6, 3.31.10, 3.31.11 and 3.31.12 for the 3.31 series, but not 3.31.1.
Nothing in this repo changed - the last green run was 2026-08-29.
Moves to 3.31.12, the newest patch in the same minor series, deliberately
staying off cmake 4.x: 4.0 dropped compatibility with
`cmake_minimum_required(VERSION < 3.5)`, which the OpenMS build still relies on.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016Qn3mLqr7zBr7rgCokx6ku
* fix(tests): make the streamlit mock in test_tool_instance_name actually bind
The module swapped `sys.modules['streamlit']` for a MagicMock, imported
ParameterManager, restored the real streamlit, and only then dropped the cached
`src.workflow` modules. That ordering is wrong: a module binds `st` once, at
import time, so if any earlier-collected test had already imported
`src.workflow.ParameterManager` against the real streamlit, the import here was
just a cache hit and the mock never took effect.
The result was a test that passed alone and failed in a full run - the six
`TestSaveParametersWithInstanceName` cases asserted against
`mock_streamlit.session_state` while ParameterManager was reading the real one.
Drops the cached modules *before* the import so the mock binds, and again
afterwards so later test files re-import against the real streamlit.
pytest tests/test_results_helpers.py tests/test_tool_instance_name.py
before: 6 failed, 6 passed after: 12 passed
pytest tests/test_workflow_manager_stop.py tests/test_tool_instance_name.py
before: 6 failed, 5 passed after: 11 passed
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016Qn3mLqr7zBr7rgCokx6ku
* fix(ci): retarget test_gui.py and drop tests for deleted pages
The template sync in #30 restored the template's own `test_gui.py` and two
tests aimed at example pages quantms-web deleted long ago, so the
`continuous-integration` job has been red on main since 2026-08-29:
34 failed, 89 passed, 4 errors
28 of those failures and all 4 errors were FileNotFoundError for pages that
do not exist here - content/documentation.py, content/simple_workflow.py,
content/run_subprocess.py, content/topp_workflow_*.py, content/file_upload.py,
content/raw_data_viewer.py, content/run_example_workflow.py and
content/download_section.py.
Rewrites test_gui.py to smoke-test the pages app.py actually registers, the
same approach f43b0ad took the last time a sync reintroduced these files, and
deletes tests/test_run_subprocess.py and tests/test_simple_workflow.py.
Only workflow_fileupload, workflow_configure and workflow_run load standalone;
the other 14 call st.page_link and need the navigation context that exists only
when app.py runs (loading them directly raises KeyError: 'url_pathname'), so
they stay covered indirectly by test_app_loads.
Adds test_registered_pages_exist, which parses app.py and asserts every
registered page exists on disk - a direct guard against the next sync
reintroducing this class of breakage.
pytest test_gui.py tests/ before: 34 failed, 89 passed, 4 errors
after: 100 passed
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016Qn3mLqr7zBr7rgCokx6ku
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
feat: integrate LFQ and TMT workflows (#27)merge andreintroduce LFQ/TMT support via an independent, corrected implementation:
an analysis-mode split (LFQ vs TMT) in
configure(), per-mode TOPP tooltabs, sample/TMT-channel group assignment,
reactivesupport and animpl/fragment split for
input_TOPP(conditional UI), and boolean CLIflag parameter support for TOPP tools.
src/workflow/StreamlitUI.py,ParameterManager.py,CommandExecutor.py, Dockerfiles, docs, tests,etc.) with the current
streamlit-templatebase, while explicitlypreserving everything quantms-web-specific:
content/,app.py,src/WorkflowTest.py,k8s/,.claude/,.github/,.streamlit/,README.md,presets.json, anddefault-parameters.json.openms-insight-powered differential analysis pipeline on top ofthe LFQ/TMT abundance data: Filtering → Imputation → Normalization →
Statistical Inference → Volcano Plot / PCA / Heatmap / Clustered Heatmap
→ GO Enrichment, via new pages (
content/filtering.py,imputation.py,normalization.py,statistical.py,enrichment.py,results_heatmap_clustered.py) and updates to the existing results pages(
results_abundance.py,results_heatmap.py,results_pca.py,results_volcano.py,results_proteomicslfq.py).pyopenms==3.5.0,streamlit==1.49.1,streamlit==0.2.0etc.)and add
polars-runtime-compatto avoid an AVX-related native crash(
0xC0000005in_polars_runtime.pyd) seen on some Windows CPUs duringpolars dataframe operations.