From e06cc51225cf17c3ab2f06d3fcacff52fa092f1b Mon Sep 17 00:00:00 2001 From: Richard Abrich Date: Fri, 12 Jun 2026 21:02:00 -0400 Subject: [PATCH] fix: phantom grounding/retrieval exports, headless version/doctor; check all sibling seams Extending the #999 guards to every sibling seam the meta-package touches. The extended checks immediately found more live bugs, all fixed here: - openadapt/__init__.py lazy __getattr__ exported Grounder, OmniGrounder, and GeminiGrounder from openadapt-grounding and DemoRetriever/DemoLibrary from openadapt-retrieval. None of these five names exist in those packages; every one always raised. Replaced with the real exports (ElementLocator, OmniParserClient, MultimodalDemoRetriever) and pointed DemoLibrary at openadapt-evals where it actually lives. - `openadapt version` imported each sibling package to read __version__. Importing openadapt-capture takes a screenshot at module scope (recorder.py), which crashes headless environments including CI. Now reads importlib.metadata instead of executing package code. Same fix for `openadapt doctor` (find_spec instead of __import__). - tests/test_import_integrity.py: EXTERNAL_PACKAGES extended from just openadapt_ml to all six sibling packages. - CI installs all six siblings so every seam is checked on every PR. The capture-side import side effect (screenshot at module scope) gets its own fix in openadapt-capture. Co-Authored-By: Claude Fable 5 --- .github/workflows/main.yml | 15 ++++++++---- openadapt/__init__.py | 33 ++++++++++++++++--------- openadapt/cli.py | 44 ++++++++++++++++++++-------------- tests/test_import_integrity.py | 12 +++++++++- 4 files changed, 69 insertions(+), 35 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 045ed6295..ac32d5b7a 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -29,11 +29,16 @@ jobs: - name: Install dependencies run: | python -m pip install --upgrade pip - # openadapt-ml is installed explicitly so the cross-package - # seam tests (tests/test_import_integrity.py and the cmd_serve - # contract in tests/test_cli_smoke.py) run instead of skipping. - # Issue #999 shipped because this seam was never tested in CI. - pip install -e ".[dev]" openadapt-ml + # Sibling packages are installed explicitly so the + # cross-package seam tests (tests/test_import_integrity.py and + # the cmd_serve contract in tests/test_cli_smoke.py) run + # instead of skipping. Issue #999 shipped because the + # openadapt-ml seam was never tested in CI; the lazy + # __getattr__ in openadapt/__init__.py imports from all of + # these. + pip install -e ".[dev]" openadapt-ml openadapt-capture \ + openadapt-evals openadapt-viewer openadapt-grounding \ + openadapt-retrieval - name: Lint with ruff run: | diff --git a/openadapt/__init__.py b/openadapt/__init__.py index 7d6a2a868..87b646e19 100644 --- a/openadapt/__init__.py +++ b/openadapt/__init__.py @@ -65,12 +65,11 @@ def __getattr__(name: str): return QwenVLAdapter # Grounding package (optional) - if name in ("Grounder", "OmniGrounder", "GeminiGrounder"): + if name in ("ElementLocator", "OmniParserClient"): try: from openadapt_grounding import ( # noqa: F401 - GeminiGrounder, - Grounder, - OmniGrounder, + ElementLocator, + OmniParserClient, ) return locals()[name] @@ -81,17 +80,29 @@ def __getattr__(name: str): ) # Retrieval package (optional) - if name in ("DemoRetriever", "DemoLibrary"): + if name == "MultimodalDemoRetriever": try: - from openadapt_retrieval import DemoLibrary, DemoRetriever # noqa: F401 + from openadapt_retrieval import MultimodalDemoRetriever # noqa: F401 - return locals()[name] + return MultimodalDemoRetriever except ImportError: raise ImportError( f"{name} requires openadapt-retrieval. " "Install with: pip install openadapt[retrieval]" ) + # Demo library lives in openadapt-evals + if name == "DemoLibrary": + try: + from openadapt_evals import DemoLibrary # noqa: F401 + + return DemoLibrary + except ImportError: + raise ImportError( + f"{name} requires openadapt-evals. " + "Install with: pip install openadapt[evals]" + ) + raise AttributeError(f"module 'openadapt' has no attribute '{name}'") @@ -115,10 +126,10 @@ def __getattr__(name: str): # From ml "QwenVLAdapter", # From grounding (optional) - "Grounder", - "OmniGrounder", - "GeminiGrounder", + "ElementLocator", + "OmniParserClient", # From retrieval (optional) - "DemoRetriever", + "MultimodalDemoRetriever", + # From evals "DemoLibrary", ] diff --git a/openadapt/cli.py b/openadapt/cli.py index d707f6149..cfc69dbb9 100644 --- a/openadapt/cli.py +++ b/openadapt/cli.py @@ -468,25 +468,30 @@ def serve(port: int, output: str, open: bool): @main.command() def version(): """Show version information for all packages.""" + # Read distribution metadata instead of importing the packages: + # importing executes package code (openadapt-capture takes a + # screenshot at import time, which crashes in headless environments + # like CI), and metadata is what we actually want here. + from importlib.metadata import PackageNotFoundError + from importlib.metadata import version as dist_version + click.echo("OpenAdapt Ecosystem Versions:") click.echo("=" * 40) packages = [ - ("openadapt", "openadapt"), - ("openadapt-capture", "openadapt_capture"), - ("openadapt-ml", "openadapt_ml"), - ("openadapt-evals", "openadapt_evals"), - ("openadapt-viewer", "openadapt_viewer"), - ("openadapt-grounding", "openadapt_grounding"), - ("openadapt-retrieval", "openadapt_retrieval"), + "openadapt", + "openadapt-capture", + "openadapt-ml", + "openadapt-evals", + "openadapt-viewer", + "openadapt-grounding", + "openadapt-retrieval", ] - for name, module in packages: + for name in packages: try: - mod = __import__(module) - ver = getattr(mod, "__version__", "unknown") - click.echo(f" {name}: {ver}") - except ImportError: + click.echo(f" {name}: {dist_version(name)}") + except PackageNotFoundError: click.echo(f" {name}: not installed") @@ -510,11 +515,15 @@ def doctor(): "openadapt_evals", "openadapt_viewer", ] + from importlib.util import find_spec + for pkg in required: - try: - __import__(pkg) + # find_spec checks installability without executing package code + # (importing openadapt-capture screenshots at import time, which + # crashes headless environments) + if find_spec(pkg) is not None: click.echo(f" [OK] {pkg}") - except ImportError: + else: click.echo(f" [MISSING] {pkg}") # Check optional packages @@ -524,10 +533,9 @@ def doctor(): "openadapt_retrieval", ] for pkg in optional: - try: - __import__(pkg) + if find_spec(pkg) is not None: click.echo(f" [OK] {pkg}") - except ImportError: + else: click.echo(f" [--] {pkg} (not installed)") # Check GPU diff --git a/tests/test_import_integrity.py b/tests/test_import_integrity.py index a8124267c..74ccf65d8 100644 --- a/tests/test_import_integrity.py +++ b/tests/test_import_integrity.py @@ -21,7 +21,17 @@ LOCAL_PACKAGE = "openadapt" LOCAL_ROOT = Path(__file__).resolve().parent.parent / LOCAL_PACKAGE -EXTERNAL_PACKAGES = ("openadapt_ml",) +# Every sibling package the meta-package imports from (cli.py and the +# lazy __getattr__ in __init__.py). Cross-package checks skip gracefully +# for any that aren't installed; CI installs all of them. +EXTERNAL_PACKAGES = ( + "openadapt_ml", + "openadapt_capture", + "openadapt_evals", + "openadapt_viewer", + "openadapt_grounding", + "openadapt_retrieval", +) PHANTOM_IMPORT_ALLOWLIST: set[tuple[str, str]] = set()