refactor(registry): populate _module_meta in register/register_internal too

Cross-repo alignment with apcore-typescript: make python's manual
register() and register_internal() entry points populate _module_meta
via merge_module_metadata, just like _register_in_order (the discovery
path). After this change every registered module — discovery, manual,
internal — has a fully merged metadata dict in _module_meta, so
get_definition() can read uniformly from `meta` without fallback.

Changes:

- Registry.register() — pre-compute merge_module_metadata(module, metadata or {})
  outside the lock, store under the lock alongside _modules /
  _lowercase_map, and pop on on_load() rollback.

- Registry.register_internal() — same pre-compute + store, with
  meta={} since the privileged sys-module path supplies no YAML.

- Registry._register_in_order() — pass the instance (not the class)
  to merge_module_metadata, matching the new uniform contract. The
  instance picks up __init__-set attributes, and getattr falls
  through to class attrs anyway, so this is a strict superset of
  the prior class-only pass.

- get_definition() — drop the `if "annotations" in meta else
  getattr(module, ...)` fallback chain. With the invariant above,
  meta always has all canonical keys, so reads collapse to plain
  meta.get(...) calls. Mirrors apcore-typescript Registry.getDefinition
  after its 64491b7 cleanup.

merge_module_metadata robustness fixes (uncovered by the test suite
when this change widened the set of inputs reaching it):

- Add _coerce_code_annotations helper that narrows the raw
  `module.annotations` attribute to ModuleAnnotations | None. Real
  ModuleAnnotations instances pass through; dict-style annotations
  get filtered to canonical fields and re-constructed; everything
  else (None, MagicMock test stubs, custom objects) becomes None
  instead of crashing merge_annotations downstream.

- Defensive isinstance() guards on every code-side fallback so a
  test stub returning a MagicMock for `description`/`name`/`tags`/
  `version`/`metadata`/`documentation` does not propagate through
  the merged dict.

- Accept either an instance or a class as the first parameter
  (renamed from `module_class` to `module` to reflect this).

Tests: pytest 2252 pass / 2 xfail. pyright clean. ruff + black clean.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: tercel

src/apcore/registry/metadata.py

@@ -2,13 +2,15 @@
 
 from __future__ import annotations
 
+import dataclasses
 import logging
 from pathlib import Path
 from typing import Any
 
 import yaml
 
 from apcore.errors import ConfigError, ConfigNotFoundError
+from apcore.module import ModuleAnnotations
 from apcore.registry.types import DependencyInfo
 from apcore.schema.annotations import merge_annotations, merge_examples
 
@@ -64,44 +66,76 @@ def parse_dependencies(deps_raw: list[dict[str, Any]]) -> list[DependencyInfo]:
     return result
 
 
-def merge_module_metadata(module_class: type, meta: dict[str, Any]) -> dict[str, Any]:
+def _coerce_code_annotations(raw: Any) -> ModuleAnnotations | None:
+    """Narrow a module's `annotations` attribute to ``ModuleAnnotations | None``.
+
+    Modules express annotations in three forms in the wild:
+
+    - ``ModuleAnnotations(...)`` instance — pass through.
+    - ``dict[str, Any]`` (dict-style annotations) — filter to canonical
+      field names and construct a ModuleAnnotations.
+    - Anything else (None, MagicMock test stubs, custom objects) —
+      return None so :func:`merge_annotations` does not crash on
+      ``getattr`` chasing nonexistent fields.
+    """
+    if isinstance(raw, ModuleAnnotations):
+        return raw
+    if isinstance(raw, dict):
+        valid = {f.name for f in dataclasses.fields(ModuleAnnotations)}
+        return ModuleAnnotations(**{k: v for k, v in raw.items() if k in valid})
+    return None
+
+
+def merge_module_metadata(module: Any, meta: dict[str, Any]) -> dict[str, Any]:
     """Merge YAML metadata over code-level attributes per spec §4.13.
 
+    Accepts either a module instance OR a class. ``getattr`` resolves
+    instance-level attributes first and then falls through to class
+    attributes via Python's normal lookup chain, so passing the instance
+    correctly handles modules that set their version/annotations/etc. in
+    ``__init__``.
+
     Scalar fields (description, name, tags, version, documentation) follow
     "YAML wins, code is the fallback". The ``annotations`` field uses
     field-level merging via :func:`apcore.schema.merge_annotations` so a YAML
     annotation override does not blow away unrelated code-set flags. The
     ``examples`` field uses :func:`apcore.schema.merge_examples` (YAML wins
     fully when present). The ``metadata`` field is a shallow dict merge.
     """
