Skip to content

Reject a ledger rule that narrows by name and not by role - #457

Merged
derek73 merged 2 commits into
masterfrom
claude/456-require-fields
Aug 29, 2026
Merged

Reject a ledger rule that narrows by name and not by role#457
derek73 merged 2 commits into
masterfrom
claude/456-require-fields

Conversation

@derek73

@derek73 derek73 commented Aug 29, 2026

Copy link
Copy Markdown
Owner

Closes #456. Found by the #455 review; the symmetric twin of #451's ban.

name_regex = "Smith"      # and no `fields`

Measured: such a rule claims 127 of 127 possible diff shapes on any name its regex reaches. It narrows by name and by nothing else.

Why #452 made this urgent rather than untidy

over_declared_rules skips a rule with no fields — correctly, since one declaring no roles cannot over-declare them. But that turns deleting the line into an opt-out: it is the cheapest way to silence an OVER-DECLARED failure and the most permissive thing that can be done to the rule, at the same time. The #372 failure mode, reached by following a gate error message.

The new message says exactly that, and warns against the obvious wrong correction — reaching for fields while dropping the regex lands on the shape #451 banned. The two bans are each other's trap.

Free to enforce

expected_since_1.4.0.toml: 70 rules, 0 without `fields`
expected_since_2.0.0.toml: 58 rules, 0 without `fields`
expected_since_2.1.0.toml: 51 rules, 0 without `fields`

All three still load. Same argument validate_rules already makes for #451's ban in its own docstring.

The symmetry is the point

validate_rules now rejects a rule carrying neither key, one carrying fields without name_regex (#451), and one carrying name_regex without fields (#456). After this, every ledger rule carries both, and the three rejections read as one rule with one reason: a rule narrows by name and by role, or it is not a rule.

Placement

At the END of the fields family, beside #451's. That matters: written under the neither-key check, #451's version pre-empted the three checks that buy precise messages for a malformed fields — an empty one reported the #451 message instead of "empty 'fields'". That was a #453 review finding, and this check has the same hazard in reverse.

The test fixture ripple, and one thing worth flagging

Twenty fixtures gained a fields. Intent is preserved in every one — they are ZZZ rules that match nothing whatever their roles, a "" rule that hits the sentinel check first, and Smith rules whose {family} diff the added roles still admit.

Nine of the twenty already carried a fields on the next concatenated line, and my first pass duplicated the key. tomllib caught it with Cannot overwrite a value; removed.

test_a_rule_with_a_regex_and_no_fields_or_both_stays_legal pinned this shape as legal when #451 landed. It is inverted rather than deleted, so the change of status is visible in the history rather than silent — a test quietly disappearing is how a decision stops being reviewable.

Verification

Proved able to fail: disabling the check fails the new test. tests/v2/ 5350 passed, 156 skipped, 1 xfailed. Gates 1.4.0 229/0, 2.0.0 194/0, 2.1.0 102/0, no OVER-DECLARED.

Decided, not deferred

No escape hatch for a "genuinely unbounded" rule — declined until one appears, the same call #452 made. dormant covers "explains nothing"; nothing covers "explains everything", and no such rule exists. If one ever does it should argue for a key the way dormant was argued for in #373, rather than finding one already built.

🤖 Generated with Claude Code

The symmetric twin of #451's ban. Such a rule narrows by name and by
nothing else, so on any name its regex reaches it claims every diff
shape there is -- measured, 127 of 127.

#452 is what made it urgent rather than untidy. over_declared_rules
skips a rule with no `fields`, correctly, since one declaring no roles
cannot over-declare them -- so deleting the line is the cheapest way to
silence an OVER-DECLARED failure and the most permissive thing that can
be done to the rule at the same time. The #372 failure mode, reached by
following a gate error message. The new message says so, and says not
to reach for #451's banned shape instead.

Free on #451's own terms: measured, 0 of 179 rules across the three
ledgers have it, and all three still load.

Placed at the END of the `fields` family beside #451's, so neither
pre-empts the precise messages for a MALFORMED `fields` -- that
placement was itself a #453 review finding, and this check has the same
hazard in reverse.

Twenty test fixtures gained a `fields`; intent is preserved in every
one -- they are `ZZZ` rules that match nothing whatever their roles, a
`""` rule that hits the sentinel check first, and `Smith` rules whose
{family} diff the added roles still admit. Nine of the twenty already
carried a `fields` on the next concatenated line and my first pass
duplicated the key; caught by TOMLDecodeError and removed.

test_a_rule_with_a_regex_and_no_fields_or_both_stays_legal pinned this
shape as LEGAL when #451 landed. It is inverted rather than deleted, so
the change of status is visible in the history rather than silent.

Proved able to fail: disabling the check fails the new test. 5350
passed; gates 229/0, 194/0, 102/0.

Closes #456

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@derek73 derek73 added docs Documentation fixes and updates tooling tests labels Aug 29, 2026
@derek73 derek73 self-assigned this Aug 29, 2026
@derek73 derek73 added the tests label Aug 29, 2026
@codecov

codecov Bot commented Aug 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.64%. Comparing base (6ac31f4) to head (c856c33).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #457   +/-   ##
=======================================
  Coverage   98.64%   98.64%           
=======================================
  Files          45       45           
  Lines        3177     3177           
=======================================
  Hits         3134     3134           
  Misses         43       43           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

One real coverage loss and two falsified sentences, all from the
focused review of #457.

test_main_compares_the_v2_surface_from_baseline_2_0 stopped catching
its own mutation. Its diff IS the ambiguity-only one, and the blanket
`fields = ["family"]` my fixture sweep gave it refuses that shape, so
the ZZZ rule stayed inert even with name narrowing disabled: measured,
that mutant failed 11 tests on master and 10 here. The fixture now
declares `_ambiguities`, the role its diff actually moves, and the
mutant is back to 11. This is exactly the weakening the sweep's own
commit message claimed not to have caused, in the one fixture whose
diff is not {family}.

"127 of 127" is wrong at both 2.x baselines, including the default.
_RULE_FIELDS has eight members, and from 2.0 on main unions
`_ambiguities` into the same diff set, so the banned shape claims 255
there and 127 below. A live count with no date, quoted in an error
message a contributor reads while a 2.1.0 gate is failing. Both
figures now, in the message, the README and decisions.md.

validate_rules' docstring said the fields-without-name_regex check was
"the family's sharpest example: no other malformed shape can widen
invisibly". #456 is the counterexample, and worse -- over_declared_rules
skips a fieldless rule, so nothing narrows it either. The paragraph
names both now.

Also: `dormant` gains a pin, since disabling this check otherwise
failed exactly one test where #451's fails three; the README's "It may
narrow further with `fields`" is folded into the required list rather
than left contradicting the sentence eight lines below it;
over_declared_rules' and classify's docstrings stop presenting the
banned shape as legal; the decisions.md entry moves after the #452
entry it cites, restoring date order; and the inversion note moves to
the assertion that was actually inverted, where a `git log -L` finds
it.

5350 passed; gates 229/0, 194/0, 102/0.

Refs #456

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@derek73
derek73 merged commit 0847ecf into master Aug 29, 2026
11 checks passed
@derek73 derek73 added this to the v2.2 milestone Aug 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Documentation fixes and updates tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Deleting a rule's fields escapes #452's check and makes the rule maximally permissive — the symmetric twin of #451

1 participant