From 2fdecdacf5b4bc5290ab7f84f46b3f990b6786d1 Mon Sep 17 00:00:00 2001 From: Helmut Hoffer von Ankershoffen Date: Sun, 7 Dec 2025 15:10:25 +0100 Subject: [PATCH 1/2] chore(nicegui): bump to 3.3.1 - Adapt application GUI for NiceGUI 3.3.x @ui.refreshable changes - Fix closure capture issue: use mutable dict for download_options (qupath_project, marimo) - Use mutable dict for folder_state to properly track folder selection - Define 30s page response timeouts for application and run describe pages - Update Claude PR review workflow to use sticky comments --- .github/workflows/_claude-code.yml | 7 + .github/workflows/_package-publish.yml | 10 +- .../claude-code-automation-pr-review.yml | 5 + CLI_REFERENCE.md | 2 +- pyproject.toml | 4 +- src/aignostics/application/_cli.py | 22 +- .../_gui/_page_application_run_describe.py | 205 ++++++++++-------- .../application/_gui/_page_builder.py | 7 +- src/aignostics/application/_service.py | 28 ++- src/aignostics/bucket/_service.py | 2 +- src/aignostics/dataset/_service.py | 4 +- src/aignostics/platform/_settings.py | 4 +- src/aignostics/qupath/_service.py | 2 +- src/aignostics/utils/_gui.py | 3 +- tests/CLAUDE.md | 72 +++++- tests/aignostics/application/gui_test.py | 11 +- tests/aignostics/qupath/gui_test.py | 3 + uv.lock | 10 +- 18 files changed, 262 insertions(+), 139 deletions(-) diff --git a/.github/workflows/_claude-code.yml b/.github/workflows/_claude-code.yml index c387c3403..a754cd0c9 100644 --- a/.github/workflows/_claude-code.yml +++ b/.github/workflows/_claude-code.yml @@ -26,6 +26,11 @@ on: required: false default: true type: boolean + use_sticky_comment: + description: 'Use just one comment to deliver PR comments (only applies for pull_request event workflows)' + required: false + default: false + type: boolean secrets: ANTHROPIC_API_KEY: required: true @@ -85,6 +90,7 @@ jobs: --model claude-sonnet-4-5-20250929 --allowed-tools "${{ inputs.allowed_tools }}" --system-prompt "Read the CLAUDE.md file in the root folder of this repository and explicitely acknowledge you will apply **all** guidance therein and in **all** linked documents." + - name: Run Claude Code (Automation Mode) if: inputs.mode == 'automation' uses: anthropics/claude-code-action@v1.0.22 @@ -92,6 +98,7 @@ jobs: github_token: ${{ secrets.GITHUB_TOKEN }} anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} track_progress: ${{ inputs.track_progress }} + use_sticky_comment: ${{ inputs.use_sticky_comment }} additional_permissions: | actions: read allowed_bots: "dependabot[bot],renovate[bot]" diff --git a/.github/workflows/_package-publish.yml b/.github/workflows/_package-publish.yml index 5d447509d..e5bb8c30d 100644 --- a/.github/workflows/_package-publish.yml +++ b/.github/workflows/_package-publish.yml @@ -32,15 +32,15 @@ jobs: experimental: [false] include: - runner: ubuntu-24.04-arm - experimental: true + experimental: false - runner: macos-latest - experimental: true + experimental: false - runner: macos-13 - experimental: true + experimental: false - runner: windows-latest - experimental: true + experimental: false - runner: windows-11-arm - experimental: true + experimental: false permissions: attestations: write diff --git a/.github/workflows/claude-code-automation-pr-review.yml b/.github/workflows/claude-code-automation-pr-review.yml index 43f386190..55d5f5929 100644 --- a/.github/workflows/claude-code-automation-pr-review.yml +++ b/.github/workflows/claude-code-automation-pr-review.yml @@ -11,10 +11,15 @@ concurrency: jobs: claude-review: + if: | + contains(github.event.pull_request.labels.*.name, 'claude') || + github.event.action == 'ready_for_review' uses: ./.github/workflows/_claude-code.yml with: mode: 'automation' + allowed_tools: 'mcp__github_inline_comment__create_inline_comment,Read,Write,Edit,MultiEdit,Glob,Grep,LS,WebFetch,WebSearch,Bash(git:*),Bash(bun:*),Bash(npm:*),Bash(npx:*),Bash(gh:*),Bash(uv:*),Bash(make:*)' track_progress: ${{ github.event_name == 'pull_request' && contains(fromJSON('["opened", "synchronize", "ready_for_review", "reopened"]'), github.event.action) }} + use_sticky_comment: true prompt: | # PR REVIEW FOR AIGNOSTICS PYTHON SDK diff --git a/CLI_REFERENCE.md b/CLI_REFERENCE.md index 0f7678e06..24428b07c 100644 --- a/CLI_REFERENCE.md +++ b/CLI_REFERENCE.md @@ -14,7 +14,7 @@ $ aignostics [OPTIONS] COMMAND [ARGS]... * `--show-completion`: Show completion for the current shell, to copy it or customize the installation. * `--help`: Show this message and exit. -🔬 Aignostics Python SDK v0.2.226 - built with love in Berlin 🐻 +🔬 Aignostics Python SDK v0.2.226 - built with love in Berlin 🐻 // Python v3.14.1 **Commands**: diff --git a/pyproject.toml b/pyproject.toml index 85ddf815b..bad8b8946 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -77,7 +77,7 @@ dependencies = [ # From Template "fastapi[all,standard]>=0.123.10", "humanize>=4.14.0,<5", - "nicegui[native]>=3.1.0,<3.2.0", # Regression in 3.2.0 + "nicegui[native]>=3.3.1,<4", "packaging>=25.0,<26", "platformdirs>=4.5.1,<5", "psutil>=7.1.3,<8", @@ -153,7 +153,7 @@ dev = [ "pip-licenses @ git+https://github.com/neXenio/pip-licenses.git@master", # https://github.com/raimon49/pip-licenses/pull/224 "pre-commit>=4.5.0,<5", "pyright>=1.1.406,<1.1.407", # Regression in 1.1.407, see https://github.com/microsoft/pyright/issues/11060 - "pytest>=9.0.1,<10", + "pytest>=9.0.2,<10", "pytest-asyncio>=1.3.0,<2", "pytest-cov>=7.0.0,<8", "pytest-docker>=3.2.5,<4", diff --git a/src/aignostics/application/_cli.py b/src/aignostics/application/_cli.py index 13fc18e59..6ca925746 100644 --- a/src/aignostics/application/_cli.py +++ b/src/aignostics/application/_cli.py @@ -746,7 +746,7 @@ def run_submit( # noqa: PLR0913, PLR0917 ) except ValueError as e: logger.warning( - "Bad input to create run for application '%s' (version: '%s'): %s", application_id, application_version, e + "Bad input to create run for application '{}' (version: '{}'): {}", application_id, application_version, e ) console.print( f"[warning]Warning:[/warning] Bad input to create run for application " @@ -755,7 +755,7 @@ def run_submit( # noqa: PLR0913, PLR0917 sys.exit(2) except NotFoundException as e: logger.warning( - "Could not find application version '%s' (version: '%s'): %s", application_id, application_version, e + "Could not find application version '{}' (version: '{}'): {}", application_id, application_version, e ) console.print( f"[warning]Warning:[/warning] Could not find application '{application_id} " @@ -773,10 +773,10 @@ def run_submit( # noqa: PLR0913, PLR0917 try: metadata_dict = read_metadata_csv_to_dict(metadata_csv_file=metadata_csv_file) if not metadata_dict: - console.print("Could mot read metadata file '%s'", metadata_csv_file) + console.print(f"Could not read metadata file '{metadata_csv_file}'") sys.exit(2) logger.trace( - "Submitting run for application '%s' (version: '%s') with metadata: %s", + "Submitting run for application '{}' (version: '{}') with metadata: {}", application_id, app_version.version_number, metadata_dict, @@ -809,7 +809,7 @@ def run_submit( # noqa: PLR0913, PLR0917 return application_run.run_id except ValueError as e: logger.warning( - "Bad input to create run for application '%s' (version: %s): %s", + "Bad input to create run for application '{}' (version: {}): {}", application_id, app_version.version_number, e, @@ -821,7 +821,7 @@ def run_submit( # noqa: PLR0913, PLR0917 sys.exit(2) except Exception as e: logger.exception( - "Failed to create run for application '%s' (version: %s)", application_id, app_version.version_number + "Failed to create run for application '{}' (version: {})", application_id, app_version.version_number ) console.print( f"[error]Error:[/error] Failed to create run for application " @@ -1059,7 +1059,7 @@ def run_cancel_by_filter( # noqa: C901, PLR0912, PLR0915 sys.exit(1) logger.trace( - "Canceling runs with filters: tags=%s, application_id=%s, application_version=%s, limit=%s, dry_run=%s", + "Canceling runs with filters: tags={}, application_id={}, application_version={}, limit={}, dry_run={}", tags, application_id, application_version, @@ -1207,7 +1207,7 @@ def run_update_item_metadata( sys.exit(2) except ValueError as e: logger.warning( - "Run ID '%s' or item external ID '%s' invalid or metadata invalid: %s", + "Run ID '{}' or item external ID '{}' invalid or metadata invalid: {}", run_id, external_id, e, @@ -1219,7 +1219,7 @@ def run_update_item_metadata( sys.exit(2) except Exception as e: logger.exception( - "Failed to update custom metadata for item '%s' in run with ID '%s'", + "Failed to update custom metadata for item '{}' in run with ID '{}'", external_id, run_id, ) @@ -1278,8 +1278,8 @@ def result_download( # noqa: C901, PLR0913, PLR0915, PLR0917 ) -> None: """Download results of a run.""" logger.trace( - "Downloading results for run with ID '%s' to '%s' with options: " - "create_subdirectory_for_run=%s, create_subdirectory_per_item=%s, wait_for_completion=%s, qupath_project=%r", + "Downloading results for run with ID '{}' to '{}' with options: " + "create_subdirectory_for_run={}, create_subdirectory_per_item={}, wait_for_completion={}, qupath_project={}", run_id, destination_directory, create_subdirectory_for_run, diff --git a/src/aignostics/application/_gui/_page_application_run_describe.py b/src/aignostics/application/_gui/_page_application_run_describe.py index c398122b5..6abb2d3af 100644 --- a/src/aignostics/application/_gui/_page_application_run_describe.py +++ b/src/aignostics/application/_gui/_page_application_run_describe.py @@ -157,8 +157,17 @@ async def _delete(run_id: str) -> bool: ui.notify(f"Failed to delete results of application run: {e}.", type="warning") return False + # Mutable container for qupath_project and marimo options that persists across @ui.refreshable calls. + # Defined OUTSIDE the @ui.refreshable function so closures (like start_download) always read + # from the same dict instance, even after refresh() is called with new parameter values. + download_options: dict[str, bool] = {"qupath_project": False, "marimo": False} + @ui.refreshable def download_run_dialog_content(qupath_project: bool = False, marimo: bool = False) -> None: # noqa: C901, PLR0915 + # Update the shared options dict so closures read the current values + download_options["qupath_project"] = qupath_project + download_options["marimo"] = marimo + if qupath_project: ui.markdown( "##### Visualize results in QuPath with one click \n" @@ -182,7 +191,11 @@ def download_run_dialog_content(qupath_project: bool = False, marimo: bool = Fal "2. A subfolder with the application run will be created and all results downloaded there. \n" ) - selected_folder = ui.input("Selected folder", value="").classes("w-full").props("readonly") + # Use the input field directly as the source of truth for folder selection. + # We bind to folder_state dict so start_download can read the current value. + folder_state: dict[str, str] = {"value": ""} + folder_input = ui.input("Selected folder", value="").classes("w-full").props("readonly") + folder_input.bind_value(folder_state, "value") with ui.row().classes("w-full"): @@ -190,18 +203,19 @@ async def _select_download_destination() -> None: result = await GUILocalFilePicker(str(Path.home()), multiple=False) # type: ignore[misc] if result and len(result) > 0: folder_path = Path(result[0]) - if folder_path.is_dir(): - selected_folder.value = str(folder_path) - else: - selected_folder.value = str(folder_path.parent) - ui.notify(f"Using custom directory: {selected_folder.value}", type="info") + folder_value = str(folder_path) if folder_path.is_dir() else str(folder_path.parent) + folder_state["value"] = folder_value + folder_input.set_value(folder_value) + ui.notify(f"Using custom directory: {folder_value}", type="info") download_button.enable() else: ui.notify("No folder selected", type="warning") - async def _select_data() -> None: # noqa: RUF029 - """Open a file picker dialog and show notifier when closed again.""" - selected_folder.value = str(get_user_data_directory("results")) + def _select_data() -> None: + """Select the Launchpad data directory.""" + folder_value = str(get_user_data_directory("results")) + folder_state["value"] = folder_value + folder_input.set_value(folder_value) ui.notify("Using Launchpad results directory", type="info") download_button.enable() @@ -225,105 +239,114 @@ async def _select_data() -> None: # noqa: RUF029 download_artifact_progress = ui.linear_progress(value=0, show_value=False).props("instant-feedback") download_artifact_progress.set_visibility(False) - async def start_download() -> None: # noqa: C901, PLR0915 - if not selected_folder.value: + # Create a mutable container for the progress queue that will be set when download starts + # This allows the timer callback to access the queue that's created during start_download + progress_state: dict[str, Any] = {"queue": None} + + def update_download_progress() -> None: # noqa: C901, PLR0912 + """Update the progress indicator with values from the queue.""" + progress_queue = progress_state.get("queue") + if progress_queue is None: + return + while not progress_queue.empty(): + progress = progress_queue.get() + if progress.status is DownloadProgressState.DOWNLOADING_INPUT: + status_text = ( + f"Downloading input slide {progress.item_index + 1} of {progress.item_count}" + if progress.item_index is not None and progress.item_count + else "Downloading input slide ..." + ) + elif progress.status is DownloadProgressState.DOWNLOADING and progress.total_artifact_index is not None: + status_text = ( + f"Downloading artifact {progress.total_artifact_index + 1} of {progress.total_artifact_count}" + ) + else: + status_text = progress.status + + download_item_status.set_text(status_text) + download_item_status.set_visibility(True) + download_item_progress.set_value(progress.item_progress_normalized) + download_artifact_progress.set_value(progress.artifact_progress_normalized) + if progress.status is DownloadProgressState.INITIALIZING: + download_artifact_status.set_visibility(False) + download_item_progress.set_visibility(False) + download_artifact_progress.set_visibility(False) + elif progress.status is DownloadProgressState.DOWNLOADING_INPUT: + if progress.input_slide_path: + download_artifact_status.set_text(f"Input: {progress.input_slide_path.name}") + download_artifact_status.set_visibility(True) + download_item_progress.set_visibility(True) + download_artifact_progress.set_visibility(True) + elif progress.status is DownloadProgressState.DOWNLOADING: + if progress.artifact_path: + download_artifact_status.set_text(str(progress.artifact_path)) + download_artifact_status.set_visibility(True) + download_item_progress.set_visibility(True) + download_artifact_progress.set_visibility(True) + elif progress.status is DownloadProgressState.QUPATH_ADD_INPUT and progress.qupath_add_input_progress: + download_artifact_status.set_text(progress.qupath_add_input_progress.status) + download_artifact_status.set_visibility(True) + download_item_progress.set_visibility(True) + download_artifact_progress.set_visibility(False) + elif ( + progress.status is DownloadProgressState.QUPATH_ADD_RESULTS and progress.qupath_add_results_progress + ): + download_artifact_status.set_text(progress.qupath_add_results_progress.status) + download_artifact_status.set_visibility(True) + download_item_progress.set_visibility(True) + download_artifact_progress.set_visibility(False) + elif ( + progress.status is DownloadProgressState.QUPATH_ANNOTATE_INPUT_WITH_RESULTS + and progress.qupath_annotate_input_with_results_progress + ): + download_artifact_status.set_text(progress.qupath_annotate_input_with_results_progress.status) + download_artifact_status.set_visibility(True) + download_item_progress.set_visibility(True) + download_artifact_progress.set_visibility(True) + else: + download_artifact_status.set_text("") + download_item_progress.set_visibility(False) + download_artifact_progress.set_visibility(False) + + # Create the timer during dialog construction (in valid slot context), but inactive initially + # This avoids RuntimeError when trying to create timer inside async event handler + progress_timer = ui.timer(0.1, update_download_progress, active=False) + + async def start_download() -> None: + # Read from download_options dict (defined outside @ui.refreshable) to get current values + current_qupath_project = download_options["qupath_project"] + current_marimo = download_options["marimo"] + current_folder = folder_state["value"] + if not current_folder: ui.notify("Please select a folder first", type="warning") return ui.notify("Downloading ...", type="info") progress_queue = Manager().Queue() + progress_state["queue"] = progress_queue # Store queue so timer callback can access it - def update_download_progress() -> None: # noqa: C901, PLR0912 - """Update the progress indicator with values from the queue.""" - while not progress_queue.empty(): - progress = progress_queue.get() - # Determine status text based on progress state - if progress.status is DownloadProgressState.DOWNLOADING_INPUT: - status_text = ( - f"Downloading input slide {progress.item_index + 1} of {progress.item_count}" - if progress.item_index is not None and progress.item_count - else "Downloading input slide ..." - ) - elif ( - progress.status is DownloadProgressState.DOWNLOADING - and progress.total_artifact_index is not None - ): - status_text = ( - f"Downloading artifact {progress.total_artifact_index + 1} " - f"of {progress.total_artifact_count}" - ) - else: - status_text = progress.status - - download_item_status.set_text(status_text) - download_item_status.set_visibility(True) - download_item_progress.set_value(progress.item_progress_normalized) - download_artifact_progress.set_value(progress.artifact_progress_normalized) - if progress.status is DownloadProgressState.INITIALIZING: - download_artifact_status.set_visibility(False) - download_item_progress.set_visibility(False) - download_artifact_progress.set_visibility(False) - elif progress.status is DownloadProgressState.DOWNLOADING_INPUT: - if progress.input_slide_path: - download_artifact_status.set_text(f"Input: {progress.input_slide_path.name}") - download_artifact_status.set_visibility(True) - download_item_progress.set_visibility(True) - download_artifact_progress.set_visibility(True) - elif progress.status is DownloadProgressState.DOWNLOADING: - if progress.artifact_path: - download_artifact_status.set_text(str(progress.artifact_path)) - download_artifact_status.set_visibility(True) - download_item_progress.set_visibility(True) - download_artifact_progress.set_visibility(True) - elif ( - progress.status is DownloadProgressState.QUPATH_ADD_INPUT and progress.qupath_add_input_progress - ): - download_artifact_status.set_text(progress.qupath_add_input_progress.status) - download_artifact_status.set_visibility(True) - download_item_progress.set_visibility(True) - download_artifact_progress.set_visibility(False) - elif ( - progress.status is DownloadProgressState.QUPATH_ADD_RESULTS - and progress.qupath_add_results_progress - ): - download_artifact_status.set_text(progress.qupath_add_results_progress.status) - download_artifact_status.set_visibility(True) - download_item_progress.set_visibility(True) - download_artifact_progress.set_visibility(False) - elif ( - progress.status is DownloadProgressState.QUPATH_ANNOTATE_INPUT_WITH_RESULTS - and progress.qupath_annotate_input_with_results_progress - ): - download_artifact_status.set_text(progress.qupath_annotate_input_with_results_progress.status) - download_artifact_status.set_visibility(True) - download_item_progress.set_visibility(True) - download_artifact_progress.set_visibility(True) - else: - download_artifact_status.set_text("") - download_item_progress.set_visibility(False) - download_artifact_progress.set_visibility(False) - - ui.timer(0.1, update_download_progress) + # Activate the timer now that download is starting + progress_timer.activate() try: download_button.disable() download_button.props(add="loading") results_folder = await nicegui_run.cpu_bound( Service.application_run_download_static, run_id=run.run_id, - destination_directory=Path(selected_folder.value), + destination_directory=Path(current_folder), wait_for_completion=True, - qupath_project=qupath_project, + qupath_project=current_qupath_project, download_progress_queue=progress_queue, ) if not results_folder: message = "Download returned without results folder." raise ValueError(message) # noqa: TRY301 - if qupath_project: + if current_qupath_project: if results_folder: ui.notify("Download and QuPath project creation completed.", type="positive") download_item_status.set_text("Opening QuPath ...") await open_qupath(project=results_folder / "qupath", button=download_button) - elif marimo: + elif current_marimo: ui.notify("Download and Notebook preparation completed.", type="positive") download_item_status.set_text("Opening Notebook ...") open_marimo(results_folder=results_folder, button=download_button) @@ -332,7 +355,11 @@ def update_download_progress() -> None: # noqa: C901, PLR0912 show_in_file_manager(str(results_folder)) except ValueError as e: ui.notify(f"Download failed: {e}", type="negative", multi_line=True) + progress_timer.deactivate() + progress_state["queue"] = None return + progress_timer.deactivate() + progress_state["queue"] = None download_button.props(remove="loading") download_button.enable() download_item_status.set_visibility(False) @@ -361,7 +388,9 @@ def update_download_progress() -> None: # noqa: C901, PLR0912 .props("color=primary") .mark("DIALOG_BUTTON_DOWNLOAD_RUN") ) - download_button.disable() + if not folder_state["value"]: # Disable download button initially, will be enabled when folder is selected + download_button.disable() + ui.space() ui.button("Close", on_click=download_run_dialog.close).props("flat") @@ -516,8 +545,6 @@ def open_marimo(results_folder: Path, button: ui.button | None = None) -> None: lambda e: expansion.classes(add="w-full" if e.value else "", remove="w-full" if not e.value else "") ) with expansion: - # Display run metadata, including duration if possible, using humanize - submitted_at = run_data.submitted_at.astimezone() terminated_at = run_data.terminated_at.astimezone() if run_data.terminated_at else None if submitted_at and terminated_at: diff --git a/src/aignostics/application/_gui/_page_builder.py b/src/aignostics/application/_gui/_page_builder.py index 6169985e3..622b653e9 100644 --- a/src/aignostics/application/_gui/_page_builder.py +++ b/src/aignostics/application/_gui/_page_builder.py @@ -4,6 +4,9 @@ from aignostics.utils import BasePageBuilder +APPLICATION_DESCRIBE_PAGE_TIMEOUT = 30 +RUN_DESCRIBE_PAGE_TIMEOUT = 30 + class PageBuilder(BasePageBuilder): @staticmethod @@ -19,7 +22,7 @@ async def page_index(client: Client, query: str | None = None) -> None: await _page_index(client, query=query) - @ui.page("/application/{application_id}") + @ui.page("/application/{application_id}", response_timeout=APPLICATION_DESCRIBE_PAGE_TIMEOUT) async def page_application_describe(application_id: str) -> None: """Describe Application. @@ -30,7 +33,7 @@ async def page_application_describe(application_id: str) -> None: await _page_application_describe(application_id) - @ui.page("/application/run/{run_id}") + @ui.page("/application/run/{run_id}", response_timeout=RUN_DESCRIBE_PAGE_TIMEOUT) async def page_application_run_describe(run_id: str) -> None: """Describe Application Run. diff --git a/src/aignostics/application/_service.py b/src/aignostics/application/_service.py index b8999588f..ae065cfb7 100644 --- a/src/aignostics/application/_service.py +++ b/src/aignostics/application/_service.py @@ -482,7 +482,7 @@ def application_run_upload( # noqa: PLR0913, PLR0917 }) file_size = source_file_path.stat().st_size logger.trace( - "Uploading file '%s' with size %d bytes to '%s' via '%s'", + "Uploading file '{}' with size {} bytes to '{}' via '{}'", source_file_path, file_size, platform_bucket_url, @@ -1163,7 +1163,7 @@ def application_run_update_item_custom_metadata( """ try: logger.trace( - "Updating custom metadata for item '%s' in run with ID '%s'", + "Updating custom metadata for item '{}' in run with ID '{}'", external_id, run_id, ) @@ -1172,7 +1172,7 @@ def application_run_update_item_custom_metadata( custom_metadata, ) logger.trace( - "Updated custom metadata for item '%s' in run with ID '%s'", + "Updated custom metadata for item '{}' in run with ID '{}'", external_id, run_id, ) @@ -1375,6 +1375,16 @@ def application_run_download( # noqa: C901, PLR0912, PLR0913, PLR0914, PLR0915, NotFoundException: If the application run with the given ID is not found. RuntimeError: If run details cannot be retrieved or download fails. """ + logger.trace( + "Downloading application run '{}' to '{}', create_subdirectory_for_run={}, " + "create_subdirectory_per_item={}, wait_for_completion={}, qupath_project={}", + run_id, + destination_directory, + create_subdirectory_for_run, + create_subdirectory_per_item, + wait_for_completion, + qupath_project, + ) if qupath_project and not has_qupath_extra: message = "QuPath project creation requested, but 'qupath' extra is not installed." message += 'Start launchpad with `uvx --with "aignostics[qupath]" ....' @@ -1429,7 +1439,7 @@ def application_run_download( # noqa: C901, PLR0912, PLR0913, PLR0914, PLR0915, item.external_id = str(local_path) # Update external_id so subsequent code uses the local path except Exception as e: logger.warning( - "Failed to download input slide from '%s' to '%s': %s", item.external_id, local_path, e + "Failed to download input slide from '{}' to '{}': {}", item.external_id, local_path, e ) if qupath_project: @@ -1476,7 +1486,7 @@ def update_qupath_add_input_progress(qupath_add_input_progress: QuPathAddProgres if run_details.state == RunState.TERMINATED: logger.trace( - "Run '%s' reached final status '%s' with message '%s' (%s).", + "Run '{}' reached final status '{}' with message '{}' ({}).", run_id, run_details.state, run_details.error_message, @@ -1486,7 +1496,7 @@ def update_qupath_add_input_progress(qupath_add_input_progress: QuPathAddProgres if not wait_for_completion: logger.trace( - "Run '%s' is in progress with status '%s' and message '%s' (%s), " + "Run '{}' is in progress with status '{}' and message '{}' ({}), " "but not requested to wait for completion.", run_id, run_details.state, @@ -1496,7 +1506,7 @@ def update_qupath_add_input_progress(qupath_add_input_progress: QuPathAddProgres break logger.trace( - "Run '%s' is in progress with status '%s', waiting for completion ...", run_id, run_details.state + "Run '{}' is in progress with status '{}', waiting for completion ...", run_id, run_details.state ) progress.status = DownloadProgressState.WAITING update_progress(progress, download_progress_callable, download_progress_queue) @@ -1568,6 +1578,10 @@ def update_qupath_annotate_input_with_results_progress( message = f"Added {added} annotations to input slides." logger.debug(message) + else: + logger.trace("QuPath project creation not requested, skipping ...") + + logger.trace("Completed downloading application run '{}' to '{}'", run_id, final_destination_directory) progress.status = DownloadProgressState.COMPLETED update_progress(progress, download_progress_callable, download_progress_queue) diff --git a/src/aignostics/bucket/_service.py b/src/aignostics/bucket/_service.py index ac82d9e4a..a6d6a324a 100644 --- a/src/aignostics/bucket/_service.py +++ b/src/aignostics/bucket/_service.py @@ -519,7 +519,7 @@ def download( return DownloadResult(downloaded=[], failed=[]) logger.trace( - "Found %d objects matching '%s' in bucket, downloading to '%s'...", + "Found {} objects matching '{}' in bucket, downloading to '{}'...", len(matched_objects), what, destination, diff --git a/src/aignostics/dataset/_service.py b/src/aignostics/dataset/_service.py index 56899d196..775e6d211 100644 --- a/src/aignostics/dataset/_service.py +++ b/src/aignostics/dataset/_service.py @@ -255,7 +255,7 @@ def check_and_download(column_name: str, item_ids: list[str], target_directory: ) else: logger.trace( - "Starting download subprocess with executable '%s' and script:\n%s", sys.executable, script_content + "Starting download subprocess with executable '{}' and script:\n{}", sys.executable, script_content ) process = subprocess.Popen( # noqa: S603 [sys.executable, "-c", script_content], @@ -284,7 +284,7 @@ def check_and_download(column_name: str, item_ids: list[str], target_directory: stdout_output = process.stdout.read() if process.stdout else "No stdout output" stderr_output = process.stderr.read() if process.stderr else "No stderr output" logger.error( - "Download subprocess failed with code '%d'\n\nstdout:\n\n%sstdin:\n\n%s\n\n", + "Download subprocess failed with code '{}'\n\nstdout:\n\n{}stdin:\n\n{}\n\n", return_code, stdout_output, stderr_output, diff --git a/src/aignostics/platform/_settings.py b/src/aignostics/platform/_settings.py index cd629a71c..b6c586100 100644 --- a/src/aignostics/platform/_settings.py +++ b/src/aignostics/platform/_settings.py @@ -1,12 +1,12 @@ """Settings of the Python SDK.""" -import logging import os from pathlib import Path from typing import Annotated, TypeVar from urllib.parse import urlparse import platformdirs +from loguru import logger from pydantic import ( BeforeValidator, Field, @@ -49,8 +49,6 @@ ) from ._messages import UNKNOWN_ENDPOINT_URL -logger = logging.getLogger(__name__) - T = TypeVar("T", bound=BaseSettings) TIMEOUT_MIN_DEFAULT = 0.1 # seconds diff --git a/src/aignostics/qupath/_service.py b/src/aignostics/qupath/_service.py index b6ddc8e59..9cbe7506b 100644 --- a/src/aignostics/qupath/_service.py +++ b/src/aignostics/qupath/_service.py @@ -549,7 +549,7 @@ def get_app_dir( system = platform.system() if platform_system is None else platform_system machine = platform.machine() if platform_machine is None else platform_machine logger.trace( - "Getting QuPath application directory for version '%s', installation path '%s' on system '%s'", + "Getting QuPath application directory for version '{}', installation path '{}' on system '{}'", version, installation_path, system, diff --git a/src/aignostics/utils/_gui.py b/src/aignostics/utils/_gui.py index 016aebee9..e539d63a7 100644 --- a/src/aignostics/utils/_gui.py +++ b/src/aignostics/utils/_gui.py @@ -7,6 +7,7 @@ from ._di import locate_subclasses WINDOW_SIZE = (1280, 768) # Default window size for the GUI +BROWSER_RECONNECT_TIMEOUT = 60 * 60 * 24 * 7 # 7 days class BasePageBuilder(ABC): @@ -86,7 +87,7 @@ def gui_run( # noqa: PLR0913, PLR0917 show_welcome_message=native is False, show=show, window_size=WINDOW_SIZE if native else None, - reconnect_timeout=60 * 60 * 24 * 7, + reconnect_timeout=BROWSER_RECONNECT_TIMEOUT, ) diff --git a/tests/CLAUDE.md b/tests/CLAUDE.md index 8c98802a0..ad8a368ae 100644 --- a/tests/CLAUDE.md +++ b/tests/CLAUDE.md @@ -934,6 +934,70 @@ def test_api_response_structure(snapshot): snapshot.assert_match(response.json()) ``` +## NiceGUI Troubleshooting + +### Symptom: Element State Lost After User Interaction (NiceGUI 3.0+) + +**Problem:** After upgrading to NiceGUI 3.0+, tests fail because UI element state (e.g., button enabled/disabled, input values) is lost after user interactions inside `@ui.refreshable` functions. + +**Root Cause:** NiceGUI 3.0.0 introduced observable props/classes/styles that automatically sync UI updates. When modifying `.value` on elements inside `@ui.refreshable`, the framework may trigger element recreation, causing local variables to reset. + +**Symptom Example:** + +```python +@ui.refreshable +def dialog_content() -> None: + selected_folder = ui.input("Folder", value="") # Local variable resets on recreation! + download_button = ui.button("Download").props("disabled") + + def on_select(): + selected_folder.value = "/path/to/folder" # This may trigger refresh + download_button.enable() # Button recreated, enable() lost +``` + +**Solution:** Use a mutable dictionary container instead of `ui.state()` when the `@ui.refreshable` function has parameters that must be preserved: + +```python +@ui.refreshable +def dialog_content(qupath_project: bool = False) -> None: + # Use mutable dict instead of ui.state() to avoid triggering refresh + # which would reset qupath_project to its default value. + # ui.state() triggers refresh() internally when set_folder() is called, + # but refresh() without arguments uses the original call arguments. + folder_state: dict[str, str] = {"value": ""} + + selected_folder = ui.input("Folder", value=folder_state["value"]) + download_button = ui.button("Download") + if not folder_state["value"]: + download_button.disable() + + def on_select(): + folder_value = "/path/to/folder" + folder_state["value"] = folder_value # Update dict without triggering refresh + selected_folder.value = folder_value + download_button.enable() +``` + +**Why This Works:** + +1. Mutable dict container stores state without triggering `refresh()` +2. `ui.state()` internally calls `refresh()` when setter is invoked, which uses **original** function arguments +3. For `@ui.refreshable` functions with parameters, `ui.state()` setter can reset those parameters to defaults +4. The dict pattern preserves both local state AND function arguments + +**CRITICAL:** Do NOT use `ui.state()` in `@ui.refreshable` functions that accept parameters and are called with `refresh(param=value)`. The `set_state()` function triggers `refresh()` without arguments, resetting all parameters to their defaults. + +**When to use each pattern:** + +| Pattern | Use Case | +|---------|----------| +| `ui.state()` | `@ui.refreshable` with no parameters or when refresh resets are acceptable | +| Mutable dict | `@ui.refreshable` with parameters that must be preserved after state updates | + +**Reference:** [NiceGUI 3.0.0 Release Notes](https://github.com/zauberzeug/nicegui/releases/tag/v3.0.0) + +--- + ## Debugging Test Failures ### Verbose Output @@ -980,10 +1044,10 @@ def test_complex_logic(): ### Test Hygiene -- Remove obsolete tests -- Update mocks when API changes -- Maintain test documentation -- Regular dependency updates +* Remove obsolete tests +* Update mocks when API changes +* Maintain test documentation +* Regular dependency updates --- diff --git a/tests/aignostics/application/gui_test.py b/tests/aignostics/application/gui_test.py index 72c71a2d9..06b6dddf4 100644 --- a/tests/aignostics/application/gui_test.py +++ b/tests/aignostics/application/gui_test.py @@ -349,7 +349,7 @@ async def test_gui_download_dataset_via_application_to_run_cancel_to_find_back( @pytest.mark.e2e @pytest.mark.long_running @pytest.mark.flaky(retries=1, delay=5) -@pytest.mark.timeout(timeout=60 * 5) +@pytest.mark.timeout(timeout=60 * 10) @pytest.mark.sequential # Helps on Linux with image analysis step otherwise timing out async def test_gui_run_download( # noqa: PLR0915 user: User, runner: CliRunner, tmp_path: Path, silent_logging: None, record_property @@ -365,6 +365,7 @@ async def test_gui_run_download( # noqa: PLR0915 application_id=HETA_APPLICATION_ID, application_version=HETA_APPLICATION_VERSION, external_id=SPOT_0_GS_URL, + tags=["scheduled"], has_output=True, limit=1, ) @@ -397,15 +398,15 @@ async def test_gui_run_download( # noqa: PLR0915 await user.should_see(marker="BUTTON_DOWNLOAD_RUN", retries=100) user.find(marker="BUTTON_DOWNLOAD_RUN").click() - # Step 3: Select Data + # Step 3: Check download button is initially disabled, then select Data folder download_run_button: ui.button = user.find(marker="DIALOG_BUTTON_DOWNLOAD_RUN").elements.pop() assert not download_run_button.enabled, "Download button should be disabled before selecting target" await user.should_see(marker="BUTTON_DOWNLOAD_DESTINATION_DATA", retries=100) user.find(marker="BUTTON_DOWNLOAD_DESTINATION_DATA").click() + await assert_notified(user, "Using Launchpad results directory") - # Step 3: Trigger Download - await sleep(2) # Wait a bit for button state to update so we can click - download_run_button: ui.button = user.find(marker="DIALOG_BUTTON_DOWNLOAD_RUN").elements.pop() + # Step 4: Trigger Download - wait for button to be enabled + download_run_button = user.find(marker="DIALOG_BUTTON_DOWNLOAD_RUN").elements.pop() assert download_run_button.enabled, "Download button should be enabled after selecting target" user.find(marker="DIALOG_BUTTON_DOWNLOAD_RUN").click() await assert_notified(user, "Downloading ...") diff --git a/tests/aignostics/qupath/gui_test.py b/tests/aignostics/qupath/gui_test.py index ee9d12860..4c38790f1 100644 --- a/tests/aignostics/qupath/gui_test.py +++ b/tests/aignostics/qupath/gui_test.py @@ -165,6 +165,7 @@ async def test_gui_run_qupath_install_to_inspect( # noqa: C901, PLR0912, PLR091 application_id=HETA_APPLICATION_ID, application_version=HETA_APPLICATION_VERSION, external_id=SPOT_0_GS_URL, + tags=["scheduled"], has_output=True, limit=1, ) @@ -175,6 +176,8 @@ async def test_gui_run_qupath_install_to_inspect( # noqa: C901, PLR0912, PLR091 run_id = runs[0].run_id + run_id = "0717d2ca-2bc8-4f0f-815a-928553660e05" # For debugging only + # Explore run run = Service().application_run(run_id).details() print( diff --git a/uv.lock b/uv.lock index f23979507..dc039b53e 100644 --- a/uv.lock +++ b/uv.lock @@ -185,7 +185,7 @@ requires-dist = [ { name = "loguru", specifier = ">=0.7.3,<1" }, { name = "marimo", marker = "extra == 'marimo'", specifier = ">=0.18.3,<1" }, { name = "matplotlib", marker = "extra == 'marimo'", specifier = ">=3.10.7,<4" }, - { name = "nicegui", extras = ["native"], specifier = ">=3.1.0,<3.2.0" }, + { name = "nicegui", extras = ["native"], specifier = ">=3.3.1,<4" }, { name = "openslide-bin", specifier = ">=4.0.0.10,<5" }, { name = "openslide-python", specifier = ">=1.4.3,<2" }, { name = "packaging", specifier = ">=25.0,<26" }, @@ -235,7 +235,7 @@ dev = [ { name = "pip-licenses", git = "https://github.com/neXenio/pip-licenses.git?rev=master" }, { name = "pre-commit", specifier = ">=4.5.0,<5" }, { name = "pyright", specifier = ">=1.1.406,<1.1.407" }, - { name = "pytest", specifier = ">=9.0.1,<10" }, + { name = "pytest", specifier = ">=9.0.2,<10" }, { name = "pytest-asyncio", specifier = ">=1.3.0,<2" }, { name = "pytest-cov", specifier = ">=7.0.0,<8" }, { name = "pytest-docker", specifier = ">=3.2.5,<4" }, @@ -4091,7 +4091,7 @@ wheels = [ [[package]] name = "nicegui" -version = "3.1.0" +version = "3.3.1" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "aiofiles" }, @@ -4115,9 +4115,9 @@ dependencies = [ { name = "uvicorn", extra = ["standard"] }, { name = "watchfiles" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/f8/23/1a709ac5ae3b91674b7fb75b7eb2db851cf09b55a809936c106efa46c52f/nicegui-3.1.0.tar.gz", hash = "sha256:1496b9719292cdd64fb89a9e560a197f517347e8808c829fae9ba28a294d78ae", size = 20342929, upload-time = "2025-10-22T08:33:07.869Z" } +sdist = { url = "https://files.pythonhosted.org/packages/e3/ee/43ecb61be17b3487d9bc17bf493970714dfb85bda32bff2f268cd9304262/nicegui-3.3.1.tar.gz", hash = "sha256:0de625b836acf1c36875dc290ae769ef114aba9535e33d302566b19381b206fa", size = 20353012, upload-time = "2025-11-17T10:24:16.452Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/a6/4e/ce491f04b07d2530a441f01fa028c936c353ee92e0ad153bf6b7cf2c3e9d/nicegui-3.1.0-py3-none-any.whl", hash = "sha256:c5ad4ac120eaec138bee71514e9cba2f9d27384b7e9201c11f309a90a128a96f", size = 20996176, upload-time = "2025-10-22T08:33:05.178Z" }, + { url = "https://files.pythonhosted.org/packages/d7/5c/fccb05fd8e9e9bdb5dd1531af1d40bcfbb3d7e850b2d43902ca117becde9/nicegui-3.3.1-py3-none-any.whl", hash = "sha256:7eb4e35936958c1df4b0fa6f5f4ed51d6e060c2938aca09b229abfec6b25e35e", size = 21007617, upload-time = "2025-11-17T10:24:13.13Z" }, ] [package.optional-dependencies] From a4a5aa6c677e48f8641f114972f7c07c880118b2 Mon Sep 17 00:00:00 2001 From: Helmut Hoffer von Ankershoffen Date: Sun, 7 Dec 2025 16:31:50 +0100 Subject: [PATCH 2/2] test(qupath): remove local test run id --- tests/aignostics/qupath/gui_test.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/aignostics/qupath/gui_test.py b/tests/aignostics/qupath/gui_test.py index 4c38790f1..c7dde0613 100644 --- a/tests/aignostics/qupath/gui_test.py +++ b/tests/aignostics/qupath/gui_test.py @@ -176,8 +176,6 @@ async def test_gui_run_qupath_install_to_inspect( # noqa: C901, PLR0912, PLR091 run_id = runs[0].run_id - run_id = "0717d2ca-2bc8-4f0f-815a-928553660e05" # For debugging only - # Explore run run = Service().application_run(run_id).details() print(