-    code_desc = getattr(module_class, "description", "")
-    code_name = getattr(module_class, "name", None)
-    code_tags = getattr(module_class, "tags", [])
-    code_version = getattr(module_class, "version", "1.0.0")
-    code_annotations = getattr(module_class, "annotations", None)
-    code_examples = getattr(module_class, "examples", [])
-    code_metadata = getattr(module_class, "metadata", {})
-    code_docs = getattr(module_class, "documentation", None)
+    code_desc = getattr(module, "description", "")
+    code_name = getattr(module, "name", None)
+    code_tags = getattr(module, "tags", [])
+    code_version = getattr(module, "version", "1.0.0")
+    code_annotations = _coerce_code_annotations(getattr(module, "annotations", None))
+    code_examples = getattr(module, "examples", [])
+    code_metadata = getattr(module, "metadata", {})
+    code_docs = getattr(module, "documentation", None)
 
     yaml_metadata = meta.get("metadata", {})
     merged_metadata = {**(code_metadata or {}), **(yaml_metadata or {})}
 
     yaml_annotations = meta.get("annotations")
-    merged_annotations: Any
+    merged_annotations: ModuleAnnotations | None
     if yaml_annotations is None and code_annotations is None:
         merged_annotations = None
     else:
         merged_annotations = merge_annotations(yaml_annotations, code_annotations)
 
+    # Defend against unusable code_examples (e.g. MagicMock test stubs):
+    # only forward when it is actually a list, otherwise treat as absent.
+    safe_code_examples = code_examples if isinstance(code_examples, list) else None
+
     return {
-        "description": meta.get("description") or code_desc,
-        "name": meta.get("name") or code_name,
-        "tags": meta.get("tags") if meta.get("tags") is not None else (code_tags or []),
-        "version": meta.get("version") or code_version,
+        "description": meta.get("description") or (code_desc if isinstance(code_desc, str) else ""),
+        "name": meta.get("name") or (code_name if isinstance(code_name, (str, type(None))) else None),
+        "tags": (
+            meta.get("tags") if meta.get("tags") is not None else (code_tags if isinstance(code_tags, list) else [])
+        ),
+        "version": meta.get("version") or (code_version if isinstance(code_version, str) else "1.0.0"),
         "annotations": merged_annotations,
-        "examples": merge_examples(meta.get("examples"), code_examples or None),
-        "metadata": merged_metadata,
-        "documentation": meta.get("documentation") or code_docs,
+        "examples": merge_examples(meta.get("examples"), safe_code_examples),
+        "metadata": merged_metadata if isinstance(merged_metadata, dict) else {},
+        "documentation": meta.get("documentation") or (code_docs if isinstance(code_docs, str) else None),
     }
 
 

src/apcore/registry/registry.py

