fix(registry): keep consumer-provider attrs under strict attribute scoping (#265) - #268
Merged
Conversation
Fable cross-port investigation: strict scoping's provenance-blind prune (FR-033 B2b) deletes consumer registry.extend() vocabulary. Not "Python only" — Python+C# reject (prune bug), Java/Kotlin accept-too-weakly (consumer path skips scoping), TS is the reference. Fix: stamp provider provenance, prune only library-origin attrs; add Java composeMetamodelRegistry(extras) seam; 4 new provider-composition-conformance fixtures to gate it across all five ports. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: <session-url>
…tures) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: <session-url>
…me, per-port recipes Fable "ready-with-fixes": (R1) new fixtures go in a compose-load/ subdir so the non-recursive un-updated runners don't red on the new shape; (R2) Task 4 reframed overload-first-scaffolding with correct RED (fixtures 1-3 fail under the prune, fixture 4 is a seam regression-lock); (R3) unstamped/build-time defaults to LIBRARY (prunable); (R4) Java two-entry-point diff-stamp (register + extendType); (R5/R6) inline Java+C# strict-load construction recipes; (R7) extend-spec-subtype declares no deps, composeWithCore orders; (R8) core_providers list, per-port from_string calls, field.currency fixtures, expectedError optional. Design doc gains the B2a structural-children residual + convergent base-subtype note. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: <session-url>
…es (TS reference green) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HLoJkFSyoticveo5ehMUAr
…mer extends Strict attr scoping (_apply_strict_attr_scoping, FR-033 B2b) pruned any attr whose name wasn't in the shipped spec allow-list from spec-declared subtypes, blind to who registered it — so it deleted attrs a consumer provider added via registry.extend(). Stamp each attr with the provider id that registered it (TypeRegistry._attr_provenance, set around each provider's register_types() turn in compose_registry) and prune only LIBRARY-origin attrs (unstamped or one of LIBRARY_PROVIDER_IDS); a consumer-origin attr now survives the prune. Extends the provider-composition-conformance runner to cover the new fixtures/provider-composition-conformance/compose-load/ corpus (4 fixtures, shared with the TS reference runner). registry-conformance stays byte-identical (library-only composition is a no-op under the new guard). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HLoJkFSyoticveo5ehMUAr
…extends Strict attr scoping (ApplyStrictAttrScoping, FR-033 B2b) pruned any attr whose name wasn't in the shipped spec allow-list from spec-declared subtypes, blind to who registered it — so it deleted attrs a consumer provider added via registry.Extend(). Stamp each attr with the provider id that registered it (TypeRegistry.CurrentProviderId, set around each provider's RegisterTypes() turn in Provider.ComposeRegistry) and prune only LIBRARY-origin attrs (unstamped or one of the four DefaultRegistry provider ids, now exposed as LibraryProviderIds); a consumer-origin attr now survives the prune. Mirrors the Python fix (dd172d6) — same mechanism. Extends the provider-composition-conformance runner to cover the new fixtures/provider-composition-conformance/compose-load/ corpus (4 fixtures, shared with the TS reference runner). registry-conformance stays byte-identical (library-only composition is a no-op under the new guard); full C# suite green (1509 tests across 4 projects, 1 pre-existing skip). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HLoJkFSyoticveo5ehMUAr
…r onto CoreTypes (C#) Review fix on the prior provenance-scoped strict-attr-prune commit (eb3d896): Registry.cs::ApplyStrictAttrScoping referenced MetaObjects.Loader.MetaDataLoader.LibraryProviderIds, which was the FIRST back-reference from the core MetaObjects composition layer into MetaObjects.Loader — the pre-existing dependency direction was strictly Loader -> Registry/Provider. Python's equivalent constant (LIBRARY_PROVIDER_IDS) is homed in core_types.py, a peer/lower module, not the loader; the original placement here followed the brief's literal "beside DefaultRegistry" wording instead of Python's actual layering. Mechanical relocation, no behavior change: LibraryProviders / LibraryProviderIds now live on CoreTypes (a peer of Registry.cs/Provider.cs, same MetaObjects namespace), right after CoreTypesProvider. MetaDataLoader.DefaultRegistry() reads CoreTypes.LibraryProviders; Registry.cs::ApplyStrictAttrScoping reads CoreTypes.LibraryProviderIds as a same-namespace sibling reference. The sole Registry -> Loader reference is gone; the graph is back to strictly Loader -> Registry/Provider/CoreTypes. Verified no-behavior-change: provider-composition conformance 11/11, registry-conformance byte-match gate 3/3 (Emit_matches_the_committed_cross_port_canonical green), full C# suite unchanged (1509/1510, 1 pre-existing unrelated skip). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HLoJkFSyoticveo5ehMUAr
… seam (Java/Kotlin) Two bugs, one fix. (1) MetaDataRegistry.applyStrictAttrScoping (FR-033 B2b) pruned any attr not in the spec allow-list from a spec-declared subtype's direct AND inherited requirement maps, blind to WHO registered it — deleting a consumer's extendType() addition on a core subtype (e.g. view.currency @dECIMALS). (2) The sanctioned MetaDataLoader.setTypeRegistry(...) extension seam had no "library-set-plus-extras" helper, so a consumer either bypassed spec scoping entirely via raw MetaDataRegistry.compose(...) (a WEAKER strict mode — zero ERR_UNKNOWN_ATTR protection) or hand-assembled the provider list. RegistryManifest gains composeMetamodelRegistry(Collection<MetaDataTypeProvider> extra) — metamodelProviders() + extra, run through the same spec-description + attr-scoping pipeline, unsealed — as the seam adopters use; the no-arg overload now delegates to it. MetaDataRegistry stamps every attr requirement with the provider id that added it (registerProviders sets currentProviderId around each provider's turn; register() blanket-stamps since it always replaces a type's complete definition; extendType() diff-stamps since it rebuilds-and-merges from an already-registered definition — blanket-stamping there would mis-attribute every pre-existing attr and reassign a same-name redefinition's origin). applyStrictAttrScoping's prune now additionally requires library-origin (unstamped, the _LIBRARY sentinel, or one of RegistryManifest's memoized metamodelProviders() ids) — a consumer-origin attr survives. Library-id set lives in the composition layer (RegistryManifest), not the loader, per the layering direction a prior port (C#, d9815ce) had to fix after getting it backwards. ProviderCompositionConformanceTest restructured to @RunWith(Enclosed.class) with two nested Parameterized classes (FlatCorpus: existing 5-fixture corpus, unchanged; ComposeLoad: new 4-fixture fixtures/provider-composition-conformance/ compose-load/ corpus) so both stay under one -Dtest=ProviderCompositionConformanceTest invocation — JUnit4's Parameterized runner allows only one @parameters source per class. Enclosed sweeps every PUBLIC nested class into its suite, so the existing probe classes (CompositionProbeTemplate/SealProbeTemplate) were demoted from public to package-private (verified no external references first). RED baseline (seam + runner only, provenance fix reverted via stash): 2/9 fail — ComposeLoad's registry+strict-load fixtures (decimals pruned, then rejected as ERR_UNKNOWN_ATTR); the typo-rejected and misplaced-core-attr fixtures already passed (neither depends on the prune bug). GREEN after the fix: 9/9. Full metadata module suite: 1276/1276. registry-conformance byte-match (composing zero extras) reconfirmed unchanged. Full JVM reactor compile + test-compile (14 modules, Kotlin included) green — Kotlin needs no separate code, it shares this loader/registry. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HLoJkFSyoticveo5ehMUAr
…trs (Java) Review fix on the prior provenance-scoped-prune commit (8e367fb): stampNewAttrProvenance's "already existed" base set was built from existing.getDirectChildRequirements() (direct-only) but diffed against extended.getDirectChildRequirements(). extended comes from TypeDefinitionBuilder.from(existing).build(), and from() copies existing.getChildRequirements() (direct + INHERITED) into one flat builder map that build() writes into the rebuilt definition's direct requirements (inherited left empty) — so every previously-inherited library attr was seen as "new" and stamped with the extending provider's id. For a consumer provider, that flips isLibraryOrigin false for attrs it never touched, so strict scoping wrongly spared them (e.g. object.base's @Discriminator, inherited onto object.projection and scoped by B2b to object.entity only, would wrongly survive once ANY provider extendType()'d object.projection for an unrelated reason). Latent for the shipped 4-fixture corpus but reachable via the public composeMetamodelRegistry(extra) seam. Fix: the base-set loop now walks existing.getChildRequirements() (direct + inherited), matching how from() actually seeds extended. New regression test (Java-local — this direct/inherited split doesn't exist in the flat Python/C#/TS registries): ProviderCompositionConformanceTest $InheritedAttrProvenanceRegression. Finding a reproducing scenario took real digging — field.currency/@maxlength (the initially-suggested example) does NOT reproduce the bug here, because field.* is a wildcard extends target of three library concern providers (metaobjects-ui/-prompt/-db), each of which flattens+library-stamps every field subtype's inherited attrs via register() (not extendType()) before any consumer's turn, masking the diff-base bug. object.value is masked the same way (metaobjects-prompt extends it with @normalize). The one clean, unmasked target found by grepping every concern provider's extends list plus CoreDBMetaDataProvider's two explicit extendType() calls: object.projection, never touched by any library register()/extendType()/applyProviderExtends call after its own initial registration, yet it still inherits object.base's @Discriminator (scoped to object.entity only). The test extendType()s object.projection with an attr unrelated to @Discriminator, composes via the real composeMetamodelRegistry(extra) seam, and asserts against the registry's declared-attr lookup directly (no loader/document needed, sidestepping object.projection's orthogonal FR-024 authoring-validity rules). Verified RED (temporarily reverted the one-line fix + throwaway debug prints) before GREEN. Full re-verification: ProviderCompositionConformanceTest 10/10, RegistryManifestConformanceTest 3/3 (byte-match unchanged), full metadata module suite 1277/1277 (was 1276), full 14-module JVM reactor compile + test-compile green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HLoJkFSyoticveo5ehMUAr
…odelRegistry seam Documents that extending a spec-declared core subtype via registry.extend() and strict-loading metadata that uses it is supported and conformance-gated (previously forced a whole-file --lax fallback on Python/C#), and that Java adopters composing extra vocabulary should use loader.setTypeRegistry(RegistryManifest.composeMetamodelRegistry(extras)) rather than a raw MetaDataRegistry.compose(...), which skips spec scoping. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HLoJkFSyoticveo5ehMUAr
Hardening flagged by the final whole-branch review, ahead of #267 extending this same compose-load corpus. ComposeLoad.providerCompositionComposeLoad() appended the caught MetaDataException's code AND every code from loader.getErrors() into actualCodes unconditionally — if a future fixture ever hit a code that a validation phase both addError()-recorded and that also terminated the load via throw, the same underlying failure would be double-counted and expectErrors would spuriously fail. Latent for the current 4 fixtures (3 surface ERR_UNKNOWN_ATTR purely via getErrors(), no throw). Fix: capture the thrown exception into a local; add getErrors()'s codes first; add the thrown exception's code only when it is not the exact same object already in getErrors() (MetaDataException has no equals()/hashCode() override, so List#contains here is a reference-identity check — it only suppresses a true double-record-then-throw of the SAME exception instance, never two distinct exceptions that happen to share a .code, keeping the list a sorted multiset comparison like the TS reference runner's expectErrors check). ProviderCompositionConformanceTest still 10/10; RegistryManifestConformanceTest (byte-match) still 3/3 (test-only change). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HLoJkFSyoticveo5ehMUAr
… + Java compose seam
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.
Intent
Fix GitHub #265: strict attribute scoping (FR-033 B2b applyStrictAttrScoping) wrongly pruned attributes a consumer provider added via registry.extend() to a spec-declared core subtype — blind to who registered it — forcing whole-file --lax. It was a THREE-way cross-port divergence on identical input: TypeScript accepted (correct/reference), Python + C# rejected with ERR_UNKNOWN_ATTR (the prune bug), Java/Kotlin accepted but only because the sanctioned consumer registry path skipped spec scoping entirely (a weaker strict mode — a second bug). Fix: stamp each per-type attr with its contributing provider id at registration (compose loop sets currentProviderId around each registerTypes; register/extend record (type,subType,attr)->id in a side map; unstamped/build-time defaults to a LIBRARY sentinel), and the B2b prune drops ONLY library-origin attrs so consumer extensions survive. Java also adds a RegistryManifest.composeMetamodelRegistry(extraProviders) seam so JVM consumer registries (setTypeRegistry) get provenance-safe scoping instead of skipping it; raw MetaDataRegistry.compose is unchanged. TypeScript has NO product change (it is the reference + gate lane). Java's extendType diff-stamp must diff new attr names against existing.getChildRequirements() (direct+inherited) because TypeDefinitionBuilder.from flattens inherited into the rebuilt direct tier — a subtle latent over-stamp fixed + pinned with a Java-local regression (object.projection/@Discriminator). Invariants held and verified per port: NO new error codes; the registry-conformance manifest byte-match is UNCHANGED in every port (the guard only spares consumer attrs; library-only composition is a structural no-op — the library-id set is DERIVED from the same provider list that seeds compose); the strict CHECK stays own-attrs-only (ADR-0039, untouched). Gated by 4 new fixtures in fixtures/provider-composition-conformance/compose-load/ (a NEW subdir invisible to the non-recursive un-updated runners); fixtures 1+4 span all three divergent behaviors. Verified: all 4 fixtures + registry-conformance byte-match green in TS/Python/C#/Java(+Kotlin). Went through full subagent-driven development: per-task reviews (C# fixed a Registry->Loader layering inversion; Java fixed the extendType diff-base over-stamp) + a final whole-branch review (Ready to merge: Yes). PR should Close #265. Coordinated cross-port patch (PyPI + NuGet + Maven; npm reference-only, no product change) when Doug cuts it. Accepted residuals (documented in the design non-goals + docs/features/extending-with-providers.md): the core-attr-NAME ERR_PROVIDER_ATTR_CONFLICT divergence, the B2a structural-children twin, and the convergent base-subtype behavior.
What Changed
registry.extend()to a spec-declared subtype survive. Closes the cross-port divergence (TS accepted; Python + C# rejected withERR_UNKNOWN_ATTR; Java/Kotlin accepted only by skipping spec scoping entirely). In C#,LibraryProviders/LibraryProviderIdsmove off the Loader ontoCoreTypes(fixing a Registry→Loader layering inversion).RegistryManifest.composeMetamodelRegistry(extraProviders)seam so JVM consumer registries get provenance-safe scoping, and fixextendType's diff-base to include inherited attrs soTypeDefinitionBuilder-flattened inherited library attrs aren't mis-stamped as consumer origin.compose-load/subdir of 4 cross-port conformance fixtures and extend the provider-composition suites in every port; no new error codes and the registry-conformance manifest byte-match is unchanged in all ports.Closes #265.
Risk Assessment
✅ Low: A well-bounded, faithfully-implemented cross-port fix with a thorough design: I source-verified the load-bearing invariants (no new error codes; library-only composition stamps every attr library-origin so the B2b prune and registry-conformance manifest are byte-identical; the strict check is untouched; MetaDataTypeId is a record so the provenance lookup does not silently no-op; Java's extendType diff-base correctly uses existing.getChildRequirements() direct+inherited and is pinned by a regression test), found no correctness bug in any port, and the single finding is informational about a niche library-internal path no consumer uses.
Testing
Exercised the #265 fix across every port with focused conformance runs and two before/after bug reproductions. TS reference (11/11, test-only, no product change), Python (11/11, with a provenance-blind-prune revert showing the consumer
decimalsattr was wrongly pruned → ERR_UNKNOWN_ATTR pre-fix and survives post-fix), C# (11/11, registry-inspection + strict-load paths), and Java (10/10, via the new composeMetamodelRegistry seam, with a revert of the one-line extendType diff-base fix reproducing the object.projection/@Discriminator over-stamp). Verified the held invariants directly: registry-conformance manifest byte-match unchanged (Python/C#/Java 3/3 each, expected-registry.json untouched), no new error codes, raw compose() untouched, and the typo/misplaced-attr cases still reject (strict check untouched). All targeted tests pass; working tree left clean.Evidence: Python #265 before/after (prune-bug divergence reproduction)
AFTER #265: declared view.currency attrs = ['locale','decimals']; strict-load @decimals:2 -> [] PASS; typo @decimalz -> ERR_UNKNOWN_ATTR PASS; @maxLength on field.boolean -> ERR_UNKNOWN_ATTR PASS. BEFORE #265 (provenance-blind prune reverted): declared = ['locale'] (decimals pruned); strict-load @decimals:2 -> ['ERR_UNKNOWN_ATTR'] FAIL (the reported divergence). Compose-load fixtures 1+2 fail on reverted code, 3+4 stay green — tests gate the fix.Evidence: Java #265 extendType diff-base over-stamp before/after
Java-local regression InheritedAttrProvenanceRegression.extendingASubtypeDoesNotSpareItsInheritedOutOfScopeAttrs. AFTER (getChildRequirements direct+inherited): 10/10 PASS. BEFORE (getDirectChildRequirements direct-only, reverted): 9/10, regression FAILS 'expected null, but was: <optional child[name=discriminator]>' — inherited @discriminator wrongly survives on object.projection after an unrelated extendType. Bug latent for the shipped 4-fixture corpus (other 9 pass), reachable only via the composeMetamodelRegistry(extra) seam.Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
server/java/metadata/src/main/java/com/metaobjects/registry/MetaDataRegistry.java:478- The provenance stamp has two definition-rebuild entry points that diff asymmetrically. extendType() (line ~379) correctly diffs new attr names againstexisting.getChildRequirements()(direct+inherited) before stamping via stampNewAttrProvenance — so the flattened-inherited library base attrs keep library origin and stay prunable. But the data-driven concern-provider path applyExtendsAttrs() (line 478) routes through register(), whose recordAttrProvenance() blanket-stamps the rebuilt definition's ENTIRE direct-requirement set (which TypeDefinitionBuilder.from(existing) has flattened inherited attrs into) with currentProviderId. For the only current callers — the library's own ui/prompt concern providers, invoked during their own registerTypes turn — currentProviderId is a library id, so the stamp is correct and the registry-conformance manifest stays byte-identical. The gap is latent: applyProviderExtends is public, so a downstream provider that extended a spec subtype through that mechanism (rather than the sanctioned extendType()/extend() path) would mis-stamp its flattened-inherited library attrs as its own and spare them from the B2b prune — the exact over-spare class extendType's diff was added to prevent. No library flow is affected; this documents the coverage boundary of the provenance mechanism on a non-documented, library-internal API.✅ **Test** - passed
✅ No issues found.
cd server/typescript && bun test packages/metadata/test/provider-composition-conformance.test.ts (11 pass, 0 fail — TS reference lane)cd server/python && uv run pytest tests/conformance/test_provider_composition_conformance.py -v (11 passed)cd server/python && uv run python /tmp/repro_265.py (4/4 end-user scenarios PASS: registry holds decimals; strict-load @decimals:2 clean; typo @decimalz rejected; @maxLength on field.boolean rejected)Python BEFORE/AFTER: temporarily reverted spec_metamodel._apply_strict_attr_scoping prune to pre-#265 (direct-only, provenance-blind) form — repro scenarios 1+2 then FAIL (@decimals pruned -> ERR_UNKNOWN_ATTR), 3+4 stay green; restored via git checkout and re-confirmed 11 passedcd server/python && uv run pytest tests/conformance/test_provider_composition_conformance.py -k compose_load with buggy prune reverted -> fixtures 1+2 FAIL (proves tests gate the fix), 3+4 passcd server/csharp && dotnet test MetaObjects.Conformance.Tests --filter ProviderComposition (Passed: 11, Failed: 0)cd server/java && mvn -pl metadata test -Dtest=ProviderCompositionConformanceTest (Tests run: 10, Failures: 0)Java BEFORE/AFTER: reverted stampNewAttrProvenance diff-base existing.getChildRequirements()->getDirectChildRequirements() (commit 0f67aa7b4 one-line fix) -> InheritedAttrProvenanceRegression.extendingASubtypeDoesNotSpareItsInheritedOutOfScopeAttrs FAILS (expected null but was @discriminator), other 9 pass; restored, re-confirmed 10/10cd server/python && uv run pytest tests/conformance/test_registry_conformance.py (3 passed — byte-match UNCHANGED)cd server/csharp && dotnet test MetaObjects.Conformance.Tests --filter RegistryManifestConformance (Passed: 3, Failed: 0)cd server/java && mvn -pl metadata test -Dtest=RegistryManifestConformanceTest (Tests run: 3, Failures: 0)git diff base..target -- fixtures/registry-conformance/ (empty — expected-registry.json untouched)git diff base..target grep for added ERR_ codes not already-existing (empty — NO new error codes)git diff base..target -- server/typescript/ (only the test file changed — NO TS product change)✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.