resolver: fix provenance and resolution of attributes inherited from dependencies - #1669
resolver: fix provenance and resolution of attributes inherited from dependencies#1669lmolkova wants to merge 3 commits into
Conversation
Attributes reaching a group through a refinement arrive as full definitions (AttributeSpec::Id) built from the dependency's resolved signal, which lost the fact that they came from a dependency. They were then recorded as defined by the refining group, so the resolved schema attributed them to the local file and dropped their source registry. Carry the defining registry on UnresolvedAttribute, set it when building dependency group summaries, propagate it through inheritance, and use it when registering the root attribute.
An attribute inherited through a refinement enters resolution as a full definition (AttributeSpec::Id), so it registered itself as the root attribute for its key. Two consequences: a bare `ref` elsewhere in the registry resolved against it even though the registry has no route to the real definition, and it resolved to the refinement's own copy, silently picking up a signal-local brief for an attribute another registry owns. Record such attributes as non-definitions. They keep their provenance, so resolved schemas are unchanged, but references no longer resolve against them and a definition always wins a conflict against one. A reference that only worked through this path now fails, which is the honest answer: a published registry exposes an attribute it inherited rather than defined only through the signals that carry it.
V1Schema::lookup_attribute falls back to scanning resolved groups when a key is not a definition. That scan finds attributes a refinement inherited, hands them out as definitions, and takes the source from the refining group - so a dependency consumed from source re-exported an attribute it does not own, and attributed it to itself instead of the registry that defines it. Stop at the fallback when the schema knows the key but knows it is not a definition. The scan stays for schemas without root attributes, such as ones deserialized from a published artifact. A registry now resolves the same way whether its dependency is consumed from source or from a package.
Pull request dashboard statusWaiting on the author · refreshed 2026-08-05 05:17 UTC Respond to 1 review item (e.g. link a commit, explain why not, ask a follow-up):
Status above doesn't look right?
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1669 +/- ##
=====================================
Coverage 81.4% 81.4%
=====================================
Files 131 131
Lines 11714 11754 +40
=====================================
+ Hits 9538 9577 +39
- Misses 2176 2177 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR fixes attribute provenance and reference resolution when attributes are inherited through dependency refinements, ensuring provenance points to the true defining registry and preventing “inherited-through-refinement” attribute instances from being treated as definitions a bare ref can resolve against. It also aligns behavior between dependencies consumed from source and from published artifacts, as described in issue #1658.
Changes:
- Track an attribute’s defining registry (“origin”) through inheritance/refinement so resolved provenance is attributed to the correct schema URL.
- Prevent inherited-through-refinement attributes from becoming root definitions usable for bare
refresolution, avoiding incorrect resolution against a refinement’s local copy. - Add regression tests + fixtures covering transitive provenance, “inherited is not a definition”, and v1-dependency provenance.
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| crates/weaver_resolver/src/registry.rs | Propagates origin through inheritance and passes it into attribute reference resolution. |
| crates/weaver_resolver/src/lib.rs | Adds regression tests for transitive provenance and “inherited attribute is not a definition” behavior. |
| crates/weaver_resolver/src/dependency.rs | Determines attribute origin when building refinement summaries from v1/v2 dependencies. |
| crates/weaver_resolver/src/attribute.rs | Introduces is_definition handling for root attributes so inherited instances don’t satisfy bare ref resolution. |
| crates/weaver_resolver/data/registry-test-v2-dep/v1_dep_registry/registry/registry.yaml | New fixture: v2 registry refining a metric from a v1 dependency. |
| crates/weaver_resolver/data/registry-test-v2-dep/v1_dep_registry/registry/manifest.yaml | New fixture manifest for v1-dependency provenance test. |
| crates/weaver_resolver/data/registry-test-v2-dep/v1_dep_registry/expected-schema.yaml | Expected resolved output validating correct provenance across v1 + transitive dependencies. |
| crates/weaver_resolver/data/registry-test-v2-dep/span_registry/expected-schema.yaml | Updates expected outputs to include provenance on catalog attributes. |
| crates/weaver_resolver/data/registry-test-v2-dep/metric_registry/expected-schema.yaml | Updates expected outputs to include provenance on catalog attributes. |
| crates/weaver_resolver/data/registry-test-v2-dep/event_registry/expected-schema.yaml | Updates expected outputs to include provenance on catalog attributes. |
| crates/weaver_resolver/data/registry-test-v2-dep/entity_registry/expected-schema.yaml | Updates expected outputs to include provenance on catalog attributes. |
| crates/weaver_resolver/data/registry-test-v2-dep/deep_registry/registry/registry.yaml | New fixture: transitive dependency provenance through deeper attribute definition. |
| crates/weaver_resolver/data/registry-test-v2-dep/deep_registry/registry/manifest.yaml | New fixture manifest for deep/transitive provenance test. |
| crates/weaver_resolver/data/registry-test-v2-dep/deep_registry/expected-schema.yaml | Expected resolved output validating provenance points at the deeper defining registry. |
| crates/weaver_resolver/data/inherited-ref-test/user/registry.yaml | New fixture: refinement brings inherited attr into scope; bare ref must still fail. |
| crates/weaver_resolver/data/inherited-ref-test/user/manifest.yaml | New fixture manifest for inherited-ref test (published dependency form). |
| crates/weaver_resolver/data/inherited-ref-test/refiner/registry.yaml | New fixture: refiner that refines but defines nothing. |
| crates/weaver_resolver/data/inherited-ref-test/refiner/manifest.yaml | New fixture manifest for refiner-from-source scenario. |
| crates/weaver_resolver/data/inherited-ref-test/refiner_user/registry.yaml | New fixture: consumes refiner from source and tries to ref inherited attribute (must fail). |
| crates/weaver_resolver/data/inherited-ref-test/refiner_user/manifest.yaml | New fixture manifest for refiner_user. |
| crates/weaver_resolver/data/inherited-ref-test/middle/registry.yaml | New fixture: middle uses base attribute without defining it. |
| crates/weaver_resolver/data/inherited-ref-test/middle/manifest.yaml | New fixture manifest for middle. |
| crates/weaver_resolver/data/inherited-ref-test/middle_published/resolved.yaml | New fixture: published-form resolved schema used as dependency in tests. |
| crates/weaver_resolver/data/inherited-ref-test/middle_published/manifest.yaml | New fixture manifest for published middle. |
| crates/weaver_resolver/data/inherited-ref-test/base/registry.yaml | New fixture: base defines base.attr. |
| crates/weaver_resolver/data/inherited-ref-test/base/manifest.yaml | New fixture manifest for base. |
| crates/weaver_resolved_schema/src/catalog.rs | Adds RootAttribute and a root_attribute_definition API to distinguish definitions vs inherited instances. |
| crates/weaver_resolved_schema/src/attribute.rs | Extends UnresolvedAttribute with origin to track defining schema URL through resolution. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| a.provenance | ||
| .source | ||
| .and_then(|dep| schema.dependencies.iter().nth(dep.0 as usize).cloned()) | ||
| .unwrap_or_else(|| schema.schema_url.clone()) |
Related to #1658
Case 3 only happened when the dependency was consumed from source; a published one already refused. Now both behave the same.