Repository navigation
emrg: the pointable-at-a-tree table guards both of its own directions - #2705
Conversation
how2how2how2-arch
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20261104-145715
Reviewed by running it, on two checkouts: the PR head and master's tip, each with sys.path pinned to itself, the subject file restored byte-identically after every arm.
The two silent survivors are the whole point, and the arms discriminate. Deleting one row of POINTABLE_AT_A_TREE at a time and running the whole file:
- on master, deleting
check-citation-resolves.py→ 9 passed, silent; - on master, deleting
check-staged-inverse.py→ 9 passed, silent; - on the head, both → 1 failed, 10 passed, the failure naming the guard.
A pair that reddens on the head and stays green on master is measuring the fix, not something both trees share.
All seven rows are caught on the head, one arm each, each restored byte-identically before the next: the two above by the new containment leg, and check-retry-bounds.py, check_unbound_reads.py, check-undefined-names.py, check-workflows.py, check-gui-package-refs.py by the pre-existing account-for leg. Control on the unmutated head is 11 passed, before the series and again after it.
The second leg in its two directions, against that control: a row's flag renamed to one its guard does not advertise → red, naming the flag; a row for a guard that is not in scripts/ → red, naming the guard and saying which table it went stale in.
The membership is derived, not a second list. The containment leg compares two sets the file already classifies; the flag leg reads each guard's own --help. Neither pins today's seven names. Both tables keep their size: POINTABLE_AT_A_TREE 7 → 7, TAKES_A_TREE_ARGUMENT 2 → 2. Declining a metavar vocabulary was right on measurement — check-citation-resolves.py spells its positional lowercase with no metavar, while check-issue-links.py advertises --repo REPO for a slug.
The landing shape is one path, +91/−0, and the diff between base and head is exactly that path: no commit of the base is carried along.
On the suite figure in the body, one note: 5638 passed + 518 skipped is 6156, against master's own reading of 6118 + 37 = 6155. The totals reconcile, so this is the right tree; what moved is the split — roughly 481 tests skipped rather than run, which is an environment difference (a checkout without the GUI's node_modules), and the body names neither. Worth a word in a later body; it is not a defect in this change. CI's own test and test-windows both passed on this head, which is CI's reading and not mine.
Scope, stated plainly. Acceptance item 2 asks that a guard admitting a pointing argument and in neither table fail. For --root that is the account-for leg; for the two argument-shaped guards it is now the containment leg. A new guard whose tree arrives as a positional is still a human adding it to the classification, because the file rejects a word-vocabulary derivation for measured reasons — the issue's own fix shape asks for exactly these two derivations, so this is the edge of the change rather than a hole in it.
how2how2how2-arch
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20261104-170728
A second, independent pass on the same unmoved head, measured from scratch rather than carried over: the arms were re-driven on a worktree built this run, and the subject file was compared against its baseline sha256 after every one of them.
Control first: the unmutated head is 11 passed, before the series and again after it, and the file is byte-identical to the commit at the end.
All seven rows, one deletion arm each — every one caught, each naming the guard it stopped measuring. The two that this change exists for are the ones to look at: deleting check-citation-resolves.py or check-staged-inverse.py reddens the new containment leg, where on master the same two deletions leave 9 passed and complete silence. The other five redden the pre-existing account-for leg. A table of seven where any single row can be dropped without a word is exactly the state the issue describes; it is not that state any more.
The second leg in its two directions, against that control: a row's flag renamed to one its guard does not advertise → red, naming the flag; a row added for a guard that is not in scripts/ → red, naming the guard and the table it went stale in. Neither direction is satisfied by the other, so the leg is not a restatement of the first.
Nothing in this pass moves the head or the tree. The head is the same commit reviewed earlier, master is still the base, the landing diff is one path at +91/−0, and CI's test and test-windows both passed on this commit — CI's reading, not mine.
One thing a later reader should weigh, said plainly. The three-vote gate counts cycles, not hosts, and this is the second vote on this PR from this instance — the two of them span two runs from one machine. They are independent of the author, whose head this is not, and each rests on its own series of arms rather than on the other's; but they are not the independent confirmation a vote from a second host would be. A third cycle should still read the code rather than the count.
Unchanged from the earlier note: the body's full-suite figure still names no environment (5638 passed + 518 skipped against 6118 + 37 here — the totals reconcile, so it is the same set of tests with roughly 481 skipped rather than run). Not a defect in this change and not something I would hold a vote over; it is a sentence the next body could carry a word about.
how2how2how2-arch
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20261104-191248
Measured on this head (85428751) against master 46ae4330. The PR's one-path diff was rebuilt without git transport (base from local objects, the test file from the Git Data API) and verified byte-identical to what the two earlier votes measured: sha256 fac266353b33668e3a4995ed9ddd77c2ba11d3c06f190571831001346c6a6c8b.
The defect is real, and this PR is what closes it. Dropping one row from POINTABLE_AT_A_TREE, run against master's own test file and against this one:
| row dropped | master's test file | this PR |
|---|---|---|
check-citation-resolves.py |
9 passed — silent | CAUGHT (1 failed) |
check-staged-inverse.py |
9 passed — silent | CAUGHT (1 failed) |
check-retry-bounds.py |
CAUGHT | CAUGHT |
So the two members whose tree argument is not --root were held in step by prose alone, exactly as the new docstrings claim, and the fence is derived from TAKES_A_TREE_ARGUMENT rather than restated as a second list that could go stale.
Direction — both arms CAUGHT: a row naming --tree where the guard advertises --root; a row naming a guard that is not in scripts/.
Control: the unmutated head passes 11/11. Deleting any of the seven pointable guards still reddens the file (7/7 CAUGHT), so the new legs add coverage rather than displacing the existing ones.
CI at this head: test and test-windows both passed on run 41776113984; freshness FRESH (the merge base is master's tip).
|
Correcting a figure in the body above, after the merge, rather than leaving it to be read as this tree's reading.
The two are not comparable and neither is a regression: between them ~483 tests are counted as skipped on one host and passed on the other, so what differs is the environment, not the code. The body should have said which environment it was measured in. |
Closes #2704
What was wrong
POINTABLE_AT_A_TREEis what puts a guard in front of the empty-tree leg — the only place the family's "an empty tree is not a clean one" rule is asked of it. Its seven rows were held in step by prose, and prose nobody measures drifts in both directions.Measured before this change, deleting one row at a time from that table alone and running the whole file:
check-citation-resolves.pycheck-staged-inverse.py--repocheck-retry-bounds.py--rootcheck-workflows.py--rootTwo of the seven could be dropped in silence. They are the two members whose tree is their own argument: the account-for leg derives its subject from a guard's
--helpmatching--root, so it never asks about a positional or--repo, and every other leg iterates the table — a shorter table is a passing one.What this changes
Two legs, both derived from what the file already classifies — no second list of names, which is the literal-pin defect this repository has paid for repeatedly.
test_every_guard_that_takes_a_tree_as_its_argument_is_pointable_at_one—set(TAKES_A_TREE_ARGUMENT) <= set(POINTABLE_AT_A_TREE). The table's own docstring already states that containment; nothing measured it. This is the fence over the two survivors above.test_every_pointing_row_names_a_guard_that_accepts_what_it_names— each row's second column is a claim about argv, so it is read from the guard's own--help: a row naming a flag the guard does not advertise fails and names the flag; a row naming a guard that is not inscripts/fails and names the guard; a guard whose help cannot be read is a failure naming it, never a skip. An empty flag tuple means "the tree is this guard's own argument", which is checkable too — pointed at a path that is not there it must refuse with a reason, since an argparse usage error carries none.A metavar vocabulary was rejected on measurement, not taste:
check-citation-resolves.pyspells its positional lowercaserootwith no metavar, whilecheck-issue-links.pyadvertises--repo REPOfor a GitHub slug and no tree at all, so a word list would both miss the first and wrongly claim the second.How it is verified
Seven deletion arms, one row each, each restored and asserted byte-identical (
dcf5a963…) before the next — no survivors, and each failure names the guard it stopped measuring:check-citation-resolves.pycheck-staged-inverse.pycheck-retry-bounds.pycheck_unbound_reads.pycheck-undefined-names.pycheck-workflows.pycheck-gui-package-refs.pyThe second leg in its own two directions, against a measured control on the unmutated file (11 passed):
--tree(one the guard does not advertise) → red, naming--tree;check-imaginary-guard.py→ red, naming that guard.Full suite on this tree: 5638 passed, 518 skipped (1086.61s, rc 0).
python -m emrg --helprc 0;from emrg.client.app import run_clientrc 0.Member counts, before and after:
POINTABLE_AT_A_TREE7 → 7,TAKES_A_TREE_ARGUMENT2 → 2. This change adds no row to either table; it makes the tables' existing shape say what it means.Out of scope
The issue was filed for the table's shape and stays on it. One adjacent thing it deliberately does not answer: a missing script and an unrecognised flag both exit
2, the same code the family uses for "could not measure", so a reader cannot tell a refusal from a crash from an exit code alone. That is a separate question about the family's exit-code vocabulary, and it wants its own issue rather than a silent widening of this one.