feat: add opt-in lenient mode with neutral semantics - #12
Conversation
Introduce NewEngineWithOptions and WithLenientMode Option. The Evaluator gains a lenient bool field (zero allocation cost) wired from the Engine. NewEngine() behavior is unchanged (strict mode); lenient is opt-in only.
Add lenientCompare() that applies null-aware semantics when at least one operand is missing: eq null<->value is false (null<->null is true), ne flips accordingly, not in becomes true, and ordering/string/datetime ops stay false. Strict mode is unchanged: the new branch only runs when e.lenient is set.
Add Lenient* fixtures covering eq/ne/lt/gt/le/ge/co/sw/ew/in/not_in with missing attributes, both-sides-missing, nil-vs-concrete, and/or/not, nested missing chains, datetime operators and presence (unaffected). Wire a TestRulesLenient runner using NewEngineWithOptions(WithLenientMode).
Add Example_lenientMode with a verified // Output block that contrasts strict vs lenient behavior for missing attributes, demonstrating ne/not in/eq null.
Add a 'Lenient Mode (Null-Aware Semantics)' section with the SQL-ish operator table, enabling snippet, example and notes. Register NewEngineWithOptions in the API section, add the feature to the Exclusive Features table, and link it from the TOC.
Add lenient mode bullet to the supported operations list and document SQL-ish null semantics under Type System Compliance. Also fix linter findings (exhaustive nolint, wsl_v5 whitespace) introduced by the new branch.
|
Warning Review limit reached
Next review available in: 56 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. 📝 WalkthroughWalkthroughAdds an opt-in lenient mode for missing-attribute comparisons. Strict mode remains the default. The change adds engine options, evaluator behavior, operator fixtures, executable coverage, examples, and documentation. ChangesLenient comparison mode
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
PLAN.md was a working draft of the implementation plan and should not ship in the repository.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@PLAN.md`:
- Around line 5-7: Atualize o objetivo em PLAN.md para alinhar a descrição do
modo “lenient” ao contrato explícito de nil: remova “ou nil” da regra que
transforma campos ausentes em null, ou declare claramente a exceção de que nil
explícito permanece presente, mantendo a definição detalhada das linhas
posteriores consistente.
In `@README.md`:
- Line 946: Reconcile the exclusive-feature count in README.md: the table now
lists seven capability rows when “rule.D Type Alias” is excluded, while the
summary reports six. Update the summary to reflect the actual count, or
explicitly define a counting rule that explains the exclusion.
In `@test/lenient_fixtures.go`:
- Around line 149-156: Extend LenientPresenceTests with explicit-nil comparison
cases that execute lenientCompare: add x eq y using rule.D{"x": nil} with y
absent and expect false, and x ne y with the same data and expect true. Keep the
existing pr cases unchanged and use the established test-case structure.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 54e83c91-12c5-47e7-9277-13654b4ca0b6
📒 Files selected for processing (8)
CLAUDE.mdPLAN.mdREADME.mdengine.goevaluator.goexample_test.gotest/lenient_fixtures.gotest/rule_engine_test.go
Address CodeRabbit review: extend LenientPresenceTests with x eq y /
x ne y cases using rule.D{"x": nil} (present) vs y absent, expecting
false and true respectively. These exercise lenientCompare when only
one operand is missing. Also fix the README exclusive-features count
(6 -> 7 major extensions) now that Lenient Mode is listed.
|
Addressing CodeRabbit review (3 actionable findings) per the "verify against current code, fix only still-valid issues" guidance: 1. 2. 3.
These exercise Validation: |
A comparison predicate over a missing attribute now returns true (neutral) across all comparison operators (eq, ne, lt, gt, le, ge, co, sw, ew, in, not in, datetime). This is the identity element for AND-chains, the dominant pattern in betting lifecycle rules, so a missing optional field drops out of the conjunction instead of failing it. The SQL-ish per-operator logic (null eq value -> false, null ne value -> true, ...) is removed in favor of a single result.Bool = true branch. Neutrality only holds inside an AND-chain: under not, or, or list quantifiers it becomes decisive (documented in code + tests). pr, logical and quantifier operators are unaffected. Tests in test/lenient_fixtures.go and Example_lenientMode are updated to the neutral truth table; the old SQL-ish expectations are replaced.
matheosu
changed the title
The lenientCompare function was removed when switching to uniform neutral semantics; update the note to refer to 'the lenient branch' instead.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/lenient_fixtures.go`:
- Around line 20-23: Add executable quantifier fixture cases to TestRulesLenient
for any, all, and none using missing comparison attributes, covering
short-circuit behavior and vacuous truth as documented. Ensure the assertions
match the existing lenient semantics and keep the fixture group integrated with
the current test structure.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 020844e9-b136-4cb1-b400-0e003105e5e6
📒 Files selected for processing (6)
CLAUDE.mdREADME.mdengine.goevaluator.goexample_test.gotest/lenient_fixtures.go
🚧 Files skipped from review as they are similar to previous changes (3)
- example_test.go
- CLAUDE.md
- engine.go
| * Composition consequences (documented, intentional): | ||
| * not (x eq 10) -> not true -> false | ||
| * x eq 10 or y eq 20 -> OR short-circuits to true | ||
| * selections none (r gt 0) -> sees true per element where r is absent -> false |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add executable quantifier fixtures.
Lines 20-23 document none behavior, but TestRulesLenient does not run a quantifier fixture group. Add any, all, and none cases with missing comparison attributes. Verify short-circuiting and vacuous truth where applicable.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/lenient_fixtures.go` around lines 20 - 23, Add executable quantifier
fixture cases to TestRulesLenient for any, all, and none using missing
comparison attributes, covering short-circuit behavior and vacuous truth as
documented. Ensure the assertions match the existing lenient semantics and keep
the fixture group integrated with the current test structure.
Source: Coding guidelines
3 participants
📝 Summary
This PR adds an opt-in lenient mode to the rule engine where a comparison predicate involving a missing attribute returns
true(neutral), instead of being coerced tofalseunconditionally. This is the identity element forand-chains — the dominant pattern in betting lifecycle rules — so a missing optional field drops out of the conjunction instead of failing it.Motivation
In the current strict (default) mode, when an attribute is missing from the context, every comparison involving it returns
false. This is fast and predictable but failsand-chains as soon as any optional field is absent:For a betting lifecycle, "settled_at absent" should mean "no time constraint", not "the rule fails". Lenient mode fixes this.
API
NewEngine()andNewEngineWithOptions()with no options keep the original strict behavior, so existing code is 100% unaffected.Semantics (neutral)
The table applies symmetrically — "missing operand" covers null on either side (left or right). The lenient path is only triggered when an attribute is missing from the context (key absent from the map); an explicit
nilvalue is still considered present (see Notes).eq/==truene/!=truelt,gt,le,getrueco,sw,ewtrueintruenot intruedq…dg)trueprfalseif missing /trueif present, incl.nil)and,or,notand the list quantifiers (any/all/none) are unaffected — they keep their normal truthiness behavior. This leads to intentional composition consequences:not (x eq 10)withxabsent →not true→falsex eq 10 or y eq 20withxabsent →orshort-circuits totrueselections none (r gt 0)per element whereris absent →noneseestrue→ returnsfalseThese are deliberate: neutrality is neutral only inside an
and-chain. For negation/quantifier-heavy rules, prefer strict mode or keep the relevant fields present.Example
Implementation
Evaluatorgains alenient boolfield (zero allocation cost — field read by value).evaluateComparisonOperator: when an operand is missing ande.lenientis set,result.Bool = true. The neutral behavior is uniform across all comparison operators.pr(presence) is handled in a separate code path and is intentionally unchanged.nilvalues are still considered present (IsValid=true) and dispatched to the normal comparison path — they do not go through the lenient branch.What we did (commits)
engine.go—Option,WithLenientMode(),NewEngineWithOptions()evaluator.go—lenient boolfield + neutral branch inevaluateComparisonOperatortest/lenient_fixtures.go— ~70 cases covering eq/ne/lt/gt/le/ge/co/sw/ew/in/not_in with missing attributes, both-sides-missing, missing-on-right, nil-vs-concrete, and/or/not composition, nested missing chains, datetime operators and presencetest/rule_engine_test.go—TestRulesLenientrunner usingNewEngineWithOptions(WithLenientMode())example_test.go—Example_lenientModewith verified// Output:blockREADME.md— new "🟢 Lenient Mode (Neutral Semantics)" section, TOC, API entry, Exclusive Features tableCLAUDE.md— lenient mode spec under Type System Compliance + features list⚡ Performance Impact
Strict mode is bit-for-bit identical (the new branch only runs when
e.lenientis set). Lenient mode adds a single predictable branch in one hot-path function and a field read — no allocations.Benchmarks (Apple M4 Pro), strict path unchanged:
All core evaluations remain sub-100ns with 0 allocs/op (the
In1-alloc andComplexBetting2-allocs are pre-existing Go runtime[]anyhandling, documented in CLAUDE.md, and untouched by this PR).✅ Checklist
golangci-lint run ./...→ 0 issues)gofmtcleanFuzzRuleExecution, 20s, no panics)Evaluatorotherwise stateless per evaluation)NewEngine()API-compatible — lenient is opt-in onlySummary by CodeRabbit
New Features
nilvalues.Tests