A covariate check that cannot report is worse than no covariate check - #18
Merged
Conversation
`check_covariates` decided path-opening as
separated_alone = d_separated(back_door, target, outcome) # computed ONCE
opens = separated_alone and not d_separated(back_door, target, outcome, (name,))
so whenever the target and outcome were ALREADY d-connected in the back-door graph --
which is what an unmeasured common cause produces, and therefore the normal case in any
real system -- `opens` was False for every covariate and the report read
Admissible: ['A', 'B', 'C', 'W']
with both finding sections empty. Three defects at once, and the second is what makes this
more than cosmetic:
1. Nothing distinguished "tested and clean" from "the test could not fire".
2. A collider the module flags correctly on a clean graph goes UNFLAGGED the moment a
confounder exists anywhere. The finding is not merely unstated, it is LOST.
3. `W` is unobserved. Covariates were validated against `variable_set`, never
`observed_set`, so the report advised conditioning on a latent variable.
The module's own docstring warned about exactly this shape for the name-clash case -- "a
check that runs, passes, and cannot report -- every covariate would come back admissible
for the wrong reason" -- and the guard was installed there and not here.
★ The repair is not a warning label. Path opening asks whether conditioning BREAKS a
separation; when nothing was separated there is nothing to break, and the question with an
answer is whether the covariate CLOSES the open path -- which is the entire purpose of an
adjustment set, and which a check that only ever asks "does this open a path?" cannot
express. So the previously-vacuous branch now answers rather than merely refusing.
back_door_open graph-level finding, reported once, leading the summary
blocks_path names the covariates that close the open path
undecided no path-level verdict was reachable; never admissible
observed unobserved is a STRUCTURAL refusal, not an undecided one, so the
report does not file a hard fact under an "unanswerable" heading
⚠️ FOUND WHILE MUTATION-TESTING, and unrelated to the above: the O(n^2) gate on
`mechanism_dependencies` had stopped gating anything. It timed CONSTRUCTION, on the stated
grounds that "every MechanismGraph validates acyclicity at construction" -- true when
written, and PR #9 made it false by lifting C1 to a query-time condition. Restoring the
pairwise implementation left construction at 0.0038s/0.0154s, unchanged, while
`mechanism_dependencies` itself went 0.0006s -> 0.2764s at n=1500. The gate is now timed on
the routine (mutation-tested: fires, 28s vs 0.08s), and a companion test COUNTS the calls
to pin the premise, because a timing proxy there would be the same mistake one layer down.
That gate also took min-of-5 after a warm-up: a single 4ms sample was dominated by whatever
ran before it, and it fired when this branch merely ADDED a test file.
Five mutations on the covariate repair, all dead, restored and verified by content:
path_test_applicable := True (the bug reinstated), admissible := True in the open branch,
drop the observed check, back_door_open := False, closes := False.
README gains the third finding with its real output, gated by a new `test_readme_smoke`
case including `Closes it: ['donor']`. 340 passed, 1 xfailed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
The defect
check_covariatesdecided path-opening asSo whenever the target and outcome were already d-connected in the back-door graph — what an unmeasured common cause produces, and therefore the normal case in any real system —
openswasFalsefor every covariate and the report read:with both finding sections empty. Three defects at once, and the second is what makes this more than cosmetic:
Wis unobserved. Covariates were validated againstvariable_set, neverobserved_set, so the report advised conditioning on a latent variable.The module's own docstring warns about exactly this shape for the name-clash case — "a check that runs, passes, and cannot report — every covariate would come back admissible for the wrong reason" — and the guard was installed there and not here.
The repair is not a warning label
Path opening asks whether conditioning breaks a separation. When nothing was separated there is nothing to break — and the question that does have an answer is whether the covariate closes the open path. That is the entire purpose of an adjustment set, and a check that only ever asks "does this open a path?" cannot express it. So the previously-vacuous branch now answers rather than merely refusing.
back_door_openblocks_pathundecidedobservedThe O(n²) gate on
mechanism_dependencieshad stopped gating anything. It timed construction, on the stated grounds that "every MechanismGraph validates acyclicity at construction" — true when written, and PR #9 made it false by lifting C1 to a query-time condition. Restoring the pairwise implementation left construction at 0.0038s/0.0154s, unchanged, whilemechanism_dependenciesitself went 0.0006s → 0.2764s at n=1500.Now timed on the routine (mutation-tested: fires, 28s vs 0.08s), with a companion test that counts the calls to pin the premise — a timing proxy there would be the same mistake one layer down. It also takes min-of-5 after a warm-up: a single 4 ms sample was dominated by whatever ran before it, and it fired when this branch merely added a test file.
Verification
Five mutations on the covariate repair, all dead, restored and verified by content:
path_test_applicable := True(the bug reinstated)admissible := Truein the open-back-door branchback_door_open := Falsecloses := FalseREADME gains the third finding with its real output, gated by a new
test_readme_smokecase includingCloses it: ['donor']. The 2,596-verdict conformance sweep is unchanged and still green.340 passed, 1 xfailed.ruff check .clean.🤖 Generated with Claude Code