fix(trust): rank a declared arrival so a narrowing is not charged as a weakening - #869
Conversation
|
Warning Review limit reachedNext included review available in 27 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Every other check on this head is Not re-running it: a re-run on a draft reproduces the same cancellation, and CI on this branch is meaningless until the PR is readied. That is blocked on something outside the diff, recorded in the PR body — no Generated by Claude Code |
`rule-predicate-changed` fired on any difference in a rule's predicate columns, so tightening a gate cost the same admission as loosening one: a groomed `Weakens:` pair plus a matching trailer, with the groom predating the work. This repository has twice paid that by shaping the config around the gate rather than touching a row (batten.toml:925-933, :951-960), and where a new row is not available — a field on an existing row — there is no such route. `RULE_NARROWING_ON_ARRIVAL` names the keys whose ARRIVAL can only add refusals, beside `RULE_NON_PREDICATE` and with the same census. A key it names, filled where the base said nothing, is not charged; a changed value, a departure, a resized list and an undeclared arrival stay unrankable and still fire. Declared rather than inferred, because "an optional absence-preserving key can only narrow" is false here three times over: `bypass_env` makes a row suppressible (batten.toml:415-430), `key_shape` resolves a non-matching subject to allow by its own doc, and `when_present` narrows which calls a row gates at all. A ranking reading "unrankable" as "narrowing" would turn this gate's one false refusal into a false pass. Absent and `null` are one reading: a rule column carries `skip_serializing_if` and a nested row like `CaptureQuery` does not, so a ranking seeing one spelling would rank the rule and not the row inside it. Closes CLOUD-1394
85a28f5 to
48aab71
Compare
|
❌ The last analysis has failed. |
|
/fast-forward |
1 participant
Closes CLOUD-1394
The defect
rule-predicate-changedfires on any difference in a rule's predicate columns (trust.rsserializes eachRuleto JSON and compares per-column). It has no direction, so tightening a gate costs the same admission as loosening one: a groomedWeakens:pair plus a matching commit trailer, with the groom predating the work — which cannot be satisfied retroactively.The repository has already paid for this twice by shaping the config around the gate.
batten.toml:925-933and:951-960both record splitting a check into a newreceiptrow rather than widening an existingcheckslist, explicitly because a new row raises nothing. Where a new row is not available — a field on an existing row, which is PR #842's case — there is no such route.The fix
RULE_NARROWING_ON_ARRIVAL, besideRULE_NON_PREDICATEand with the same census discipline. A key the table names, arriving where the base said nothing, is not charged. Everything else — a changed value, a departure, a resized list, an undeclared key arriving — stays unrankable and still fires.The ranking is declared rather than inferred, and the counterexamples are why. The tempting structural rule — an optional key whose absence preserved the prior behaviour can only narrow — is false here three times over, each already written down in this repository:
bypass_envbatten.toml:415-430calls the smell correct "in its own terms")key_shapewhen_presentmediated_callrow gates, so the calls it stops gating are no longer refusedEach is an added, optional, absence-preserving key that widens. A partial ranking that read "unrankable" as "narrowing" would convert this gate's one false refusal into a false pass, which is the direction it has always been right about.
Seeded with two keys, each carrying its argument in the tuple:
max_age(a receipt this row accepted can only stop qualifying by aging out; absent, existence is the verdict) andrequires_field(three-valued and fails open on an absent field, so it can only add theRefuteddeny it introduces).Absent and
nullare one reading. A rule column carriesskip_serializing_if, so a fresh optional field is missing from the base object; a nested row likeCaptureQuerycarries none and serializes the same field asnull. A ranking that saw only one spelling would rank the rule and not the row inside it — which is exactly the shape CLOUD-1387 has.What this does not do
Weaken the admission for changes that genuinely loosen a rule, or touch
lint.rs's groom/trailer plumbing (CLOUD-1221's family, and correct).key_atis not declared here, because it does not exist onmain. PR #842 adds the field and its one-line row together, which is the unblocking.Verification
trust.rs: six new cases — the narrowing, the same key moving after it arrived, the key departing, the undeclared arrival (bypass_env, the measured counterexample), the nested-row reading, and the census.crates/batten/tests/it/config_lint.rs: against a real base ref viapr_fixture. One case carries all three arms, because a case asserting only the narrowing passes trivially under a gate that stopped firing at all.RULE_NARROWING_ON_ARRIVALemptied, the narrowing case fails and all three discriminating mirrors stay green.mise run verifygreen.