Skip to content

Commit e31c3e9

Browse files
abrichrclaude
andauthored
fix: phantom sibling exports, headless version/doctor; check all seams in CI (#1003)
* 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 <noreply@anthropic.com> * test: give the CI sibling-install check teeth test_external_packages_installed_in_ci was a no-op: it only skipped or passed, so it could never fail and verified nothing. Convert it to a CI-gated assertion that fails if any sibling package is missing - which is the actual risk worth guarding (seam tests silently degrading to skips = the false-green failure mode #999 was about). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent b1ac1d5 commit e31c3e9

4 files changed

Lines changed: 82 additions & 40 deletions

File tree

.github/workflows/main.yml

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,16 @@ jobs:
2929
- name: Install dependencies
3030
run: |
3131
python -m pip install --upgrade pip
32-
# openadapt-ml is installed explicitly so the cross-package
33-
# seam tests (tests/test_import_integrity.py and the cmd_serve
34-
# contract in tests/test_cli_smoke.py) run instead of skipping.
35-
# Issue #999 shipped because this seam was never tested in CI.
36-
pip install -e ".[dev]" openadapt-ml
32+
# Sibling packages are installed explicitly so the
33+
# cross-package seam tests (tests/test_import_integrity.py and
34+
# the cmd_serve contract in tests/test_cli_smoke.py) run
35+
# instead of skipping. Issue #999 shipped because the
36+
# openadapt-ml seam was never tested in CI; the lazy
37+
# __getattr__ in openadapt/__init__.py imports from all of
38+
# these.
39+
pip install -e ".[dev]" openadapt-ml openadapt-capture \
40+
openadapt-evals openadapt-viewer openadapt-grounding \
41+
openadapt-retrieval
3742
3843
- name: Lint with ruff
3944
run: |

openadapt/__init__.py

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -65,12 +65,11 @@ def __getattr__(name: str):
6565
return QwenVLAdapter
6666

6767
# Grounding package (optional)
68-
if name in ("Grounder", "OmniGrounder", "GeminiGrounder"):
68+
if name in ("ElementLocator", "OmniParserClient"):
6969
try:
7070
from openadapt_grounding import ( # noqa: F401
71-
GeminiGrounder,
72-
Grounder,
73-
OmniGrounder,
71+
ElementLocator,
72+
OmniParserClient,
7473
)
7574

7675
return locals()[name]
@@ -81,17 +80,29 @@ def __getattr__(name: str):
8180
)
8281

8382
# Retrieval package (optional)
84-
if name in ("DemoRetriever", "DemoLibrary"):
83+
if name == "MultimodalDemoRetriever":
8584
try:
86-
from openadapt_retrieval import DemoLibrary, DemoRetriever # noqa: F401
85+
from openadapt_retrieval import MultimodalDemoRetriever # noqa: F401
8786

88-
return locals()[name]
87+
return MultimodalDemoRetriever
8988
except ImportError:
9089
raise ImportError(
9190
f"{name} requires openadapt-retrieval. "
9291
"Install with: pip install openadapt[retrieval]"
9392
)
9493

94+
# Demo library lives in openadapt-evals
95+
if name == "DemoLibrary":
96+
try:
97+
from openadapt_evals import DemoLibrary # noqa: F401
98+
99+
return DemoLibrary
100+
except ImportError:
101+
raise ImportError(
102+
f"{name} requires openadapt-evals. "
103+
"Install with: pip install openadapt[evals]"
104+
)
105+
95106
raise AttributeError(f"module 'openadapt' has no attribute '{name}'")
96107

97108

@@ -115,10 +126,10 @@ def __getattr__(name: str):
115126
# From ml
116127
"QwenVLAdapter",
117128
# From grounding (optional)
118-
"Grounder",
119-
"OmniGrounder",
120-
"GeminiGrounder",
129+
"ElementLocator",
130+
"OmniParserClient",
121131
# From retrieval (optional)
122-
"DemoRetriever",
132+
"MultimodalDemoRetriever",
133+
# From evals
123134
"DemoLibrary",
124135
]

openadapt/cli.py

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -468,25 +468,30 @@ def serve(port: int, output: str, open: bool):
468468
@main.command()
469469
def version():
470470
"""Show version information for all packages."""
471+
# Read distribution metadata instead of importing the packages:
472+
# importing executes package code (openadapt-capture takes a
473+
# screenshot at import time, which crashes in headless environments
474+
# like CI), and metadata is what we actually want here.
475+
from importlib.metadata import PackageNotFoundError
476+
from importlib.metadata import version as dist_version
477+
471478
click.echo("OpenAdapt Ecosystem Versions:")
472479
click.echo("=" * 40)
473480

