Skip to content

A computed fact for routing-assertion findings (#395) - #399

Open
patelankeet2 wants to merge 2 commits into
mainfrom
feature/395-computed-facts-every-finding-type
Open

patelankeet2 wants to merge 2 commits into
mainfrom
feature/395-computed-facts-every-finding-type

Conversation

@patelankeet2

Copy link
Copy Markdown
Collaborator

Part of #395 (US-62), not closing it.

What #395 actually covers

ai/explain.py had three computed-fact functions for one class of mistake: a model attributing which of two opposite states is which (_compute_dead_rule_outcome, _compute_policy_outcome, _compute_expected_actual_outcome). Before writing anything, I inventoried every status="found" evidence shape across access_control.py, policy_compliance.py, routing.py and change_impact.py to see which ones have no equivalent guard yet -- six shapes, not one.

Alternatives considered for scope

  1. Cover all six shapes in one PR, to literally satisfy "every finding type" as the acceptance criteria is worded.
  2. Cover only the shapes with a real two-sided attribution risk (the same risk class the existing three guard against), and explicitly document why the rest don't need one.
  3. One shape at a time, in review-sized increments, starting with the highest-confidence/highest-risk one.

Went with a version of 3, informed by 2. Three of the six shapes (access_control's GUARANTEES proof, undefined-reference, and unused-structure findings) are flat, one-sided facts with nothing to attribute to the wrong side -- adding a computed-fact function there would be guarding against a risk that doesn't exist, which isn't what these functions are for. That leaves three real candidates: the routing-assertion shape (this PR), and two change_impact.py shapes (compareFilters, differentialReachability) as follow-ups. Landing them one at a time, each independently live-tested and reviewed, matches how the first three functions were added and keeps each PR small enough to actually verify rather than trust.

What this PR does

_compute_routing_outcome() for routing.py's assertion findings ("Expected REACHABLE/UNREACHABLE for a flow from X to Y, but traceroute...").

A different failure than the one I was looking for

Live-tested a real routing violation (routing-missing-route, RT-001) six times with no computed fact, expecting to find the same "which state is which" inversion the other three guards catch. That never happened -- 0 of 6 runs inverted REACHABLE vs UNREACHABLE.

What happened instead: 3 of 6 runs inverted the flow's source and destination. "The HQ network cannot reach the branch network" (correct) became "The HQ network cannot be REACHED FROM the branch network" -- which describes the opposite flow, branch-to-HQ instead of HQ-to-branch. Both IP addresses were genuine in every run, so _names_an_invented_fact() would not have caught it: the addresses were just attributed to the wrong role in the relationship, not invented.

run 1: "The HQ network cannot reach the branch network."                    correct
run 2: "The HQ network cannot be reached FROM the branch network."          inverted
run 3: "The HQ network cannot be reached FROM the branch network."          inverted
run 4: "The HQ network cannot reach the branch network."                    correct
run 5: "The HQ network cannot reach the branch network."                    correct
run 6: "The HQ network cannot be reached BY the branch network."            inverted

So the computed fact states the direction explicitly ("the flow FROM {src} TO {dst} -- that direction, not the reverse") as well as the real disposition, rather than only the disposition the way the other three functions state only their two-state fact.

Re-verified after the fix

Same finding, six more live runs, fix in place:

run 1-6: all state "The HQ network cannot reach the branch network" / "the flow from 10.10.10.5 to 10.20.20.5"

0 of 6 inverted the direction.

A gap I found in my own first commit

Mutation-tested by removing _compute_routing_outcome() from _build_prompt()'s chain -- and my own unit tests for the function (tested in isolation) didn't catch it, because none of them went through the actual wiring. Each of the other three shapes has a dedicated test_prompt_includes_the_computed_outcome_for_a_X_finding test that exercises the real chain; routing was missing its equivalent. Added it in a second commit, re-ran the same mutation, caught correctly the second time. Recording this because a mutation survivor from your own code is easy to wave past, and this one would have shipped a function that looked tested but wasn't wired to anything.

Tests against the real producer, not a hardcoded literal

_real_routing_detail() calls the genuine routing._evaluate() to build the evidence string, the same discipline _real_detail() already established for policy_compliance (#145/#169) -- a future rewording of _evaluate() fails these tests loudly instead of leaving them green while testing a string that no longer exists.

Verified

python -m pytest tests/test_explain_safety_net.py tests/test_routing_classification.py -q
  97 passed

python -m pytest tests/ -q
  1877 passed, 13 skipped, 0 failed

No em-dashes in either changed file.

What this deliberately does not do

Does not cover the two change_impact.py shapes (compareFilters, differentialReachability) -- planned as follow-up PRs against the same issue, see "Alternatives considered" above. Does not touch _compute_dead_rule_outcome, _compute_policy_outcome, or _compute_expected_actual_outcome. Does not add a computed-fact function for the three flat-fact shapes, with the reasoning for each documented in the issue discussion rather than silently skipped.

Part of #395. Does not close it.

ai/explain.py had three computed-fact functions (dead rules, policy
compliance, access_control policy statements) for one class of
mistake: a model attributing which of two opposite states is which.
Routing assertions had no equivalent, and this one found a different
failure, not the one it was built looking for.

Live-tested a real routing violation six times with no computed fact:
the reachability direction itself (reachable vs not) was never
inverted, but 3 of 6 runs inverted the flow's source and destination --
'the HQ network cannot be reached FROM the branch network' instead of
the real 'HQ cannot reach branch'. Both IPs were genuine in every run,
so the existing invented-fact guard would not have caught it -- the
addresses were just attributed to the wrong role in the relationship.

_compute_routing_outcome() states the flow's direction explicitly
(source before destination, 'that direction, not the reverse') as
well as its real disposition, rather than only the disposition the
way the other three functions state only their two-state fact.

Re-ran the same live finding six times with the fix in place: 0 of 6
inverted the direction.

Tests use the real routing._evaluate() to build the evidence string,
not a hand-typed literal, same discipline test_the_safeguard_can_read_
what_policy_compliance_actually_writes already established (#145/#169)
-- a future rewording of _evaluate() fails these tests loudly instead
of leaving them green while testing a string that no longer exists.

python -m pytest tests/test_explain_safety_net.py tests/test_routing_classification.py -q
  96 passed.
Found by mutation-testing my own previous commit: removing
_compute_routing_outcome() from _build_prompt()'s chain broke nothing
in the direct unit tests, because they call the function in isolation
rather than through the chain it is supposed to be wired into. Each
of the other three computed-fact functions already has a dedicated
test_prompt_includes_the_computed_outcome_for_a_X_finding test that
exercises the real wiring -- routing was missing its equivalent.

python -m pytest tests/test_explain_safety_net.py tests/test_routing_classification.py -q
  97 passed (was 96).
@patelankeet2 patelankeet2 added enhancement New feature or request layer: ai Layer 2 — local LLM explanation and Q&A phase: 3 Market readiness -- what stands between us and a product someone could buy. labels Oct 13, 2026
@patelankeet2 patelankeet2 self-assigned this Oct 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request layer: ai Layer 2 — local LLM explanation and Q&A phase: 3 Market readiness -- what stands between us and a product someone could buy.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant