Skip to content

Commit e06cc51

Browse files
abrichrclaude
andcommitted
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>
1 parent b1ac1d5 commit e06cc51

4 files changed

Lines changed: 69 additions & 35 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: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,17 @@
2121
LOCAL_PACKAGE = "openadapt"
2222
LOCAL_ROOT = Path(__file__).resolve().parent.parent / LOCAL_PACKAGE
2323

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

2636
PHANTOM_IMPORT_ALLOWLIST: set[tuple[str, str]] = set()
2737

0 commit comments

Comments
 (0)