Feat/inlined multivalued element identity - #107
Open
Kapersyx wants to merge 58 commits into
Open
Conversation
Element identity per inlined multivalued slot comes from a key, a unique_keys composed key, or a diff.linkml.io/opaque annotation; an opt-in linter flags models that still allow ambiguous deltas. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Phone numbers resolve via a unique_keys declaration of the existing SHACL rule; the coordinate rings resolve by lifting the positioning system identity up a layer and holding opaque lists of a bare Vertex class, leaving SpotLocation and the coordinate hierarchy untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ings Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ntity Diff's (List, List) arm now derives element identity uniformly: a key/identifier slot first, else the range class's merged `unique_keys` (single-slot keys use the bare scalar, composite keys the JSON array encoding of the values in `unique_key_slots` order). Keyed matching applies iff every element on both sides yields an identity label AND the labels are unique within each side. The guard is uniform: key/identifier labels are checked for duplicates exactly as unique_keys labels are, removing the silent collapse of lists that repeat a key. Lists that fail the guard fall back to positional matching with plain numeric segments — the old opportunistic mixing of key values into positional paths is gone, since a label that failed the guard cannot address an element unambiguously. The label helpers are extracted from the inline closure as element_key_label / element_unique_key_label / element_identity_label / scalar_slot_string for reuse by the patch resolver and the linter. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…mbiguity Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Opt-in schema lint surfacing multivalued inlined slots whose element
identity comes from nowhere. Without the flag output is unchanged; with
it, warnings print but never affect the exit code.
Text mode prints one `warning[Class.slot]: <detail>` line per finding.
JSON mode wraps the existing document so the result stays a single
parseable object: `{"status":"valid","identity_warnings":[...]}`.
Two behaviours worth calling out:
- The lint is skipped when the schema does not validate. Its answers
would be wrong on an incomplete graph (an unresolved import makes a
class range look like a non-class), and running it on a class with a
dangling `slots:` reference panics in SlotView::definition, which
indexes an empty definition list.
- Warnings are sorted by subject. ClassView::slots() is backed by a
HashMap, so the linter's per-class slot order varies between runs.
The lint reads the same SchemaView the validation used, so classes
reached through `imports:` and resolve_schemas are linted too.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The flagged JSON surface varied by case: identity_warnings was absent when the lint was skipped, and the skip was signalled by a prose string under the flag-named key. Both flagged cases now emit the same five keys, so a consumer reads one shape unconditionally: status, errors, identity_warnings, identity_lint_skipped, identity_lint_skipped_reason identity_lint_skipped is a real boolean with the reason in its own field, and identity_warnings distinguishes null (skipped, unknown) from [] (ran, found nothing). Output without the flag is unchanged, verified byte-for-byte against the pre-flag build in all four cases. Also drop exact duplicate warnings. lint_element_identity iterates get_class_ids(), which holds one id per class URI, so a class declaring an explicit class_uri is visited under both that and its default URI and every one of its warnings is emitted twice (personinfo.yaml: 10 warnings, 6 distinct). Correct the sort comment: the library does sort the classes it visits; only a single class's slot order is HashMap-derived. The sort now also lives inside the flag branch, so nothing runs when the flag is off. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…orted Two defects, both surfaced by running the linter over a real schema through linkml-schema-validate. get_class_ids() yields one id per class URI, and a class declaring an explicit class_uri is indexed under both that URI and its default one. Walking those ids visited such a class twice and reported every one of its slots twice (personinfo.yaml: 10 warnings for 6 distinct findings). Key a seen-set on the canonical URI, which is the same for both ids — the idiom ClassView::unique_keys already uses to walk a hierarchy. The class ids were sorted, but a class's own slots come from ClassView::slots(), which is HashMap-backed, so per-class warning order varied between runs. Sort the returned Vec by subject once, in the library, so every consumer inherits a stable order rather than each re-sorting. Service in the test fixture now declares class_uri, which is what exercises the first defect; the exact-set assert in schema_lint_flags_exactly_the_undeclared_positional_slots fails on the duplicates without the fix. diff_opaque and diff_unique_keys share the fixture and are unaffected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
lint_element_identity now dedupes visits and returns a sorted Vec, so the CLI can render what the library hands it. Output is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The round-2 dedupe keyed its seen-set on ClassView::canonical_uri(), which returns the explicit class_uri when one is declared. LinkML lets distinct classes declare the same class_uri -- meta.yaml's Anything and extensions.yaml's AnyValue both declare linkml:Any -- and both stay reachable through their own default URIs. The second such class hit seen.insert == false and had every one of its warnings dropped, with which class survived depending on hash order. A silent false negative is worse in a lint than the duplicate reporting the dedupe was added to fix. Key on (schema_id, name) instead, which is unique per class by construction and still collapses the two URIs of a single class. New fixture identity_shared_class_uri.yaml pulls in both directions at once: SharedUriA and SharedUriB share one class_uri and must both be reported, while TwoUris is indexed under two URIs of its own and must be reported once. Keeping them in one schema means neither fix can be made by breaking the other. The regression lives in its own fixture rather than in identity.yaml so that identity.yaml keeps yielding exactly the two documented Service warnings, which is the CLI's acceptance criterion. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…relative to source files
Two defects kept a multi-file schema with sibling imports from ever
loading, found while linting a real asset360 model.
resolve_schemas snapshotted get_unresolved_schemas() once and iterated
that snapshot. An import only becomes visible after the schema declaring
it is loaded, so imports-of-imports were never attempted: a root
importing ./rsm resolved rsm, but rsm's own ./types stayed unresolved.
Resolution now repeats until nothing is unresolved or a round achieves
nothing. A single failure also returned Err immediately, abandoning the
rest of the pass; failures are now collected per round and only become
the error when a round resolves nothing, so an import that failed while
other work was still progressing gets retried.
Nothing recorded where a schema was read from, so a relative import was
joined onto parent(get_resolution_uri_of_schema(S)), which is None for
the root (degenerating to the process CWD) and the original relative
import string for everything else ("./types" -> parent "." -> CWD
again). Resolution now tracks schema id -> canonicalized source
directory for every schema it loads from a file, and tries bases in
order: recorded source dir, then the old resolution-uri parent, then
CWD. Keeping CWD last preserves schemas that import CWD-relative paths.
New resolve_schemas_from(sv, root_source) seeds the root's own directory,
which is the only thing the caller knows and the view cannot. Plain
resolve_schemas delegates unseeded, so existing callers are unaffected.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Every bin takes a schema path and then resolved imports without telling the resolver where that file lives, so a schema importing ./sibling only loaded when the process happened to run from the schema's directory. All five now call resolve_schemas_from with args.schema. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…class A flagged slot is inherited by every descendant, so the lint reported it once per class and buried the single declaration the author would edit. On the asset360 model that meant 48 warnings covering 25 distinct (slot, element class) pairs: quantity 7 times via ObservableProperty's coordinate subclasses, elementCollections 8, and pictures / secondaryImages / hasCoveredSections 4 each. A flagged slot is now reported only where it is introduced: skipped when the direct is_a parent carries a slot of the same name that is flagged for the same reason. Applied at every level this leaves the topmost flagged declarer. The "same reason" half matters. A subclass whose slot_usage changes the identity answer is judged on its own merits in both directions: narrowing a range to a keyed class makes the subclass silent while its parent stays reported, and widening a clean slot to an identity-less range makes the subclass the introducer and reports it there. Only the is_a chain is walked. A slot arriving from a mixin is still reported on the class using the mixin as well as on the mixin itself: a mixin can be applied to unrelated classes, so there is no single owning declaration to point at. Documented in the rustdoc rather than guessed at. asset360 goes from 48 warnings to 25 with all 25 distinct pairs intact. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add `lint_element_identity(schema_view)` and `lint_instance_identity(instance)` to the `_native` module, wrapping the runtime lints and reusing `validation_results_to_py`; add matching stubs to `_native.pyi`. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Delta's path rustdoc now spells out how a unique_keys-derived segment is encoded (bare value for a single-slot key, JSON array in unique_key_slots order for a composite key), and diff's rustdoc states that an opaque slot stops recursion and yields one whole-value Update at the slot path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…dentity `diff` decided keyed matching from BOTH lists, while `patch` decides how to address a list from the ONE list in front of it. When the source list was label-addressed but the target repeated (or lacked) a label, diff emitted positional segments that patch, resolving the source by label only, refused: `patch(a, diff(a, b)) == b` broke and the edit was dropped. Emit one whole-slot `Update` for that quadrant instead — "this list stopped having coherent element identity" is honestly a whole-value change. All other quadrants keep today's behaviour. The one-list predicate is now shared: `list_is_keyed_shaped` backs both diff's fallback and `resolve_list_index`, so they cannot drift apart. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`navigate_path` ignored identity labels entirely and tried the numeric index first, so a `unique_keys` delta segment was invisible to it and a numeric segment silently returned the element at that *position* even when a sibling's identity label was that same number — the wrong element, reported as success. Both list resolvers are now one function, `diff::resolve_list_segment`: what diff emits, patch applies and navigate finds by construction, not by three implementations agreeing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Element identity derived from `unique_keys` comes from the name-sorted first entry: the metamodel does not preserve declaration order and delta paths have to be stable. A class declaring two entries therefore has an identity chosen alphabetically, and adding an earlier-sorting one silently re-addresses every delta path for every slot ranged on it — schema evolution with no signal. `lint_element_identity` now warns for such a class, naming the load-bearing entry. It flags slots the identity-less rule *passes*, so no existing warning changes; a class with a key/identifier slot is exempt (the key outranks every `unique_keys`, so none of them is load-bearing). The introducing-class rule now applies to both lint rules. `Delta`'s path documentation states the selection rule and the hazard. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The `--lint-identity --output json` payload spelled `type` with `Debug` formatting (`AmbiguousElementIdentity`) while the Python binding reported `ambiguous_element_identity` for the same value: two machine surfaces, two spellings, and a variant rename would have silently changed the CLI contract. `ValidationProblemType::label()` now lives next to the enum and both surfaces call it. The CLI's `type` becomes snake_case — the flag shipped inside this branch, so no external consumer exists. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`patch` resolves each list segment against the list's current state as the deltas apply, not against a snapshot of the list they were produced from. On a list whose elements carry duplicate identity labels that is observable: a delta removing the duplication flips the list to identity-addressed mid-sequence, and numeric segments still queued in the same patch are then reported failed. It fails loudly and never guesses, and it is confined to lists the identity linters exist to flag, so document the behaviour rather than build snapshot resolution for a zone the design tells authors to model their way out of. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…sed lists `python_navigate.rs` still walked `['objects','2',…]`. `objects` ranges on NamedThing, which declares an `id` identifier, so the shared list-segment rule addresses it by label and the numeric segment no longer resolves — CI's bare `cargo test --workspace` would have gone red where the local gates could not, the python crate not linking here. Uses 'P:002', and asserts the numeric segment now resolves to nothing. The assertions live in a `py_run!` string that cannot execute without a linking python crate, so `navigate_basic` mirrors them one-to-one against `navigate_path`, which `PyLinkMLInstance::navigate` delegates straight to. Also fixes the crate's one rustdoc warning: `[`crate::diff`]` was ambiguous between the module and the re-exported function. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…r key A class keyed by its own `designates_type` slot looks, to the identity-less rule, like a class that declares its identity. In a homogeneous inlined list it does not: the designator's value is fixed per class, so an N-vertex ring yields N identical labels, the diff engine's uniqueness guard falls back to positional addressing, and the declaration is misleading rather than load-bearing. Flag the list form only. The dict form keyed by the same designator is a different and legitimate model — at-most-one-element-per-subtype — and stays silent, as do `opaque`/`ignore` slots and ordinary discriminating keys. The key outranks `unique_keys` in `element_identity_label`, so this fires (and the several-unique_keys rule does not) when a class declares both. Reported once, at the class introducing the slot. On the asset360 model this catches `Polyline.PolyLine_coordinates` and `Polygon.Polygon_coordinates`, both ranged on `PositioningSystemCoordinate`, whose key is the `typeURI` discriminator. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The fixture only covered `key: true` and `designates_type: true` on the same
attribute. The shape in the wild — asset360's PositioningSystemCoordinate —
declares the designator on a base class and promotes it to the key with
`slot_usage` on the subclass. The rule already handles that through
`SlotView::definition()`'s is_a chain merge, but nothing pinned it: a
regression in that merging would have produced a silent false negative with
every test still green.
Add `TypedThing` (designator, no key) and `KeyedTypedThing` (`slot_usage:
typeURI: {key: true}`), range a ring-style list slot on the subclass, and
extend the exact-subject assertion plus a check that the message names
'KeyedTypedThing.typeURI'. Inverting the designator check drops
`inheritedVertices` from the flagged set, so the new assertion is not vacuous.
Also correct two stale docs: `lint_element_identity`'s rustdoc described two
rules, and the introducing-class comment said "both rules below". There are
three.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`element_key_label` skipped nothing: a key (or identifier) that is also the class's type designator labelled every element of a homogeneous list alike, and shadowed the `unique_keys` such a class may really be identified by. A designator's value is a function of the element's *class*, so it can never tell two elements apart — spec addendum rule 1 (D3). The engine now looks past a designator key (`identity_key_slot`): identity falls through to `unique_keys`, else the list is positional. Behaviour deltas: a designator-keyed class WITH `unique_keys` is now matched by them (reorder is a no-op, edits are addressed by the real label, for diff, patch and navigate alike); a polymorphic designator-keyed list, which used to pass as one-element-per-subtype, is positional. The schema lint keeps exactly one voice per designator-keyed slot. Its identity-less and several-`unique_keys` rules now use the engine's notion of a key, which the designator-keyed shape newly satisfies, so the reporting loop asks the designator question first and alone — the sharpest diagnosis, and the only one naming the declaration the author would edit. Its wording is updated to describe what the engine now does with such a key. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… hedge `identity_key_slot` is the chokepoint for the *labelling* question only. The `treat_changed_identifier_as_new_object` branch deliberately keeps asking the metamodel's key: it asks whether this is still the same thing, and a changed designator value means a different class — a whole-element replacement, which spec addendum rule 3 will make general. Routing it through `identity_key_slot` would drop that replacement and recurse across two classes. Comment at the site, and scope the rustdoc that implied otherwise. The designator fixture's comments still described the pre-rule-1 engine: the ring "collapsing to one element", the designator key "outranking unique_keys" (now exactly backwards), and positional fallback stated unconditionally. The lint's new wording over-claimed past the uniqueness guard: unique_keys address the list when their labels are unique within it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`ClassView::get_uri(native: true, expand: true)` returned the *canonical* URI, so for a class that declares a `class_uri` the schema-native URI was unreachable through the public API. Its only caller is `get_accepted_type_designator_values`, whose rustdoc promises "the canonical URI, native URI, and CURIE forms so that data produced with different prefix settings can be round-tripped" — it delivered three of the four, and the missing one was exactly the spelling a schema's own generator produces. The consequence was silent: a designator spelled `https://w3id.org/linkml/examples/personinfo/Organization` matched no accepted value, so `select_class` fell through to slot-shape matching and loaded the element as the base class `NamedThing`. Spec addendum rule 2 makes that set load-bearing — a spelling missing from it is not just ignored, it now decides which class's designator value gets stored — so the gap has to close before designator canonicalisation lands. `validation_issue_paths_include_list_indices_once` filtered for `UndeclaredSlot` at `objects.1.primary_email`, which only existed because the object had been loaded as `NamedThing`. The fixture is named `container_person_bad_email.yaml` and now behaves like it: the bad email is a pattern violation on a declared slot. The test is about the path carrying the list index exactly once, so it no longer filters by problem type. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…labels
Spec addendum rule 2 (findings D1 + D6): identity compares meaning, not
spelling. Two halves that only work together, so they land together.
**Designator values are canonicalised at the boxing chokepoint.** A
`uriorcurie`-ranged designator can say "this is a `Circle`" as
`canon:Circle`, as the expanded IRI, or by saying nothing at all; stored
verbatim those were three different strings, so the same element authored
by two producers diffed as a whole-element Remove + Add, and the RDF
loader — which never harvests the designator predicate and always fills it
from the class — disagreed with the JSON loader on every document.
`canonicalize_type_designator` runs at all four `Object`-building sites
(`parse_object_fixed_class`, `parse_object_value`, both arms of
`build_mapping_entry_for_slot`), mirroring what `coerce_scalar_to_range`
does for the int/float ambiguity. A supplied value that is no accepted
designator value of the selected class is a load-time **warning**, not an
error, and the slot then carries the canonical value — the loader-tolerance
precedent, and the same posture rule 5 takes for a disagreeing dict key.
This is also what makes diff's changed-key check ("a changed designator
means a different class", spec rule 3) unconditionally true instead of
true only for consistently-spelled data; no change was needed there.
**Identity-label components that denote IRIs are expanded before
comparison.** `ex:WGS84` and `https://example.org/canon/WGS84` are one
IRI. The expansion lives in `scalar_slot_string`, the single function every
identity label is built from, so all resolve sites move at once: diff
emission, `resolve_list_segment` for patch and navigate, and the instance
lint. The segments diff emits are then exactly what the resolver computes,
by construction rather than by three coincidences; a round-trip test over a
mixed-spelling pair pins it. A bare name and a CURIE with an unregistered
prefix are left verbatim — inventing an expansion would rename identities
the schema never claimed were IRIs. Only the identity *comparison* is
normalised: the stored value of an ordinary `uri`-ranged slot is the
author's data, unlike a designator, whose value is a function of the class.
`example_personinfo_data{,_2}.yaml` spelled `objecttype` with the
schema-native URI of classes that declare a `class_uri`; canonicalisation
rewrites those on load, so the round-trip fixture pair now spells the
canonical value it round-trips to. The other personinfo fixtures are left
as they are, as live evidence of the canonicalisation.
Downstream differential harness: corpus A byte-identical, all 95 self-diffs
empty, and the `designator-to-curie` mutation — re-spelling every `typeURI`
as a CURIE — drops from 46 non-zero diffs to zero across all 59 derivable
instances. The 13 instances whose classes declare neither `id` nor
`typeURI` were already at zero and did not move.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…t symmetry Review follow-up on 11cf89d. **Cost.** `canonicalize_type_designator` computed the canonical value *and* the full accepted set before it had even looked for the slot, so every object of a designator-carrying class paid two uncached `type_ancestors` walks and up to five `get_uri` calls — including the two commonest cases, an absent designator and an already-canonical one, where the answer is "do nothing". Now: find the scalar, read it, compare against the canonical value; compute the accepted set only when the two differ. An `Err` from `get_accepted_type_designator_values` also no longer collapses to "nothing is accepted" — an unknown accepted set is not an empty one, and treating it as empty warned about, and rewrote, data that may well have been right. It returns early, the same posture the canonical-value line takes. **Dict scalar arm.** `build_mapping_entry_for_slot`'s compact arm hardwires the slot's range class, but `find_scalar_slot_for_inlined_map` picks the first non-key scalar slot — which can be the designator. `{"w1": "canon:FancyWidget"}` then names its own class exactly as the object form does, and canonicalising against the range class rewrote it to `Widget` and warned: the misclassification 2b3f918 exists to prevent, reintroduced one arm over. That arm now selects the class from the scalar, scoped to the designator shape only — a compact entry whose scalar is an ordinary slot names no class and keeps the range class exactly as before. **Segment symmetry.** Labels were IRI-expanded on the way out but `resolve_list_segment` compared the caller's segment raw, so a stored delta or hand-written patch spelling a CURIE landed in `trace.failed` against expanded labels. Spec rule 2 says a curie and its expansion are one identity, in both directions. `segment_matches_label` normalises the incoming segment through the *same* slot the label came from (`identity_label_slot`, mirroring `element_identity_label`'s precedence), so the comparison is symmetric. Segments diff emits equal the label outright and never reach the expansion. Also: `identity_canonical.yaml` gains `BareLeaf` — a subclass with a `class_uri` and no distinguishing slot, so its schema-native URI is the only thing that can name it. `a_subclass_is_selected_by_its_native_uri_alone` fails loudly if `ClassView::get_uri(native, expand)` ever regresses; verified by reverting that line. And `validation_issue_paths_include_list_indices_once` pins `SlotRangeViolation` again rather than filtering nothing. Harness output is byte-identical to the 11cf89d capture: none of these five changes moves the downstream corpus. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…soft Spec addendum rules 3 and 4 (spike findings D2). Rule 3: when diff pairs two objects of different classes, the element was not edited, it was replaced. Recursing field-by-field across the two class definitions produced deltas describing one class's slots on the other's element (`thread` on a `Nut`) — a diff that no patch could apply, by construction. It now emits one whole-element Update, guarded by `equals` so the "objects the crate considers equal produce no delta" invariant survives. Class identity is the schema-qualified name; `equals`'s canonical-URI comparison is deliberately the looser test. The changed-key branch stays: it still owns a changed key value *within* one class, and its designator case is now subsumed (a canonicalised designator value is a function of the class). Rule 4: a delta whose payload cannot be built at the location it addresses records its path in `PatchTrace::failed` and leaves the tree untouched, instead of propagating the builder's Err and voiding the whole batch. Err is now reserved for infrastructure failure. Builds happen before any mutation, so "failed" always means "nothing happened". Also: `resolve_list_segment` tries exact label equality before the normalising scan — an element addressed by its own label is never shadowed by a sibling whose differently-spelled label normalises to the same string (heterogeneous lists only), and the common case stops re-deriving each element's label slot through its merged `unique_keys`. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`patch` never hard-errors on an unappliable delta: it records the path and applies the rest of the batch. The CLI dropped the trace, so a patch that silently skipped half its deltas exited 0 and wrote a file that looked clean. The paths now go to stderr, leaving stdout byte-identical for a fully applied patch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
0 = every delta applied, 2 = partial application, 1 = hard failure (bad arguments, unreadable files, schema or parse errors). Partial application is designed operation under drift, so it is not an error exit — but the removed builder-error `Err` did at least give it a non-zero status, and a script should not have to parse stderr to notice that half its deltas were skipped. A code of its own restores the signal without conflating the two. The codes are in the `--help` text, the patched document is written before the exit, and the writer is flushed explicitly since `process::exit` runs no destructors. The CLI test now asserts the codes, and covers a genuine BUILD-failure delta (a payload with no slot to box it against) alongside the missing-target one: that shape used to propagate as an `Err` and void the batch, so it is what pins rule 4's soft path at the CLI surface. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Three rustdoc corrections at the sites task 13 touched: - `patch` claimed `Err` was "reserved for infrastructure failure", but with rule 4 it no longer fails at all — `LinkMLError` carries validation problems, not infrastructure ones. The `LResult` return is retained for API stability; an `Err` is unreachable, not the place to look for a rejected delta. - `diff` stated rule 3 unconditionally; it is `equals`-guarded, so two classes sharing a `class_uri` with identical content emit nothing. - Rule 3 qualifies class identity by schema *id*, so a genuinely cross-schema pairing (v1 against v2, or two schemas declaring one class name) coarsens every paired object to a whole-element Update — correct and patchable, just coarse. Two `SchemaView`s over the SAME schema do not: they qualify identically and diff as finely as ever, which the new test pins. Also completes the segment-resolution perf fix: `resolve_list_segment` built the labels and then called `list_is_keyed_shaped`, which derived every label again to answer a question about the labels already in hand. A `list_is_keyed_shaped_from_labels` sibling takes the labels; the existing predicate delegates to it for callers that have only the elements. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`2` was already taken: clap exits 2 on an unknown flag or a missing argument (verified for both), so the documented contract made a typo'd flag indistinguishable from a partially applied patch — the one confusion a script following the contract could least afford — and the help line claiming `1` for bad arguments was simply false. Partial application is now `3`. The help text lists all four: 0 every delta applied, 1 hard failure from this tool, 2 argument/usage error (the parser's convention, not ours to choose), 3 partial application with the document still written. A new test pins the distinction by asserting that a usage error exits 2, not 3. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ild failures `list_is_keyed_shaped(values)` had been reduced to "materialise every label, then ask the labels", which threw away `all()`'s short-circuit: a long list whose FIRST element carries no label used to settle the question in one derivation and had started paying for all of them. It gets its lazy body back; `list_is_keyed_shaped_from_labels` remains for the resolver, which genuinely holds the labels already. Also completes the cross-schema note: coarsening every paired object to a whole-element Update is patchable only where the two schemas still agree about the element's shape. Where they genuinely disagree the payload fails to build and lands in `trace.failed` — still better than the field-level recursion it replaced, which produced deltas that could not apply and had no single path to report. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…yloads Spec addendum rule 5 (finding D5). The LinkML `inlined` dict form says the mapping key *is* the element's key/identifier value, but the loader never wrote it back: a `required` key slot produced a MissingSlotValue error on legal data, and the loaded object did not carry its own key. `build_mapping_entry_for_slot` now takes the entry's dict key and injects it into the key slot when the payload omits it — before class selection (a designator-keyed dict selects the class its key names), before the object constraints (the required key is satisfied), and before canonicalization (an injected designator value is canonicalized like any other). When the payload does supply the key slot, two warnings can fire, never errors: a value that disagrees with the dict key (compared through `canonical_identity_component`, so a CURIE key and its expanded payload stay one identity), and — for a designator key — a dict key that is no accepted designator value of the selected class. The payload value is stored as the element's data; the mapping stays addressed by its dict key. `example_personinfo_data_2.yaml` gains the `role` key it always implied: it is the expected side of the linkml-patch CLI round-trip, so it must state its own normal form. The source file still omits `role`. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review fix. `reconcile_dict_key_with_payload` compared the dict key against the RAW payload, before `canonicalize_type_designator` ran. For a designator key that was a false positive in one direction: key `ex:Linear` + payload `<native URI>` are two accepted spellings of one class, so canonicalization rewrote the payload onto the key's own spelling — yet the load warned, and the message said "the payload value is stored" while naming a value that was not what got stored. Reloading the emitted document was silent. The divergence half now runs after both designator passes and reads the key slot out of `child_values`: warn when the entry, as it will be written back out, contradicts the key it is written under. The message names the stored value. The injected case is still skipped — it agrees by construction. The asset360 divergence (native-URI key vs canonical `RSM:#EAID_CB107995…`) is unaffected: canonicalization moves that payload away from the key, not onto it. Both directions are now tests, plus the round-trip invariant that makes the silence correct. Also folded in from the review: a comment on why the accepted-designator half stays a raw string compare (it must match what canonicalization matches, or one fact gets two voices); a test for the compact arm's key-is-designator path; and a rustdoc note that the key slot is read from the RANGE class only, so a subclass-only `slot_usage` key gets neither injection nor checks. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…s, shared class_uri Spec addendum rule 6 (D7, D4d, D8, D9). Three new lint rules and the rustdoc that lists all of them. Instance lint, "positional despite a declared identity" (D7): an inlined list some of whose elements yield no identity label, although the element class declares one, is addressed positionally after all. An optional key left empty is valid data, so neither the schema lint nor validation can see it. One warning per container, counting the unlabelled elements; half-labelled lists are the same defect and are counted the same way. Unlike the duplicate rule it honours `opaque`/`ignore` and skips reference lists: it claims only that the list is addressed positionally, and for those slots that claim is false. The unguarded version fired on exactly one list in the downstream corpus — asset360's `NetElement.ports`, a reference list — and on nothing else. Schema lint, `unique_keys` across descendants (D4d): a list ranged on a class holds elements of every class descending from it, each labelled by its own merged `unique_keys`, so the several-entries rule now unions candidates over the whole family. A new, additional warning fires when the family resolves DIFFERENT load-bearing entries — one list, two label spaces, where a path written against one cannot address an element of the other and two elements can collide on a label without violating either class's constraint. Each rule gates on its own introduces-predicate. Schema lint, shared `class_uri` under a designator (D8): two classes of one `is_a` hierarchy answering to one class URI while the hierarchy designates its type. The loader resolves such a value to one class, stably but by an ordering the schema does not state. Warning only; the loader is deliberately unchanged. Class-level, so it is emitted once per (hierarchy, shared URI) instead of being gated on an introducing slot. D9 (`slot_usage: designates_type: false` leaving a key nothing fills) is documented as a sharp edge that collapses into the instance rule, not detected separately. Harness: asset360 27 warnings before and after, rinf 19 before and after, self diffs empty. The only downstream-visible movement is the reworded several-entries message on an in-repo fixture, plus the three new fixtures. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Four review items on the rule-6 lint extensions. Gate subtraction (real lost warning). `introduces_flagged_slot` asks "is the parent's slot flagged for the same reason?", and rule 1's predicate subtracts the designator case from that question. Rules 3 and 4 were gated on the raw predicates, but since the designator key stopped shadowing its class's `unique_keys` (spec addendum rule 1) a designator-keyed range class satisfies both raw shapes — while the designator rule is asked first and is the slot's only voice, so the parent emitted neither warning. A subclass that slot_usage-retargets the slot onto a keyless multi-entry class was therefore suppressed by a parent warning that does not exist, and the ambiguity was reported nowhere. Both gates now subtract the designator case, symmetric with rule 1. Key-labelled classes are their own split group. The divergence rule skipped family members with a non-designator key, claiming their entries "cannot split anything". Their *entries* are indeed not load-bearing — which is why the several-entries rule still skips them — but the key itself labels the elements and occupies its own label space: `Plate` elements addressed by a `plate_identity` value and `StampedPlate` elements by a `stampId` collide across the two spaces without breaking either class's constraint. Grouping is now on `identity_labelling`, which renders the declaration `element_identity_label` actually reads, so a key and an entry are different groups and never collide as group names. `declared_identity_description` collapses onto the same function — it asks the identical question — which also makes the instance rule's message distinguish `key` from `identifier`. Docs: the module rustdoc summary miscounted the rules (four + two = six; it is five + two = seven), and the Python binding docstrings plus the matching .pyi stubs predated rules 5-7. Sanctioned exact-set change: `Depot.keyed` now warns (divergence only, not several-entries), so `schema_lint_counts_...` was narrowed to the candidate-set claim and a new test asserts the divergence warning. Harness: fixture-block movement only; asset360 27 and rinf 19 warnings unchanged, self-diffs empty — no downstream schema has a key-vs-entry family. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`lint_instance_identity` was reachable from no CLI at all, so the two instance rules — a list repeating a declared identity, and one addressed positionally because some element leaves the identity slot empty (spec addendum rule 6, D7) — were invisible to the downstream differential harness however the corpus was exercised. Only the schema lint had a CLI. Modelled on `linkml-schema-validate --lint-identity`: opt-in, warnings never change the exit code, and deliberately skipped when the data does not validate (the lint asks how a list's elements are addressed, and a tree whose loading already went wrong answers about the damage rather than about the data). Text output uses the same `warning[<subject>]: <detail>` form; JSON gains `identity_warnings` / `identity_lint_skipped` / `identity_lint_skipped_reason`, using `ValidationProblemType::label()` as that binary already does. The existing `issues` shape is untouched. Without the flag the output is byte-identical to before — verified by running the pre-change and post-change binaries over valid and invalid data, in text and JSON, and diffing including exit codes. The JSON lint keys are absent, not null, when the flag is off. `identity_missing_labels_data.json` is the harness canary: data that trips both instance rules while its guard-rail slots (undeclared range, scalar range, reference list, `opaque`) stay silent, so a zero warning count can never mean "the flag stopped being passed". Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two sentences in the pre-addendum spec text no longer read true after the 2026-08-19 addendum landed: - The Non-goal section defined the identity label as "key/identifier first, else the class's unique_keys" and said path segments are the labels. Addendum rule 1 excludes a key that is the element class's type designator, and rule 2 IRI-expands label components whose slot ranges on uri/uriorcurie. Both are now named inline. - Option 1 claimed a truthful key always means "nothing needs to change". That is false for the one slot rule 1 excludes, in list form. The dict form the option actually exhibits (SpotLocation_coordinates) is unaffected, and the added paragraph says which is which. Docs only; no code, test or fixture touched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`severity_label` and `identity_warnings_json` were duplicated verbatim in `linkml_validate` and `linkml_schema_validate`, and `format_path` existed privately in the tools lib beside a copy of `severity_label`. All three are rendering decisions both CLIs must make identically — two binaries printing one lint two ways is a difference a reader has to explain to themselves — so they belong in the one place the binaries already share. No output changes: the moved bodies are the ones that were there. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`is_valid = validation_issues.is_empty()` predates the spec addendum. Until the addendum every diagnostic the loader could produce was an error, so "no diagnostics" and "no errors" were one predicate; rules 2 and 5 added the first three warning-severity issues (designator canonicalisation, dict-key divergence, unaccepted designator key) and the two parted company. A document whose only finding was a warning was therefore reported as invalid: exit 1, `valid: false`, the warning printed in the error list, and `--lint-identity` suppressed with "fix the validation errors above first" — an error message about a document that has none. Validity is now decided by errors alone (`LoadResult::has_errors`), which also un-gates the lint: a warning says the document is unusual, not that it failed to load, and the lint's answers over it are exactly as sound as over a silent one. Non-error issues are printed in the valid branch too, marked with their severity so they cannot be read as errors; the error branch keeps its published unmarked shape, so no document that already failed moves a line. In JSON, `valid` reflects errors and the issues array is untouched (it always carried `severity`); the skip reason now names errors, since errors are what gates it. Byte-identical for every warning-free document, which is every document that predates the addendum — pinned by a test, and by the differential harness. Fixtures live under src/tools/tests/data rather than the runtime fixture directory: the differential harness treats the latter as corpus. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`treat_changed_identifier_as_new_object` compared the two sides' key scalars as raw JSON values — rule 2's last unconverted resolve site. A `uri`-ranged non-designator key respelled curie↔uri yields ONE identity label, so the list matched the two elements as one element; this branch then compared the same values as strings, found them different, and emitted a whole-element `Update` at a path that addresses the element by the identity it had just declared changed. The delta contradicted itself, and a re-spelling that should have diffed as one field became a replacement of the whole element. Both sides now go through `scalar_slot_string`, the function every identity label is built from. The branch keeps reading the metamodel's key slot rather than `identity_key_slot` — the two questions differ in *which* slot to read, never in what makes two values of it the same value. A key that is a genuinely different IRI is still a replacement. `unique_keys`-derived identity never reached this branch, which is why the case survived rule 2's first pass; the fixture gains `KeyedSystem` to express the same component as a `key`. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The shared-`class_uri` warning's subject was the bare list of sharing classes. Every other rule in the module has a `[class_name, slot_name]` subject, and both CLIs render a subject by joining its segments with `.`, so `[Alpha, Beta]` printed as `warning[Alpha.Beta]` — indistinguishable from a slot warning about `Beta` on class `Alpha`, and just as indistinguishable to anything grouping findings by subject. The subject now leads with a `shared_class_uri` marker. It cannot collide with a class name, since it is not one, and it costs the rest of the rule nothing: the detail, the problem type and the emission gate are unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`resolve_list_segment`'s keyed branch took the FIRST normalising match while its positional branch refused two — one function answering "which element does this segment mean?" two ways. Both branches now share one helper, so neither can drift into resolving an ambiguity the other refuses. The second hit is barely reachable in the keyed branch (labels are unique there, so two of them can only normalise alike under different converters — a heterogeneous list drawn from schemas that disagree about a prefix), which is why it is a refusal and not a `debug_assert`: the case exists, and asserting it away would turn "I cannot tell which element you mean" into a crash. Left untested for the same reason it is hard to reach; the shared helper is what keeps the rule true. Also records, in `reconcile_dict_key_with_payload`'s rustdoc, why its two checks compare by different notions of equality: set membership is the raw match the canonicaliser performs, while "do these two spellings name one element" is `canonical_identity_component`'s question everywhere else. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The previous commit marked severity only in the valid branch, leaving a warning
listed beside errors in the invalid branch spelled exactly like them — so the
Critical's own complaint ("the CLI prints a warning as if an error") survived
for documents that carry both, which is the shape asset360's committed
signal-obj-with-track.json has.
Both lists now go through one printer: an error keeps the bare
`Type at path: detail` line, so no document whose diagnostics are all errors
moves a byte, and anything that is not an error says what it is.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ts address ae8e8d0 made every `Update` whose list address resolves to nothing report, which breaks the multi-source merge it matters most for: one source drops an element, another that still holds it describes it as an `Update` (its delta was computed against an older golden), and the element silently disappears from the merged record. The payload decides instead of the op. An element carries its own identity — key/identifier, else a `unique_keys`-derived label — so it can be checked against the address it was filed under: - identity names the address -> append, the re-add the merge intends - identity names a different element -> report; appending would duplicate a label and knock the list out of keyed matching entirely - no identity at all -> append only in a positional or emptied list, never in an identity-addressed one This keeps what ae8e8d0 was really protecting: a stale positional `Update` into a keyed list still reports rather than overwriting whichever element moved into that index. `resolve_list_index` is what refuses that address, not the leaf guard, so restoring the append does not restore the clobber. A build failure on this path now reports rather than erroring the whole patch — the value was never going to be applied. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…elta Clippy (-D warnings) rejects the helper's 8 arguments; its sibling delta-application helpers in this file already carry the same allow. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
2 participants
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.
Element identity for inlined multivalued slots
Goal is to answer the question What is the identity of an element in an inlined multivalued slot, this is needed to have unambiguous deltas when there are multiple sources in play that can update a list at the same time.
See design doc at 2026-08-17-inlined-multivalued-element-identity-design.md
What's new
(["Emergency","02/111.11.11"]); patch and navigate_path resolve them through one shared resolver.
multi-unique_keys ambiguity, split label spaces (incl. key-vs-entry), shared class_uri hierarchies, duplicate identities, missing labels. Exposed via linkml-schema-validate
--lint-identity, linkml-validate --lint-identity, and Python lint_element_identity / lint_instance_identity.
linkml-validate --lint-identity, and Python lint_element_identity / lint_instance_identity.
IRI-canonically (curie ≡ expansion); a class change is a whole-element replacement; inlined-dict keys are injected and validated against payloads.
get_uri(native, expand) defect that misclassified subclasses.
Breaking changes
The old style inference mixing optional keys with positional indexing, still allowed too much ambiguity, so we break compatibility with them, meaning old deltas generated with previous versions of the diff engine are assumed broken.
paths; no more silent collapse of duplicate keys).
only — this branch introduces the first load-time warnings).
Verification
TDD throughout (fresh gates: 301 tests, 0 failures) plus an empirical differential harness over the real consolidator-server corpus (59 instance files, two schema roots): every changed
output line across the entire branch is attributed to a numbered spec rule; asset360's schema-lint count is stable at 27; all self-diffs empty.