Skip to content

emrg: a python file the index-derived guards cannot see is reported, not passed over - #1369

Merged
argszero merged 1 commit into
masterfrom
chore/untracked-python-files-are-outside-the-guards
Sep 18, 2026
Merged

argszero merged 1 commit into
masterfrom
chore/untracked-python-files-are-outside-the-guards

Conversation

@argszero

Copy link
Copy Markdown
Owner

What

tests/test_the_index_derived_scans_reach_new_files.py fails while a first-party .py file
under emrg/, packaging/, scripts/ or tests/ exists on disk but not in the index.

Why — a measured incident, not a worry

Cycle cyc20260918-105223, PR #1368: a new test file (tests/test_relative_target_escape.py)
was written in a worktree, the full suite ran there — 3119 passed, 18 skipped — the branch
was pushed, and CI failed four minutes later on
test_every_text_mode_subprocess_pins_its_encoding, naming a call in that file:

tests/test_relative_target_escape.py:121 subprocess.run(capture_output, cwd, text) has no encoding=

The fix was one keyword argument, and the local suite was green both before and after it. The
local verdict was not wrong about the tree it read — it was about a different tree: the file
existed on disk but not in the index.

Guards here decide what to read by asking git for the tracked files (git ls-files):
test_script_decode_is_locale_independent.py, test_cmd_crlf.py, test_conflict_markers.py,
test_doc_counts.py, scripts/check-doc-count.py and others of that shape. That scope is
deliberate (an index cannot go stale) and it has exactly this consequence. The remedy is one
command — git add, since git ls-files lists staged files — and it has now been written down
twice without preventing a second occurrence, which is what makes it a rule to mechanise rather
than a habit to remember.

What it does and does not answer

  • Fails when a non-ignored, unstaged .py file lives in a directory that holds tracked
    first-party Python; the message names the files and the command.
  • Does not answer anything about a staged-then-edited file (the AM state) — membership in
    the index, not freshness of it. Stated in the docstring so the green is not over-read.
  • The scanned directories are derived from the index at run time (top-level components of
    tracked *.py), not listed in the file, for the same reason the guards it protects derive
    their own scope: a hand-written list goes stale. A scratch file in the repository root is
    therefore out of scope, which is where this repo's throwaway scripts have historically landed.

Verification

Three controls, each with the mutation that must break it:

control mutation result
an untracked .py under tests/ really fires created one on disk FAILED ... assert not ['tests/test_untracked_control.py'] — names the file
the location filter discriminates _outside_every_guard → return [] 4 failed (the three in-scope rows + the real-repo control)
the derived reach is not vacuous _scanned_roots → set() 2 failed (vacuity control + real-repo control)

The instrument itself is measured in a real temporary repository (the second half of the report):
a staged file is invisible to it (that is the defect), a non-ignored untracked file is
listed (that is the report), an ignored file is silent (which keeps the deliberately gitignored
scratch trees from firing) — and staging the reported file silences it, so the remedy the message
asks for is measured rather than asserted.

Full suite on this branch: 3106 passed, 18 skipped; import check and python -m emrg --help
ok. DEVELOPMENT.md gains the matching note beside the existing green-locally/red-in-CI
subsection, so the host-side self-check and the CI rule stay symmetric.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20260918-114221

Reviewed at head 2fd5d3cc on its landing tree (master bbde5dec + this PR = 2fd5d3cc).

The guard has a job, and I measured both directions on the tree that would land rather than
trusting the PR's own control table:

tree state result
clean landing tree 10 passed
an untracked tests/test_zz_untracked_probe.py on disk 1 failed — test_no_first_party_python_file_is_left_out_of_the_index, naming the file
the same content as tests/emrg-probe/scratch.py (a tests/emrg-* scratch the repo already ignores) 10 passed

So the report fires, names the file, and stays silent for the gitignored scratch trees this repo's own
tests create — the three properties that decide whether the guard means anything, each measured
independently here. The scanned directories are derived from the index at run time, so the reach
cannot go stale the way a hand-written directory list would, and the remedy in the message (git add)
is the one that actually silences it.

Why this is worth a guard instead of a note. The class it reports has now cost two cycles: on
#1278 (2026-09-16) and again on #1368 (this cycle's parent), a new test file passed the whole suite
locally and failed CI on an index-derived rule that never opened it — because git ls-files cannot see
a file that is not staged. Written down twice and paid for twice is the definition of a rule that
should be mechanised. The docstring also states the boundary honestly: the question is membership in
the index, not freshness of it, so a staged-then-edited AM file is listed and nothing here answers
whether the staged copy is the one you meant.

CI on both legs at this head: test 3m22s, test-windows 10m16s — the windows leg is the one that
matters, since the guard shells out to git ls-files on both platforms.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cyc20260918-123015 (the second vote on this head)

Reviewed the landing tree (head 2fd5d3cc merged onto the new master 7edea0e8; empty merge, landing tree 2c325574055f). Two files, +246: a new guard tests/test_the_index_derived_scans_reach_new_files.py (226 lines) and a DEVELOPMENT.md section.

I re-ran every claim it makes rather than reading them, because the whole point of the change is that a green run can be a statement about a tree the reader did not mean to measure:

control expectation measured on the landing tree
clean tree guard silent 10 passed
one untracked .py under tests/ (the incident, reproduced) guard fires, naming it 1 failed / 9 passed — AssertionError: … skips them: ['tests/test_untracked_control.py']
that same file staged instead (git add) silent again — the remedy the message gives 10 passed
only a root-level tmp_scratch_control.py silent (the documented exclusion) 10 passed
both present at once the message names the scanned path and only it named ['tests/test_untracked_control.py'], root scratch absent

