From 432496b9d8398802678b876b5f42e98e34d8feda Mon Sep 17 00:00:00 2001 From: mauanga Date: Mon, 14 Sep 2026 00:08:57 +0200 Subject: [PATCH 1/2] fix(validation): require breadcrumb item on every entry but the last --- AGENTS.md | 30 ++ ...est_breadcrumb_listitem_item_validation.py | 312 +++++++++++++++++- tests/test_shacl_generator.py | 104 +++++- wordlift_sdk/validation/generator.py | 105 ++++-- .../validation/shacls/google-breadcrumb.ttl | 53 ++- 5 files changed, 540 insertions(+), 64 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index f0c17d1..17f287a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -55,6 +55,36 @@ lists when building property alternatives, and downgrades conditional required prose ("required when/if", "only required if") to warning-level constraints to avoid unconditional errors for context-dependent sections. +- The SHACL generator special-cases Google's breadcrumb rule that the *last* + trail entry may omit `item`. Instead of a plain `sh:minCount 1` on + `ListItem.item`, `BreadcrumbList` gets a `sh:sparql` order constraint + (`:google_BreadcrumbListItemOrderConstraint`): an entry may omit `item` only if + it holds the highest `schema:position` *uniquely*. Google never states a limit on + how many entries may omit `item`, so no count rule stands in for the real rule. + `ItemList` (carousels) is untouched and still requires `item` on every entry. +- Ranking is only as good as the positions, so a second constraint + (`_BREADCRUMB_POSITION_SELECT`) reports any `ListItem.position` that would not + rank: a lexical form `xsd:double` rejects, and `NaN`, which fails a self-equality + test. Its guard is the cast the order constraint uses, so the reported set is + exactly the unrankable set — a regex would be narrower and wrongly flag `INF` or + non-ASCII digits, which do rank. Google types `position` as `Integer`, but + `FeatureData` keeps only which properties are required, never the Type column the + Google page states, and no `google-*.ttl` shape holds a datatype constraint — + value typing otherwise lives in the schema.org grammar, permissive by design. + Generating it from the Type column for every feature is the wider fix. +- `_emit_node` skips `sh:minCount 1` on `ListItem.item` under a `BreadcrumbList` + parent, but both breadcrumb constraints are emitted only from the top-level + `BreadcrumbList` branch in `_write_feature`, so a nested breadcrumb would lose + `item` enforcement entirely. `BreadcrumbList` is never a child type in + `_SCOPED_CHILD_RULES` today, guarded by + `test_breadcrumb_list_is_not_yet_a_scoped_child_type`. +- SPARQL constraints are emitted as *named* top-level resources (for example + `:google_BreadcrumbListItemOrderConstraint`) that `sh:sparql` points at, never + as inline blank nodes. pyshacl stringifies the source constraint into every + result it produces, and a blank node expands to its whole property list — the + `sh:select` body included — so an inline constraint repeats the whole query + once per offending entry in `report_text`, which is surfaced to users and fed + to the quality agent. A named node prints as a single token. - Google SHACL type-context parsing is now constrained to explicit type definitions (`must be based on one of the following schema.org types`, `full definition of ... is available/provided`) and scoped plain headings diff --git a/tests/test_breadcrumb_listitem_item_validation.py b/tests/test_breadcrumb_listitem_item_validation.py index 13fcf2e..2a3869d 100644 --- a/tests/test_breadcrumb_listitem_item_validation.py +++ b/tests/test_breadcrumb_listitem_item_validation.py @@ -1,3 +1,12 @@ +"""Google lets the *last* breadcrumb entry omit ``item``; every other entry must +carry it. + +https://developers.google.com/search/docs/appearance/structured-data/breadcrumb +""" + +from rdflib import URIRef +from rdflib.namespace import SH + from wordlift_sdk.validation.shacl import ( PreparedShaclValidator, _load_graph_from_jsonld, @@ -12,6 +21,18 @@ def validator() -> PreparedShaclValidator: return PreparedShaclValidator(prepare_shapes(["google-breadcrumb"])) +@pytest.fixture(scope="module") +def carousel_validator() -> PreparedShaclValidator: + return PreparedShaclValidator(prepare_shapes(["google-carousel"])) + + +@pytest.fixture(scope="module") +def both_validator() -> PreparedShaclValidator: + return PreparedShaclValidator( + prepare_shapes(["google-breadcrumb", "google-carousel"]) + ) + + def _list_item(position: object, item: str | None = None) -> dict: node: dict = {"@type": "ListItem", "position": position, "name": "X"} if item is not None: @@ -27,33 +48,54 @@ def _breadcrumb(elements: list[dict]) -> dict: } -def _conforms(validator: PreparedShaclValidator, elements: list[dict]) -> bool: +def _validate(validator: PreparedShaclValidator, elements: list[dict]): graph = _load_graph_from_jsonld(_breadcrumb(elements)) - return validator.validate_graph(graph).conforms + return validator.validate_graph(graph) + + +def _conforms(validator: PreparedShaclValidator, elements: list[dict]) -> bool: + return _validate(validator, elements).conforms + + +A = "https://example.com/a" +B = "https://example.com/b" +C = "https://example.com/c" @pytest.mark.parametrize( "elements", [ pytest.param( - [ - _list_item(1, "https://example.com/a"), - _list_item(2, "https://example.com/b"), - ], + [_list_item(1, A), _list_item(2, B)], id="every-entry-has-item", ), pytest.param( - [_list_item(1, "https://example.com/a"), _list_item(2)], + [_list_item(1, A), _list_item(2)], id="final-entry-omits-item", ), + pytest.param([_list_item(1)], id="single-entry-omits-item"), + # "Last" is the highest ``position``, not the last one written down. pytest.param( - [_list_item(1), _list_item(2, "https://example.com/b")], - id="first-entry-omits-item", + [_list_item(2), _list_item(1, A)], + id="highest-position-declared-first", + ), + # Real-world markup types `position` inconsistently; positions are + # compared numerically, so these all behave like 1, 2. + pytest.param( + [_list_item("1", A), _list_item("2")], + id="string-positions-final-omits-item", + ), + pytest.param( + [_list_item(1.0, A), _list_item(2.0)], + id="float-positions-final-omits-item", + ), + pytest.param( + [_list_item("1", A), _list_item(2.0, B), _list_item(3)], + id="mixed-position-types-final-omits-item", ), - pytest.param([_list_item(1)], id="single-entry-omits-item"), ], ) -def test_one_entry_may_omit_item( +def test_trail_conforms_when_only_the_last_entry_omits_item( validator: PreparedShaclValidator, elements: list[dict] ) -> None: assert _conforms(validator, elements) @@ -63,7 +105,15 @@ def test_one_entry_may_omit_item( "elements", [ pytest.param( - [_list_item(1, "https://example.com/a"), _list_item(2), _list_item(3)], + [_list_item(1), _list_item(2, B)], + id="first-entry-omits-item", + ), + pytest.param( + [_list_item(1, A), _list_item(2), _list_item(3, C)], + id="middle-entry-omits-item", + ), + pytest.param( + [_list_item(1, A), _list_item(2), _list_item(3)], id="two-entries-omit-item", ), pytest.param( @@ -71,22 +121,50 @@ def test_one_entry_may_omit_item( id="no-entry-has-item", ), pytest.param( - [_list_item(1.0), _list_item(2.0), _list_item(3.0)], - id="float-positions", + [_list_item("1"), _list_item("2", B)], + id="string-positions-first-omits-item", + ), + pytest.param( + [_list_item(1.0), _list_item(2.0, B)], + id="float-positions-first-omits-item", + ), + pytest.param( + [_list_item("1"), _list_item(2.0, B)], + id="mixed-position-types-first-omits-item", ), pytest.param( - [_list_item("1"), _list_item("2"), _list_item("3")], - id="string-positions", + [_list_item(1), _list_item(1)], + id="tied-positions-both-omit-item", + ), + # A tie means there is no single highest position, so neither entry can + # be the exempt last one. + pytest.param( + [_list_item(1), _list_item(1, A)], + id="tied-positions-one-omits-item", + ), + # The entry omitting `item` carries two positions. It must rank against + # its sibling, never against itself. + pytest.param( + [_list_item(1, A), _list_item([1, 5])], + id="entry-with-two-positions-ties-its-sibling", ), - pytest.param([_list_item(1), _list_item(1)], id="tied-positions"), ], ) -def test_two_or_more_entries_omitting_item_fail( +def test_trail_is_reported_when_a_non_final_entry_omits_item( validator: PreparedShaclValidator, elements: list[dict] ) -> None: assert not _conforms(validator, elements) +def test_violation_is_an_error_not_a_warning( + validator: PreparedShaclValidator, +) -> None: + result = _validate(validator, [_list_item(1), _list_item(2, B)]) + assert not result.conforms + assert result.warning_count == 0 + assert "sh:Violation" in result.report_text + + def test_missing_position_is_still_reported( validator: PreparedShaclValidator, ) -> None: @@ -94,3 +172,201 @@ def test_missing_position_is_still_reported( result = validator.validate_graph(graph) assert not result.conforms assert "position" in result.report_text + + +def _carousel(elements: list[dict]) -> dict: + return { + "@context": "https://schema.org", + "@type": "ItemList", + "itemListElement": elements, + } + + +def _carousel_entry(position: int, url: str, with_item: bool = True) -> dict: + node: dict = {"@type": "ListItem", "position": position, "url": url} + if with_item: + node["item"] = {"@type": "Movie", "name": "M", "url": url} + return node + + +def test_carousel_still_requires_item_on_every_entry( + carousel_validator: PreparedShaclValidator, +) -> None: + complete = _carousel([_carousel_entry(1, A), _carousel_entry(2, B)]) + assert carousel_validator.validate_graph(_load_graph_from_jsonld(complete)).conforms + + last_omits_item = _carousel( + [_carousel_entry(1, A), _carousel_entry(2, B, with_item=False)] + ) + assert not carousel_validator.validate_graph( + _load_graph_from_jsonld(last_omits_item) + ).conforms + + +def test_breadcrumb_rule_does_not_leak_into_carousels( + both_validator: PreparedShaclValidator, +) -> None: + """Loading both shapes together must not swap their verdicts. The carousel + verdict on its own is covered above. + + Scope is the bare data graph. A graph that *also* asserts + `BreadcrumbList rdfs:subClassOf ItemList` still loses the exemption under + `inference="rdfs"`, because the entailed `rdf:type ItemList` hands the trail + to the carousel shape. That predates this constraint and is not covered here. + """ + breadcrumb = _breadcrumb([_list_item(1, A), _list_item(2)]) + assert both_validator.validate_graph(_load_graph_from_jsonld(breadcrumb)).conforms + + carousel = _carousel( + [_carousel_entry(1, A), _carousel_entry(2, B, with_item=False)] + ) + assert not both_validator.validate_graph(_load_graph_from_jsonld(carousel)).conforms + + +@pytest.mark.parametrize( + "elements", + [ + pytest.param([_list_item("9", A), _list_item("10")], id="string-9-then-10"), + pytest.param( + [_list_item(str(i), A) for i in range(1, 10)] + [_list_item("10")], + id="string-1-to-10", + ), + pytest.param([_list_item(9, A), _list_item(10)], id="int-9-then-10"), + ], +) +def test_multi_digit_positions_are_ordered_numerically( + validator: PreparedShaclValidator, elements: list[dict] +) -> None: + """The final entry omits `item`, so these must conform. Ordering positions + lexicographically would rank "9" after "10" and reject them.""" + assert _conforms(validator, elements) + + +@pytest.mark.parametrize( + "elements", + [ + pytest.param( + [_list_item(-3, A), _list_item(-2), _list_item(-1, A)], + id="negative-positions", + ), + pytest.param( + [_list_item(" 1", A), _list_item(" 2"), _list_item(" 3", A)], + id="whitespace-padded-positions", + ), + pytest.param( + [_list_item("01", A), _list_item("02"), _list_item("03", A)], + id="zero-padded-positions", + ), + pytest.param( + [_list_item(0, A), _list_item(1), _list_item(2, A)], + id="zero-indexed-positions", + ), + ], +) +def test_signed_and_padded_positions_are_still_ranked( + validator: PreparedShaclValidator, elements: list[dict] +) -> None: + """A numeric guard narrower than the xsd:double cast would drop the sign or + the padding and silently exempt these.""" + assert not _conforms(validator, elements) + + +@pytest.mark.parametrize( + "elements", + [ + pytest.param( + [_list_item("a", A), _list_item("b"), _list_item("c", A)], + id="non-numeric-positions", + ), + # "NaN" reaches the same place by a different route than "a"/"b"/"c": + # it *is* a valid xsd:double lexical form, so the cast succeeds rather + # than raising, and NaN then loses every comparison. + pytest.param( + [_list_item("NaN"), _list_item(2, A)], + id="nan-position-omits-item", + ), + pytest.param( + [_list_item("a"), _list_item("b")], + id="non-numeric-positions-two-omit", + ), + ], +) +def test_unrankable_positions_are_reported( + validator: PreparedShaclValidator, elements: list[dict] +) -> None: + """A position the order constraint cannot rank is reported as a position, not + left to exempt the entry beside it. The guard is the same cast, so the set it + reports is exactly the set that would not rank.""" + result = _validate(validator, elements) + assert not result.conforms + assert "position must be a number" in result.report_text + + +@pytest.mark.parametrize( + "elements,conforms", + [ + # -INF is the lowest position, so the entry omitting `item` is first. + pytest.param([_list_item("-INF"), _list_item(2, A)], False, id="minus-inf"), + # INF is the highest, so the entry omitting `item` really is last. + pytest.param([_list_item("INF"), _list_item(2, A)], True, id="inf"), + ], +) +def test_infinite_positions_rank_as_expected( + validator: PreparedShaclValidator, elements: list[dict], conforms: bool +) -> None: + """Unlike NaN, the infinities cast cleanly *and* order, so they are ranked + rather than exempted.""" + assert _conforms(validator, elements) is conforms + + +def test_every_result_reports_a_path_the_focus_node_actually_has( + validator: PreparedShaclValidator, +) -> None: + """sh:resultPath is a path from sh:focusNode — the BreadcrumbList, not the + entry. The offending entry travels in sh:value instead.""" + result = _validate(validator, [_list_item(1), _list_item(2), _list_item(3, C)]) + + assert not result.conforms + paths = set(result.report_graph.objects(None, SH.resultPath)) + assert paths == {URIRef("http://schema.org/itemListElement")} + + +def test_report_text_does_not_repeat_the_sparql_query( + validator: PreparedShaclValidator, +) -> None: + """The constraint is named rather than inline so that report_text, which is + shown to users and fed to the quality agent, does not carry a copy of the + query per result. See the note in generator.py.""" + result = _validate(validator, [_list_item(1), _list_item(2), _list_item(3, C)]) + + assert not result.conforms + assert "SELECT $this" not in result.report_text + # A full IRI, not the `:` prefix: the prepared shapes graph merges every + # shape file and carries no single file's prefix bindings. + source_constraints = [ + line.strip() + for line in result.report_text.splitlines() + if "Source Constraint:" in line + ] + assert source_constraints + assert all( + line.endswith("google_BreadcrumbListItemOrderConstraint>") + for line in source_constraints + ) + + +def test_each_offending_entry_is_reported_once( + validator: PreparedShaclValidator, +) -> None: + """Positions 2 and 3 both omit `item`; only position 2 is wrong. Naming + position 3 would tell the quality agent that consumes report_text to add + `item` to the one crumb Google exempts.""" + result = _validate(validator, [_list_item(1, A), _list_item(2), _list_item(3)]) + + assert not result.conforms + # report_graph keeps the internal urn:wl:node: ids; only report_text is scrubbed. + values = sorted( + str(v).removeprefix("urn:wl:node:") + for v in result.report_graph.objects(None, SH.value) + ) + assert values == ["/itemListElement/1"] diff --git a/tests/test_shacl_generator.py b/tests/test_shacl_generator.py index 97212c2..a97bef9 100644 --- a/tests/test_shacl_generator.py +++ b/tests/test_shacl_generator.py @@ -1,8 +1,13 @@ +import re from pathlib import Path from rdflib import URIRef -from wordlift_sdk.validation.generator import FeatureData, _write_feature +from wordlift_sdk.validation.generator import ( + _SCOPED_CHILD_RULES, + FeatureData, + _write_feature, +) def _read_output(tmp_path: Path, feature: FeatureData) -> str: @@ -356,7 +361,9 @@ def test_keeps_listitem_shape_without_itemlist(tmp_path: Path) -> None: assert "sh:targetClass schema:ListItem" in content -def test_breadcrumb_listitem_item_is_exempted_for_one_entry(tmp_path: Path) -> None: +def test_breadcrumb_listitem_item_is_not_unconditionally_required( + tmp_path: Path, +) -> None: feature = FeatureData( url="https://example.com", types={ @@ -370,10 +377,11 @@ def test_breadcrumb_listitem_item_is_exempted_for_one_entry(tmp_path: Path) -> N content = _read_output(tmp_path, feature) - assert "sh:qualifiedMaxCount 1 ;" in content - assert "sh:not [" in content - node_shape = content.split("sh:qualifiedValueShape")[0] + # The nested ListItem shape must not require `item` outright — the SPARQL + # constraint decides, so that the last entry may omit it. + node_shape = content.split("sh:sparql")[0] assert "sh:path schema:item ;\n sh:minCount 1 ;" not in node_shape + assert "sh:sparql :google_BreadcrumbListItemOrderConstraint ;" in content def test_itemlist_listitem_item_stays_unconditional(tmp_path: Path) -> None: @@ -389,3 +397,89 @@ def test_itemlist_listitem_item_stays_unconditional(tmp_path: Path) -> None: assert "sh:qualifiedMaxCount" not in content assert "sh:path schema:item ;\n sh:minCount 1 ;" in content + + +def test_breadcrumb_item_order_is_checked_with_sparql(tmp_path: Path) -> None: + feature = FeatureData( + url="https://example.com", + types={ + "BreadcrumbList": {"required": {"itemListElement"}, "recommended": set()}, + "ListItem": { + "required": {"position", "name", "item"}, + "recommended": set(), + }, + }, + ) + + content = _read_output(tmp_path, feature) + + # A named constraint, not an inline blank node: pyshacl stringifies the + # source constraint into every result, and a blank node drags the whole + # query along with it. + assert "sh:sparql :google_BreadcrumbListItemOrderConstraint ;" in content + assert ":google_BreadcrumbListItemOrderConstraint\n a sh:SPARQLConstraint ;" in ( + content + ) + # The ordering half of the rule needs sibling positions, not just a count. + assert "" in content + assert "" in content + + +def test_itemlist_gets_no_breadcrumb_sparql_exemption(tmp_path: Path) -> None: + feature = FeatureData( + url="https://example.com", + types={ + "ItemList": {"required": {"itemListElement"}, "recommended": set()}, + "ListItem": {"required": {"position", "item"}, "recommended": set()}, + }, + ) + + content = _read_output(tmp_path, feature) + + assert "sh:sparql" not in content + + +def test_checked_in_breadcrumb_shape_matches_generator_output(tmp_path: Path) -> None: + """The shipped .ttl is regenerated from Google's docs, so a hand edit would be + silently dropped on the next run. Comparing the whole file — not just the + breadcrumb block — catches an edit anywhere in it, in either direction: a + generator change that was never written out, and a .ttl change the generator + would not produce. + + The ``# Generated:`` header is stamped with the wall clock on every run, so + it is the one line that cannot match. + """ + feature = FeatureData( + url="https://developers.google.com/search/docs/appearance/structured-data/breadcrumb", + types={ + "BreadcrumbList": {"required": {"itemListElement"}, "recommended": set()}, + "ListItem": { + "required": {"position", "name", "item"}, + "recommended": set(), + }, + }, + ) + output_path = tmp_path / "google-breadcrumb.ttl" + assert _write_feature(feature, output_path, overwrite=True) + + def _without_timestamp(text: str) -> str: + return re.sub(r"^# Generated: .*$", "# Generated: -", text, flags=re.M) + + shipped = Path("wordlift_sdk/validation/shacls/google-breadcrumb.ttl").read_text( + encoding="utf-8" + ) + assert _without_timestamp(output_path.read_text(encoding="utf-8")) == ( + _without_timestamp(shipped) + ) + + +def test_breadcrumb_list_is_not_yet_a_scoped_child_type() -> None: + """Tripwire. The order constraint is wired up only from _write_feature's + top-level BreadcrumbList branch, so nesting BreadcrumbList here would leave + nested trails with no `item` enforcement. Emit it from _emit_node before + updating this assertion.""" + for parent_type, rules in _SCOPED_CHILD_RULES.items(): + for prop, child_types in rules.items(): + assert "BreadcrumbList" not in child_types, ( + f"{parent_type}.{prop} now nests BreadcrumbList" + ) diff --git a/wordlift_sdk/validation/generator.py b/wordlift_sdk/validation/generator.py index ed4aa5a..b2b60f2 100644 --- a/wordlift_sdk/validation/generator.py +++ b/wordlift_sdk/validation/generator.py @@ -733,24 +733,72 @@ def _emit_listitem_name_or_item_name(lines: list[str], indent: int) -> None: lines.append(f"{sp}) ;") -def _emit_breadcrumb_item_exemption(lines: list[str], indent: int) -> None: - sp = " " * indent - lines.append(f"{sp}sh:property [") - lines.append(f"{sp} sh:path schema:itemListElement ;") - lines.append(f"{sp} sh:qualifiedValueShape [") - lines.append(f"{sp} sh:not [") - lines.append(f"{sp} sh:property [") - lines.append(f"{sp} sh:path schema:item ;") - lines.append(f"{sp} sh:minCount 1 ;") - lines.append(f"{sp} ] ;") - lines.append(f"{sp} ] ;") - lines.append(f"{sp} ] ;") - lines.append(f"{sp} sh:qualifiedMaxCount 1 ;") - lines.append( - f'{sp} sh:message "Required by Google: item on every ListItem ' - 'except the final one." ;' - ) - lines.append(f"{sp}] ;") +_XSD_DOUBLE = f"<{XSD.double}>" + +# Google types ListItem.position as Integer; nothing else asserts it, because +# FeatureData keeps which properties are required, never the Type column. Self +# equality holds only for a position that ranks: a form the cast rejects errors the +# filter and drops the row, and NaN fails against itself. +_BREADCRUMB_POSITION_SELECT = f"""SELECT $this ?value ?path +WHERE {{ + BIND( AS ?path) + $this ?value . + ?value ?position . + FILTER NOT EXISTS {{ + FILTER ({_XSD_DOUBLE}(str(?position)) = {_XSD_DOUBLE}(str(?position))) + }} +}}""" + +# Google lets the *last* breadcrumb entry omit `item`. RDF has no array order, so +# "last" means "uniquely highest schema:position", which core SHACL cannot express. +# ?path is bound because sh:resultPath runs from the BreadcrumbList, not the entry. +# `>=` makes ties rank, so a trail with no unique highest reports every item-less +# entry; sameTerm keeps a literal entry from raising a type error. Positions that +# would not rank are reported by _BREADCRUMB_POSITION_SELECT instead. +_BREADCRUMB_ITEM_ORDER_SELECT = f"""SELECT $this ?value ?path +WHERE {{ + BIND( AS ?path) + $this ?value . + FILTER NOT EXISTS {{ ?value ?item }} + FILTER EXISTS {{ + ?value ?position . + $this ?other . + FILTER (!sameTerm(?other, ?value)) + ?other ?otherPosition . + FILTER ({_XSD_DOUBLE}(str(?otherPosition)) >= {_XSD_DOUBLE}(str(?position))) + }} +}}""" + + +# Named constraints, not inline blank nodes: pyshacl stringifies the source +# constraint into every result, and a blank node expands to its whole property +# list — the query included. +_BREADCRUMB_CONSTRAINTS = ( + ( + ":google_BreadcrumbListItemOrderConstraint", + "Required by Google: item on every ListItem except the last " + "(highest position) one.", + _BREADCRUMB_ITEM_ORDER_SELECT, + ), + ( + ":google_BreadcrumbListPositionConstraint", + "Required by Google: position must be a number on every ListItem.", + _BREADCRUMB_POSITION_SELECT, + ), +) + + +def _emit_breadcrumb_constraints(lines: list[str]) -> None: + """Emit the named SPARQLConstraints the BreadcrumbList shape points at.""" + for name, message, select in _BREADCRUMB_CONSTRAINTS: + lines.append(name) + lines.append(" a sh:SPARQLConstraint ;") + lines.append(f' sh:message "{message}" ;') + lines.append(' sh:select """') + lines.extend(f" {line}" for line in select.split("\n")) + lines.append(' """ ;') + lines.append(".") + lines.append("") def _emit_node( @@ -777,7 +825,9 @@ def _emit_node( and type_name == "ListItem" and prop == "item" ): - # Google: the final breadcrumb entry may omit `item`. + # Google: the final breadcrumb entry may omit `item`. The order + # constraint stands in for this minCount, emitted only from + # _write_feature's top-level BreadcrumbList branch. continue if type_name == "ListItem" and prop == "name": # Google: ListItem.name is only required when `item` is a bare URL; @@ -799,10 +849,6 @@ def _emit_node( ) if type_name == "ListItem" and "name" in bucket["required"]: _emit_listitem_name_or_item_name(lines, indent) - if type_name == "BreadcrumbList" and "item" in buckets.get("ListItem", {}).get( - "required", set() - ): - _emit_breadcrumb_item_exemption(lines, indent) for prop in sorted(bucket["recommended"]): child_types = child_rules.get(prop) @@ -902,10 +948,12 @@ def _write_feature(feature: FeatureData, output_path: Path, overwrite: bool) -> ) if type_name == "ListItem" and "name" in bucket["required"]: _emit_listitem_name_or_item_name(lines, 2) - if type_name == "BreadcrumbList" and "item" in feature.types.get( - "ListItem", {} - ).get("required", set()): - _emit_breadcrumb_item_exemption(lines, 2) + emit_breadcrumb_constraints = type_name == "BreadcrumbList" and ( + "item" in feature.types.get("ListItem", {}).get("required", set()) + ) + if emit_breadcrumb_constraints: + for name, _, _ in _BREADCRUMB_CONSTRAINTS: + lines.append(f" sh:sparql {name} ;") for prop in sorted(bucket["recommended"]): child_types = _SCOPED_CHILD_RULES.get(type_name, {}).get(prop) @@ -946,6 +994,9 @@ def _write_feature(feature: FeatureData, output_path: Path, overwrite: bool) -> lines.append(".") lines.append("") + if emit_breadcrumb_constraints: + _emit_breadcrumb_constraints(lines) + recommended_one_of_groups = feature.one_of_recommended.get(type_name, []) for idx, group in enumerate(recommended_one_of_groups, start=1): shape_name = f":google_{type_name}RecommendedOneOf{idx}Shape" diff --git a/wordlift_sdk/validation/shacls/google-breadcrumb.ttl b/wordlift_sdk/validation/shacls/google-breadcrumb.ttl index b195d76..a0a23fb 100644 --- a/wordlift_sdk/validation/shacls/google-breadcrumb.ttl +++ b/wordlift_sdk/validation/shacls/google-breadcrumb.ttl @@ -3,7 +3,7 @@ @prefix schema: . # Source: https://developers.google.com/search/docs/appearance/structured-data/breadcrumb -# Generated: 2026-09-09T08:57:24Z +# Generated: 2026-09-16T12:45:54Z # Notes: required properties => errors; recommended properties => warnings. :google_BreadcrumbListShape @@ -40,17 +40,42 @@ ) ; ] ; ] ; - sh:property [ - sh:path schema:itemListElement ; - sh:qualifiedValueShape [ - sh:not [ - sh:property [ - sh:path schema:item ; - sh:minCount 1 ; - ] ; - ] ; - ] ; - sh:qualifiedMaxCount 1 ; - sh:message "Required by Google: item on every ListItem except the final one." ; - ] ; + sh:sparql :google_BreadcrumbListItemOrderConstraint ; + sh:sparql :google_BreadcrumbListPositionConstraint ; +. + +:google_BreadcrumbListItemOrderConstraint + a sh:SPARQLConstraint ; + sh:message "Required by Google: item on every ListItem except the last (highest position) one." ; + sh:select """ + SELECT $this ?value ?path + WHERE { + BIND( AS ?path) + $this ?value . + FILTER NOT EXISTS { ?value ?item } + FILTER EXISTS { + ?value ?position . + $this ?other . + FILTER (!sameTerm(?other, ?value)) + ?other ?otherPosition . + FILTER ((str(?otherPosition)) >= (str(?position))) + } + } + """ ; +. + +:google_BreadcrumbListPositionConstraint + a sh:SPARQLConstraint ; + sh:message "Required by Google: position must be a number on every ListItem." ; + sh:select """ + SELECT $this ?value ?path + WHERE { + BIND( AS ?path) + $this ?value . + ?value ?position . + FILTER NOT EXISTS { + FILTER ((str(?position)) = (str(?position))) + } + } + """ ; . From 5221f2ae8449b9ae64b7116300b82cc0ce746fc0 Mon Sep 17 00:00:00 2001 From: mauanga Date: Fri, 18 Sep 2026 11:42:41 +0200 Subject: [PATCH 2/2] chore(release): v8.4.7 --- CHANGELOG.md | 10 ++++++++++ pyproject.toml | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 36a86fb..aebccc7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,15 @@ # Changelog +## 8.4.7 - 2026-09-18 + +### Fixed + +- Breadcrumb SHACL validation now requires `item` on every `ListItem` except + the one with the highest `position`, matching Google's rule that only the + last entry may omit it. A trail missing `item` on a first or middle entry + used to validate clean. A `position` that cannot be ranked numerically is + reported too. + ## 8.4.6 - 2026-09-11 ### Fixed diff --git a/pyproject.toml b/pyproject.toml index 19700fb..3536dd3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "wordlift-sdk" -version = "8.4.6" +version = "8.4.7" description = "Python toolkit for orchestrating WordLift imports and structured data workflows." authors = ["David Riccitelli "] readme = "README.md"