Fix FLASHDeconv Sequence View ions not updating on scan change#94
Conversation
Rebuild js-component/dist from the fixed Vue source (submodule fix/seqview-scan-reactivity @ d98f15a) so the Sequence View recomputes fragment ions when the selected scan changes. Root cause was a Vue reactivity freeze in SequenceView (selectedScanIndex computed pinned at 0 for the single-row FLASHDeconv per_scan_data, and no per_scan_data watcher), detailed in the submodule PR. Bundle rebuilt via Docker from local submodule source. Also document in CLAUDE.md that the Vue bundle must always be built via Docker (with the local-source recipe), not a local Node toolchain. Submodule pointer bumped 57c9f6f -> d98f15a (depends on the submodule PR into FVdeploy; update to the merged commit once it lands). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Z9gnbqpmuU9z6BmoaXmuY
📝 WalkthroughWalkthrough
ChangesFLASHApp documentation and submodule update
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@CLAUDE.md`:
- Line 30: The Docker build example currently shows a raw GitHub token being
pasted directly into the command, which should be replaced with an
environment-variable based example. Update the documented command in CLAUDE.md
to reference GITHUB_TOKEN via an existing shell variable (for example, in the
Docker build invocation) so the guidance points users to export the token first
and pass it through the build arg without exposing the secret.
- Line 35: Clarify the Python 3.11 note in CLAUDE.md so it is explicitly scoped
to Docker/runtime rather than implying all tooling or CI uses 3.11. Update the
wording around the Python pin and GITHUB_TOKEN requirement to reflect that CI
linting may still use a different version, and reference the surrounding
environment/setup guidance in CLAUDE.md so contributors do not assume parity
with the pylint workflow.
- Line 68: The fenced code block opened in CLAUDE.md is missing a language
identifier, which triggers markdownlint MD040. Update the opening fence to
include a language hint such as text, and keep the rest of the fenced block
unchanged so the documentation stays lint-clean.
🪄 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: e7348652-82b2-4116-acde-ad2700ceb848
⛔ Files ignored due to path filters (3)
js-component/dist/assets/index-801f5ae0.cssis excluded by!**/dist/**js-component/dist/assets/index-ac66dd94.jsis excluded by!**/dist/**js-component/dist/index.htmlis excluded by!**/dist/**
📒 Files selected for processing (2)
CLAUDE.mdopenms-streamlit-vue-component
| st.number_input("X", value=params["my-param"], key="my-param") | ||
| save_params(params) | ||
| # Docker (full image with OpenMS + TOPP tools + Vue build) | ||
| docker build -f Dockerfile --no-cache -t flashapp:latest --build-arg GITHUB_TOKEN=<gh-token> . |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Avoid documenting raw token pasting in shell commands.
Line 30 currently suggests placing a token directly on the command line, which risks credential leakage via shell history/process lists. Prefer env-var based usage in the example (for example, --build-arg GITHUB_TOKEN="$GITHUB_TOKEN" after exporting it).
🤖 Prompt for 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.
In `@CLAUDE.md` at line 30, The Docker build example currently shows a raw GitHub
token being pasted directly into the command, which should be replaced with an
environment-variable based example. Update the documented command in CLAUDE.md
to reference GITHUB_TOKEN via an existing shell variable (for example, in the
Docker build invocation) so the guidance points users to export the token first
and pass it through the build arg without exposing the secret.
| - `results()` — display outputs | ||
|
|
||
| Each workflow gets 4 content pages (upload, configure, run, results) that call `wf.show_*_section()`. | ||
| Python is pinned to **3.11** (matches the Docker runtime). `GITHUB_TOKEN` is required at build time to fetch the private `openms-streamlit-vue-component` submodule and OpenMS resources. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Clarify Python version scope (runtime vs CI).
Line 35 states Python is pinned to 3.11, but CI lint workflow uses Python 3.10 (.github/workflows/pylint.yml, Line 15). Please clarify this is Docker/runtime pinning so contributors don’t assume CI/tooling parity.
🤖 Prompt for 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.
In `@CLAUDE.md` at line 35, Clarify the Python 3.11 note in CLAUDE.md so it is
explicitly scoped to Docker/runtime rather than implying all tooling or CI uses
3.11. Update the wording around the Python pin and GITHUB_TOKEN requirement to
reflect that CI linting may still use a different version, and reference the
surrounding environment/setup guidance in CLAUDE.md so contributors do not
assume parity with the pylint workflow.
| {"key": "my-param", "value": 5, "name": "My Parameter", "help": "Description", | ||
| "min": 1, "max": 100, "step_size": 1, "widget_type": "slider"}, | ||
| ] | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a language identifier to the fenced code block.
Line 68 opens a fenced block without a language, which triggers markdownlint MD040. Use a language hint (for example, text) to keep docs lint-clean.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 68-68: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for 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.
In `@CLAUDE.md` at line 68, The fenced code block opened in CLAUDE.md is missing a
language identifier, which triggers markdownlint MD040. Update the opening fence
to include a language hint such as text, and keep the rest of the fenced block
unchanged so the documentation stays lint-clean.
Source: Linters/SAST tools
1 participant
Problem
In the FLASHDeconv Viewer's Sequence View, fragment-ion markers (a/b/c/x/y/z) never appear — even after selecting an MS2 scan. FLASHTnT works.
Root cause (Vue reactivity freeze)
prepareFragmentTable()(the only ion-marker producer) runs only from watchers. FLASHDeconv always slicesper_scan_datato one row, which pins theselectedScanIndexcomputed at0, so it never changes on a scan switch and its watcher never fires; there was noper_scan_datawatcher. The match was computed once and frozen. FLASHTnT was unaffected because protein selection changes theproteinIndex-keyedsequencecomputed.Change
Vue source fix is in t0mdavid-m/openms-streamlit-vue-component#29 (→
FVdeploy); this PR carries the rebuilt bundle + the submodule pointer bump:js-component/dist/rebuilt from the fixed source (built via Docker from local submodule source — verified the bundle contains only the intended deltas).57c9f6f → d98f15a.CLAUDE.md: document that the Vue bundle must always be built via Docker (not a local Node toolchain), with the local-source recipe.Watcher set rationalized for the only two consumers (FLASHTnT, FLASHDeconv): add a shallow
per_scan_datawatcher →recomputeFragments(); drop the deadselectedScanIndexand debugfontSizewatchers; extract the shared recompute into a helper.Merge order
Depends on the submodule PR. After #29 merges into
FVdeploy, update this submodule pointer to the merged commit so the full Docker image build (which clonesFVdeploy) and the committed bundle stay in sync.Verification
recomputeFragments×6,fontSizedebug log removed,prepareFragmentTable8→4).🤖 Generated with Claude Code
https://claude.ai/code/session_016Z9gnbqpmuU9z6BmoaXmuY
Summary by CodeRabbit
Documentation
Chores