Skip to content

fix(ranking): prune zero-support rules in prune_harmful_rules#26

Open
AZERDSQ131 wants to merge 1 commit into
KRLabsOrg:mainfrom
AZERDSQ131:fix/tighten-dead-rule-pruning
Open

fix(ranking): prune zero-support rules in prune_harmful_rules#26
AZERDSQ131 wants to merge 1 commit into
KRLabsOrg:mainfrom
AZERDSQ131:fix/tighten-dead-rule-pruning

Conversation

@AZERDSQ131

Copy link
Copy Markdown

Problem

Rules with validated_support == 0 never match anything on the dev split, but prune_harmful_rules only drops a rule when its marginal F1 is known and strictly negative. A zero-support rule's marginal F1 is exactly 0.0 (removing it changes nothing), which is not < min_marginal_f1, so it survives as dead weight.

Fix

Added a min_support: int = 1 parameter to prune_harmful_rules. Rules whose validated_support is below this threshold are now dropped too, independent of the existing marginal-F1 logic (unchanged). Default of 1 is conservative — a single dev match is enough to keep a rule.

Tests

  • test_prunes_zero_support_rule: a rule that never matches the dataset gets dropped with the default min_support=1.
  • test_min_support_zero_keeps_zero_support_rule: min_support=0 disables the new check, preserving the old behavior.
  • Existing prune_harmful_rules tests still pass unchanged (their rules all have support ≥ 1).

pytest tests/ -q → 313 passed, 1 skipped. ruff check and mypy rulechef/ranking.py clean.

Closes #4

Rules with validated_support == 0 never match anything on the dev
split, so removing them changes ensemble F1 by exactly 0.0 -- which
isn't < min_marginal_f1, so they survived as dead weight cluttering
the rule report and rule count.

Add a min_support parameter (default 1, conservative: a single dev
match is enough to keep a rule) that also drops rules below that
support threshold, independent of the existing marginal-F1 check.

Closes KRLabsOrg#4
@adaamko

adaamko commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Reviewed — this is right, and it matches what #4 asked for: zero-support rules have marginal F1 of exactly 0.0 so the old threshold never caught them. Verified the only library call site (engine.rank_rules) stamps validated_support via rank_rules before pruning, so the new default is safe there. Tests and typecheck are green. One thing blocks the merge: the rights checkbox in the PR body (the failing required check). Please tick the '- [x] I certify...' box in the PR description (same for your other open PRs) and this can go in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tighten dead-rule pruning

2 participants