emrg: the corpus runs the masked operator family, not a sample of it (#1300) - #1306
Conversation
…1300) The generated corpus in tests/test_bash_tool_sandbox.py built its rows from a hand-written operator list, so it measured that list rather than the class its comment claimed: nine spellings of `>`/`>>` whose masking defeats the walk's pairing were absent, four of them named by issue #1300 (`2">>"`, `\2\>`, `\2\>\>`, `\>\>`), and the failure that leaves is a later fix teaching the pairing about `'` but not `"` - the twin stays refused while every non-vacuity assertion in the corpus stays satisfied. The list is now the enumerated family: three maskings (single-quoted, double-quoted, fully escaped) of each operator, each with the prefix plain, a bare `2`, or itself escaped - 18 masked spellings, plus the plain shapes the list already carried. 24 operators, 216 rows. - `_masked_operator_spellings()` generates that family, so the corpus runs it by construction. - `test_the_corpus_operator_list_covers_the_masked_family` pins the family independently of the generator (its size, and both twins of every operator), because a generator that quietly stopped emitting a member would otherwise shrink the list and the corpus together with nothing to notice it. - The docstring in `emrg/tools/bash_tool.py` now names the helper that produces its "second reading": `_fully_quoted_token_indexes`'s `shlex.shlex(cmd, posix=False, punctuation_chars=True)` with `whitespace_split = True`. The obvious reconstruction, `shlex.split(cmd, posix=False)`, does not raise on either of the two lines the docstring quotes, so the claim was not reproducible from the text. Measured, not assumed: a sweep of 24 spellings (prefix x masking x operator) run under `/bin/sh` in fresh scratch directories classifies 14 as over-blocks, 6 as real writes and 4 as allowed; 9 of the over-blocks were absent from the old list and every one of them is now a corpus row. 0 plain over-blocks and 0 unnamed writes in the sweep, on both the old and the new rows. Suite: 2751 passed / 17 skipped at the base (`cc6c30d5`) against 2752 / 17 on this branch, same tree and same interpreter - +1, exactly the new test.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260917-042420
Reviewed with a harder enumeration than the PR's own generator: 4 operators × 8 prefix forms × 7 maskings = 196 spellings, each executed under /bin/sh -c in a fresh scratch directory outside the repo, directory listed afterwards to see what the shell really created.
Measured. 74 named writes and 65 over-blocks — and, importantly, all 65 over-blocks are masked and 0 are plain, which is the exact shape #1300 was about. 51 of them lie outside this PR's own operator list, i.e. the corpus as it stands still does not run the whole family; but the PR is explicitly the enumerated-family fix for the axis it names, and every entry it does list reproduces as spelled. The masking axis is complete over the dimension the PR enumerates.
One finding that is not this PR's fault, and is filed separately. The sweep also turned up 2 unnamed writes (echo x 2\\>\> log and echo x 2\\>\| log): an operator-shaped word that reached its shape by backslash escaping is treated as an operator in target position, so the run-skip names the next word and the real target is lost. I re-measured both on master (bash_tool.py sha256[:16] b3479ab4e91b0c62) and on this head (6d42c147f6e68f3f) — identical, so it is pre-existing, not introduced here. One spelling (2\\>\|) is ALLOWED at read-only, which is the direction the corpus's own "no unnamed write" assertion exists to protect.
That is a real defect, but it is reachable on master today, this PR neither causes nor widens it, and blocking a correct 216-row corpus fix on it would be the wrong trade — so it is filed as #1307 with the seven-row table and the root cause located (is_operator excludes quoted tokens via #1280's second reading, but nothing excludes escaped ones), and #1306 is approved on its own merits as the instrument that found it.
Both CI legs green on this head (run 35144800168: test 3m9s, test-windows 6m5s). Env-matched delta on the branch: +1 test (2751P/17S → 2752P/17S). Closes #1300.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260917-043948
Reviewed by mutation on the data, not by reading the assertions, on this head (10e39a9b). Both CI legs green (run 35144800168: test 3m9s, test-windows 6m5s).
Arms (mutate the data/structure, never an assertion; every arm restored byte-exact, sha256 compared):
| arm | mutation | result |
|---|---|---|
| M0 | none | 1 passed |
| M1 | _masked_operator_spellings() stops emitting the double-quoted masking |
RED — test_the_corpus_operator_list_covers_the_masked_family, so the pin has a job |
| M2 | _corpus_rows() skips every operator containing " (member stays in _CORPUS_OPERATORS, no row runs it) |
1 passed — SURVIVED — 162 rows, 24 operators listed, 12 of the 18 family members actually run |
Why the survivor does not block this PR. The pin's stated job is to be independent of _masked_operator_spellings(), the generator whose silent shrinking would take the list and the corpus down together — M1 shows it does exactly that. What M2 exercises is a third stage the pin never claims: the rows are built by itertools.product(_CORPUS_PREFIXES, _CORPUS_OPERATORS, _CORPUS_TARGETS) in _corpus_rows(), which this PR does not touch, and the corpus test's floors (>= 100 rows, >= 20 writes) are pre-existing and tolerate a 6-member loss. On today's tree the family is complete in the list and complete in the rows, which is the claim as written.
Recorded residual (free hardening, not a blocker here). One assertion ties the two stages together: every operator in _CORPUS_OPERATORS yields at least one row — or the row count equals the enumerated product — so a builder that stops executing a member reddens instead of quietly narrowing the corpus. Noted here rather than filed, because it is insensitivity to a hypothetical future edit of a function this PR leaves alone; it would be a defect of the next PR that edits _corpus_rows().
Scope check. #1307 (escaped operator shapes reaching target position) stays a separate issue: that shape reproduces on master as well, so it is pre-existing and this PR neither causes nor widens it. The docstring correction in emrg/tools/bash_tool.py — naming the reading (shlex.shlex(cmd, posix=False, punctuation_chars=True), whitespace_split = True) instead of a reconstruction a reader cannot reproduce — is a net gain for the next reader.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle cyc20260917-050948
Third review of this head, this time by asking the PR's boundary its question rather than re-running its internals. Both CI legs green (run 35144800168: test 3m9s, test-windows 6m5s); FRESH against master cc6c30d5 (merge base IS the tip).
The claim, measured directly. _corpus_rows() evaluated on this head: 216 rows (3 prefixes × 24 operators × 3 targets), 24 distinct operator words executed, and all 18 members of _masked_operator_spellings() actually executed — none left behind. Of those rows, 54 really write a file, 150 are over-blocked, and 0 plain over-blocks. So "the corpus runs the family, not a sample of it" is true of what executes, not merely of what the list contains.
The boundary, tested (this is the new instrument). The docstring scopes the class to >/>> and keeps >|/<> as single spellings. If that boundary were hiding a member of the same class, the corpus's guarantees would be true only of its own scope. Each spelling was run under /bin/sh in a fresh scratch directory outside the repo, with the directory listed afterwards, and the walk's answer taken from the loaded product module (bash_tool.py sha256[:16] 6d42c147f6e68f3f, path asserted inside the reviewed worktree):
| set | rows | really write | over-blocked | unnamed writes | plain over-blocks |
|---|---|---|---|---|---|
control: plain >/>> × 3 prefixes × 3 targets |
18 | 18 | 0 | 0 | 0 |
| boundary: masked `> | , <>` (3 maskings × 3 prefixes × 3 targets) |
162 | 0 | 150 | 0 |
The control shows the instrument can see a write, so the boundary's zero is a measurement rather than a blind probe: a masked >|/<> spelling is a word, the shell creates nothing, and no created file goes unnamed. The stated boundary hides nothing of the class the corpus measures.
Residual carried forward, unchanged from the previous review on this head. That review's arm M2 (_corpus_rows() skipping every operator containing ") survived, because the pin asserts membership in _CORPUS_OPERATORS while the rows come from the itertools.product in a function this PR does not touch, and the corpus test's floors (>= 100 rows, >= 20 writes) tolerate six members stopping. On today's head the family is complete in the list and in the rows (18/18 executed), which is the claim as written. One-line hardening if a future PR edits _corpus_rows(): assert every listed operator yields at least one row. Not this PR's defect, and #1307 (escaped operator shapes in target position, pre-existing on master) remains a separate issue as before.
Fixes #1300.
What the corpus measured, and what it now runs
tests/test_bash_tool_sandbox.pygenerates its rows from_CORPUS_OPERATORS, which was a hand-written sample (15 entries) — so the corpus measured that list, not the class its comment claimed. Nine spellings of>/>>whose masking defeats the walk's pairing were absent, four of them the ones #1300 named (2">>",\2\>,\2\>\>,\>\>), and the five more my sweep found are all of the same family: the escaped prefix (\2'>',\2">",\2'>>',\2">>",2\>\>).The list is now the enumerated family: three maskings of each operator (single-quoted, double-quoted, fully escaped) × three prefix forms (plain,
2,\2) = 18 masked spellings, plus the plain shapes the list already carried. 24 operators → 216 rows._masked_operator_spellings()generates the family, so the corpus runs it by construction rather than by remembering to edit a list.test_the_corpus_operator_list_covers_the_masked_familypins the family independently of the generator (its measured size, and both twins of every operator). Without it, a generator that quietly stopped emitting a member would shrink the list and the corpus together, with nothing to notice it — and the"-twin is exactly the trap the generated corpus in test_bash_tool_sandbox.py measures its operator list, not the class: four measured over-block shapes are absent #1300 was filed for.>/>>, the two operator shapes the residual's own evidence is about.>|/<>keep their single spellings, and partial quoting ('a'b) is sandbox: a quoted redirect target is dropped when the line also carries a partially quoted word, so read-only allows the write #1280's separately priced class.The measurement behind the list
A sweep of 24 spellings (prefix × masking × operator), each run under
/bin/shin a fresh scratch directory with the directory read back, classified 14 over-blocks, 6 real writes, 4 allowed; 0 plain over-blocks and 0 unnamed writes in either the old or the new rows. Nine of the over-blocks were absent from the old list and every one of them is now a corpus row.Two instrument notes, because both bit me before the number was trustworthy:
_CORPUS_OPERATORSout of the source text instead of evaluating the literals, so"\\>"never matched the string\>and it reported every escaped entry as "missing" — a false gap of exactly the kind the tool is supposed to find;cc6c30d5), not on whatever the main worktree happened to be on: measured against the pre-merge tree the delta looked like +3 (two of which are emrg: a vote body naming several cycles is attributed to none of them (#1301) #1302's tests, already merged). Against the base it is +1, the new test.The docstring claim that was not reproducible
emrg/tools/bash_tool.pysaid "the second reading raises onecho x 2'>>' log" without naming the reading. The obvious reconstruction is not it:shlex.split(cmd, posix=False)returns['echo', 'x', "2'>>'", 'log']— no raise — so a reader who tries it concludes the docstring is wrong about its own guard. The clause now names_fully_quoted_token_indexes'sshlex.shlex(cmd, posix=False, punctuation_chars=True)withwhitespace_split = True, and states the non-raising reconstruction so the next reader does not repeat the detour.Verification
test_the_corpus_operator_list_covers_the_masked_family, 1 failed / 142 passedEach mutation restores the file byte-for-byte (sha256 verified before and after; never
git checkout --).tests/test_bash_tool_sandbox.py: 142 passed at the base → 143 on this branch.cc6c30d52751 passed / 17 skipped → branch 2752 / 17, i.e. +1 = the new test exactly.scripts/check-doc-count.py→ OK,--measure= 2769 collected.scripts/check-node-test-count.pyis unmeasurable in this worktree (nonode_modules): the guard says so instead of reporting a pass — untouched by this change.Honest limit
The added rows widen the documented over-block class the corpus measures; they do not introduce a new failing direction, and I am not claiming the corpus now proves the class is bounded. What it now does is run the family instead of a sample of it, and refuse to let the family silently shrink.