From 10b07fd5ed3afe9597439ee395d653d257290932 Mon Sep 17 00:00:00 2001 From: Tom David Mueller Date: Mon, 31 Aug 2026 15:08:54 +0200 Subject: [PATCH 1/3] 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) Claude-Session: https://claude.ai/code/session_016Qn3mLqr7zBr7rgCokx6ku --- src/workflow/StreamlitUI.py | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/src/workflow/StreamlitUI.py b/src/workflow/StreamlitUI.py index f9adf5ec..69cf3264 100644 --- a/src/workflow/StreamlitUI.py +++ b/src/workflow/StreamlitUI.py @@ -559,7 +559,9 @@ def _select_input_file_impl(self, key, name, multiple, display_file_path, reacti if not path.exists(): st.warning(f"No **{name}** files!") return - options = [str(f) for f in path.iterdir() if "external_files.txt" not in str(f)] + options = sorted( + str(f) for f in path.iterdir() if "external_files.txt" not in str(f) + ) # Check if local files are available external_files = Path( @@ -675,11 +677,21 @@ def format_files(input: Any) -> List[str]: key = f"{self.parameter_manager.param_prefix}{key}" + # Streamlit ignores a widget's initial-value argument (value=/default=/index=) + # once that key already exists in session state -- but on Streamlit < 1.50 the + # argument is still hashed into the widget's element id. Since this method feeds + # the persisted parameter straight back in as that argument, the id changed on + # every interaction, the following interaction arrived under the now-stale id and + # was silently dropped: selecting six mzML files kept only three. Seed the widget + # on first render only; from then on session state owns the value. + def seed(**kwargs: Any) -> dict: + return {} if key in st.session_state else kwargs + if widget_type == "text": - st.text_input(name, value=value, key=key, help=help, on_change=on_change) + st.text_input(name, key=key, help=help, on_change=on_change, **seed(value=value)) elif widget_type == "textarea": - st.text_area(name, value=value, key=key, help=help, on_change=on_change) + st.text_area(name, key=key, help=help, on_change=on_change, **seed(value=value)) elif widget_type == "number": number_type = float if isinstance(value, float) else int @@ -693,27 +705,27 @@ def format_files(input: Any) -> List[str]: name, min_value=min_value, max_value=max_value, - value=value, step=step_size, format=None, key=key, help=help, on_change=on_change, + **seed(value=value), ) elif widget_type == "checkbox": - st.checkbox(name, value=value, key=key, help=help, on_change=on_change) + st.checkbox(name, key=key, help=help, on_change=on_change, **seed(value=value)) elif widget_type == "selectbox": if options is not None: st.selectbox( name, options=options, - index=options.index(value) if value in options else 0, key=key, format_func=format_files, help=help, on_change=on_change, + **seed(index=options.index(value) if value in options else 0), ) else: st.warning(f"Select widget '{name}' requires options parameter") @@ -723,11 +735,11 @@ def format_files(input: Any) -> List[str]: st.multiselect( name, options=options, - default=value, key=key, format_func=format_files, help=help, on_change=on_change, + **seed(default=value), ) else: st.warning(f"Select widget '{name}' requires options parameter") @@ -744,12 +756,12 @@ def format_files(input: Any) -> List[str]: name, min_value=min_value, max_value=max_value, - value=value, step=step_size, key=key, format=None, help=help, on_change=on_change, + **seed(value=value), ) else: st.warning( @@ -757,12 +769,12 @@ def format_files(input: Any) -> List[str]: ) elif widget_type == "password": - st.text_input(name, value=value, type="password", key=key, help=help, on_change=on_change) + st.text_input(name, type="password", key=key, help=help, on_change=on_change, **seed(value=value)) elif widget_type == "auto": # Auto-determine widget type based on value if isinstance(value, bool): - st.checkbox(name, value=value, key=key, help=help, on_change=on_change) + st.checkbox(name, key=key, help=help, on_change=on_change, **seed(value=value)) elif isinstance(value, (int, float)): self._input_widget_impl( key, From 33696a575efc8cddbe16caf296ae2a42005601ba Mon Sep 17 00:00:00 2001 From: Tom David Mueller Date: Mon, 31 Aug 2026 19:32:46 +0200 Subject: [PATCH 2/3] 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) Claude-Session: https://claude.ai/code/session_016Qn3mLqr7zBr7rgCokx6ku --- .github/workflows/build-windows-executable-app.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build-windows-executable-app.yaml b/.github/workflows/build-windows-executable-app.yaml index 99b41773..03748875 100644 --- a/.github/workflows/build-windows-executable-app.yaml +++ b/.github/workflows/build-windows-executable-app.yaml @@ -55,7 +55,7 @@ jobs: shell: bash run: | choco install ccache ninja -y --no-progress - choco install cmake --version=3.31.1 -y --no-progress --force + choco install cmake --version=3.31.12 -y --no-progress --force ## GH CLI "SHOULD BE" installed. Sometimes I had to manually install nonetheless. Super weird. # https://github.com/actions/runner-images/blob/main/images/win/scripts/Installers/Install-GitHub-CLI.ps1 echo "C:/Program Files (x86)/GitHub CLI" >> $GITHUB_PATH From 3a18fd94345007f9fcac51d03e48dfd532e49127 Mon Sep 17 00:00:00 2001 From: Tom David Mueller Date: Mon, 31 Aug 2026 19:32:46 +0200 Subject: [PATCH 3/3] 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) Claude-Session: https://claude.ai/code/session_016Qn3mLqr7zBr7rgCokx6ku --- tests/test_tool_instance_name.py | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/tests/test_tool_instance_name.py b/tests/test_tool_instance_name.py index cd060cac..d1eef17a 100644 --- a/tests/test_tool_instance_name.py +++ b/tests/test_tool_instance_name.py @@ -24,6 +24,23 @@ _original_streamlit = sys.modules.get('streamlit') sys.modules['streamlit'] = mock_streamlit + +def _drop_cached_workflow_modules() -> None: + """Forget any cached src.workflow modules. + + A module binds `st` once, at import time. If an earlier test file has already + imported src.workflow.ParameterManager against the real streamlit, the import + below is just a cache hit and the mock never takes effect - which is why these + tests passed when run alone but failed in a full-suite run. + """ + for _key in list(sys.modules.keys()): + if _key.startswith('src.workflow'): + sys.modules.pop(_key, None) + + +# Drop first, so the import below really binds the mock. +_drop_cached_workflow_modules() + from src.workflow.ParameterManager import ParameterManager if _original_streamlit is not None: @@ -31,10 +48,9 @@ else: sys.modules.pop('streamlit', None) -# Remove cached src.workflow modules -for _key in list(sys.modules.keys()): - if _key.startswith('src.workflow'): - sys.modules.pop(_key, None) +# Drop again, so later test files re-import against the real streamlit instead of +# the mock-bound modules this file just created. +_drop_cached_workflow_modules() @pytest.fixture