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..1fa81da07 100644 --- a/tests/test_import_integrity.py +++ b/tests/test_import_integrity.py @@ -14,6 +14,7 @@ import ast import importlib.util +import os from pathlib import Path import pytest @@ -21,7 +22,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() @@ -139,11 +150,18 @@ def _local_modules() -> list[tuple[str, Path]]: def test_external_packages_installed_in_ci(): - """The cross-package checks are only meaningful with openadapt-ml - present. Locally this may skip; CI installs it.""" - for pkg in EXTERNAL_PACKAGES: - if importlib.util.find_spec(pkg) is None: - pytest.skip(f"{pkg} not installed; cross-package checks limited") + """In CI, every sibling package must be importable so the + cross-package seam checks actually run instead of silently + degrading to skips. (The #999 meta-lesson: a green check that + verifies nothing is worse than no check.) Locally this is allowed + to skip.""" + if not os.environ.get("CI"): + pytest.skip("sibling install only enforced in CI") + missing = [p for p in EXTERNAL_PACKAGES if importlib.util.find_spec(p) is None] + assert not missing, ( + f"CI must install all sibling packages so the cross-package " + f"seam tests run, but these are not importable: {missing}" + ) def test_no_phantom_imports():