From 9b4bd0d1ea5a24ccb76c960ac50bed749e588e22 Mon Sep 17 00:00:00 2001 From: Doug Mealing Date: Sun, 14 Jun 2026 11:58:17 -0400 Subject: [PATCH] fix(codegen): resolve @objectRef to bare class name + use resolution_key for header FQN Two multi-file / nested-ref codegen correctness fixes: - @objectRef is FQN-expanded at load time; the value/entity generator emitted list[pkg::Thing] and a 'from .pkg::Thing import' line (invalid Python). The type annotation and relative import now use the bare class name. - The four header-FQN helpers did a naive nearest-ancestor package walk, so after a multi-file merge every object folded onto the alphabetically-first file's package. Route them all through MetaData.resolution_key() (own -> file-default -> ancestor), which already handles the merged-tree case. Regression tests added for both. Co-Authored-By: Claude Opus 4.8 --- .../codegen/generators/entity_model.py | 19 +++++------ .../generators/filter_allowlist_generator.py | 12 +++---- .../generators/payload_vo_generator.py | 22 ++++++------- .../codegen/generators/router_generator.py | 12 +++---- .../src/metaobjects/codegen/type_map.py | 5 ++- .../python/tests/codegen/test_entity_model.py | 32 +++++++++++++++++++ 6 files changed, 63 insertions(+), 39 deletions(-) diff --git a/server/python/src/metaobjects/codegen/generators/entity_model.py b/server/python/src/metaobjects/codegen/generators/entity_model.py index b2b058ff8..bf757a3cd 100644 --- a/server/python/src/metaobjects/codegen/generators/entity_model.py +++ b/server/python/src/metaobjects/codegen/generators/entity_model.py @@ -112,7 +112,10 @@ def _field_line(field: MetaField, imports: set[str], config: GenConfig) -> tuple if field.sub_type == fc.FIELD_SUBTYPE_OBJECT: ref = field.attr(fc.FIELD_ATTR_OBJECT_REF) if ref: - imports.add(f"from .{ref} import {ref}") + # @objectRef is FQN-expanded at load time; the generated VOs live + # flat in one package, so import by the bare class name. + ref_name = str(ref).split("::")[-1] + imports.add(f"from .{ref_name} import {ref_name}") required = field.attr(fc.FIELD_ATTR_REQUIRED) is True default_raw = field.attr(fc.FIELD_ATTR_DEFAULT) has_default = default_raw is not None @@ -152,14 +155,12 @@ def _field_line(field: MetaField, imports: set[str], config: GenConfig) -> tuple def _effective_fqn(entity: MetaObject) -> str: - """`package::name`, resolving the package from the nearest ancestor that carries - one (objects inherit the file/root package). Falls back to the bare name.""" - pkg = entity.package - parent = entity.parent - while pkg is None and parent is not None: - pkg = parent.package - parent = parent.parent - return f"{pkg}{PACKAGE_SEP}{entity.name}" if pkg else entity.name + """`package::name` via the canonical :meth:`MetaData.resolution_key` — own + package, else the file-default package (captured at parse), else the nearest + ancestor's. The ``file_default_package`` step is load-bearing after a multi-file + merge: every node hangs under one package-less merged root, so the ancestor walk + alone would fold each object onto the alphabetically-first file's package.""" + return entity.resolution_key() class EntityModelGenerator: diff --git a/server/python/src/metaobjects/codegen/generators/filter_allowlist_generator.py b/server/python/src/metaobjects/codegen/generators/filter_allowlist_generator.py index 30aa37f3a..e2fb28829 100644 --- a/server/python/src/metaobjects/codegen/generators/filter_allowlist_generator.py +++ b/server/python/src/metaobjects/codegen/generators/filter_allowlist_generator.py @@ -57,14 +57,10 @@ def _effective_fqn(entity: MetaObject) -> str: - """``package::name``, resolving package from the nearest ancestor that carries - one. Mirror of the router generator's helper of the same name.""" - pkg = entity.package - parent = entity.parent - while pkg is None and parent is not None: - pkg = parent.package - parent = parent.parent - return f"{pkg}{PACKAGE_SEP}{entity.name}" if pkg else entity.name + """``package::name`` via the canonical :meth:`MetaData.resolution_key` (own + package, else file-default, else ancestor) — multi-file-merge safe. Mirror of + the router generator's helper of the same name.""" + return entity.resolution_key() def _primary_source_rdb(entity: MetaObject) -> MetaSource | None: diff --git a/server/python/src/metaobjects/codegen/generators/payload_vo_generator.py b/server/python/src/metaobjects/codegen/generators/payload_vo_generator.py index 302e0f31e..7b8141859 100644 --- a/server/python/src/metaobjects/codegen/generators/payload_vo_generator.py +++ b/server/python/src/metaobjects/codegen/generators/payload_vo_generator.py @@ -571,19 +571,15 @@ def render_payload_vo( def _effective_fqn_for(template: MetaTemplate, payload: MetaObject) -> str: - """``package::name`` for the doc-header. Prefer the template's own package - chain; fall back to the payload's; finally the bare template name.""" - - def _walk(node: MetaData) -> str | None: - pkg = node.package - parent = node.parent - while pkg is None and parent is not None: - pkg = parent.package - parent = parent.parent - return pkg - - pkg = _walk(template) or _walk(payload) - return f"{pkg}{PACKAGE_SEP}{template.name}" if pkg else template.name + """``package::name`` for the doc-header, via the canonical + :meth:`MetaData.resolution_key` (own package, else the file-default captured at + parse, else the nearest ancestor) — multi-file-merge safe. Falls back to the + payload's package only if the template resolves to a bare (package-less) name.""" + key = template.resolution_key() + if PACKAGE_SEP in key: + return key + payload_pkg = payload.package or payload.file_default_package + return f"{payload_pkg}{PACKAGE_SEP}{template.name}" if payload_pkg else template.name # --------------------------------------------------------------------------- diff --git a/server/python/src/metaobjects/codegen/generators/router_generator.py b/server/python/src/metaobjects/codegen/generators/router_generator.py index 293f6ec67..981d76682 100644 --- a/server/python/src/metaobjects/codegen/generators/router_generator.py +++ b/server/python/src/metaobjects/codegen/generators/router_generator.py @@ -54,14 +54,10 @@ def _effective_fqn(entity: MetaObject) -> str: - """``package::name``, resolving package from the nearest ancestor that carries - one. Mirrors the entity-model generator's helper of the same name.""" - pkg = entity.package - parent = entity.parent - while pkg is None and parent is not None: - pkg = parent.package - parent = parent.parent - return f"{pkg}{PACKAGE_SEP}{entity.name}" if pkg else entity.name + """``package::name`` via the canonical :meth:`MetaData.resolution_key` (own + package, else file-default, else ancestor) — multi-file-merge safe. Mirrors the + entity-model generator's helper of the same name.""" + return entity.resolution_key() def _primary_source_rdb(entity: MetaObject) -> MetaSource | None: diff --git a/server/python/src/metaobjects/codegen/type_map.py b/server/python/src/metaobjects/codegen/type_map.py index 867d5c58d..b6a101303 100644 --- a/server/python/src/metaobjects/codegen/type_map.py +++ b/server/python/src/metaobjects/codegen/type_map.py @@ -67,7 +67,10 @@ def py_type_for(field: MetaField) -> PyType: ``str``.""" if field.sub_type == fc.FIELD_SUBTYPE_OBJECT: ref = field.attr(fc.FIELD_ATTR_OBJECT_REF) - base = PyType(str(ref)) if ref else PyType("object") + # @objectRef is expanded to a package-qualified FQN at load time + # (e.g. ``app::pkg::Thing``); the emitted VOs all live flat in one + # generated package, so type by the bare class name. + base = PyType(str(ref).split("::")[-1]) if ref else PyType("object") elif field.sub_type == fc.FIELD_SUBTYPE_ENUM: values = effective_enum_values(field) if values: diff --git a/server/python/tests/codegen/test_entity_model.py b/server/python/tests/codegen/test_entity_model.py index 1234ed098..1c02ae7c0 100644 --- a/server/python/tests/codegen/test_entity_model.py +++ b/server/python/tests/codegen/test_entity_model.py @@ -74,6 +74,38 @@ def test_nested_object_array_imports_ref() -> None: assert "posts: list[PostBrief] | None = None" in out +def test_header_fqn_folds_file_default_package_after_merge() -> None: + """After a multi-file merge an object hangs under one package-less merged + root, so the naive nearest-ancestor walk would fold every object onto the + alphabetically-first file's package. The header FQN must instead use the + canonical ``resolution_key`` (own → file-default → ancestor), so an object + keeps its true source-file package regardless of merge order.""" + from metaobjects.codegen.generators.entity_model import _effective_fqn + + obj = _entity("ClaimComplexity", [_f("confidence", fc.FIELD_SUBTYPE_DOUBLE)]) + # Post-merge shape: no own package, captured file-default from its real file, + # parent is the merged super-root carrying a DIFFERENT (first-file) package. + obj.file_default_package = "app::reasoning" + merged_root = _entity("_merged", [], package="app::orchestration") + merged_root.add_child(obj) + + assert _effective_fqn(obj) == "app::reasoning::ClaimComplexity" + + +def test_object_ref_fqn_is_shortened_to_class_name() -> None: + """@objectRef is FQN-expanded at load time (``app::pkg::Thing``); the type + annotation and the relative import must use the bare class name, not the + raw FQN (which is not valid Python). Regression: object.value VOs emitted + ``list[app::pkg::Thing]`` and ``from .app::pkg::Thing import ...``.""" + e = _entity("Catalog", [ + _f("tools", fc.FIELD_SUBTYPE_OBJECT, object_ref="myapp::orch::ToolDescriptor", is_array=True), + ]) + out = render_entity_model(e) + assert "from .ToolDescriptor import ToolDescriptor" in out + assert "tools: list[ToolDescriptor] | None = None" in out + assert "::" not in out + + def test_field_default_is_emitted_as_literal() -> None: """@default renders a Python-literal default and makes the field non-Optional.""" flag = MetaField(TYPE_FIELD, fc.FIELD_SUBTYPE_BOOLEAN, "flag")