From 9b0516e9caba9a9ab421a1add4442e8fc2e03b03 Mon Sep 17 00:00:00 2001 From: Vasilev Dmitrii Date: Fri, 4 Sep 2026 03:26:51 +0700 Subject: [PATCH 1/2] gates empty: say whether a passing gate could reach a tree at all Closes #3057 `tri gates empty` reported 5 invocations that passed over an empty tree. Going through them by hand found ZERO defects: three never touch a tree, and the two that do print their scope or their population -- `Scope: this tests the two shell forms, not the live workflow`, and `tracked files read 7741` here against `tracked files read 0` in an empty tree built to check. The discriminator is not "did it pass over nothing" but "can this reach a tree", and the output did not say it. One column, decided from the script's source, with the marker list in a named constant so it can be argued with. Two states, plus "source not read" for a command with no script in it -- never false, because a file nobody opened cannot be reported as one that touches nothing. A THIRD STATE WAS TRIED AND REMOVED, and that is the part worth keeping. "Reads one and builds one" was meant for pack_index_consistency_gate.py --selftest, whose os.listdir is aimed at a mkdtemp of its own. It also swallowed check_conflict_markers.py, which reads 7741 tracked files and merely uses a TemporaryDirectory inside its --self-check at line 141. The bucket held two members and neither belonged in it, while the count of the category worth reading went to zero. A file-level marker cannot answer an invocation-level question; the limitation is stated in the doc comment with --selftest named in it, and the removal has its own test so its absence is a decision. Three mutants. Emptying the marker list kills all three tests. Restoring the third state kills exactly the test written for it, 2 passed 1 failed. The first attempt at that round used `cargo test ... reach`, which matched sixteen tests in leanreach and modreach and none of mine -- the filter is a substring, the mutant read as surviving, and what caught it was expecting 3 and reading 16. --- .claude/skills/ci-gates/SKILL.md | 46 ++++++ cli/tri/src/gates.rs | 150 +++++++++++++++++- ...9-04-two-states-and-a-stated-limitation.md | 25 +++ 3 files changed, 220 insertions(+), 1 deletion(-) create mode 100644 docs/now/2026-09-04-two-states-and-a-stated-limitation.md diff --git a/.claude/skills/ci-gates/SKILL.md b/.claude/skills/ci-gates/SKILL.md index 3eecd1db1a..9728c33bd3 100644 --- a/.claude/skills/ci-gates/SKILL.md +++ b/.claude/skills/ci-gates/SKILL.md @@ -12447,3 +12447,49 @@ That is §464 arriving for the third time. **The list is the check.** The co prints `--list` for every step counted and `--excluded` for every line it refused, because a census that prints only its totals cannot be argued with -- and this one was wrong in its totals while every total looked plausible. + + +## 999. A second heuristic to cover the first one's false positive + +`tri gates empty` reports every gate invocation that PASSES against a tree with +nothing in it -- five of them. That is a shape and not a verdict, and going +through the five by hand took an hour and found **zero** defects: three never +touch a tree at all, and the two that do are honest about it. One prints +`Scope: this tests the two shell forms, not the live workflow`; the other prints +`tracked files read 7741` here and `tracked files read 0` in an empty tree I +built to check. The discriminator is not "did it pass over nothing" but **can +this thing reach a tree, and does it say what it read.** + +So: put the first half in the command, as a column decided from the script's +source. Two states, plus *source not read* -- never `false`, because a file +nobody opened cannot be reported as one that touches nothing. + +It printed **2** where my hand pass had said 1. The extra was +`pack_index_consistency_gate.py --selftest`, whose `os.listdir` at line 164 is +aimed at a `tempfile.mkdtemp` of its own. Both readings were right about +different subjects: the FILE reads a directory, the INVOCATION reads its own. + +**Then I did the wrong thing, and the wrong thing is the section.** I added a +third state -- *reads one and builds one* -- keyed on `mkdtemp` and +`TemporaryDirectory`. It captured the selftest, and it also captured +`check_conflict_markers.py`, which really does read 7741 tracked files and +merely uses a `TemporaryDirectory` inside its `--self-check` at line 141. The +new bucket held two members and **neither belonged in it**, while the count of +the category actually worth reading went from 2 to **zero**. The output looked +richer and said less. + +A file-level marker cannot answer an invocation-level question. A second +heuristic stacked on the first to cover its false positive does not narrow the +error; it moves it somewhere with no name. **Two states and a stated limitation +beat three states and a hidden one** -- the limitation is now a sentence in the +doc comment with `--selftest` named in it, and the removal has its own test so +that its absence is a decision rather than an omission. + +Two process notes from the same hour, both my own rules arriving again. The +mutation round for this ran under `cargo test ... reach`, which matched +**sixteen** tests in `leanreach` and `modreach` and **none of mine**: the filter +is a SUBSTRING, the mutant looked like it survived, and what caught it was +expecting 3 and reading 16. And the earlier `--base origin/` mistake +in the same session was the same species one level up -- a flag pointed at the +wrong subject, producing a correct-looking answer about something nobody asked +about. diff --git a/cli/tri/src/gates.rs b/cli/tri/src/gates.rs index ffac366bd7..beaf4b8b90 100644 --- a/cli/tri/src/gates.rs +++ b/cli/tri/src/gates.rs @@ -4832,6 +4832,89 @@ fn find_invocation(line: &str) -> Option { None } + +/// What a gate's script can reach, decided from its SOURCE. +/// +/// THE SUBJECT IS THE SCRIPT, NOT THE INVOCATION, and the difference is not +/// academic: `tools/pack_index_consistency_gate.py` calls `os.listdir` at line +/// 164, so the file reads a directory -- and the invocation CI runs is +/// `--selftest`, which points it at a `tempfile.mkdtemp` of its own. Both +/// readings are correct about different things. A script that reads a directory +/// AND builds one is reported separately for exactly that reason. +/// +/// `gates empty` reports every invocation that passed over nothing. That is a +/// shape, not a verdict, and going through the five it found by hand showed why: +/// four of them never touch the tree -- they are self-tests with their own +/// fixtures, and three say so in their own last line ("Scope: JavaScript source +/// in actions/github-script", "selftest OK: the gate is falsifiable"). Only +/// `tools/check_conflict_markers.py` reads the tree, and it prints the +/// population it read (`tracked files read 7741` here, `0` in the empty tree), +/// so a reader can tell. The honest count of defects among the five was 0. +/// +/// This column is the part of that pass a command can do. It separates "passed +/// over nothing because there was nothing for it to do" from "passed over +/// nothing and would have passed over anything", which is the only one worth a +/// reader's time. +/// +/// `None` means the script could not be read -- an absent file, a command with +/// no script in it. Not `false`: a source nobody read cannot be reported as one +/// that touches nothing. +/// +/// SYNTACTIC, and the list is printed so it can be argued with. A script that +/// reaches the tree through a name not in `TREE_READERS` reads as `no`, and a +/// script that mentions one in a comment reads as `yes`. +#[derive(PartialEq, Clone, Copy)] +pub enum Reach { + /// No call in this file reads a directory. A pass says nothing about a tree. + SelfContained, + /// Some call in this file reads a directory. + ReadsADirectory, +} + +pub fn reach_of(text: &str) -> Reach { + if TREE_READERS.iter().any(|m| text.contains(m)) { + Reach::ReadsADirectory + } else { + Reach::SelfContained + } +} + +fn reads_the_tree(root: &std::path::Path, cmd: &str) -> Option { + let script = cmd + .split_whitespace() + .find(|t| t.ends_with(".py") || t.ends_with(".sh"))?; + let text = std::fs::read_to_string(root.join(script)).ok()?; + Some(reach_of(&text)) +} + +// A THIRD STATE WAS TRIED AND REMOVED, which is worth more than the two that +// stayed. "Reads a directory AND builds one" was meant to catch +// `pack_index_consistency_gate.py --selftest`, whose `os.listdir` is aimed at a +// `mkdtemp` of its own. It did -- and it also swallowed +// `check_conflict_markers.py`, which really does read 7741 tracked files and +// merely uses a `TemporaryDirectory` inside its `--self-check` at line 141. The +// bucket ended with two members and neither belonged in it: the count of the +// category worth reading went to ZERO while the output looked richer. +// +// A file-level marker cannot answer an invocation-level question, and a second +// heuristic added to cover the first one's false positive produced a category +// with no correct members. Two states and a stated limitation beat three states +// and a hidden one. + +/// Ways a gate script in this repository reaches the working tree. Taken from +/// the five `gates empty` reports plus the twenty-two it skips, not invented. +const TREE_READERS: [&str; 9] = [ + "glob(", + "rglob(", + "iterdir(", + "os.walk(", + "listdir(", + "ls-files", + "read_dir(", + "git diff", + "git show", +]; + fn empty(verbose: bool) -> Result<()> { let root = repo_root()?; let wf = root.join(".github/workflows"); @@ -4908,8 +4991,30 @@ fn empty(verbose: bool) -> Result<()> { " PASSED over nothing {}", passed.len() ); + let marks: Vec<(String, Option)> = passed + .iter() + .map(|(c, _)| (c.clone(), reads_the_tree(&root, c))) + .collect(); + let n = |r: Reach| marks.iter().filter(|(_, m)| *m == Some(r)).count(); + println!( + " ... the script reads a directory {} <- a pass here may be about a tree", + n(Reach::ReadsADirectory) + ); + println!( + " ... self-contained {} <- a pass says nothing about any tree", + n(Reach::SelfContained) + ); + let unread = marks.iter().filter(|(_, m)| m.is_none()).count(); + if unread > 0 { + println!(" ... source not read {unread} <- no reading either way"); + } for (cmd, text) in &passed { - println!(" {cmd}"); + let mark = match reads_the_tree(&root, cmd) { + Some(Reach::ReadsADirectory) => "reads a directory", + Some(Reach::SelfContained) => "self-contained", + None => "source not read", + }; + println!(" {cmd} [{mark}]"); if verbose { for l in text.lines().take(3) { println!(" {l}"); @@ -5039,6 +5144,49 @@ mod empty_tests { /// An empty tree that carries the data is not an empty tree. The first /// version copied every file, and two gates read backwards because of it. + /// A pass over nothing is a shape, not a verdict. This is the column that + /// separates the two, and each state has a case. + #[test] + fn a_script_that_reads_a_directory_is_told_from_one_that_cannot() { + assert!(matches!( + super::reach_of("import os\nfor f in os.listdir(d): pass\n"), + super::Reach::ReadsADirectory + )); + assert!(matches!( + super::reach_of("print('four payloads matched their declared class')\n"), + super::Reach::SelfContained + )); + assert!(matches!( + super::reach_of("subprocess.run(['git', 'ls-files'])\n"), + super::Reach::ReadsADirectory + )); + } + + /// The third state that was tried and removed, pinned so its absence is a + /// decision. `check_conflict_markers.py` reads 7741 tracked files AND uses a + /// `TemporaryDirectory` inside its self-check; bucketing those together took + /// the count of the category worth reading to zero. + #[test] + fn a_temporary_directory_does_not_excuse_a_script_that_reads_one() { + assert!(matches!( + super::reach_of( + "import tempfile, os\n with tempfile.TemporaryDirectory() as d: pass\n for f in os.listdir(root): pass\n" + ), + super::Reach::ReadsADirectory + )); + } + + /// SYNTACTIC, and that is stated rather than discovered. A mention in a + /// comment counts, and a reader who expects otherwise should meet this test + /// instead of a surprise. + #[test] + fn the_column_is_syntactic_and_a_comment_counts() { + assert!(matches!( + super::reach_of("# this used to call os.walk(root)\nprint(1)\n"), + super::Reach::ReadsADirectory + )); + } + #[test] fn only_scripts_are_carried_into_the_empty_tree() { let from = std::env::temp_dir().join(format!("tri_copy_from_{}", std::process::id())); diff --git a/docs/now/2026-09-04-two-states-and-a-stated-limitation.md b/docs/now/2026-09-04-two-states-and-a-stated-limitation.md new file mode 100644 index 0000000000..c4ba39963f --- /dev/null +++ b/docs/now/2026-09-04-two-states-and-a-stated-limitation.md @@ -0,0 +1,25 @@ +# NOW -- Two states and a stated limitation (2026-09-04) + +## `gates empty` says whether a passing gate could reach a tree at all + +- `tri gates empty` reported 5 invocations that passed over an empty tree. Going + through them by hand found **0 defects**: three never touch a tree, and the + two that do print their scope or their population. +- The discriminator is "can this reach a tree", and the output did not say. That + column is now in the command, decided from the script's source, with the + marker list printed so it can be argued with. +- Two states, plus *source not read* -- never `false`, because a file nobody + opened cannot be reported as one that touches nothing. +- A third state was tried and REMOVED, and that is the finding. "Reads one and + builds one" was meant for `pack_index_consistency_gate.py --selftest`, whose + `os.listdir` is aimed at its own `mkdtemp`. It also swallowed + `check_conflict_markers.py`, which reads 7741 tracked files and merely uses a + `TemporaryDirectory` in its self-check. The bucket held two members, neither + belonged, and the count of the category worth reading went to zero while the + output looked richer. +- The limitation is stated instead: the column is about the SCRIPT, and an + invocation's flags can narrow it. Its removal has a test, so the absence is a + decision. +- The mutation round ran under `cargo test ... reach` first, which matched 16 + tests in `leanreach` and `modreach` and none of mine. The filter is a + substring; expecting 3 and reading 16 is what caught it. From bd533c88c013762a0d1f039137df49186ba057e4 Mon Sep 17 00:00:00 2001 From: Vasilev Dmitrii Date: Fri, 4 Sep 2026 03:27:14 +0700 Subject: [PATCH 2/2] Renumber the section from its 999 placeholder to 473 Refs #3057 `tri skill renumber` is on an unmerged branch, so this worktree's `tri` does not have it and the previous commit shipped the literal placeholder. `tri skill check` passed on it: 999 is unique and it is larger than every other number, so both properties the check tests were satisfied by a number that is obviously wrong. A placeholder that survives the check is a placeholder that needs the check to know it is one. --- .claude/skills/ci-gates/SKILL.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.claude/skills/ci-gates/SKILL.md b/.claude/skills/ci-gates/SKILL.md index 9728c33bd3..ef484d4e06 100644 --- a/.claude/skills/ci-gates/SKILL.md +++ b/.claude/skills/ci-gates/SKILL.md @@ -12449,7 +12449,7 @@ because a census that prints only its totals cannot be argued with -- and this o wrong in its totals while every total looked plausible. -## 999. A second heuristic to cover the first one's false positive +## 473. A second heuristic to cover the first one's false positive `tri gates empty` reports every gate invocation that PASSES against a tree with nothing in it -- five of them. That is a shape and not a verdict, and going