@@ -473,7 +473,10 @@ def _register_in_order(
                 logger.error("Failed to instantiate module '%s': %s", mod_id, e)
                 continue
 
-            merged_meta = merge_module_metadata(cls, meta)
+            # Pass the instance (not the class) so __init__-set attributes
+            # are picked up; getattr falls through to class attrs anyway.
+            # Aligned with manual register() / register_internal().
+            merged_meta = merge_module_metadata(module, meta)
             with self._lock:
                 self._modules[mod_id] = module
                 self._module_meta[mod_id] = merged_meta
@@ -539,6 +542,17 @@ def register(
 
         is_versioned = version is not None
 
+        # Pre-compute the merged metadata view OUTSIDE the lock so the
+        # critical section stays minimal. merge_module_metadata is pure
+        # (no I/O, no shared state). Pass the *instance* so getattr()
+        # picks up instance-level attribute overrides (e.g. modules that
+        # set self.version in __init__) and still falls back to class
+        # attributes via Python's normal lookup chain. This populates
+        # _module_meta even for the manual register() path, matching
+        # apcore-typescript Registry.register() so get_definition()
+        # never has to fall back to the raw module object.
+        merged_meta = merge_module_metadata(module, metadata or {})
+
         with self._lock:
             # For explicit versioned registration, skip conflict check when
             # the module_id already exists (we allow multiple versions).
@@ -568,6 +582,7 @@ def register(
             latest = self._versioned_modules.get_latest(module_id)
             if latest is not None:
                 self._modules[module_id] = latest
+            self._module_meta[module_id] = merged_meta
             self._lowercase_map[module_id.lower()] = module_id
 
         # Call on_load if available
@@ -580,6 +595,7 @@ def register(
                     self._versioned_meta.remove(module_id, effective_version)
                     if not self._versioned_modules.has(module_id):
                         self._modules.pop(module_id, None)
+                        self._module_meta.pop(module_id, None)
                 raise
 
         self._trigger_event("register", module_id, module)
@@ -728,19 +744,23 @@ def get_definition(self, module_id: str, version_hint: str | None = None) -> Mod
         if deprecation:
             sunset_date = deprecation.get("sunset_date")
 
-        # `meta` is populated by merge_module_metadata at registration time and
-        # already implements spec §4.13 (YAML > code > defaults). Read from it
-        # directly so YAML annotations and examples are honored.
+        # INVARIANT: every registration site (`register`, `register_internal`,
+        # `_register_in_order`) populates `_module_meta` via
+        # `merge_module_metadata`, so `meta` always contains the full set of
+        # canonical keys including the merged `annotations` slot. Read all
+        # merged-meta fields straight from it. Schemas come straight from
+        # the module instance because they are not part of the merged
+        # metadata payload. Aligned with apcore-typescript Registry.getDefinition.
         return ModuleDescriptor(
             module_id=module_id,
-            name=meta.get("name") or getattr(module, "name", None),
-            description=meta.get("description") or getattr(module, "description", ""),
-            documentation=meta.get("documentation") or getattr(module, "documentation", None),
+            name=meta.get("name"),
+            description=meta.get("description") or "",
+            documentation=meta.get("documentation"),
             input_schema=input_json,
             output_schema=output_json,
-            version=meta.get("version") or getattr(module, "version", "1.0.0"),
-            tags=list(meta.get("tags") or getattr(module, "tags", None) or []),
-            annotations=meta.get("annotations") if "annotations" in meta else getattr(module, "annotations", None),
+            version=meta.get("version") or "1.0.0",
+            tags=list(meta.get("tags") or []),
+            annotations=meta.get("annotations"),
             examples=list(meta.get("examples") or []),
             metadata=effective_metadata,
             sunset_date=sunset_date,
@@ -1127,13 +1147,21 @@ def register_internal(self, module_id: str, module: Any) -> None:
         _validate_module_id(module_id, allow_reserved=True)
 
         _ensure_schema_adapter(module)
+
+        # Mirror register(): populate _module_meta via the same merge path
+        # so get_definition() reads merged metadata uniformly regardless of
+        # which registration entry point was used. Pass the instance so
+        # __init__-set attributes are picked up.
+        merged_meta = merge_module_metadata(module, {})
+
         with self._lock:
             if module_id in self._modules:
                 # Aligned with apcore-typescript / apcore-rust and the canonical
                 # message produced by detect_id_conflicts for the public
                 # `register()` path.
                 raise InvalidInputError(message=f"Module ID '{module_id}' is already registered")
             self._modules[module_id] = module
+            self._module_meta[module_id] = merged_meta
             self._lowercase_map[module_id.lower()] = module_id
         self._trigger_event("register", module_id, module)