Skip to content

tests/test_path_guard_hygiene.py: each stat helper is loaded by its AST and asked with its stat refused, the scan follows every binding #1111 left out, and parity takes the correct spellings it refused (#1134) - #1149

Merged
brettheap merged 8 commits into
mainfrom
fix/1134-hygiene-test-nits
Oct 23, 2026

Conversation

@brettheap

@brettheap brettheap commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

What

The follow-ups r1111b found when it re-checked #430's slice Z (#1111) (its NITs 1 to 6), and r1111c's NIT 1, collected in #1134. This is a test-only change: one file, tests/test_path_guard_hygiene.py. No shipped tool changes, and no --help text either. It is outside v1.2.0 by Brett Heap's ruling of 2026-10-21T10:32:04Z ("leave the follow-ups out of v1.2.0"), and it has no milestone.

NIT 1: a behaviour test

test_every_stat_helper_answers_absent_false_and_raises_a_refusal asks each _is_a/is_a that _helpers finds. That is the twelve, and any added later.

File Statements run
adopt-project.py, scripts/family.py import errno, _ABSENT_ERRNOS, _ABSENT_WINERRORS, _absent, _is_a
update-shape.py import errno, import os, the two constants, _says_absent, _mode_of, _is_a
shape-doctor.py import errno, ABSENT_ERRNOS, ABSENT_WINERRORS, refused_read, is_a
scripts/bump-leg.py, scripts/shape_advisory.py, the assembly root's validate-pins.py, validate-manifest.py and bootstrap.py, the family root's validate-family.py, bootstrap.py and siblings.py import errno, the two constants, _is_a
  • What it asks. os.stat and os.lstat are patched to raise for one name only; Path.stat and Path.lstat call them. Each case below is asked with stat.S_ISREG and with stat.S_ISDIR (r1149's S2).
    • ENOENT, ENOTDIR, EBADF and ELOOP must answer False.
    • Windows' 21, 123 and 1921, each set as winerror beside EINVAL, an errno that is not absent, must answer False.
    • Every other errno in errno.errorcode (126 here: EACCES, EPERM, EIO, EINVAL and ESTALE among them), and EACCES with winerror 5 and with 32 (Windows' access denied and sharing violation), must come back raised, as the very exception the stat raised (is). A set widened after it is set is named whichever errno it adds (r1149's S1).
    • A real link to nothing is asked all of the above with only the stat that follows it refused, so its lstat answers a link. A helper that answers the refusal from that lstat is named (r1149's S2). The link is made in the test's folder and skipped where none can be made; it is asked without a flag only, because a flag may ask lstat on purpose (bootstrap.py's link).
    • A file, a folder and nothing must answer as the disk does.
    • A keyword-only bool flag with a default is asked both ways: the assembly root's bootstrap.py has link=False.
    • A failing run prints the first 20 problems and counts the rest, since a helper wrong for every errno gives hundreds.
  • Its own proof.
    • ROUTES holds 18 changes to the HELPER sample. They are the routes tests/test_path_guard_hygiene.py: a behavioural parity test, the scanner's other alias forms, the correct helpers parity refuses, and four self-test gaps (r1111b's NITs; follow-up of #430 slice Z) #1134 lists (or isinstance(exc, PermissionError), a dead return False before the raise, _ABSENT_ERRNOS += (errno.EACCES,), exc.args[0] ==, exc.strerror ==, contextlib.suppress(PermissionError), a method, a lambda, _is_a = _lenient), three more: absent and refused swapped, a Windows error dropped, and a branch only a flag takes; and six from r1149: ESTALE and EINVAL added to the errno set, winerror 5 and 32 added to the Windows set, and a refusal read as absent for S_ISDIR only and for S_ISREG only. Each is named by the error it answers wrongly, a kind route for its kind and no other.
    • LINK_ROUTES holds 4 changes that only a link shows: a refusal answered from os.lstat, from Path.lstat or from os.stat(..., follow_symlinks=False), and a link to nothing read as there (path.is_symlink()). Each is right for a name nothing is at and is named for the link alone. test_the_dangling_link_is_a_link_to_nothing holds the link itself.
    • test_a_helper_that_stats_through_os_stat_is_right_for_a_link_too: os.stat(path) and os.stat(path, follow_symlinks=True) pass, so the link's refusal is of a stat that follows it and of no other.
    • test_a_helper_added_later_is_asked_like_the_twelve: a file HELPERS does not name is asked all the same, and its wrong helper is named by its file.
    • absent_constant_problems: the constants test is a function now, so that the right sets, a wrong set and a top-level += or |= on _ABSENT_ERRNOS, _ABSENT_WINERRORS or the doctor's spellings are proved on planted text.
    • test_the_behaviour_test_loads_only_what_the_helper_reads: a module's sys.path.insert, its sibling import and its main do not run, and a helper that needs repo_shape is refused by name.
    • test_the_behaviour_test_asks_the_disk_too: a helper that answers the wrong way round is named by the three disk controls.
  • The routes killed on a real helper. I applied each route to a scratch copy of ONE helper's file, adopt-project.py, and ran the hygiene file. Under base's test file (038dafba) every route survives; under this head every one is killed:
Id Route in adopt-project.py Base Head
N1 _absent: or isinstance(exc, PermissionError) survived killed
N2 _is_a: a dead return False before the raise survived killed
N3 _ABSENT_ERRNOS += (errno.EACCES,) after the constants survived killed
N4 _absent: or exc.args[0] == errno.EACCES survived killed
N5 _absent: or exc.strerror == os.strerror(errno.EPERM) survived killed
N6 _is_a: with contextlib.suppress(PermissionError): raise, then return False survived killed
N7 _is_a = _lenient at the end of the module survived killed
N8 _is_a: or _R1134_POLICY.lenient(exc), a method survived killed
N9 _absent: _r1134_lenient(exc) or ..., a lambda survived killed
N10 _is_a: if not _absent(exc), absent and refused swapped survived killed

r1111b's parity probe variants tell the same story. Its ten "wrong" helpers that pass static parity are all named by the behaviour test. One of its "correct" variants, os.stat(path), is named too, because the sample has no import os and the call raises NameError.

NIT 2: what the scanner missed

  • Now followed. _os_path_names now returns a Reach: each name, and each attribute name, bound to os, to os.path (or its twins), or to a guard. It follows:

    • o = os;
    • from os.path import * and from os import *;
    • an alias bound by tuple unpacking, a walrus, a for target over a literal, a with target (its value, or an argument of the call that makes it), a default parameter, or a class or instance attribute;
    • importlib.import_module, __import__ and sys.modules[...] or .get(...), each by a constant name;
    • every later call of a guard bound once to a name (EX = os.path.exists, chk = isfile, IS_DIR = Path.is_dir, class C: ex = os.path.exists).

    PLANTED holds 23 self-tests, one or more per form (r1149's NIT 3 added nested tuples, comprehension targets, keyword-only defaults, an annotated class attribute and the bare posixpath, ntpath and genericpath). Each plants an example and asserts the exact sites found in its file, and nothing else.

  • importlib and sys.modules: detected. A constant name is cheap to read: _looked_up is about twenty lines. A name that is not a constant, or an alias of importlib or sys, stays on the known-limit list in the docstring.

  • The _os_path_names docstring is now true. It lists what is not caught. It no longer says "in no shipped tool today": it names the three Windows-only getattr(os.path, "isjunction", lambda _: False) lookups (shape-doctor.py's unclonable, and siblings.py's not_a_clone and family_link). Those three are the only ones r1111b's missforms.py and a grep find in today's tree.

  • Re-scan of today's 25 tools. The new scanner finds the same 190 sites in 18 files as base's does, compared site for site: none only on head, none only on base. So it makes no new finding, and no BENIGN or KNOWN_OPEN entry changes for NIT 2.

  • Measured against r1111b's alias_probe.py. Base misses 14 of its cases; head misses none. The 4 cases it over-flags (a parameter named path, a local o, a local exists, and a stored o.exists) are unchanged from base. They are the safe direction r1111b noted.

NIT 3: correct helpers that parity refused

On base, r1111b's parity_probe.py gets 6 false refusals of correct helpers. On head it gets none. test_the_parity_test_takes_a_correct_spelling_and_refuses_its_twin asks each correct spelling of both the parity test and the behaviour test. Each is taken by both. Its nearest wrong twin is still refused by parity, by name, and named by the behaviour test (a twin further off is not always refused: see the known limits):

Taken now How Its nearest wrong twin, still refused
errno not in NotIn is read like In not in (errno.ENOENT, errno.EACCES): its errnos are wrong
raise exc _reraises takes raise <the handler's name> raise FileNotFoundError(...): it does not re-raise
raise exc, against another name the same, and no other name other = FileNotFoundError(...); raise other (r1149's V22)
a frozenset({...}) constant _members unwraps frozenset(...) and set(...) around a literal frozenset({...}) | {errno.EACCES}: no literal set
import errno as E _errno_names reads every name import errno binds a class E whose names are all errno.EACCES: neither errno.<NAME> nor a number
except (FileNotFoundError, NotADirectoryError) HANDLERS takes the two subclasses that are absent errnos except (FileNotFoundError, PermissionError)
an unrelated except KeyError in a function the helper calls handlers are held only in a try that makes a call (_calls_out), which is the way from the stat to the answer _note's try: x.stat() with except PermissionError

NIT 4: self-test gaps

Mutant What kills it now
R2, the alias fixpoint cut to one pass SPELLINGS binds late = early before early = os.path
R3, AnnAssign not followed SPELLINGS has typed: Any = os.path
R7, _helpers without ClassDef test_a_class_or_an_async_def_by_a_helpers_name_is_held_to_parity
R13, _members skipping an element a parity row adds EACCES by name, which must be refused

R2 and R3 are r1111b's edits to code this change rewrote, so they ran adapted to the new code with the same meaning (R2a, R3a).

NIT 5: the KNOWN_OPEN owner check

NIT 6

The HELPER comment now says what the sample is: a helper in adopt-project.py's and scripts/family.py's shape (slices H and G), with its sets as module constants asked through an _absent of its own. It also says slice A's scripts/bump-leg.py asks the same sets inline.

r1111c's NIT 1: a MODEL tag

The two _CloneGit.is_dir entries (_PlannedRoot._into_git and _PlannedRoot._no_directory) are now tagged MODEL: "not a filesystem call at all: a method of the run's own model in memory that shares a guard's name and asks no stat".

I chose a new tag over widening OWN. OWN claims something about whose path a guard asks: one the tool made or owns. These two ask no path at all; the scan flags them only because the method is named is_dir. Widening OWN would blur that claim and hide this second class inside it. A tag of its own keeps it countable, and makes a reviewer ask "is this really not the disk?" separately. BENIGN keeps the same 175 entries (179 sites). By tag, as entries (sites), they are now OWN 55 (56), MODEL 2 (2), DELIBERATE 63 (65), ASKED 25 (25), ROOT 20 (21), DEST 9 (9) and PINNED 1 (1). KNOWN_OPEN keeps its 11.

Review round 1: r1149's findings

r1149 read the PR at ccbd0eb0 and found it READY on condition: no MUST, two SHOULDs and four NITs. This round answers each one in the same file and changes nothing else. A scratch copy of a shipped tool carries each mutant; no shipped tool changes.

Finding What changed Killed, measured
SHOULD 1: the behaviour test asked no refused Windows error and no errno but EACCES, EPERM and EIO, so a set widened after it was set passed both tests It asks every errno outside the four absent ones (126 here) and EACCES with winerror 5 and 32, and expects the same exception back. The constants test also refuses a top-level += or ` =on anABSENT_` constant (the optional part), because only two refused Windows errors are asked
SHOULD 2: a refused stat was asked with S_ISREG only, and a fallback to lstat passed Each case is asked with S_ISREG and S_ISDIR, and again of a real link to nothing whose stat alone is refused (skipped where no link can be made) U2 (a branch for S_ISDIR), U3 (os.lstat fallback), r1149's G4 (posix.lstat) and G5 are killed by the behaviour test on the real file. All 12 real helpers pass
NIT 1: the stale docstring of test_every_stat_helper_holds_the_one_errno_set It says in and not in; FileNotFoundError and NotADirectoryError beside OSError and ValueError; and raise or raise exc the docstring only
NIT 2: the six parity relaxations trust a name "Each wrong twin" is now "the nearest wrong twin", and a known limit says so: r1149's T1 to T6 pass parity and are named by the behaviour test the behaviour test (r1149 measured it; I re-ran twins_probe.py on the head: all six named)
NIT 3: eleven survivors with no self-test one row each, below all 11 killed
NIT 4: only keyword-only bool flags are flipped a known limit; today only bootstrap.py's link exists none needed

NIT 3's rows:

Mutant Row that kills it
V7, _reads ignores global test_the_slice_reads_a_global_a_function_changes_and_not_a_local
V10, nested tuples PLANTED["nested-tuple"]
V12, comprehension targets PLANTED["comprehension-target"]
V13, keyword-only defaults PLANTED["keyword-only-default"]
V14, an annotated class attribute PLANTED["annotated-class-attribute"]
V15, _members unwraps any one-argument call test_members_unwrap_a_frozenset_or_a_set_and_no_other_call
V16, the nothing disk control test_the_behaviour_test_asks_the_disk_too: a helper right only for the names the test refuses
V18, the "helper added later" path test_a_helper_added_later_is_asked_like_the_twelve
V19, import x of a non-stdlib module test_the_behaviour_test_loads_only_what_the_helper_reads: a plain import repo_shape
V20, bare posixpath, ntpath, genericpath PLANTED["bare-os-path-twins"]
V22, raise <another name> the twin row raise-other-name

Tests

CI's first run on cd935163 (Python 3.14.8, run 39753985022) found one failure: ast.dump leaves out ctx=Load() from Python 3.13, so the text a parity row hard-coded no longer matched, and the three messages that name a piece of the helper (_members twice, _caught) now use ast.unparse, which reads the same on every Python; the row, one row for each of the other two sites, and the _members table assert that text.

Mutants

Each mutant was applied to a scratch clone, the hygiene file was run on it (-x), and the file was restored. Its sha256 then matched the HEAD blob each time: 136 of 136 on head and 20 of 20 on base.

143 killed of 143 on head fa67fac7, the head after CI's 3.14 fix, with 0 survivors. The 143 are the 136 below plus B1 to B7, each of which breaks one of the three ast.unparse messages; R13 is adapted to ast.unparse. On cd935163: 136 killed of 136 on head cd935163; no survivor. The first run of this round, on the same code before its last four tests, killed 126 of 130. The four survivors were mine (F4b: only S_ISDIR asked; F8b: the link made to a file that is there; F10: the += check not applied; F15: no absent case asked of the link). Each now has a test, below, and the whole set was run again.

On head the 136 are: the 74 of the first writer's run (as it ran them, but for W15, W16 and W24, adapted to code this round moved, same meaning), r1149's 22 V and 3 U, six of its wrong-helper routes (G2, G2b, G4, G5, G6, G11), and 31 of mine on this round's code (F1 to F21, with variants). On base 038dafba I ran 20 of the first 74: r1111b's ten survivors that apply to this file (R2, R3, R7, R13, Z1, Z2, Z3, Z6, Z7, Z11) and N1 to N10. Every one survived there. At ccbd0eb0 r1149 measured 11 of its 22 V killed and 0 of its 3 U.

Group Base 038dafba Head cd935163 What kills it on head
r1111b's survivors R2, R3, R7, R13 4 survived 4 killed (R2 and R3 as R2a and R3a: same meaning, on rewritten code) SPELLINGS (R2a, R3a); the _helpers self-test (R7); a parity row (R13)
r1111b's Z1, Z2, Z3 (one route each in bump-leg.py, validate-pins.py, shape_advisory.py) 3 survived 3 killed the behaviour test
r1111b's Z6, Z7 (o = os, from os.path import * in path_classify.py) 2 survived 2 killed the hygiene test, by file and line
r1111b's Z11 (FOLLOW_UP = "#1101") survived killed entry_problems: the docstring names #1110
N1 to N10, NIT 1's routes in adopt-project.py (table above) 10 survived 10 killed the behaviour test
r1111b's other 20: 18 as it wrote them (X3, X4, X5, X6, X15, X18, T2, T3, R14, R15, R16, Z4, Z5, Z8, Z9, Z10, Z14, Z15), R1 as R1a, and T6 as W21 (counted below) not run 19 killed here as on #1111
W1 to W34 (35, W33 in two), the first writer's, on the new code n/a 35 killed each by a named self-test: PLANTED (W1-W7, W22, W23, W25-W30), the twin table (W9, W10, W12, W14, W15, W18, W31, W32), the parity rows (W8, W13), ROUTES (W16, W17, W19, W24, W34), the loader test (W20), entry_problems (W21, W33a, W33b), the helper parity test (W11); these are the killers named on ccbd0eb0, and -x reports the first failing test in file order, so a test of this round can come first (W31: the _members table)
r1149's V1 to V22, on the new test code not run (11 of 22 killed at ccbd0eb0) 22 killed the eleven survivors by the rows above; the rest as before
r1149's U1, U2, U3 in adopt-project.py not run (all 3 survived at ccbd0eb0) 3 killed the behaviour test on the real file (U1 also the += check)
G2 (ESTALE), G2b (EINVAL), G4 (posix.lstat), G5 (os.lstat), G6 (primary posix.stat), G11 (a fresh equal exception) in adopt-project.py not run 6 killed the behaviour test (G2, G2b also the += check; G11 also parity)
F1 to F21, mine, on this round's code (31 with variants) n/a 31 killed the first failing test in file order: ROUTES (F1, F3, F3a, F3b, F3c, F4, F4b, F19), a right helper given a false problem in the parity twin table (F2, F3d, F7b), LINK_ROUTES (F5, F6, F7, F15), the link test (F8, F8b, F8c), the whole behaviour test on the real bootstrap.py with its flag (F9), the self-test of absent_constant_problems (F10, F11, F12, F16, F20, F21), the constants test (F13), the _shown test (F14, F14b, F14c), the os.stat spellings test (F17), the added-later test (F18)

Because the file is run with -x, a mutant is killed by the first failing test in file order. U1, G2 and G2b are killed first by the += check, which runs before the behaviour test. I ran them again with the += check left out (only the behaviour test): U1 is named for EACCES with winerror 5, G2 for ESTALE and G2b for EINVAL, each for S_ISREG and S_ISDIR. A Windows error that is neither 5 nor 32 (+= (53,)) passes the behaviour test and is named by the += check alone: see the known limits.

My first full run of the first round, on b8f78bdb, killed 72 of 73. Its one survivor, W33, deleted the two FOLLOW_UP asserts. They passed on the real value whether they ran or not. ccbd0eb0 moves them into entry_problems, which is proved on planted input, and W33a and W33b are killed there.

Mine on the first round, in short: the walrus, __import__, default parameters, class attributes, tuple unpacking, the two * imports, attrs[GUARD], sys.modules, the attribute lookup, for, with, a guard bound to a name, and o = os, each dropped from the scan (W1-W7, W22, W23, W25-W30). Then: _calls_out always False or always True (W8, W9); raise exc refused or any raise taken (W10, W11); any <name>.<NAME> taken as errno (W12); PermissionError in HANDLERS (W13); not in refused again (W14); frozenset refused again (W31); import errno as E ignored (W32). On the behaviour side: any raise taken for a refusal (W15); no Windows cases (W16); no flag variants (W17); _slice cut to one pass (W18); an AugAssign skipped (W19); a non-stdlib import taken (W20); absent cases unchecked (W24); a later _is_a = ... dropped (W34). Last, the owner check loosened (W21), and each FOLLOW_UP check dropped (W33a, W33b).

Mine on this round: the refused errnos cut to the old three or made to include the absent ones (F1, F2); no refused Windows error, or only 5, or only 32, or carried by EINVAL, or read as absent (F3 to F3d); only one kind asked (F4, F4b); the link never asked, its lstat refused too, follows ignored, inverted or defaulted to False, the link asked with the flags too (F5, F6, F7, F7b, F17, F9); _dangling_link making no link, linking to a file that is there, or making the link again (F8, F8b, F8c); the += check not applied, not limited to the top level, reading one name, or no AugAssign (F10 to F13, F20, F21); the set comparison dropped (F16); _shown dropping the count, counting at the boundary or cutting nothing (F14 to F14c); the kind left out of the message (F18); the link's absent cases unchecked (F15).

Gate

All runs used the probe environment (stub gh first on PATH, GH_TOKEN bogus, an empty GH_CONFIG_DIR, GIT_CONFIG_GLOBAL=/dev/null, GIT_SSH_COMMAND=false, EMAIL set, LANES_LANE unset), with Python 3.12.3 and git 2.43.0, every call on the final head cd935163, each under 9 minutes and at most 4 pytest calls at once:

Call Head Result
tests/test_path_guard_hygiene.py, whole cd935163 119 passed
tests/test_path_guard_hygiene.py, whole fa67fac7 121 passed (two new rows pin the ast.unparse messages)
test_agents_md_codes, test_cli_reference, test_repo_hygiene fa67fac7 170 passed
the twelve helpers' own unreadable-path files (14): test_adopt_unsearchable_source.py, test_adopt_unenterable_source_and_locale.py, test_update_shape_unreadable.py, test_doctor_unreadable_paths.py, test_doctor_unreadable_pin.py, test_bump_leg_unopenable.py, test_bump_leg_unreadable.py, test_bump_leg_unsearchable.py, test_assembly_root_unreadable_paths.py, test_validate_pins_unreadable_workflow.py, test_validate_pins_unreadable_workflows_dir.py, test_validate_family_unreadable.py, test_family_siblings_bootstrap_unreadable.py, test_family_py_unreadable.py cd935163 562 passed
tests/test_shape_doctor.py, 4 shards of 112 node ids each cd935163 111 passed, 1 skipped, 112 passed, 111 passed, 1 skipped, 111 passed, 1 skipped: 445 passed, 3 skipped of 448 (the skip is the Windows-only PowerShell line)
tests/test_scaffold_leg_path_collisions.py cd935163 104 passed
test_agents_md_codes, test_cli_reference, test_repo_hygiene cd935163 170 passed

No call ran the full suite.

Timing

The hygiene file, three runs each, alternating, pytest's own time. The machine was not quiet (load average 4.8 to 7 from other lanes' runs), so the seconds are only comparable within the table:

Tree Tests Runs Median
base 038dafba 49 10.20 s, 9.36 s, 9.38 s 9.38 s
head cd935163 119 12.43 s, 13.83 s, 12.76 s 12.76 s

That is 1.36x base, under twice base. The behaviour test asks about 135 cases for each of two kinds, of a name and of a link, for each of twelve helpers (about 2 s of the file), and _reached follows attribute chains in the scan.

Known limits

  • The scanner.

    • It still misses a guard named by a string (getattr, operator.methodcaller).
    • It misses the module looked up by a name that is not a constant, or through an alias of importlib or sys.
    • It misses a binding through a for over anything but a literal, *-unpacking, globals(), setattr, or a function's return.
    • It misses the other questions of the same kind (ismount, isjunction, is_fifo, os.access with F_OK, a hand-written stat whose OSError is False).

    The docstring says which of these the shipped tools hold today.

  • It still over-flags. A name bound to the module once is the module everywhere in its file. A stored o.exists = ... is flagged as a guard, as on base.

  • The parity test's handler rule now looks only at a try that makes a call. A handler around a stat reached without a call (a property, say) is left to the behaviour test, which asks the helper whatever the walk sees.

  • The parity relaxations trust a name without checking that nothing rebinds it (r1149's T1 to T6). exc = FileNotFoundError(...); raise exc, a module-level def frozenset(...) that adds EACCES, import errno as E in another function beside a top-level class E, FileNotFoundError = OSError at module level, a local name shadowing a call-free function, and an inverted not in all pass the parity test. The behaviour test names each of them (measured), and it is what names those.

  • except (OSError, PermissionError) is still refused statically. It is the same as except OSError when run, and the behaviour test passes it. r1111b listed it as wrong; I left the parity rule as it was.

  • The behaviour test needs the helper to take (path, kind). It must also stat through os.stat, os.lstat, Path.stat or Path.lstat. A helper that binds from os import stat at import would answer from the real disk, and would be named, not passed.

  • The behaviour test flips only keyword-only bool flags. A positional quiet=False, or a non-bool keyword-only flag that swallows the refusal, passes (r1149's G9 and G10). Today only the assembly root's bootstrap.py has a flag, link. It asks a Path only, and runs only the module's top-level statements the helper reads: a branch on a str path, or a rebind inside a function main calls, passes (G8, G12).

  • Windows errors are asked as an attribute, and only two of them are refused. Each winerror is set on a POSIX OSError, since CI is Linux-only (ci: run the test suite on Linux only #597). The behaviour test asks every errno but only winerror 5 and 32 with EACCES. Any other Windows error added to _ABSENT_WINERRORS after it is set is named only by the constants test, which reads a top-level += or |=; one added by a method call or a statement inside an if passes.

  • The link is asked without a flag only, since a flag may ask lstat on purpose, and it is skipped where this system makes none (Windows without the right). On Windows a refusal answered from lstat is therefore not asked.

  • The Z11 check compares two spellings. It compares FOLLOW_UP with the docstring. Only GitHub knows family.py, the family root's siblings.py, shape-doctor.py and shape_advisory.py: a name in the root that is a link into a folder this user cannot search is still read as absent at twelve sites (follow-up of #430 slice Z) #1110's number, and the tests make no network call.

Closes #1134

Lane: openreposhape-1 (openRepoShape-1)

🤖 Generated with Claude Code

Summary by Sourcery

Harden path-guard hygiene tests by expanding static binding analysis, adding runtime validation for every stat helper, and closing parity and allowlist self-test gaps.

Enhancements:

  • Strengthen path-guard hygiene analysis by following substantially more module, attribute, alias, lookup, and binding forms while documenting the scanner’s remaining limits.
  • Add AST-loaded behavioral validation for every discovered stat helper, covering absent and refused errno/Windows errors, dangling links, filesystem parity, flags, late-added helpers, and isolated loading.
  • Improve parity analysis to accept correct errno, exception, set, handler, and re-raise spellings while rejecting unsafe variants.
  • Make allowlist validation enforce follow-up issue ownership, documentation agreement, valid tags, nonempty reasons, positive counts, and helper exclusion, including a distinct MODEL tag for in-memory guards.
  • Stabilize diagnostic source text across Python versions by using ast.unparse and expand self-tests and mutation coverage for the scanner, parity checks, behavior checks, and allowlists.

Documentation:

  • Update the hygiene test documentation to describe expanded scanner coverage, behavioral validation, follow-up ownership, and known limitations.

Tests:

  • Expand the path-guard hygiene suite with planted binding cases, parity cases, behavioral routes, dangling-link checks, loader isolation checks, allowlist validation, and comprehensive mutant coverage.

Brett Heap and others added 3 commits October 22, 2026 07:27
…ST and asked, the scanner follows every binding #1111 listed as missed, and the parity rules take the correct spellings they refused (#1134)

- NIT 1: a behaviour test. Each `_is_a`/`is_a` found (the twelve and any
  added later) is loaded by its AST, with only the statements it reads
  run (`_slice`, `loaded_helper`): no shipped tool is imported, since
  each one's top level puts its folder on `sys.path` and imports
  `repo_shape` and the like. With `os.stat` and `os.lstat` refused for
  one name, each absent errno and Windows error must answer False, and
  EACCES, EPERM and EIO must come back raised, the same exception; a
  file, a folder and nothing answer as the disk does. `ROUTES` proves it
  names each route r1111b listed.
- NIT 2: the scanner follows `o = os`, `from os.path import *`, `from
  os import *`, tuple unpacking, a walrus, a `for` or `with` target, a
  default parameter, a class or instance attribute, `importlib`,
  `__import__` and `sys.modules` by a constant name, and the later calls
  of a guard bound to a name. `PLANTED` holds one self-test each. The
  `_os_path_names` docstring now names the three `isjunction` lookups.
  On today's 25 tools it finds the same 190 sites as before.
- NIT 3: parity takes `errno not in`, `raise exc`, a `frozenset({...})`,
  `import errno as E`, `except (FileNotFoundError, NotADirectoryError)`
  and an unrelated handler in a `try` that calls nothing; each beside a
  wrong twin that is still refused.
- NIT 4: self-tests for r1111b's R2, R3, R7 and R13.
- NIT 5: `FOLLOW_UP` must be `#<n>` and match the module docstring;
  `entry_problems` is proved on planted wrong entries.
- NIT 6: the `HELPER` comment names its real shape.
- r1111c's NIT 1: a `MODEL` tag for `_CloneGit.is_dir`.

Lane: openreposhape-1 (openRepoShape-1)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rs a Windows error, a branch only a flag takes and the disk's answers, and the scan's a guard bound to an attribute (#1134)

- `ROUTES` gains `winerror-dropped` (1921 answered as a refusal) and
  `flag-branch` (EACCES absent only with `link=True`), so dropping the
  Windows cases or the flag variants from `behaviour_problems` fails.
- `test_the_behaviour_test_asks_the_disk_too`: a helper whose answer is
  the wrong way round is named by the three disk controls.
- `PLANTED` gains `guard-as-attribute` (`class C: ex = os.path.exists`,
  then `self.ex(x)`), the one `Reach.attrs[GUARD]` case.

Lane: openreposhape-1 (openRepoShape-1)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ntry_problems`, so a typo in it and a value that is no issue are proved named on planted input (#1134)

The two `FOLLOW_UP` asserts (`#\d+`, and named in the module docstring as
"the follow-up") passed on the real value whether they ran or not: the
mutant that deletes them (W33) survived. They are now part of
`entry_problems`, whose self-test plants `#1101` (r1111b's Z11) and
`1110`.

Lane: openreposhape-1 (openRepoShape-1)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@sourcery-ai

sourcery-ai Bot commented Oct 22, 2026

Copy link
Copy Markdown

Reviewer's Guide

This test-only change substantially hardens path-guard hygiene by combining broader AST reachability and parity analysis with AST-isolated runtime checks for every stat helper, while adding self-tests for missed bindings, correct and incorrect spellings, allowlist integrity, and non-filesystem model methods.

File-Level Changes

Change Details Files
Added AST-isolated behavioral validation for every discovered filesystem-stat helper.
  • Load only the helper’s dependency slice, reject non-standard-library imports, and avoid module import side effects.
  • Patch os.stat/os.lstat to verify absent errors return False, refusal errors are re-raised identically, Windows error handling is correct, disk results match, and keyword-only flags are exercised.
  • Add mutation routes and disk/loader self-tests covering behavior the parity analysis cannot statically observe.
tests/test_path_guard_hygiene.py
Expanded filesystem-guard discovery to follow substantially more Python binding and lookup forms.
  • Track os, os.path, guard, and attribute reachability through aliases, fixpoint rebinding, wildcard imports, tuple/walrus/loop/with/default/class bindings, bound guards, importlib, import, and sys.modules constant lookups.
  • Add planted binding cases and document remaining known limits and current shipped-tool exceptions.
  • Preserve existing scan results while closing the missed alias and lookup cases.
tests/test_path_guard_hygiene.py
Refined parity analysis to accept correct helper spellings while rejecting semantically unsafe variants.
  • Support not in comparisons, re-raising a caught exception by name, frozenset/set wrappers, errno aliases, and absent exception subclasses.
  • Restrict handler analysis to try blocks capable of reaching a stat call and follow calls between local functions.
  • Add twin-case tests ensuring accepted spellings remain paired with rejected incorrect forms.
tests/test_path_guard_hygiene.py
Strengthened allowlist integrity checks and classified in-memory model methods separately.
  • Validate issue-number formatting, docstring/follow-up agreement, owners, reasons, counts, duplicate sites, and stat-helper placement through a shared entry_problems routine.
  • Add planted negative-entry tests and tag the two non-filesystem _CloneGit.is_dir methods as MODEL instead of OWN.
  • Update the documentation and helper sample to reflect the expanded test model.
tests/test_path_guard_hygiene.py

Assessment against linked issues

Issue Objective Addressed Explanation
#1134 Add a behavioural parity test that loads each stat helper from only the module statements it needs and verifies absent errors return False, refused errors are re-raised unchanged, and ordinary filesystem cases match the disk. ✅
#1134 Extend scanner and parity coverage for the issue’s missed alias/binding forms and valid helper spellings, and add self-tests that detect the identified scanner, helper-discovery, and set-member gaps. ✅
#1134 Strengthen allowlist ownership validation and clarify the HELPER sample’s relationship to the existing helper shapes. ✅

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@brettheap

Copy link
Copy Markdown
Contributor Author

(lane openRepoShape-1, 2026-10-22T08:35:25Z) r1149, lane 1's independent review of #1149 at ccbd0eb0: READY on condition, with no MUST, two SHOULDs and five NITs. It is not the writer w1134.

The full review

r1149: review of PR #1149 (Closes #1134) at ccbd0eb0

Verdict: READY on condition

There is no MUST. Every count in the body that I measured is true, and every gate run is green on head and on the predicted merge with #1144.

The condition is SHOULD 1: the module docstring says the new behaviour test catches "a constant changed after it is set". That holds only when the added errno is EACCES, EPERM or EIO. Either add the missing cases (a 0.5 s, roughly 6-line change) or narrow the docstring and add the gap to Known limits. SHOULD 2 should be fixed the same way or listed in Known limits. Everything else is NIT.

Findings

MUST

None.

SHOULD

1. The behaviour test asks no refused Windows error and no non-absent errno beyond EACCES, EPERM and EIO, so a set widened after it is set passes both tests.

2. The behaviour test models a refusal more narrowly than #430's headline case, and only for S_ISREG.

  • Where: :1853-1857. It refuses os.stat AND os.lstat for the name, and asks every refused case with stat.S_ISREG only.
  • Evidence, which answers the brief's "try one":
    • A helper whose primary stat goes through an unpatched spelling is safe: G6 (posix.stat, swallowing EACCES) is named, because the unpatched call sees the real disk, where the asked name does not exist.
    • A helper that falls back to lstat after the refused stat passes, through the unpatched posix.lstat (G4) and even through the patched os.lstat (U3/G5). Because the test refuses lstat too, the fallback simply re-raises.
    • U3 survives head's whole file (91 passed). On a real link into a folder at mode 000, the shipped helper raises PermissionError(13, 'Permission denied'), while U3 answers False (logs/routes_real.log).
    • U2, or (kind is stat.S_ISDIR and isinstance(exc, PermissionError)), also survives (91 passed).
  • Fix: ask each refused case with both S_ISREG and S_ISDIR, and add a stat-only refusal of a real dangling link (lstat answers a link), skipped where symlinks are unavailable. The same prototype names U2, G4 and G5 and passes all 12 real helpers. Failing that, add to Known limits: "a helper that answers a refused stat through lstat, or only for one kind, passes".

NIT

  1. A stale docstring. test_every_stat_helper_holds_the_one_errno_set (:1486-1491) still says "only as in" and "catches nothing but OSError and ValueError" (unchanged from base :1145). It should now say not in, add FileNotFoundError/NotADirectoryError, and allow raise exc.

  2. The six parity relaxations trust a name without checking it isn't rebound. I wrote six wrong twins beyond the writer's. Each is refused by base's parity, taken by head's parity, and named by the behaviour test (logs/twins_probe.log, logs/twins_base.log):

    • T1: exc = FileNotFoundError(...); raise exc (_reraises, :1374-1379);
    • T2: a module-level def frozenset(...) that adds EACCES (_members, :1316-1319);
    • T3: import errno as E inside another function, with a top-level class E (_errno_names walks the whole file, :1298-1303);
    • T4: FileNotFoundError = OSError at module level (HANDLERS, :1269);
    • T5: a local name shadowing a call-free module function (_calls_out, :1359-1371);
    • T6: an inverted not in.

    That division of labour is acceptable now that the behaviour test exists, which makes SHOULD 1-2 matter more. The body's "each wrong twin is still refused" should read "the nearest wrong twin".

  3. Pinning gaps among my mutant survivors. The code is right in each case, but no self-test pins it (see the mutant list below): nested-tuple, comprehension, keyword-only-default and annotated-class-attribute bindings; bare posixpath; raise <another name>; import x of a non-stdlib module; global in _reads; the "any helper added later" path of the behaviour test; and the nothing disk control. Each is a one-line PLANTED or self-test row.

  4. Only keyword-only bool flags are flipped (:1822-1825). A positional quiet=False (G9) or a non-bool keyword-only flag (G10) that swallows the refusal passes. Today only bootstrap's link exists, so a Known-limits line would do.

  5. GitHub appended a "Summary by Sourcery" block after the Generated line. It holds no closing keyword. It only matters if the squash message is taken from the body.

The body's claims

Claim Result
Only tests/test_path_guard_hygiene.py changes; no --help true
None of the 12 modules imports without side effects (sys.path.insert, sibling imports; the doctor's sys.exit) true, all 12 by AST; doctor :466-487
_slice per file: 5 (adopt, family), 7 (update-shape), 5 (doctor), 4 (the other eight) true. logs/slices.log: no call runs at load in any slice; only import errno (plus import os for update-shape), two tuples, and the defs
os.stat/os.lstat are patched; all 12 helpers stat through path.stat() (bootstrap's link=True through path.lstat(), which on 3.12 is os.stat(follow_symlinks=False)) true
ROUTES 12, PLANTED 18 true (counted)
N1-N10 survive base and are killed on head true: 10 of 10 each way
r1111b's ten parity-passing wrong helpers are named; os_stat_follow is named for NameError true (logs/parity_behaviour.log)
Same 190 sites in 18 of 25 files, site for site true: identical lists, order included
alias_probe: base misses 14, head 0; 4 over-flags unchanged true
Docstring names the 3 isjunction lookups, and they are the only ones true (grep: doctor :10305, siblings :822, :1393)
Base has 6 false parity refusals, head 0; each twin refused true for the writer's twins (see NIT 2)
NIT 4: R2a, R3a, R7, R13 killed by the named tests true
NIT 5 and Z11 via entry_problems true (Z11, W21, W33a, W33b killed)
MODEL: TAGS text and both reasons accurate. _CloneGit.is_dir is a set lookup (adopt-project.py:10195); BENIGN 175 (179); OWN 55 (56), MODEL 2 (2), DELIBERATE 63 (65), ASKED 25 (25), ROOT 20 (21), DEST 9 (9), PINNED 1 (1); KNOWN_OPEN 11
74 of 74 killed on head; 0 of 20 on base; sha256 restored every time true (re-run with the writer's runner on my clones)
First run 72 of 73, W33 survivor matches the writer's log; that log does not record the commit
Hygiene file 91 passed true on head
The 14 helper files: 562 passed true, on ccbd0eb0 itself
Doctor: 445 passed, 3 skipped true: 111+1s, 112, 111+1s, 111+1s (the Windows PowerShell row)
Collisions 104 true
Doc trio 170 true
Timing 1.21x base true in substance: I measured base 9.21/9.65/9.32 s (median 9.32) and head 11.32/10.90/10.31 s (median 10.90), so 1.17x. Load average was 4-6 from other lanes; the writer's exact seconds were not reproduced
Exactly one Closes #1134, no other closing keyword true: closingIssuesReferences = [1134]; none in the title or the 3 commits
Known limit: except (OSError, PermissionError) refused statically, passed by the behaviour test true (measured)
Known limit: the Z11 check compares spellings only true (:1043)
Known limit: a helper with an unpatched stat is "named, not passed" true for the primary stat (G6, G7). A fallback through lstat passes (SHOULD 2)

My mutants: 11 killed of 25

Mutants of the new test code: 11 of 22 killed.

  • Killed (11): V1, V2, V3, V4, V5, V6, V8, V9, V11, V17, V21.
  • Survived (11):
    • V7: _reads ignores global;
    • V10: _paired does not recurse into nested tuples;
    • V12: comprehension targets not followed;
    • V13: keyword-only defaults not followed;
    • V14: an annotated class attribute not followed;
    • V15: _members unwraps any one-argument call;
    • V16: the nothing disk control dropped;
    • V18: the behaviour test asks only HELPERS;
    • V19: loaded_helper does not check import x;
    • V20: bare posixpath/ntpath/genericpath not modules;
    • V22: _reraises takes raise <any name>.

Wrong-helper tool mutants in adopt-project.py: 0 of 3 killed. U1 (winerror 5), U2 (kind branch) and U3 (os.lstat fallback) all survive; these are SHOULD 1-2.

Every file was restored, and sha256 matched the HEAD blob for all 25.

Merge results

Hygiene

  • Nothing was written to GitHub; gh was used read-only from a plain shell.
  • No commit, push, rm, git config or worktree prune.
  • Every probe and test ran in the probe environment.
  • Concurrency stayed at 4 or fewer pytest calls, each under 540 s.
  • df -i was 64% before and after.
  • My worktree and all clones are clean. merged holds the predicted tree staged, as built.

Files are in /workspace/projects/openRepoShape-worktrees/_lane1-scratch/r1149:

  • Mutant runners: probes/w_mutants.py (the writer's, repointed) and probes/r_mutants.py (mine).
  • Probes: probes/routes_probe.py, probes/routes_real.py, probes/twins_probe.py, probes/twins_base.py, probes/fix_proto.py, probes/compare_scan.py, probes/slices.py.
  • Logs: logs/wmut_head.log, logs/wmut_base.log, logs/rmut_head.log, logs/gate_*.log, logs/timing.log, logs/merged_hyg.log.
  • Clones: headc, headm, basec, basep, merged.

Lane: openreposhape-1 (openRepoShape-1)

…, a refused Windows error, both kinds and a link whose `stat` alone is refused, and each survivor of r1149's mutants has a self-test (#1134)

r1149 found #1149 READY on condition: no MUST, two SHOULDs, four NITs.

SHOULD 1: the behaviour test asked EACCES, EPERM and EIO and no refused Windows
error, so `_ABSENT_WINERRORS += (5,)` or `_ABSENT_ERRNOS += (errno.ESTALE,)`
passed both tests. It now asks every errno outside the four absent ones and
EACCES with winerror 5 and 32, and expects the very exception back; a top-level
augmented assignment to an `*ABSENT_*` constant is refused by the constants
test, which now has a function and a self-test of its own.

SHOULD 2: each refused case is asked with S_ISREG and S_ISDIR, and again of a
real link to nothing whose `stat` alone is refused (its `lstat` answers a link;
skipped where no link can be made), so a refusal answered from `lstat`, or
read as absent for one kind only, is named.

NIT 1: the stale docstring of the errno-set test. NIT 3: a self-test row for
each survivor (`global` in `_reads`, nested tuples, comprehension targets,
keyword-only defaults, an annotated class attribute, `_members`' unwrap, the
`nothing` disk control, a helper added later, `import x` of a non-stdlib
module, bare `posixpath`/`ntpath`/`genericpath`, `raise <another name>`).
A failing run prints the first 20 problems and counts the rest.

Lane: openreposhape-1 (openRepoShape-1)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@brettheap

Copy link
Copy Markdown
Contributor Author

(lane openRepoShape-1, 2026-10-22T09:36:10Z) r1149b, lane 1's independent re-check of the fix round at cd935163: READY, with no MUST or SHOULD. It is not the writer f1149.

The full re-check

Verdict: READY. There is no MUST and no SHOULD. S1, S2 and all eleven NIT 3 survivors are closed at cd935163, and every count I measured in the body is true.

Scratch: /workspace/projects/openRepoShape-worktrees/_lane1-scratch/r1149/r2. The probes are in probes/ and the logs in logs/. My worktree /workspace/projects/openRepoShape-worktrees/l1-r1149 was never edited and is clean.

Checks

1. Diff.

  • git diff ccbd0eb0 cd935163 --stat shows one file, tests/test_path_guard_hygiene.py (+387/-71).
  • git diff 038dafba cd935163 --name-only shows the same single file.
  • GitHub's headRefOid is cd935163, and ls-remote agrees.

2. S1.

  • I ran every mutant against the whole hygiene file on a scratch clone, without -x. Each was KILLED:
    • U1, _ABSENT_WINERRORS += (5,);
    • _ABSENT_ERRNOS += (errno.ESTALE,), (errno.EINVAL,) and (errno.EROFS,);
    • _ABSENT_WINERRORS += (32,) (extra).
  • Each was killed by two tests, test_every_absent_constant_is_the_one_set and test_every_stat_helper_answers_absent_false_and_raises_a_refusal.
  • With the += check removed from a scratch copy of the test file, I ran only the behaviour test. It names each mutant, for S_ISREG, for S_ISDIR, and for the link:
    • ... answers False for S_ISREG where its stat raises errno EACCES with winerror 5: ... (#430)
    • the same line for errno ESTALE, errno EINVAL, errno EROFS and with winerror 32.
  • += (53,) passes the behaviour test alone and is killed by the constants test only. That matches the body's Known limit.
  • After every run, adopt-project.py and the test file in both clones matched the HEAD blob by sha256, and both clones' git status was clean:
    • adopt-project.py: 0c1c1f3d…bd7c1f
    • test file: 4e8f4821…a605240

3. S2.

  • Whole-file runs killed all of U2 (S_ISDIR branch), U2f (S_ISREG branch), U3 (os.lstat fallback), U3b (path.lstat() fallback), G4 (posix.lstat) and G5 (aliased os.lstat). The test that fails each one is test_every_stat_helper_answers_absent_false_and_raises_a_refusal.

  • routes_probe.py against head:

    Group Result
    G1, G2, G2b, G2c, G3, G4, G5 NAMED (G4 and G5 for "only the stat of a link")
    G6, G7, G11 NAMED
    G8, G9, G10, G12 pass

    G8, G9, G10 and G12 are exactly the Known limits the body lists.

  • routes_real.py, on a real link into a mode-000 folder:

    • the shipped helper raises PermissionError(13);
    • G4 and G5 answer False;
    • G1 raises on the link and answers False for winerror 5.
  • All 12 real helpers pass: 0 problems in 0.79 s.

4. NIT 3. On head, each mutant is killed by the row the report names. My runner printed "expected row among them: True" for each:

Mutant Row that kills it
V7 test_the_slice_reads_a_global_a_function_changes_and_not_a_local[global]
V10 nested-tuple
V12 comprehension-target
V13 keyword-only-default
V14 annotated-class-attribute
V15 test_members_unwrap_a_frozenset_or_a_set_and_no_other_call[sorted]
V16 test_the_behaviour_test_asks_the_disk_too
V18 test_a_helper_added_later_is_asked_like_the_twelve (I adapted it to asked_problems)
V19 test_the_behaviour_test_loads_only_what_the_helper_reads
V20 bare-os-path-twins
V22 raise-other-name

V1 to V22 are 22 of 22 killed once V1 to V3 were adapted to the moved code. twins_probe.py: T1 to T6 pass parity, and the behaviour test names all six.

5. Over-reach.

  • The 126-errno loop and the link case refuse no correct helper.

    • All 12 pass, with a link and without one (0 problems each).
    • errno.errorcode has 130 entries, so 126 are asked beside the 4 absent ones.
    • That makes 135 cases per kind per target, so the body's "about 135" is true.
  • The link case skips cleanly where no link can be made. I simulated each of these, and in each the sample helper and the 12 real helpers raised 0 problems:

    • symlink_to raising OSError(1314);
    • symlink_to raising NotImplementedError;
    • the link name already taken by a regular file.

    In each case _dangling_link returned None. A link-only wrong helper is then not asked, which is the documented Windows limit.

  • absent_constant_problems refuses no shipped +=. No shipped non-test file has a +=/|= on an *ABSENT_* name (grep), and the hygiene file passes whole.

  • Limits on the constants check (probes/overreach.py):

    Form Constants test Behaviour test
    top-level += (53,) names it passes
    += (53,) inside an if passes passes
    += (53,) inside a try passes passes
    X = X + (53,) ValueError, i.e. loud passes
    += (5,) inside an if passes names it
    except (OSError, PermissionError) parity refuses it passes

    The body's Known limit on Windows errors added "by a method call or inside an if" is true.

6. Runs on head.

  • Hygiene file, whole: 119 passed in 13.18 s.

  • The 14 helper files: 561 passed and 1 skipped, in 193 s. The skip is test_adopt_unsearchable_source.py:416, "git filter-repo is not installed". My first run had the PATH rebuilt without ~/.local/bin. Re-run with the inherited PATH, that file gives 27 passed, so 562 passed in all. This is an environment difference on my side, not a PR fault.

  • Doc trio: 170 passed in 23 s (re-run; the file changed this round, and the trio includes test_repo_hygiene).

  • Doctor and collisions: not re-run. Nothing else references test_path_guard_hygiene, and the diff is that one file.

  • Timing, hygiene file, median of 3, alternating, load about 2:

    Tree Runs Median
    base 038dafba (49 tests) 8.31, 7.80, 7.91 s 7.91 s
    head (119 tests) 10.43, 10.16, 10.89 s 10.43 s

    That is 1.32x, against the body's 1.36x, so it is under 2x. The slowest test is the behaviour test at 1.46 s.

7. Body.

  • Closing keywords:
    • The only closing keyword is line 221, Closes #1134.
    • A regex over close/fix/resolve forms before an issue number finds no other, in the body, the title or the four commits.
    • closingIssuesReferences is [1134].
    • Nothing in the body or the test file holds the operator's address.
  • Counts, from probes/counts.py:
    • ROUTES 18;
    • LINK_ROUTES 4;
    • PLANTED 23;
    • 126 errnos;
    • 135 cases;
    • 12 helpers.
  • Mutant counts:
    • The writer's mut_1 to mut_4 logs show 4 × 34 = 136 killed, 0 survived, 136 restores matching the HEAD blob.
    • The first-run logs show 126 killed of 130, with the survivors F4b, F8b, F10 and F15, as the body says.
  • Known limits:
    • NIT 2's twins: T1 to T6 pass parity and are named. True.
    • NIT 4's flags and the G8/G12 passes: true.
    • Other Windows errors: true, per the table in check 5.
    • A binding from from os import stat is "named, not passed": true (G6, G7).
  • Title is 211 characters. Draft, no milestone.

8. CI, as gh pr checks 1149 stands now, without waiting:

Check State
tests (run 39753985022, Ubuntu, full suite) pending (in progress, 18m54s)
SonarCloud Code Analysis pass
package pass
tests-git-bash pass (it runs 3 files, not this one)

Findings

MUST: none.

SHOULD: none.

NIT (optional, none blocks):

  1. tests/test_path_guard_hygiene.py:1794, REFUSED_WINERRORS = (5, 32). A refused Windows error beyond these (1920, 53) added in an if or try still passes. This is already in the body's Known limits. The cheap hardening is to add 1920 and 53 to the tuple, with a ROUTES row each.
  2. :2191-2198, test_the_dangling_link_is_a_link_to_nothing. It asserts the link exists on every non-Windows system, where LINK_ROUTES (:2207-2214) skips. A POSIX filesystem with no symlinks would fail here instead of skipping. Skipping when link is None would match; I found no CI cause for it.
  3. The timing row says 1.36x and I measured 1.32x. That is noise, and the body already notes the machine was not quiet.

Lane: openreposhape-1 (openRepoShape-1)

…iece of the helper by `ast.unparse`, not `ast.dump`, so a row passes on every Python

CI's first run of the file on cd93516, on CPython 3.14.8 (run 39753985022),
had one failure in 19769 tests:

  test_the_parity_test_refuses_another_set[change11-...`Name(id='EACCES',
  ctx=Load())` is neither `errno.<NAME>` nor a number]

The row hard-coded what `ast.dump` prints up to 3.12. From 3.13 `ast.dump`
leaves out `ctx=Load()` and the other default fields, so 3.14 built
`Name(id='EACCES')`. Base 038dafb builds its messages with `ast.dump` too,
but no row there asserts the dumped text, so main was green.

The three sites that put a node in a message now use `ast.unparse`, which
prints the source form of these expressions the same on 3.12, 3.13 and 3.14:
`_members` (an element it cannot read, and a set that is no literal) and
`_caught` (a handler that is no dotted name). The EACCES row says
`EACCES` is neither `errno.<NAME>` nor a number.

Each site is held by an exact text now, where only the EACCES row was:
- a row for `sorted(_ABSENT_ERRNOS)` (`_members`, no literal set) and one for
  `except errors():` (`_caught`, which no row asked before);
- the `frozenset({...}) | {errno.EACCES}` row and the `E.ENOENT` row of the
  twin table name the element they refuse, not just the tail of the message;
- the `sorted` and `tuple` cases of the `_members` test compare the whole
  message.

Checked the rest of the file for text that depends on the Python version: no
other row asserts an AST dump or an exception's repr, and none depends on the
order of a set or a dict.

Proof on 3.12.3, the only Python here: `ast.dump` made to print as 3.13 does
fails the old row with CI's text and passes the new file; `ast.dump` refused
outright passes the whole file; 143 of 143 mutants are killed.

Lane: openreposhape-1 (openRepoShape-1)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@brettheap

Copy link
Copy Markdown
Contributor Author

(lane openRepoShape-1, 2026-10-22T11:23:32Z) #1149's first CI run failed on Python 3.14, and fa67fac7 fixes it. The PR is READY again once tests passes there.

  • The failure. tests on cd935163 (run 39753985022, CPython 3.14.8) gave 1 failed and 19768 passed.
    • The failing row, test_the_parity_test_refuses_another_set[change11-...], was new in this PR.
    • It hard-coded ast.dump's 3.12 text, Name(id='EACCES', ctx=Load()). From 3.13 on, ast.dump leaves out ctx=Load().
    • This bench has only Python 3.12, so the writers' gates and both reviews ran on 3.12 and could not see it.
  • The fix, f1149b's fa67fac7, is test-only, 26 lines in and 11 out.
    • The three messages that name a piece of a helper now use ast.unparse, which is the same on every Python.
    • The row now expects `EACCES`.
    • Two new rows pin the other two message sites.
    • Three assertions now compare whole messages.
  • Evidence:
    • An emulation of 3.13+'s ast.dump reproduces CI's exact failure on the old file and passes the new one.
    • The whole file passes with ast.dump forbidden outright, so nothing depends on it now.
    • Mutants: 143 of 143 killed, the 136 plus B1 to B7.
    • The hygiene file: 121 passed. The doc trio: 170 passed.
  • Review of the delta: lane 1 read the whole diff. It changes only how the test's own messages are spelled, and the assertions on them. r1149b's READY at cd935163 stands for everything else.
  • Landing: tests/test_path_guard_hygiene.py: each stat helper is loaded by its AST and asked with its stat refused, the scan follows every binding #1111 left out, and parity takes the correct spellings it refused (#1134) #1149 is a DRAFT outside v1.2.0. It lands after the tag, on a later word.

Lane: openreposhape-1 (openRepoShape-1)

@brettheap

Copy link
Copy Markdown
Contributor Author

(lane openRepoShape-1, 2026-10-22T13:04:57Z) #1149 is READY at fa67fac7. CI passed on that head: tests (CPython 3.14), tests-git-bash, package and SonarCloud Code Analysis.

  • The earlier tests failure, test_the_parity_test_refuses_another_set[change11], came from ast.dump text that differs on 3.13 and later. The test now spells its messages with ast.unparse.
  • r1149b's READY stands for the rest, and lane 1 reviewed the fa67fac7 delta (issuecomment-6339538077).

#1149 stays a DRAFT with no milestone. It is outside v1.2.0 by Brett Heap's ruling of 2026-10-21T10:32:04Z, and lands after the tag on a later word.

Lane: openreposhape-1 (openRepoShape-1)

brettheap and others added 3 commits October 23, 2026 14:01
GitHub's update-branch wrote 8d1af88 at 18:01Z but left the pull request's head at fa67fac, so update-branch now refuses; this is the same clean merge of main dea964a, made by hand to resync the pull request.

Lane: openreposhape-1 (openRepoShape-1)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

@brettheap
brettheap marked this pull request as ready for review October 23, 2026 18:39
@sourcery-ai

sourcery-ai Bot commented Oct 23, 2026

Copy link
Copy Markdown

Sorry @brettheap, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 18 hours and 7 minutes by commenting @sourcery-ai review. Upgrade to get a review now.

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

Labels

None yet

Projects

None yet

1 participant