Repository navigation
Name the dead line's own stated action explicitly, not just the outcome (#384) - #401
Open
patelankeet2 wants to merge 1 commit into
Open
patelankeet2 wants to merge 1 commit into
patelankeet2 wants to merge 1 commit into
Conversation
…me (#384) _compute_dead_rule_outcome() already captured the dead line's own action via its regex ('(?P<dead_action>PERMIT|DENY)') but never used it -- the computed fact stated only the real outcome, never what the dead line itself says, on the reasoning that the action 'never takes effect, regardless of what it says' made stating it unnecessary. Found live, not assumed (#384): the model sometimes mischaracterised the dead line's own action anyway, even while correctly stating the outcome -- 'which is supposed to block traffic' for a line whose real action is PERMIT. Correct about the outcome, backwards about a different fact. Re-measured on the same real finding (AC-002, rtr-us5-messy) immediately before fixing: 3 of 8 live generations mischaracterised or were internally inconsistent about the dead line's own action, worse than the 1-in-8 originally reported in #384. Re-measured again with the fix in place: 0 of 8. Both facts are now stated explicitly, the same discipline the other three computed-fact functions already use for their own two-sided facts -- the dead line's own stated action, and the real outcome, named as two separate things rather than leaving the model to read one of them off the raw evidence and risk inverting it. python -m pytest tests/test_explain_safety_net.py -q -> 81 passed (was 78).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #384.
The bug
_compute_dead_rule_outcome()'s regex already captured the dead line's own action ((?P<dead_action>PERMIT|DENY)), but the function never used it. The computed fact stated only the real OUTCOME (what happens to matching traffic), never what the dead line itself says its own action is -- on the reasoning that the action "never takes effect, regardless of what it says", so stating it seemed unnecessary.Found live during #346's held-out set work: the model sometimes mischaracterised the dead line's own action anyway, even while correctly stating the outcome -- "which is supposed to block traffic" for a line whose real action is PERMIT. Correct about the outcome, backwards about a different, separate fact.
Re-measured before fixing
Same real finding (AC-002,
rtr-us5-messy), 8 fresh live generations against the code as it stood before this PR:3 of 8 -- worse than the 1-in-8 originally reported in #384, and good evidence this is a real, recurring failure mode worth fixing now rather than a one-off.
The fix
State both facts explicitly, the same discipline the other three computed-fact functions already use for their own two-sided facts: the dead line's own stated action, and the real outcome, named as two separate things.
Re-measured after fixing
Same finding, 8 more live generations with the fix in place: 0 of 8 mischaracterised the dead line's own action. Every run correctly described it as a permit/allow rule.
Tests
Three new tests pin the dead line's own action is named explicitly (both PERMIT and DENY cases), and that the own-action fact and the outcome fact can genuinely differ and both get stated -- not just whichever one the function used to consider more important.
Mutation-tested: reverted the return string to the original, action-less version. All three new tests failed, for the right reason (the specific
"own stated action is X"substring missing). Reverted.Verified
No em-dashes in either changed file.
What this does not do
Does not touch
_compute_policy_outcome()or_compute_expected_actual_outcome()-- both already state both sides of their own two-state fact, this brings_compute_dead_rule_outcome()to the same standard rather than changing something that already works. Does not touch the routing function added in #399, or the compareFilters investigation currently on #395 -- unrelated shapes, unrelated files beyondai/explain.pyitself.