Skip to content

fix(annotations): module annotations are needed at runtime#1377

Merged
paul-nechifor merged 1 commit intodevfrom
paul/fix/stream-annotations
Feb 28, 2026
Merged

fix(annotations): module annotations are needed at runtime#1377
paul-nechifor merged 1 commit intodevfrom
paul/fix/stream-annotations

Conversation

@paul-nechifor
Copy link
Contributor

@paul-nechifor paul-nechifor commented Feb 28, 2026

Problem

  • See Task: Create base manipulation module #1364 (comment)
  • Annotations weren't read correctly, causing problems with modules and blueprints.
  • When annotations are inside if TYPE_CHECKING, get_type_hints cannot read them.
  • This is currently handled with a hack which reads the types from the parent file, in the hope that they have it.

Closes DIM-636

Solution

Basically remove the hack and just add:

[tool.ruff.lint.flake8-type-checking]
runtime-evaluated-base-classes = ["dimos.core.module.Module"]

...to get ruff to enforce that annotations to modules always use real types, not types behind TYPE_CHECKING.

Breaking Changes

None.

How to Test

In a module file, move the import for In/Out to TYPE_CHECKING. Then run:

pre-commit run --all-files

It should be moved back outside.

Contributor License Agreement

  • I have read and approved the CLA.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 28, 2026

Greptile Summary

This PR fixes a runtime annotation resolution bug in Module subclasses by enforcing — via ruff's flake8-type-checking plugin — that any type used in a class-level In/Out annotation must be imported at runtime rather than hidden behind TYPE_CHECKING. As a result, the fragile MRO-based namespace-collection workaround in module.py could be removed entirely, replaced by a direct get_type_hints(cls, include_extras=True) call.

Key changes:

  • pyproject.toml: adds runtime-evaluated-base-classes = ["dimos.core.module.Module"] so ruff statically enforces the invariant going forward.
  • dimos/core/module.py: removes the sys.modules/MRO namespace hack in both __init_subclass__ and __init__; the simplified get_type_hints call now works because annotation types are guaranteed to be in module globals.
  • Camera and grasping modules (realsense/camera.py, zed/camera.py, graspgen_module.py, grasping.py): Out (and PoseArray where it appears in a class annotation) moved from TYPE_CHECKING to top-level imports; heavy/optional types used only in method signatures correctly remain behind TYPE_CHECKING.
  • bbox.py: Self moved under TYPE_CHECKING — safe because Detection2DBBox is not a Module subclass and from __future__ import annotations means the return annotation is already a string at runtime.

Confidence Score: 5/5

  • This PR is safe to merge — the changes are a clean, targeted fix with no logic regressions.
  • The fix is correct and principled: ruff now statically enforces the invariant, the runtime hack is removed, and all affected Module subclasses have their annotation types promoted to top-level imports. No behavioral changes exist outside of the annotation resolution path, and the exception handler (except (NameError, AttributeError, TypeError): hints = {}) provides a safe fallback for any edge cases not yet caught by the linter.
  • No files require special attention. The bbox.py change (moving Self under TYPE_CHECKING) is safe because Detection2DBBox is not a Module subclass, but reviewers may want to confirm no other code calls get_type_hints on its methods.

Important Files Changed

Filename Overview
pyproject.toml Adds runtime-evaluated-base-classes = ["dimos.core.module.Module"] to ruff's flake8-type-checking config, which is the core of this fix — it instructs ruff to enforce that any type used in a Module subclass annotation must be imported at the top level (not hidden behind TYPE_CHECKING).
dimos/core/module.py Removes the hacky MRO-based global namespace collection (sys.modules traversal) that was used to work around hidden imports. With the ruff rule now enforcing top-level imports for Module annotations, get_type_hints(cls, include_extras=True) can resolve annotations directly from the class module's namespace without the workaround.
dimos/manipulation/grasping/grasping.py Moves Out and PoseArray from TYPE_CHECKING to top-level imports so get_type_hints(GraspingModule) can resolve the grasps: Out[PoseArray] class annotation; PointCloud2 correctly remains behind TYPE_CHECKING as it appears only in method signatures.
dimos/hardware/sensors/camera/zed/camera.py Out promoted to top-level; the entire if TYPE_CHECKING block and its import of typing.TYPE_CHECKING are removed since no other conditional imports existed in that block.
dimos/perception/detection/type/detection2d/bbox.py Self from typing_extensions is moved behind TYPE_CHECKING. Detection2DBBox does not subclass Module, so Module.__init_subclass__ will not call get_type_hints on it, and from __future__ import annotations makes the return annotation a string at runtime — this change is safe but worth noting.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["Module subclass defined\n(e.g. GraspingModule)"] --> B["Module.__init_subclass__ called"]
    B --> C["get_type_hints(cls, include_extras=True)"]
    C --> D{NameError?}
    D -->|"Yes (pre-fix: type behind TYPE_CHECKING)"| E["hints = {}\nIn/Out streams NOT wired"]
    D -->|"No (post-fix: types in module globals)"| F["Iterate annotations"]
    F --> G{origin is In or Out?}
    G -->|Yes| H["setattr(cls, name, None)\n(class-level placeholder for Dask proxy)"]
    G -->|No| I[Skip]

    subgraph "ruff enforcement (new)"
        J["runtime-evaluated-base-classes:\n  dimos.core.module.Module"]
        J --> K["Ruff TCxxx rules: imports used\nin Module annotations\nMUST NOT be behind TYPE_CHECKING"]
    end

    K -.->|guarantees| C
Loading

Last reviewed commit: 3c37fac

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

8 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@paul-nechifor paul-nechifor merged commit ab5e625 into dev Feb 28, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants