[codex] Fix main CI regressions in GUI import fallbacks and docs build#16
Conversation
Root cause: - GitHub Actions could import clearex.gui.app in a PyQt-unavailable state, which left _apply_application_icon and _show_startup_splash undefined and caused GUI tests that monkeypatch those helpers to fail. - The runtime docs used malformed nested list indentation in the movie storage section, which made the Sphinx Docs workflow fail with an Unexpected indentation error. - One Dask GPU env test also depended on host-specific inherited library-path state, which made the full local pytest run unnecessarily brittle. What changed: - Add import-safe no-op fallbacks for the GUI helper functions and keep the real PyQt implementations as overrides when PyQt6 is available. - Add a regression test that simulates importing clearex.gui.app with PyQt6 blocked and asserts the helper surface still exists. - Rewrite the movie runtime storage bullets in valid reStructuredText and normalize the default_transition_frames literal markup. - Clear platform library-path env vars in the GPU worker env test before asserting the merged path value. Validation: - uv run pytest -q tests/gui/test_gui_execution.py - uv run python -m sphinx -W --keep-going -b html docs/source docs/_build/html - uv run ruff check src/clearex/gui/app.py tests/gui/test_gui_execution.py tests/io/test_experiment.py - uv run pytest -m "not biohpc" --cov=./ --cov-report=xml
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #16 +/- ##
==========================================
+ Coverage 48.97% 51.67% +2.70%
==========================================
Files 50 52 +2
Lines 17636 20894 +3258
==========================================
+ Hits 8637 10797 +2160
- Misses 8999 10097 +1098
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Fixes CI regressions by making clearex.gui.app safe to import without PyQt6 (while preserving the expected helper surface), repairing a Sphinx-breaking reStructuredText list, and hardening a GPU worker env test against inherited host environment state.
Changes:
- Add import-safe no-op fallbacks for
_apply_application_iconand_show_startup_splashinsrc/clearex/gui/app.py(PyQt6 implementations still override when available). - Add a regression test that imports
clearex.gui.appwith PyQt6 deliberately blocked and asserts helper exports remain present. - Fix docs list indentation in runtime docs and make the Dask GPU worker env test resilient to inherited library path variables.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/clearex/gui/app.py |
Defines always-available fallback GUI helper functions to prevent import-time PyQt6 absence from breaking tests/consumers. |
tests/gui/test_gui_execution.py |
Adds regression coverage for importing the GUI module when PyQt6 is unavailable. |
tests/io/test_experiment.py |
Clears inherited library-path env vars before asserting expected merged env for GPU worker config. |
docs/source/runtime/cli-and-execution.rst |
Fixes malformed nested list indentation that was breaking Sphinx builds. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ) | ||
| """ | ||
| result = subprocess.run( | ||
| [sys.executable, "-c", script], |
There was a problem hiding this comment.
The inline script passed to python -c is indented inside the triple-quoted string (leading spaces before import ...). That will raise IndentationError: unexpected indent in the subprocess. Consider left-aligning the script content or wrapping the string with textwrap.dedent()/inspect.cleandoc() before passing it to -c so the first statement starts at column 0.
| [sys.executable, "-c", script], | |
| [sys.executable, "-c", inspect.cleandoc(script)], |
| """Fallback no-op when PyQt6 is unavailable at import time. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| app : Any | ||
| Placeholder application object. | ||
|
|
||
| Returns | ||
| ------- | ||
| None | ||
| No-op fallback used until the PyQt implementation overrides it. | ||
| """ |
There was a problem hiding this comment.
The docstring says the fallback is used "until the PyQt implementation overrides it", but when HAS_PYQT6 is False the override never happens in the same interpreter session. Consider rephrasing to clarify that these are permanent no-ops in non-PyQt environments, and are only shadowed by the PyQt definitions when PyQt6 is importable during module import.
Summary
This PR fixes the red
mainCI state in both theTestsandDocsworkflows.The main test regression came from
clearex.gui.appexposing_apply_application_iconand_show_startup_splashonly whenPyQt6imported successfully at module import time. On GitHub Actions, the module could be imported in aPyQt6-unavailable state, which left those helpers undefined and caused GUI tests that monkeypatch them to fail.The docs regression came from malformed nested list indentation in the movie runtime-storage section of the runtime docs, which caused the Sphinx build to fail with
Unexpected indentation.What Changed
_apply_application_iconand_show_startup_splashinclearex.gui.app, with the existing PyQt implementations still overriding them when PyQt6 is available.clearex.gui.appwithPyQt6deliberately blocked and asserts that the fallback helper surface still exists.default_transition_frames.Impact
main.Validation
uv run pytest -q tests/gui/test_gui_execution.pyuv run python -m sphinx -W --keep-going -b html docs/source docs/_build/htmluv run ruff check src/clearex/gui/app.py tests/gui/test_gui_execution.py tests/io/test_experiment.pyuv run pytest -m "not biohpc" --cov=./ --cov-report=xml