fix(validator): fail-closed property-coverage attribution (N-7)#8
Conversation
The coverage display filtered on hardcoded field names, so violations with unmatched fields — approval_tiers.<tier>.blocks, TOML syntax, schema failures, SOUL/IDENTITY content, cross-file mismatches — flipped no property and a FAIL run could print all-green coverage (PR #4 review N-7). Exit codes were always correct; display-only. Replace the filters with per-violation attribution: every violation marks >=1 property; unrecognized fields mark every property their source file underwrites (unknown sources mark all five). Verified across all 42 fixtures + 2 examples: 36/36 negatives flip >=1 property, 8/8 positives all-green, suite READY. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the validator’s Property Coverage display logic so coverage attribution is computed per-violation with fail-closed defaults, preventing “all green” coverage output on failing runs when new/unknown violation fields appear.
Changes:
- Replace hardcoded field-list filtering with per-violation property attribution and a fail-closed fallback in
validators/validate.js. - Document the new fail-closed attribution semantics in
validators/README.md.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| validators/validate.js | Reworks Property Coverage computation to attribute every violation to ≥1 property, with fail-closed defaults by source file. |
| validators/README.md | Documents the updated Property Coverage attribution semantics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (v.file === 'ward.toml') { | ||
| if (['meta.person', 'protected.invariants'].includes(v.field)) return ['Human Belonging']; | ||
| if (v.field.startsWith('approval_tiers.') || v.field === '[approval_tiers]' | ||
| || v.field.startsWith('schema /approval_tiers') || v.field.startsWith('schema /editable') | ||
| || v.field.startsWith('schema /protected') | ||
| || ['[protected]', 'protected.files', '[editable]', 'editable.paths'].includes(v.field)) { | ||
| return ['Bounded Authority']; | ||
| } | ||
| if (v.field.startsWith('schema /meta')) return ['Human Belonging']; | ||
| // file/syntax/content/root-schema failures: ward.toml underwrites both properties | ||
| return ['Bounded Authority', 'Human Belonging']; | ||
| } |
|
Fresh-context code review at |
Validator coverage-display fix (PR #4 review N-7)
Bug (display-only; exit codes were always correct): the Property Coverage block filtered violations against hardcoded field-name lists. Violations whose fields matched no list flipped nothing:
approval_tiers.<tier>.blocks(the compiler checks added in v0.4.0),syntax,schema *, SOUL/IDENTITYcontent, missing-file variants, andcross-file. Result: a FAIL run could print all-✓ coverage — e.g.negative/09-missing-memoryFAILed with two Bounded Authority tier violations while displaying Bounded Authority ✓.Fix: per-violation property attribution with a fail-closed default — every violation marks ≥1 property; a violation no rule recognizes marks every property its source file underwrites (unknown source ⇒ all five). Targeted rules keep the display informative:
schema /meta*→ Human Belonging;schema /approval_tiers*,/editable*,/protected*→ Bounded Authority; malformed/unreadableward.toml→ both; missing/emptySOUL.md→ all three SOUL-carried properties;cross-file→ Named Identity. README documents the semantics.Not a conformance change: nothing in-tree parses the coverage block (runner greps PASS/FAIL only); no fixture, schema, or RFC text touched; no version bump needed.
Verification
bash tests/conformance/run-conformance.sh→ 6/6 + 36/36, READYNo conflict with pending PR #7 (touches validate.js banner line only; trivial either-order rebase).
Bead: threads-cgj