fix(gate-54): a correct nested relation was unrepresentable to the check (#231) - #255
Merged
Merged
Conversation
added 2 commits
August 8, 2026 15:45
…eck (#231) `property_ids` was built by a single non-recursive loop over `schema.properties.*` while `_raw_walk` recursed into the whole document. Every relation nested inside `items.properties.*` was therefore reported as "x-relation-filter is placed off a property" UNCONDITIONALLY — the collector could never contain it, so the finding was not a judgement, it was arithmetic. The finding was unfixable in the app. larpingapp's `character.requirementOverrides.items.properties.skill` is byte-for-byte the same shape as `character.skills` twenty lines above, which the gate accepts; the only difference is nesting depth. The three ways to clear it (move the filter off the property, flatten the array-of-objects, delete the filter) are all wrong. The same blind spot ran in the opposite direction for checks (b), (d) and (f): a nested relation MISSING its `$ref`, carrying a dangling `$ref` or an invalid filter token was never inspected. The gate was simultaneously false-positive on shape and false-negative on the thing that matters. One recursive collector (`_collect_properties`) fixes both directions. It descends `items.properties.*` (dict and tuple `items` forms) and `properties.*` for inline objects, guarded by an id() seen-set and a depth cap so a pathological document cannot crash the gate — a crashed gate reports nothing, which reads exactly like a pass. Invariants deliberately preserved, each pinned by a test: * `items` is a subschema, NOT a property — a filter riding on the items node is still a real rule-6 violation and is still reported. * `@object.<field>` still resolves against the ROOT schema at every depth. `@object` is the object under edit, not the array element; larpingapp's nested `@object.setting` points at `character.setting`. Resolving against the element would have manufactured a fresh false positive. * rule-10 (frozen lifecycle) stays top-level: a lifecycle `field` names a property of the schema, never an element of a nested array. Also, because a finding count is not a defect count (gate-53 turned ~132 defects into "240 violations"): audited this helper for multi-emission. Exactly one overlap — a property with `x-relation-filter` and no `$ref` matched check (b) AND check (c), one defect stated twice with one fix. Check (c)'s duplicate is now suppressed when (b) already reported that property, so the ratio is 1:1. Check (d) stays per-token by design. And the placement findings now carry a JSON pointer. They named only the FILE, so a register emitting three of them gave the reader three identical lines for three different nodes — that ambiguity is what made this issue hard to triage. Measured, larpingapp@development registers (full-tree, no diff scope): before 4 findings / after 3 — the false positive is gone and all three genuine findings survive. Fleet-wide re-measure over 19 apps' registers: the placement false positive existed only on larpingapp; the closed false negative surfaces 8 previously silent nested missing-$ref findings (scholiq 4, decidesk/hermiq/openbuild/ openconnector 1 each). Check (b) is property-level diff-scoped in CI, so these only surface on a PR that touches those lines. Tests: new suite, 0 -> 23 tests, auto-discovered by run-helper-suites.sh (28 suites passing, 2 quarantined). Every new test mutation-checked: reverting the items recursion fails 10, collecting `items` as a property fails 2 (the true-positive guards), removing the dedupe fails 1, removing the top-level guard fails 1, and the whole suite run against origin/main's helper fails 16.
Past the bound a nested property is no longer collected, so its filter is reported as misplaced — an over-report a human can dismiss. Silently accepting everything past the bound is the dangerous direction and looks exactly like a pass. Carried over from the earlier attempt on this issue, which asserted the same property.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #231. Supersedes #241 (that branch fixed only the false-positive half and shipped no runnable
def test_; it is closed in favour of this one).Cause
property_idswas built by a single non-recursive loop overschema.properties.*, while_raw_walk()recursed into the entire document and flagged everyx-relation-filterwhose containing dict was not in that set.So a relation nested in
items.properties.*was reported as "placed off a property" unconditionally. That is not a judgement, it is arithmetic —_raw_walkfinds the node, the collector can never contain it, the finding always fires.The finding was unfixable in the app. larpingapp's
character.requirementOverrides.items.properties.skillis byte-for-byte the same shape ascharacter.skillstwenty lines above, which the gate accepts; the only difference is nesting depth. The three ways to clear it — move the filter off the property, flatten the array-of-objects, delete the filter — are all wrong.The same blind spot ran the other way for checks (b), (d) and (f): a nested relation missing its
$ref, carrying a dangling$ref, or carrying an invalid filter token was never inspected at all. The gate was simultaneously false-positive on shape and false-negative on the thing that matters.Fix
One recursive collector,
_collect_properties(), replaces the flat loop and feeds all the checks. It descendsitems.properties.*(both dict and tuple-itemsforms) andproperties.*for inline objects, guarded by anid()seen-set and a depth cap — a crashed gate reports nothing, which reads exactly like a pass.Three invariants deliberately preserved, each pinned by a test:
itemsis a subschema, not a property. A filter riding theitemsnode is still a genuine rule-6 violation and is still reported.@object.<field>still resolves against the ROOT schema at every depth.@objectis the object under edit, not the array element. larpingapp's nested@object.settingpoints atcharacter.setting; resolving against the element would have manufactured a fresh false positive.fieldnames a property of the schema, never an element of a nested array.Acceptance — three proofs, all real runs
(a) True positive still caught. Never widen a checker until it catches nothing. Fixture with a filter on an
x-*block and one insideitemsbut not on a property — both still reported:(b) False positive gone.
larpingapp@development, full-tree, all register files:The one removed line is the false positive. All three genuine findings survive.
(c) False negative closed. A nested relation with no
$ref— silent onorigin/main, reported now:Finding count vs defect count
Gate-53 turned ~132 defects into "240 violations" because one missing
_noteemitted a triplet, so this helper was audited for the same shape. Exactly one overlap: a property withx-relation-filterand no$refmatched check (b) and check (c) — one defect, one fix, two messages. Check (c)'s duplicate is now suppressed when (b) already reported that property, so the ratio is 1:1. Check (d) stays per-token by design (three bad tokens are three defects).Placement findings now name the node
They named only the FILE. A register emitting three of them gave the reader three identical lines for three different nodes — that ambiguity is exactly what made #231 hard to triage. They now carry a JSON pointer.
Fleet re-measure (19 apps' registers, read-only)
The placement false positive existed only on larpingapp. Closing the false negative surfaces 8 previously-silent nested missing-
$reffindings: scholiq 4, decidesk / hermiq / openbuild / openconnector 1 each. Check (b) is property-level diff-scoped in CI, so these only surface on a PR that touches those lines — no repo goes red on landing.Honest caveat: of those 8, at least two (hermiq
CourseRecommendation.recommendations.items.courseId, openconnectorsource...credentialRef.credentialId) are cross-register FKs where a local$refcannot resolve, so they are not fixable by adding one. That is a pre-existing weakness of check (b)'s prose heuristic which this change gives more reach; it is filed separately rather than patched under this issue with a prose carve-out.Tests
New suite
test_check_relation_dialect.py, auto-discovered byrun-helper-suites.sh: 0 → 24 tests. Full suite on the rebased tree: 29 passed, 2 quarantined, 0 failed.Every new test mutation-checked (each mutation applied, suite run, mutation reverted, suite re-run green):
itemsrecursionitemsitself as a propertydepth == 0lifecycle guardorigin/main's helperTouches only
check_relation_dialect.pyand its new test file.