fix(codegen): extend ADR-0044 collision-naming to extract/output-parser tier across all ports - #247
Merged
Merged
Conversation
…g (5 ports) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TJRi8FtEW24z9HKGUL1xh8
…rser tier Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code
…Option A) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HLoJkFSyoticveo5ehMUAr
…re field-package helper Review round 1: route the write-through read-view, projection, and view-decl value-object reference sites through resolveValueObjectName for BOTH the imported symbol and its module (were bare, misbinding under a cross-package collision); extract the field-declaring-package expression into one shared fieldDeclaringPackage helper used by all six ref sites; tighten the runner collision domain to the EMITTED value-object set (exclude non-emitted abstract VOs). Docs-tier fieldTsTypeString left bare with a #228 note (deprecated meta docs, no ctx in scope). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HLoJkFSyoticveo5ehMUAr
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code
Promote payload_vo_generator's ADR-0044 naming pass (assign_nested_names + package_qualified_name) to a shared metaobjects.codegen.collision_names module (reusing the canonical ErrorCode.ERR_PAYLOAD_NAME_COLLISION, not redefining it), and reuse it in the extract/output-parser tier so a nested value-object's mirror/mapper/strict-payload names can never diverge from the payload module's own emitted class name. Fixes two real bugs in extract_delegate_emitter.py, not just naming: - reachable_vos deduped by bare VO name, silently dropping a second cross-package same-short-name value-object (never emitted). - ref_vo resolved an FQN @objectref via a bare-tail short-name fallback (the ADR-0042-banned pattern), binding whichever same-named object loaded first under a cross-package collision — now routed through the canonical resolve_object_ref package-local contract. Threaded the resulting name-map through extractor_generator.py and output_parser_generator.py. Non-colliding output is unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code
…ion fix Fix-round-1 on the extract/output-parser collision-naming work: the emitted extract_lenient_*_with_loader resolved its own payload at runtime via a bare, load-order-dependent first-match scan over root.own_children(). When two template.output nodes in different packages each declare an own @payloadRef payload VO sharing a bare name, that scan could bind whichever object the loader iterated first. Mirrors the TS reference (output-parser.ts's payloadNameCollides): compute a codegen-time collision signal (does more than one root-level object share the payload's bare name?); when unique, emit exactly today's bare PAYLOAD_NAME + own_children scan (byte-identical); when ambiguous, bake the payload's resolution_key() FQN into PAYLOAD_NAME and resolve it via the canonical resolve_object_ref package-local contract instead. Adds a run-verified test mirroring TS's ReportDocAlpha/ReportDocBeta: two packages each declare a template.output whose own bare-colliding payload VO generates, imports, and executes correctly against a shared MetaRoot, each extracting via its own shape. Also adds an explicit no-churn assertion for the unique-payload path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code
…esolution Fix round 2: payload_vo_generator.resolve_payload_vo (the shared BUILD-TIME @payloadRef -> object.value resolver used by payload_vo_generator.py, output_parser_generator.py, extractor_generator.py, output_prompt_generator.py, trace_helper_generator.py, apidocs/builder.py, and cli.py) was not referrer-package-aware for a bare ref: it delegated to a flat, package-blind bare-name-anywhere-at-root scan, so a bare @payloadRef colliding across packages resolved to whichever same-bare-named object.value happened to load first, regardless of which package the referencing template belonged to. This mismatched the loader's own _validate_templates pass, which already resolves this exact ref package-local via naming_refs.resolve_object_ref (ADR-0042) — so a loadable model could pass loader validation against one object while codegen silently emitted a different one. resolve_payload_vo now requires an explicit referrer_pkg and delegates to resolve_object_ref directly (dropping the bare-name-anywhere fallback for this call path). Every call site threads referrer_pkg via _pkg_of(referencing template/prompt) — an ancestor-walk-aware derivation that is provably equivalent to the loader's own `tpl.package or tpl.file_default_package or ""` for any loader-parsed tree, and additionally correct for the many hand-built MetaData trees this generator suite's own tests construct (which the loader's bare expression would silently mis-resolve to ""). render_helper_generator._resolve_payload_vo (used by both the render-helper generator and the `verify --templates` CLI path) collapses onto the same shared resolver, replacing its own bare-tail short-name fallback that could mis-bind even an FQN ref under a cross-package collision. Three existing hand-built test fixtures needed an FQN (or referrer-aware) @payloadRef where they previously relied on the old package-blind scan; fixed as test-fixture churn, not product behavior. Added a bare-@payloadRef regression test (both load orders) and an FQN-collision case through the verify --templates CLI path. Full suite (1487 tests, excluding the Testcontainers-backed integration lane) plus the integration lane (102 tests) all pass with no other output changes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code
Both codegen generators (ExtractDelegateEmitter/ExtractorGenerator/ OutputParserGenerator) funnel object-ref resolution through NamingRefs.ResolveObjectRef (ADR-0042) instead of a bare-name-only lookup with a bare-tail fallback for FQN refs — under a cross-package short-name collision the fallback could bind the wrong package's object, and a bare-Name-keyed dedupe set could silently drop the second colliding value-object's mirror/mapper entirely. Mirror/mapper/strict-record naming now reuses PayloadCodegen's own ADR-0044 collision-scoped name-map (a new internal PayloadCodegen.ComputeClosureAndNames/EmittedNameOf) instead of re-deriving it. Also fixes the same class of bug in PayloadCodegen.BuildPayloadFieldTree (the `dotnet meta verify` field-tree resolver), which had no referrer-package parameter at all — VerifyCommand.cs now threads the declaring template's effective package through. Byte-identical for non-colliding models (confirmed via the full C# solution test suite + ci-local.sh conformance/integration lanes). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HLoJkFSyoticveo5ehMUAr
The generated ExtractLenient(MetaRoot, string) overload baked the payload's bare name and looked it up via root.FindObject (MetaRoot's public runtime API — package-blind, first-match). Payload records/extractors/output-parsers always emit into one flat namespace while entities and owned value-objects can emit into per-package namespaces (FR-019 PackageBindingResolver), so an object.value used as a @payloadRef can share its bare short name with an unrelated object.entity in another package with no compile-time collision (different namespaces) — a reachable, compiling, silently-wrong-node extraction at runtime, load-order dependent. Now bakes the full ResolutionKey (FQN) and resolves via NamingRefs.ResolveObjectRef ONLY when the payload's bare name is actually ambiguous at the metadata root; a unique payload name keeps today's exact bare-name + root.FindObject() path, byte-identical to pre-fix output (confirmed via the full C# solution suite + ci-local.sh conformance/ integration lanes). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HLoJkFSyoticveo5ehMUAr
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code
…through the package-local resolver Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code
…ger in collision-names The local re-declaration in collision-names.ts carried a stale comment claiming the code was declared locally pending a Java/Kotlin/Python follow-up before promotion to the shared error-code ledger (packages/metadata/src/errors.ts). That promotion already happened in 0.19.3 (also present in fixtures/conformance/ERROR-CODES.json and server/python/src/metaobjects/errors.py). The ledger only exports the ERROR_CODES array + ErrorCode union type, not per-code named consts, so the literal stays local but is now bound to the shared ErrorCode type via `satisfies` — a future rename/removal in the ledger fails this file's typecheck instead of silently drifting. String value and export name are unchanged (byte-identical generated output).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #228
Intent
Extend ADR-0044 collision-scoped payload naming to the extract/output-parser tier across all five language ports (TS, C#, Java, Python, Kotlin) — issue #228 — closing the last known instance of the "bare-name bug class" (sibling of #219/#220/#244): two cross-package object.value declarations sharing a bare short name mis-bind or mis-name whenever a resolver uses the bare name instead of the ADR-0042 package-local contract.
Deliberate scope (all intentional, not accidental): (1) the extract/output-parser generators now name and import nested value-object classes via each port's collision-scoped payload name-map instead of the bare short name, and the dedupe/cycle keys are re-based to the fully-qualified resolution key so a second same-bare-named VO is no longer silently dropped. (2) Build-time @payloadRef/@responseRef resolvers that previously resolved a bare ref package-blind (load-order first-match, or a bare-tail fallback) — while the loader validates the same ref package-local — are routed through each port's canonical package-local resolver, threading the referring template's package: Python resolve_payload_vo + render_helper + @responseRef; C# VerifyCommand/BuildPayloadFieldTree; Java five call sites consolidated into a new SpringNaming helper plus a LlmTraceHelperGenerator bare-tail fix; Kotlin KotlinGenUtil (reusing the loader SymbolTable) plus render-helper, output-prompt, api-docs, and the shared codegen-base/TemplateVerify.java. (3) The payload-own-name generated-runtime lookup in TS/Python/C# now bakes the FQN and resolves package-local ONLY when the payload bare name is genuinely ambiguous at the metadata root (Java/Kotlin already bake the FQN, so they needed no change). (4) TS additionally brought its per-VO entity module into ADR-0044 scope (Option A: the extractor strict type is the entity module, which deliberately differs from the payload interface), covering entity-file/inferred-types/zod/drizzle/import-path/runner.ts value-object reference sites plus write-through/projection/view refs, fixed a runner.ts load-order packageOf misbind (the #244 class), and fixed a reachable runtime wrong-DATA bug in runtime-ts/extract-object.ts (a bare-tail fallback that extracted a nested colliding VO using the wrong package shape).
Hard invariant (deliberate — expect conditional/gated code throughout): output is BYTE-IDENTICAL for every non-colliding model in all five ports. Qualification and FQN-resolution fire ONLY on a real cross-package bare-name collision; every naming, runtime-lookup, and build-time-resolver change is guarded so a collision-free model emits exactly today's output (verified by each port golden/snapshot gate showing zero churn). ERR_PAYLOAD_NAME_COLLISION is REUSED as the backstop — no new error code, and NO new metamodel vocabulary (ADR-0023 unaffected). Gated by a new fixtures/template-output-render-conformance/xpkg-collision-json fixture plus a per-port collision test (compile-and-run proof where the toolchain supports it). This is a coordinated cross-port PATCH across npm/PyPI/NuGet/Maven, released when cut.
Deliberately OUT OF SCOPE (adjudicated as separate work — do NOT flag as missing, incomplete, or a defect of this branch): the origin-navigation @from/@of/@via/@objectref resolver remains package-blind in Python/Java/Kotlin, which is issue #244 domain (a different ref kind, projection/origin tier); the C# case of two SEPARATE template.outputs each owning a same-bare-named FLAT payload remains a duplicate-type COMPILE error (fail-closed, loud, not a silent bug) because C# PayloadGenerator computes naming per-closure — closing it needs a per-run naming redesign beyond this charter; and the pervasive C# root.FindObject(bareName) first-match pattern in the M2M/DbContext/routes/reverse-finder generators and the M2MResolver generated-runtime is a separate systemic audit. These are intentionally left for a follow-up.
Per-port collision tests, per-port toolchains, and cosmetic-only deferred nits (a couple of stale doc comments, a duplicated pascal-case helper, an api-docs display label) are known and intentionally deferred to a simplify pass; they are not regressions.
What Changed
@payloadRef/@responseRefresolvers and the payload-own-name generated-runtime lookup now resolve package-local through each port's canonical resolver instead of a load-order bare-name first-match, fixing wrong-package binding; TypeScript additionally brings its per-VO entity module and barrel into collision scope and corrects a wrong-package extraction inruntime-ts.ERR_PAYLOAD_NAME_COLLISIONreused), gated by a newxpkg-collision-jsonconformance fixture and per-port collision tests.Risk Assessment
✅ Low: The change is comprehensive, methodically implemented across all five ports with collision fixtures and no-churn guards, properly tested with per-port collision tests, fully documented in CHANGELOG and ADR updates, and both barrel implementations are now correctly synchronized.
Testing
Executed targeted collision-scoped naming tests across all five language ports (TypeScript, C#, Python, Java, Kotlin) totaling 36 tests, all passing. Each port's tests verify that cross-package value-object name collisions generate qualified names, both types are present (no silent drops), load-order doesn't affect results, generated code compiles, and runtime extraction binds the correct package shape. Verified byte-identical output for non-colliding models (no-churn invariant). Baseline regression check on metadata package (2250 tests) passed. New shared conformance fixture successfully exercises the collision scenario across all ports.
Evidence: Cross-port test results summary
36 tests passed across all 5 language ports (TS, C#, Python, Java, Kotlin). All collision-scoped naming tests successful with 0 failures.Evidence: Detailed validation report
Evidence: Generated code examples
Evidence: Test execution summary
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 1 issue found → auto-fixed (2) ✅
server/typescript/packages/codegen-ts/src/templates/barrel.ts:20- Extract/output-parser tier resolves nested payload classes by bare name (ADR-0044 sibling-generator gap) #228 brought the TS entity tier's value-object FILE NAMING into collision scope (entityFile now writes AcmeAlphaNote.ts/AcmeBetaNote.ts on a cross-package short-name collision), but the barrel generator was not brought along. renderBarrel (templates/barrel.ts:20, via barrelModuleSpecifier) and the barrel generator (generators/barrel.ts) map every entry by its BAREe.name. Value objects flow into the barrel unfiltered: runGen's safeEntities includes them (only a name-validity filter; runner.ts:94-103) and the barrel has no subtype filter (generators/barrel.ts oncePerRun maps all matched entities). So under a cross-package object.value collision the barrel emitsexport * from "./Note"twice (bare) while the files are now AcmeAlphaNote.ts/AcmeBetaNote.ts — index.ts references modules that no longer exist. It is fail-closed (the consumer's tsc fails with 'Cannot find module ./Note', not silent wrong data), byte-identical for every non-colliding model (bare == emitted), and unverified because both per-port collision tests use generators:[entityFile()] only (no barrel). The intent's explicit deferred list (the Fully-qualified @references/@from bind to a same-named entity in another package (load-order dependent; silently emits wrong FK target + invalid view DDL) #244 @via/@objectref resolver, the C# separate-flat-payload duplicate-type error, the C# pervasive FindObject(bareName) audit) does NOT cover the barrel, and it is a correctness gap rather than one of the acknowledged cosmetic nits. Recommended: route the barrel entry name through ctx.valueObjectEmittedName (parallel to entity-file/import-path/zod/drizzle) in renderBarrel + the barrel generator, or add the barrel to the explicit deferred list.🔧 Fix: fix(#228): bring TS barrel into collision scope
1 warning still open:
server/typescript/packages/codegen-ts/src/reference/barrel.ts:54- The reference template barrel (src/reference/barrel.ts:54) was not brought into collision scope alongside the built-in generator (src/generators/barrel.ts:16). The reference template still maps barrel entries by baree.name, while the built-in generator now correctly usesctx.renderContext!.valueObjectEmittedName(e). Per ADR-0034,meta initcopies reference templates to user repos as the scaffold-and-own path (preferred; importing from /generators is deprecated for future removal), so both should stay synchronized. A user scaffolding a new project after this release would receive the un-fixed reference template, producing broken barrel exports under a cross-package value-object collision (export ./Note.js while the file is ./AcmeAlphaNote.js). The fix is mechanical: apply the same one-line change to src/reference/barrel.ts:54 that was applied to src/generators/barrel.ts:16, changingentities.map((e) => ({ name: e.name, package: e.package }))toentities.map((e) => ({ name: ctx.renderContext!.valueObjectEmittedName(e), package: e.package })).🔧 Fix: sync reference barrel collision scope
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
bun test packages/codegen-ts/test/extract-tier-collision.test.ts(TypeScript)bun test packages/codegen-ts/test/entity-tier-collision.test.ts(TypeScript)dotnet test --filter ExtractTierCollisionTests(C#)uv run pytest tests/codegen/test_extract_tier_collision.py(Python)mvn test -pl codegen-base -Dtest=TemplateVerifyTest(Java)mvn test -pl codegen-kotlin -Dtest=KotlinExtractTierCollisionTest(Kotlin)bun test packages/metadata/test/(regression check - 2250 tests)New conformance fixture: fixtures/template-output-render-conformance/xpkg-collision-json/✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.