474481
packages = [
475-
("openadapt", "openadapt"),
476-
("openadapt-capture", "openadapt_capture"),
477-
("openadapt-ml", "openadapt_ml"),
478-
("openadapt-evals", "openadapt_evals"),
479-
("openadapt-viewer", "openadapt_viewer"),
480-
("openadapt-grounding", "openadapt_grounding"),
481-
("openadapt-retrieval", "openadapt_retrieval"),
482+
"openadapt",
483+
"openadapt-capture",
484+
"openadapt-ml",
485+
"openadapt-evals",
486+
"openadapt-viewer",
487+
"openadapt-grounding",
488+
"openadapt-retrieval",
482489
]
483490

484-
for name, module in packages:
491+
for name in packages:
485492
try:
486-
mod = __import__(module)
487-
ver = getattr(mod, "__version__", "unknown")
488-
click.echo(f" {name}: {ver}")
489-
except ImportError:
493+
click.echo(f" {name}: {dist_version(name)}")
494+
except PackageNotFoundError:
490495
click.echo(f" {name}: not installed")
491496

492497

@@ -510,11 +515,15 @@ def doctor():
510515
"openadapt_evals",
511516
"openadapt_viewer",
512517
]
518+
from importlib.util import find_spec
519+
513520
for pkg in required:
514-
try:
515-
__import__(pkg)
521+
# find_spec checks installability without executing package code
522+
# (importing openadapt-capture screenshots at import time, which
523+
# crashes headless environments)
524+
if find_spec(pkg) is not None:
516525
click.echo(f" [OK] {pkg}")
517-
except ImportError:
526+
else:
518527
click.echo(f" [MISSING] {pkg}")
519528

520529
# Check optional packages
@@ -524,10 +533,9 @@ def doctor():
524533
"openadapt_retrieval",
525534
]
526535
for pkg in optional:
527-
try:
528-
__import__(pkg)
536+
if find_spec(pkg) is not None:
529537
click.echo(f" [OK] {pkg}")
530-
except ImportError:
538+
else:
531539
click.echo(f" [--] {pkg} (not installed)")
532540

533541
# Check GPU

tests/test_import_integrity.py

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,25 @@
1414

1515
import ast
1616
import importlib.util
17+
import os
1718
from pathlib import Path
1819

1920
import pytest
2021

2122
LOCAL_PACKAGE = "openadapt"
2223
LOCAL_ROOT = Path(__file__).resolve().parent.parent / LOCAL_PACKAGE
2324

24-
EXTERNAL_PACKAGES = ("openadapt_ml",)
25+
# Every sibling package the meta-package imports from (cli.py and the
26+
# lazy __getattr__ in __init__.py). Cross-package checks skip gracefully
27+
# for any that aren't installed; CI installs all of them.
28+
EXTERNAL_PACKAGES = (
29+
"openadapt_ml",
30+
"openadapt_capture",
31+
"openadapt_evals",
32+
"openadapt_viewer",
33+
"openadapt_grounding",
34+
"openadapt_retrieval",
35+
)
2536

2637
PHANTOM_IMPORT_ALLOWLIST: set[tuple[str, str]] = set()
2738

@@ -139,11 +150,18 @@ def _local_modules() -> list[tuple[str, Path]]:
139150

140151

141152
def test_external_packages_installed_in_ci():
142-
"""The cross-package checks are only meaningful with openadapt-ml
143-
present. Locally this may skip; CI installs it."""
144-
for pkg in EXTERNAL_PACKAGES:
145-
if importlib.util.find_spec(pkg) is None:
146-
pytest.skip(f"{pkg} not installed; cross-package checks limited")
153+
"""In CI, every sibling package must be importable so the
154+
cross-package seam checks actually run instead of silently
155+
degrading to skips. (The #999 meta-lesson: a green check that
156+
verifies nothing is worse than no check.) Locally this is allowed
157+
to skip."""
158+
if not os.environ.get("CI"):
159+
pytest.skip("sibling install only enforced in CI")
160+
missing = [p for p in EXTERNAL_PACKAGES if importlib.util.find_spec(p) is None]
161+
assert not missing, (
162+
f"CI must install all sibling packages so the cross-package "
163+
f"seam tests run, but these are not importable: {missing}"
164+
)
147165

148166

149167
def test_no_phantom_imports():

0 commit comments

Comments
 (0)