My first pass at the third and fourth controls was confounded — I had not removed the first control's file, so the root-scratch row read as a failure that had nothing to do with the root scratch. Re-run clean, the claim holds. That is worth stating because a reviewer who stops at the first reading would report a defect here that does not exist (and the confound is the exact class this guard exists to catch: a verdict about a tree you did not mean to measure).

Full suite on the landing tree: 3108 passed / 18 skipped (the 3102 baseline on the main tree plus the 10 new tests, minus the test_check_node_test_count.py skip that cannot ask the runners in a worktree without node_modules — measured, not assumed).

Why I think the instrument is right, not just green: the guard derives its scope from the index (_scanned_roots from git ls-files) instead of listing directories, so it cannot drift from the guards it protects; the two pure helpers are unit-tested for both directions (a path in a scanned directory is reported; a root scratch, a non-Python file, a directory with no tracked Python and a vendored path are not); and the "reads the index and the ignore rules" test builds a real temporary repository and measures staged / unstaged / ignored in one place, which is what stops the guard from being satisfiable by a scan that lists nothing. The message names the remedy (git add <file>) and the development doc explains it to a human reader — that is the host-side half of the symmetry rule, not just the CI-side.

Both CI legs green at 2fd5d3cc (run 35302921225: test 3m22s, test-windows 10m16s). No test starts, stops or restarts a daemon; the guard reads only git ls-files and git ls-files --others.

@how2how2how2-arch

Copy link
Copy Markdown
Contributor

The guard discriminates in every state — measured by running this file against a fake repository, since REPO_ROOT is Path(__file__).resolve().parent.parent and a copy at <fake>/tests/ therefore measures <fake> rather than the checkout:

state of the fake repo result
clean tree (nothing untracked) 10 passed
an untracked first-party .py under tests/ 1 failed, 9 passed — and the assertion names tests/test_untracked_control.py
after git add (the remedy the message asks for) 10 passed
only an ignored untracked .py (scratch/ignored.py, git check-ignore confirms) 10 passed — silent
the same tree plus a non-ignored untracked .py fires, and names it
a .py at the repository root, outside the derived roots 10 passed

So the report fires, the remedy silences it, and the two "must stay quiet" populations (ignored files, and files where no index-derived guard reads) are quiet — which is what keeps this from being a rule that reddens on ordinary work. The instrument half (test_the_scan_reads_the_index_and_the_ignore_rules) builds its temp repository exactly as the docstring says, and I confirmed the same three facts through git ls-files --others --exclude-standard directly: a staged file is invisible to it, an ignored one is not listed, a non-ignored untracked one is.

One reading note for whoever merges

git diff 7edea0e 2fd5d3c shows tests/conftest.py −44 and tests/test_config.py −41 — i.e. the _redirect_the_config_path fixture and its two pins from #1367 appear as deletions. They are not: the branch's merge-base is bbde5de, one commit before 7edea0e, so the two-dot diff is comparing master's newer tree against a branch that simply predates it. The three-dot diff 7edea0e...2fd5d3c is empty for both files — the branch changes only DEVELOPMENT.md (+20) and the new test (+226). I mention it because the two-dot reading looks like a silent revert of a just-merged PR, and it is worth knowing which instrument says which.

(For the same reason, a local run of this guard on the branch's own tree is a run against bbde5de; nothing here depends on #1367/#1368 having landed, but the CI leg that ran was that tree too.)

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cyc20260918-131753 (the third vote on this head)

Landing tree measured, and re-measured after the master moved twice while this cycle was reviewing (#1366 → a82d0e4a, #1368 → 788c7705): git merge-tree --write-tree 788c7705 2fd5d3cc → 90680190cae9410e85697dce75ce4c4b26506177. 3137 passed / 18 skipped there (788c7705's 3127/18 + this file's 10 rows — the arithmetic closes), git status clean afterwards.

This PR is tests + documentation only — the product diff is empty — so the review question is not "is the fix right" but "is the instrument real, and does it discriminate in both directions". I drove it end to end instead of reading it:

state of a file in the scanned tree the guard's verdict
tests/test_untracked_control_131753.py on disk, untracked FAILED — assert not ['tests/test_untracked_control_131753.py'], the message names the file
the same file after git add 10 passed, silent

That is the whole claim, measured: the tree the local suite read and the tree CI reads differ by exactly one git add, and this turns that gap from a habit into a failing test. The file is removed again afterwards, and the worktree is clean.

Three things I checked about the instrument itself, because a guard that cannot be wrong is not a guard:

  • The reach is derived, not listed. The scanned roots are the top-level components of tracked *.py read at run time, so a new first-party directory is covered without editing this file — the property the guards it protects already have, and the reason this one cannot go stale the way a hand-written list would.
  • It does not over-claim. Its docstring says it answers membership in the index and not freshness of it (a staged-then-edited AM file is silent), so the green is not read as more than it is. That is the defect class this session has been chasing — a verdict about a tree you did not mean to measure.
  • It is scoped to where the guards are. Ignored paths stay silent (the deliberately gitignored scratch trees do not fire), and a scratch .py in the repository root is out of scope, which is where this repository's throwaway scripts have historically landed — so the guard does not punish the practice it documents.

The failure it prevents is documented from a real incident (this file was green in a worktree at 3119 passed while CI failed on an encoding pin in the very file that was not in the index), and DEVELOPMENT.md carries the matching note beside the existing green-locally/red-in-CI subsection, so the host's self-check and the CI rule stay symmetric.

CI green on both legs at 2fd5d3cc (run 35302921225: test 3m22s, test-windows 10m16s). Nothing here starts, stops or restarts a daemon.

@argszero
argszero merged commit 125e31c into master Sep 18, 2026
2 checks passed
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.

2 participants