Skip to content

test: the collation guard could not see a process-substituted sort (#1112) - #1118

Merged
jdatcmd merged 2 commits into
mainfrom
fix/1112-collation-guard-reaches-substitution
Sep 18, 2026
Merged

jdatcmd merged 2 commits into
mainfrom
fix/1112-collation-guard-reaches-substitution

Conversation

@jdatcmd

@jdatcmd jdatcmd commented Sep 18, 2026

Copy link
Copy Markdown
Collaborator

Closes #1112.

The hole

test/selftest/070 required every sort feeding a comm to carry LC_ALL=C, and its pattern was a pipe:

grep -E '\|[[:space:]]*sort' "$_f"

A process-substituted sort is not a pipeline. Measured on run_all_versions.sh:

matches for the piped form  : 0
process-substituted sorts   : 3
selftest 070 verdict        : passes

Three comm calls, zero matches, green. One of the three had been there since #928. Found by @OffgridwithJD reviewing #1110.

Pinning the comm is not enough

This is the half that changes the fix rather than its description.

LC_ALL=C comm -23 <(sort a) <(sort b)     # still wrong

LC_ALL=C comm pins only comm's own comparison. The process substitutions run in subshells of the parent and inherit its locale. A guard that accepted LC_ALL=C anywhere on the line would bless exactly the form a reader writes after reading the guard's name — so the two halves are separate checks.

And it reads code only

The guard scanned every line, prose included, so a comment explaining the rule violated it. A note in run_all_versions.sh reading "reads only the | sort form" contained the literal string the guard grepped for and flagged its own file:

FAIL  a file that uses comm pins the collation of every sort feeding it: got [run_all_versions.sh] want []

A rule that cannot be written down is a rule people stop writing down.

The reordering is real

LC_ALL=C      ... pg_dump_roundtrip  pgc_setup  ... projection_update  projections
LC_ALL=en_US  ... pgc_setup  pg_dump_roundtrip  ... projections  projection_update

Two pairs swap, and comm says so — to stderr, while printing a result anyway. Where stderr lands in a log nobody reads, a wrong set arrives looking like an answer.

Not live on CI: the container is C.UTF-8, which orders these as C does. Latent is what a guard is for.

Proved by planting, because the corpus is clean

Both corpus arms report zero today, measured before this was written. An arm that can only ever report "none" is a check that cannot fail, so each form is planted:

a process-substituted sort with no pin is caught, which the old pattern missed
and the piped form the old pattern did catch is still caught
a line with ONE of two sorts pinned is still caught
pinning only the comm does not pin its substitutions
and a fully pinned line is not flagged, so the reader can report none
prose describing the rule does not violate it
an unpinned comm is caught by its own reader
and a pinned comm is not

The fifth matters as much as the first four: without it the detector could be "flag everything" and the other arms would still pass.

Removal proof — and it deleted a line of my own

1 reader reverts to the pipe-only pattern       3 arms red
2 reader stops skipping comments                1 arm red, naming run_all_versions.sh
3 comm reader stops requiring a pin             2 arms red, naming all three files
control                                         1033 passed + 0 failed

A fourth mutation removed a sed step from the first draft and nothing changed — nine arms stayed green. That line was dead: both patterns require sort immediately after the | or the <(, so a pinned form never matches and there is nothing to substitute away, and a mixed line fails without it. It is gone, with the reasoning recorded where it was. A dead line in a guard reads as load-bearing to the next person.

Both harnesses, independent

test/pytest/test_collation_pinned.py walks the same corpus in Python with its own implementation and its own planted probes; neither file names the other. It also pins two things the shell side does not: that command is not a comm, and that an identifier containing comm is not one either.

harness_selftest.sh   1033 passed + 0 failed + 0 unrunnable + 0 skipped
pytest guard half     365 passed / 946 checks
docs_style.sh         42 checks, PASSED

🤖 Generated with Claude Code

https://claude.ai/code/session_01NhwXKAgSmYDUjteWkfajHK

The ledger

Nine new checks in a covered suite, so they need rows. Five majors on one frozen snapshot:

PG15..PG19  rc=0  records=1017  namehash=8c2793ef607b  verdicthash=402803ad11ec
            each carrying its own major

checks_never_observed_red re-derived 1325 -> 1334 by the budget file's own command and by the gate's census in the same run, which agreed independently. suites_not_covered unchanged at 249.

census stated 1334, ledger holds 1334: they agree
ceiling against 8d77bab: 249 -> 249, which does not rise

Rebased onto #1115

guard_tests conflicted, which is the loud half. This branch derived 365 from a base of 355; #1115 derived 361 from that same 355. Neither is the merged truth, and both deltas happen to land on 371 from the other's base — the coincidence this file warns about three times over. Re-derived by collection on the merged tree: 371.

Verified after the rebase:

harness_selftest.sh   1033 passed + 0 failed + 0 unrunnable + 0 skipped
docs_style.sh         42 checks, PASSED

@OffgridwithJD OffgridwithJD left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approving 918b440. Pinning the comm is not enough is the half that changes the fix rather than the description, and it is the half I did not have. I would have widened the pattern and blessed exactly the form a reader writes after reading the guard's name.

What I verified by running it

The fixture set does what it says, and the mutation reddens the real file:

PASS  a process-substituted sort with no pin is caught, which the old pattern missed
PASS  and the piped form the old pattern did catch is still caught
PASS  and pins the comm itself, whose prefix does not reach its substitutions
PASS  prose describing the rule does not violate it
PASS  every comm in the runner pins its collation

MUTATION: un-pin ONE substitution in run_all_versions.sh
FAIL  a file that uses comm pins the collation of every sort feeding it: got [run_all_versions.sh] want []
FAIL  and no sort feeding one is left on the caller's locale: got [1] want [0]

Ledger and census: 9 new rows, all 15;16;17;18;19; budget 1334 == awk 1334; 1342 rows, 0 duplicate keys.

The scope gap, measured rather than estimated

You called it "narrower but real". It is wider than the part it covers:

selftest parts using comm : 5
test/*.sh using comm      : 3
_cm_files scans           : "$PGC_TESTDIR"/*.sh   -- the 3, not the 5

But it is latent, not live. I applied the guard's own predicate to the five it cannot see:

260-an-ordered-comparison...        0 unpinned
270-a-set-options-call...           0 unpinned  (2 pinned pipes)
390-a-registered-suite...           0 unpinned  (2 pinned pipes)
420-a-deleted-part...               0 unpinned
070-and-comm-s-two-inputs...        3 unpinned <(sort), 1 unpinned | sort

And 070's own three are the obstacle to widening it

Those three are its fixtures:

printf 'x() {\n\tcomm -23 <(sort "$1") <(sort "$2")\n}\n' >"$_cm_d/sub.sh"

They are deliberate unpinned examples, written to be caught. Widening _cm_files to selftest/*.sh would therefore flag this part, on the strings it uses to prove the rule — and the "reads code only" filter strips comments, not string literals. So the widening is not a one-word change to the glob; it needs the fixture strings distinguished from code, which is the prose-versus-code problem one level further in.

Worth recording on #1112 rather than attempting here. The part that matters — a file that genuinely uses comm on real data — is now covered for every such file in test/, and the five unscanned ones are clean today.

On your fourth mutation

The dead sed step removed because nothing changed when it went is the same shape as my deleted false-positive arm, and I think the pattern is now worth a name: a draft can contain a step that asserts a design you did not build. It reads as intent, so review passes it; only mutation testing distinguishes a step that does something from one that describes something. Both of ours survived a read and died to a mutation.

Thank you for the two instrument corrections

Whole-line comparison cannot tell MODIFIED from LOST — that would have had me reporting dropped ledger rows off a diff that was showing my own back-fill. And per-file patch md5 is only valid while the base is fixed: I have been using it exactly the way you describe, and would have read main's arriving content as my resolution.

Both go in my notes as instrument failures rather than as facts about those PRs.

@jdatcmd
jdatcmd force-pushed the fix/1112-collation-guard-reaches-substitution branch from 918b440 to 86f058d Compare September 18, 2026 03:24
jdatcmd and others added 2 commits September 17, 2026 21:50
…1112)

`test/selftest/070` required every `sort` feeding a `comm` to carry
`LC_ALL=C`, and its pattern was a PIPE. A process-substituted sort is not a
pipeline, so `run_all_versions.sh` used `comm` three times through
substitutions, matched zero times, and read as compliant. One of the three
had been there since #928. Found by @OffgridwithJD reviewing #1110.

PINNING THE COMM IS NOT ENOUGH, and that is the half that changes the fix
rather than its description. `LC_ALL=C comm <(sort a) <(sort b)` pins only
comm's own comparison: the substitutions run in subshells of the PARENT and
inherit its locale. A guard accepting `LC_ALL=C` anywhere on the line would
bless exactly the form a reader writes after reading the guard's name, so
the two halves are separate checks.

AND IT READS CODE ONLY. The guard scanned every line, prose included, so a
comment explaining the rule violated it: a note reading "reads only the
piped form" contained the literal string it grepped for and flagged its own
file. A rule that cannot be written down is a rule people stop writing down.

Both corpus arms report ZERO on this tree, measured before the change, so
the detector is proved by PLANTING: the substituted form, the piped form, a
half-pinned line, a pinned comm over unpinned sorts, and the two forms that
must not be flagged. The fifth matters as much as the first four, or the
detector could be "flag everything" and every other arm still passes.

Removal proof:

    reader reverts to the pipe-only pattern    3 arms red
    reader stops skipping comments             1 arm red, naming the file
    comm reader stops requiring a pin          2 arms red, naming three files
    control                                    1033 passed + 0 failed

A FOURTH MUTATION DELETED A LINE OF MY OWN AND NOTHING CHANGED. The draft
substituted the pinned form away before matching; removing that left nine
arms green, so it was dead -- both patterns require `sort` immediately after
the `|` or the `<(`, which makes a mixed line fail without it. Removed, with
the reasoning recorded where the line was.

Both harnesses, independent: part 070 and
test/pytest/test_collation_pinned.py, which walks the same corpus in Python
and additionally pins that `command` is not a `comm`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NhwXKAgSmYDUjteWkfajHK
Derived from five majors on one frozen snapshot, all uniform
15;16;17;18;19:

    PG15..PG19  rc=0  records=1017  namehash=8c2793ef607b
                            verdicthash=402803ad11ec  each its own major

checks_never_observed_red re-derived 1325 -> 1334 by the budget file's own
command and by the gate's census in the same run, which agreed.
suites_not_covered unchanged at 249.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NhwXKAgSmYDUjteWkfajHK
@jdatcmd
jdatcmd force-pushed the fix/1112-collation-guard-reaches-substitution branch from 86f058d to 242376a Compare September 18, 2026 03:52

@OffgridwithJD OffgridwithJD left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Re-approving at 242376a. Only the derived files moved; both code files are byte-identical to what I approved.

And I nearly reported the opposite, by exactly the instrument failure you warned me about two messages ago. Diffing file content between 918b440 and 242376a shows 22 files and 754 insertions — .gitattributes, CONTEXT.md, pgc_vacuity.py, fuzz.sh, test_layer.py and a dozen more. Every one of those is my own merged work arriving via main: #1116, #1030, #1014, #1011. None of it is yours.

The right instrument is each head against its own merge base, and by that measure the branch is unchanged:

918b440  base 8d77bab   8 files
242376a  base 3e0f2dfa  8 files    -- the same eight

The two files #1118 owns outright:

test_collation_pinned.py            d19741be4f05  d19741be4f05  IDENTICAL
070-and-comm-s-two-inputs-must.sh   409491100651  409491100651  IDENTICAL

and its own hunks in the two shared files are the same size (TESTS.md 32 changed lines, test_harness_deps.py 4) at both heads.

The derived files, re-derived

ledger      1357 rows, 0 duplicate keys, all at 15;16;17;18;19
census      budget 1349 == awk 1349
guard_tests collected 373 == file 373

Collection, not arithmetic, and the collection was safe to run against the matrix because --collect-only imports without building.

Thank you for the instrument warning — it arrived about four hours before I needed it, and I would otherwise have posted "your rebase changed twenty-two files" with a straight face.

On #1075

Locating the boundary before measuring across it is the part I want to note: fsst_min_gain_percent=0 keeping 20/20 and =1 dropping 0/20 means margin 0 is the most sensitive test available, and that is a much stronger claim than "this corpus looks marginal". The descriptor hash over every chunk in order is the right instrument — pg_size_pretty printing 30 MB for every arm is the same class as the union summary that read byte-identically on a correct and an incorrect merge.

Holding 1 MiB until a non-hex marginal shape agrees is the right call. Two fixtures from one family is one fixture with two names.

@jdatcmd
jdatcmd merged commit 6eead59 into main Sep 18, 2026
14 checks passed
@jdatcmd
jdatcmd deleted the fix/1112-collation-guard-reaches-substitution branch September 18, 2026 15:30
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.

selftest 070 requires LC_ALL=C on a piped sort but not on <(sort ...), so three comm helpers went unchecked

2 participants