fix(gen-shacl): correct rule-converter path parity, combined bounds, and SPARQL escaping#21
Conversation
Adversarial audit of the hardening fixes (probed at this tip)Fix claims that held under attack: top-level induced-slot parity ( C1 — real bug: the parity fix stops one hop short, and the new
C2 — edge case: C3 — real bug (pre-existing; this PR's new docstring mis-states it): C4 — cosmetic: alias-form rule keys silently diverge. A rule key written Also verified: malformed Suggest a follow-up commit on this stack for C1–C3 before upstreaming (C1's fix best lands where the nested emitters live, i.e. the fallback commit, when folding for upstream submission). |
…and SPARQL escaping
Review hardening for the SHACL-SPARQL rule converters. Three defects, each
with a regression test that fails before this change:
- Induced-slot parity: _slot_uri and _resolve_enum_value_ref resolved the
*base* slot, so a slot_usage override of slot_uri (or a narrowed enum range)
made the generated SPARQL query a property/enum the data never uses while
sh:path used the induced IRI. The constraint then silently never fired
(false negative). Both now resolve the induced slot for the class, matching
the sh:path logic in the main slot loop.
- Combined operators: a single precondition / member condition dispatched on
the first matching operator, so {minimum_value: X, maximum_value: Y} dropped
the lower bound and under-constrained the trigger (false positives). A shared
_scalar_filters helper now emits every recognised operator, and still returns
None -- skip, never mis-translate -- when none is recognised.
- SPARQL string escaping: an equals_string / permissible-value name containing
a double quote, backslash or newline produced invalid, unparseable SPARQL.
New _sparql_string_literal escapes per SPARQL 1.1 section 19.7.
Tests: 6 new regression tests (structural, prepareQuery syntax, and pyshacl
end-to-end) covering all three defects; full shaclgen suite green (111).
Signed-off-by: Carlo van Driesten <carlo.van-driesten@vdl.digital>
Signed-off-by: jdsika <carlo.van-driesten@vdl.digital>
(cherry picked from commit e263def)
15c4d0d to
4b2ef26
Compare
8442eb8 to
1ea1ea2
Compare
Summary
Review-hardening pass over the rules → SHACL-SPARQL converters. Three latent
defects, each with regression tests that fail before the fix (re-verified
on this stack: running the new tests against the pre-fix generator source
yields 5 failures):
Induced-slot parity (silent false negative).
_slot_uriand_resolve_enum_value_refresolved the base slot, so aslot_usageoverride of
slot_uri(or a narrowed enumrange) made the SPARQL bodyquery a property / enum IRI the data never uses, while
sh:pathused theinduced IRI. The constraint then silently never fired. Both now resolve
the induced slot for the class, matching the
sh:pathlogic in the mainslot loop. Affected code originates in feat(gen-shacl): generate sh:sparql constraints from LinkML rules #11 (framework) — this fix must be
folded into the framework commit when submitting upstream.
Combined operators (silent under-constraint / false positives). A
single precondition or member condition dispatched on the first matching
operator, so a bounded range
{minimum_value: X, maximum_value: Y}droppedthe lower bound. A shared
_scalar_filtershelper now emits everyrecognised operator (and still returns
None— skip, never mis-translate —when none is recognised). Affected code originates in the compositional
fallback (base PR of this one).
SPARQL string escaping (invalid / injectable output). An
equals_stringor permissible-value name containing a",\, or newlineproduced unparseable SPARQL. New
_sparql_string_literalescapes perSPARQL 1.1 §19.7.
Affects every converter that renders string terms (framework,
presence-implies-value, fallback).
Stack position
Base branch:
feat/shaclgen-compositional-rule-fallback— only the hardeningdelta shows in the diff.
Kept as a separate commit (rather than folded into the feature branches) so
the defects, their failure modes, and the regression tests remain visible to
review. When upstreaming to
linkml/linkml, fold each fix into the submissionthat introduces the affected code, as noted per defect above.
Provenance
Cherry-pick of
e263def7fromfeat/shacl-rule-converters(#18), where thefull CI matrix is green.
How was this tested?
tests/linkml/test_generators/test_shaclgen.pyon this branch:106 passed (base suites + 6 regression tests: structural,
prepareQuerysyntax, and pyshacl end-to-end — including the below-threshold case proving
the lower bound is enforced, and an end-to-end proof that a
slot_usage-overridden constraint actually fires).source, the regression tests fail (5 failures) — the tests bite.
ruff checkandruff format --checkclean on both changed files.Areas of uncertainty
range_expression/has_memberconditions) and thatvalue_presence: ABSENTcombined with another operator was still translated; both are fixed with regression tests in the stacked fix(gen-shacl): enforce operator exactness, nested-slot parity, numeric bounds #22.my_slotfor a slot namedmy slot) silently fell back to the default-prefix CURIE — also fixed in the stacked fix(gen-shacl): enforce operator exactness, nested-slot parity, numeric bounds #22 (audit C4).Checklist
AI Assistance
If you used AI tools while preparing this PR, you are still the author and responsible for understanding, verifying, and defending your submission. Please engage with reviewers personally rather than through your agent during feedback and revisions. See our AI Covenant for details.