From 0ccf9c51a61291f403381ec7b6c58df18997dbef Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Thu, 17 Sep 2026 19:50:23 -0600 Subject: [PATCH 1/2] fix: pgc_ledger merge wrote a one-major row and never said so (#1071) A contributor adds checks, runs the suite on ONE major, merges that log. The row lands with `majors = 18`. The gate considers a row only where its majors intersect the run's, so `suites (PG 18)` matches it and is green while `suites (PG 17)` reads it as a check the ledger has never seen and reddens -- naming the contributor's own checks `(on major 17)`, which reads as though their suite is broken on 17 when it passes there. FIVE AUTHORS IN A ROW hit it, including the person who wrote the tool, on a PR that was itself about ledger hygiene: #1039, #1063, #1065, #1068, #1070. When everyone makes the same mistake it is the tool's shape, not five lapses. And the tool already knew: the distribution it prints for its summary line is computed from the same rows. `merge` now warns, naming the rows, the set they carry, the set the rest of the ledger carries, and the majors the gate will redden on. Four decisions, each with an arm. STRICT SUBSET rather than inequality, so a row naming a major the ledger has never carried -- how a new major legitimately enters -- is not warned about. ONLY ROWS THIS MERGE TOUCHED, or a partly-seeded ledger reprints its own history every time. NOTHING TO COMPARE AGAINST IS NOT A WARNING, so seeding an empty ledger stays quiet. REPORTING RATHER THAN A REFUSAL, because seeding one major at a time is how a contributor without five installed majors makes progress; the gate still refuses later, this only makes that refusal predictable at the moment it is caused. The prevailing set is the PLURALITY among untouched rows, not a union: a union cannot represent a minority set, which is the defect #1048 fixed in the summary line one level up. And the recipe that produced it. The gate printed `--date `, singular, so following it exactly writes the broken row. It now names one log per gated major and says why. Both harnesses, independent: selftest part 520 and six arms in test/pytest/test_mutation_ledger.py, neither naming the other. Removal proof, four mutations, each mutant asserted to parse: strict subset -> inequality the new-major arm reddens sweep all rows, not touched the pre-existing-minority arm reddens warning removed entirely four arms redden recipe reverts to one log the recipe arm reddens control 992 passed + 0 failed Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NhwXKAgSmYDUjteWkfajHK --- CHANGELOG.md | 32 ++++ test/pgc_ledger.py | 75 +++++++- test/pytest/TESTS.md | 33 ++++ test/pytest/expected_tests.txt | 16 +- test/pytest/test_mutation_ledger.py | 149 +++++++++++++++ .../520-a-merged-row-must-cover-the-majors.sh | 172 ++++++++++++++++++ test/selftest/parts.manifest | 1 + 7 files changed, 476 insertions(+), 2 deletions(-) create mode 100644 test/selftest/520-a-merged-row-must-cover-the-majors.sh diff --git a/CHANGELOG.md b/CHANGELOG.md index ced8f7b6..e521a6af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -171,6 +171,38 @@ true until the next version shipped. with it. Removal proof, four mutations, each reddening named checks and each mutant asserted to still parse -- reversing the difference, dropping the sort, restoring the subtraction, and dropping one recorded name. +- `pgc_ledger merge` wrote a row covering one major and never said so (#1071). + + A contributor adds checks, runs the suite on ONE major, merges that log. The row + lands with `majors = 18`. The gate considers a row only where its majors intersect + the run's, so `suites (PG 18)` matches it and is green while `suites (PG 17)` reads + it as a check the ledger has never seen and reddens -- naming the contributor's own + checks `(on major 17)`, which reads as though their suite is broken on 17 when it + passes there. + + FIVE AUTHORS IN A ROW hit it, including the person who wrote the tool, on a PR that + was itself about ledger hygiene: #1039, #1063, #1065, #1068 and #1070 each wrote 6 + to 10 rows at `18` against a ledger where every other row carried `15;16;17;18;19`. + When everyone makes the same mistake it is the tool's shape rather than five lapses. + + THE TOOL ALREADY KNEW. The distribution it prints for its summary line is computed + from the same rows, so `merge` could see the new row was a strict subset of what the + rest of the ledger carries, and said nothing. + + `merge` now warns, naming the rows, the set they carry, the set the rest of the + ledger carries, and the majors the gate will redden on. The predicate is STRICT + SUBSET rather than inequality, so a row naming a major the ledger has never carried + -- how a new major legitimately enters -- is not warned about. Only rows the merge + touched are candidates, or a partly-seeded ledger would reprint its own history on + every merge. + + REPORTING, NOT A REFUSAL, deliberately. Seeding one major at a time is how a + contributor without five installed majors makes progress, so refusing would block + the honest case to catch the careless one. The gate still refuses later; this makes + that refusal predictable at the moment it is caused. + + The gate's printed recipe said ``, singular, so following it exactly produced + the broken row. It now names one log per gated major and says why. - Four secret-leak claims over the PG server log could pass having read nothing (#1032). diff --git a/test/pgc_ledger.py b/test/pgc_ledger.py index a17b5335..53440cf0 100755 --- a/test/pgc_ledger.py +++ b/test/pgc_ledger.py @@ -375,6 +375,69 @@ def cmd_census(args): return 0 +def _warn_subset_majors(rows, touched): + """Say so when this merge wrote a row covering fewer majors than the ledger does. + + #1071. A contributor adds checks, runs the suite on ONE major, and merges that + log. The row lands with `majors = 18`. The gate considers a row only where its + majors intersect the run's, so `suites (PG 18)` matches it and is green while + `suites (PG 17)` reads it as a check the ledger has never seen and reddens -- + naming the contributor's own checks `(on major 17)`, which reads as though their + suite is broken on 17 when it passes there. + + FIVE AUTHORS IN A ROW, including the person who wrote this tool, on a PR that was + itself about ledger hygiene (#1039, #1063, #1065, #1068, #1070). When everyone + makes the same mistake it is the tool's shape rather than five lapses, and the + tool already had what it needed: the distribution below is computed for the + summary line, so `merge` knew the new row was anomalous and said nothing. + + THE PREDICATE IS STRICT SUBSET, not inequality. A row naming a major the ledger + has never carried is how a new major legitimately enters, and warning about that + would make this wrong in the case the project wants to encourage. + + ONLY ROWS THIS MERGE TOUCHED. Every untouched row in a partly-seeded ledger is a + subset of the prevailing set, so a sweep over the whole file would reprint the + ledger's history on every merge and bury the one row that matters. + + REPORTING, NOT A REFUSAL, deliberately. Seeding a major at a time is legitimate -- + it is how a contributor without five installed majors makes progress -- so a hard + refusal would block the honest case to catch the careless one. The gate still + refuses later; this only makes that refusal predictable at the moment it is caused. + """ + untouched = [v[0] for k, v in rows.items() if k not in touched and v[0]] + if not untouched: + # Nothing to be a subset OF. Seeding a fresh ledger must not warn, or the + # warning fires when nothing is wrong and stops being read. + return + # The PLURALITY set, matching the distribution the summary prints. A union would + # be wrong for the same reason it was wrong in the summary (#1048): it cannot + # represent what most rows actually carry. + prevailing = collections.Counter(frozenset(m) for m in untouched).most_common(1)[0][0] + + offenders = {} + for key in sorted(touched): + got = frozenset(rows[key][0]) + if got and got < prevailing: + offenders.setdefault(MAJOR_SEP.join(sorted(got)), []).append(key) + if not offenders: + return + + have = MAJOR_SEP.join(sorted(prevailing)) + n_prev = sum(1 for m in untouched if frozenset(m) == prevailing) + for got, keys in sorted(offenders.items()): + missing = MAJOR_SEP.join(sorted(prevailing - set(got.split(MAJOR_SEP)))) + print(f" WARNING: {len(keys)} row(s) written carrying majors={got}, while " + f"{n_prev} other row(s) carry {have}.") + print(f" The gate will refuse these on every major they do not " + f"name, so this reddens on {missing}.") + print(f" Merge a log from each major -- `merge` takes several at " + f"once -- or seed the rest before this lands.") + for suite, part, name in keys[:6]: + print(f" {suite}\t{part}\t{name}") + if len(keys) > 6: + print(f" ... and {len(keys) - 6} more") + + def cmd_merge(args): rows = read_ledger(args.ledger) runs = _by_run(args.logs) @@ -441,9 +504,11 @@ def cmd_merge(args): f"--mutation NAME if you broke it deliberately, or --reds-are-real if " f"this is a genuine observation of the code under test.") + touched = set() for path, seen in runs: for key, pairs in sorted(seen.items()): verdicts = [v for v, _m in pairs] + touched.add(key) if key not in rows: # A check this ledger has never seen enters as DEBT. A green run # has observed nothing go red, so merging one must never record a @@ -468,6 +533,8 @@ def cmd_merge(args): print(f" duplicate check name in one run, so one ledger row covers " f"{len(verdicts)}: {key[0]}\t{key[1]}\t{key[2]}") + _warn_subset_majors(rows, touched) + write_ledger(args.ledger, rows) seen_all = {k for _, s in runs for k in s} red = sum(1 for v in rows.values() if v[1] != NEVER) @@ -1062,7 +1129,13 @@ def cmd_gate(args): print(f" not in the ledger: {suite}\t{part}\t{name}\t(on major {major})") if unknown: print(f" {len(unknown)} check(s) the ledger has never seen. Regenerate it with:") - print(f" python3 test/pgc_ledger.py merge --ledger {args.ledger} --date ") + # PLURAL, and it is the reason this defect kept recurring (#1071). The recipe + # said ``, and following it exactly writes a row covering one major -- + # which reddens on every other leg and names the contributor's own checks. + print(f" python3 test/pgc_ledger.py merge --ledger {args.ledger} --date \\") + print(f" ") + print(f" One log per gated major. A row covers only the majors it was " + f"merged from, so a single log reddens the other legs.") rc = 1 # A CENSUS, not a ceiling. Bounding it deadlocks: every new check enters as diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index fa4da3e9..169fd37c 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -2698,6 +2698,39 @@ discrimination assertion, so what is load-bearing is the distribution and not th Reporting only. Whether merge should **refuse** a non-uniform result is a live design question and is deliberately not settled here (#1048). +### A merged row that covers fewer majors than the ledger (#1071) + +That design question is now settled the other way round, and the answer is a WARNING +rather than a refusal. + +`merge` writes a row covering only the majors it was handed. A contributor runs the +suite on ONE major, merges that log, and the row lands with `majors = 18`. The gate +considers a row only where its majors intersect the run's, so `suites (PG 18)` matches +it and is green while `suites (PG 17)` reads it as a check never seen and reddens — +naming the contributor's own checks `(on major 17)`, which reads as though their suite +is broken on 17 when it passes there. + +Five authors in a row hit it, including the person who wrote the tool, on a PR that was +itself about ledger hygiene: #1039, #1063, #1065, #1068, #1070. The tool already had +what it needed — the distribution it prints for its summary is computed from the same +rows — so it could see the new row was anomalous and said nothing. + +| test | what it holds | +| --- | --- | +| `test_merge_warns_when_it_writes_a_strict_subset_of_the_ledgers_majors` | a single-major merge into a five-major ledger warns exactly once, and the same checks merged from five logs do not warn at all | +| `test_the_warning_names_the_majors_the_gate_will_redden_on` | the warning names the missing majors, read out of the text rather than searched for loosely — `"15" in output` is also satisfied by the prevailing set printed beside it | +| `test_seeding_a_ledger_with_no_prevailing_set_is_not_warned` | an empty ledger has nothing to be a subset of, and a warning that fires when nothing is wrong stops being read | +| `test_a_row_carrying_a_major_the_ledger_has_never_seen_is_not_a_subset` | the predicate is strict subset, not inequality, so adding a new major is not warned about | +| `test_the_warning_is_not_a_refusal` | the merge still succeeds and the row is still written — seeding one major at a time is legitimate | +| `test_an_existing_rows_widening_is_not_reported_as_a_subset` | only rows the merge touched are candidates, or a partly-seeded ledger reprints its own history every time | + +Held as a **discrimination**, like the arm above it: the same checks merged correctly +must not warn. An assertion on the bad output alone would pass against a tool that +warned unconditionally, which would train the warning out of being read. + +The shell twin is `test/selftest/520-a-merged-row-must-cover-the-majors.sh`. Same +properties, own fixtures, and neither file names the other. + ## 24. test_loop_coverage_premise.py: a loop that never ran asserted nothing **Why this file exists.** `assert-inside-a-loop-over-zero-rows` in VACUITY_MODES.md 3.5 diff --git a/test/pytest/expected_tests.txt b/test/pytest/expected_tests.txt index 8ee79b82..d6b3fd08 100644 --- a/test/pytest/expected_tests.txt +++ b/test/pytest/expected_tests.txt @@ -159,7 +159,21 @@ # NO_CLUSTER is asserted against the corpus in both directions, so the declaration # and the property had to agree before this collected here at all. # Re-derived by collection on this tree, never by adding nine: `355 tests collected`. -guard_tests 355 +# 346 -> 352 when #1071's six arms landed in test_mutation_ledger.py: the warning, +# the correct-merge control that keeps it a discrimination rather than a wording, the +# majors it must name, the empty-ledger case, the new-major case, and the two that hold +# it to reporting rather than refusing. +# +# NOTE FOR WHOEVER MERGES SECOND. #1110 moves this same key to 355 from the same 346, +# for six arms of its own in a different file. Neither number survives the merge and +# adding the deltas lands on a value no tree collects -- the file says this three times +# already and it is about to be true again. Re-derive by collection on the merged tree. +# MERGED with #1110's nine arms. Both sides moved this key from 346 -- this branch to +# 352, #1110 to 355 -- and neither survives. 352 + 9 and 355 + 6 both happen to reach +# 361 here, which is a coincidence of this merge rather than a method: the deltas were +# measured against different trees. Re-derived by collection on the merged tree, which +# is the only resolution this number has. +guard_tests 361 # The complement: tests that need the driver and a throwaway cluster. Until #1016 these ran # in no CI job at all -- a quarter of the corpus, green when somebody ran them by hand and diff --git a/test/pytest/test_mutation_ledger.py b/test/pytest/test_mutation_ledger.py index a97572d8..40db0a81 100644 --- a/test/pytest/test_mutation_ledger.py +++ b/test/pytest/test_mutation_ledger.py @@ -1194,3 +1194,152 @@ def summary(out): "so a bucket lost in the display cannot leave it balanced") expect.num(stated, n_bad, "and that emitted total still accounts for every row in the ledger") + + +# ---- a row that is a strict subset of the ledger's majors (#1071) ------------ + + +def _uniform5(tmp_path, name): + return _w(tmp_path, name, + "demo\tpart1\told one\t15;16;17;18;19\tnever\t-\n" + "demo\tpart1\told two\t15;16;17;18;19\tnever\t-\n") + + +def _one_major_log(tmp_path, name, major, *checks): + body = "".join(f"RESULT\tdemo\tpart1\t{c}\tPASS\t{major}\t\n" for c in checks) + return _w(tmp_path, name, body + f"checks run: {len(checks)}\n") + + +def test_merge_warns_when_it_writes_a_strict_subset_of_the_ledgers_majors( + tmp_path, expect): + """Five authors made this mistake, including the person who wrote the tool (#1071). + + A contributor adds checks, runs the suite on ONE major, merges that log. The row + lands with `majors = 18`. `suites (PG 18)` then matches it and is green, while + `suites (PG 17)` cannot match it and reddens naming the contributor's own checks + `(on major 17)` -- so it reads as though their suite is broken on 17 when it + passes there. + + The tool already computes the distribution this needs for its summary line, so it + knows the new row is anomalous and says nothing. When five people make the same + mistake it is the tool's shape, not five lapses. + + HELD AS A DISCRIMINATION, not as a wording: the same checks merged correctly must + NOT warn. An assertion on the bad output alone would pass against a tool that + warns unconditionally, which would train the warning out of being read. + """ + bad = _uniform5(tmp_path, "bad.tsv") + bad_out, bad_rc = _run("merge", "--ledger", bad, "--date", "2026-09-13", + _one_major_log(tmp_path, "b18.log", "18", "new one", "new two")) + + good = _uniform5(tmp_path, "good.tsv") + for maj in ("15", "16", "17", "18", "19"): + _run("merge", "--ledger", good, "--date", "2026-09-13", + _one_major_log(tmp_path, f"g{maj}.log", maj, "new one", "new two")) + good_out, good_rc = _run("merge", "--ledger", good, "--date", "2026-09-13", + _one_major_log(tmp_path, "gnoop.log", "15", "new one")) + + expect.text({r[2]: r[3] for r in _rows(bad)}["new one"], "18", + "premise: the single-major merge really did write the minority set") + expect.text({r[2]: r[3] for r in _rows(good)}["new one"], "15;16;17;18;19", + "premise: and the five-log merge wrote the full set") + + expect.num(bad_out.count("WARNING"), 1, + "the single-major merge warns exactly once") + expect.num(good_out.count("WARNING"), 0, + "and the correct merge does not warn at all") + + +def test_the_warning_names_the_majors_the_gate_will_redden_on(tmp_path, expect): + """A warning that says only "not uniform" leaves the reader to work out the fix. + + The whole cost of #1071 was that the failure surfaced later, on another major, + phrased as the contributor's suite being broken. The warning has to name which + majors are missing, because that is exactly the list of legs that will redden. + + Asserted over the WARNING BLOCK, not the whole output. The merged major appears + in the summary line regardless, so a search over everything would pass for a + warning that named nothing. + """ + led = _uniform5(tmp_path, "l.tsv") + out, _ = _run("merge", "--ledger", led, "--date", "2026-09-13", + _one_major_log(tmp_path, "b18.log", "18", "new one")) + + warn = [l for l in out.splitlines() if "WARNING" in l] + expect.num(len(warn), 1, "premise: there is exactly one warning line to read") + + # THE MAJORS ARE READ OUT OF THE TEXT, not searched for loosely. `"15" in block` + # is also satisfied by the prevailing set `15;16;17;18;19` printed beside it, so + # a substring sweep would pass for a warning that named no missing major at all. + reddens = re.search(r"reddens on ([0-9;]+)", out) + expect.text("found" if reddens else "absent", "found", + "premise: the warning states which majors this reddens on") + expect.text(reddens.group(1).split(";"), ["15", "16", "17", "19"], + "every major the gate will redden on is named, and only those") + + expect.text("named" if "majors=18" in warn[0] else "absent", "named", + "and the set that WAS merged is named, so the reader sees both sides") + + +def test_seeding_a_ledger_with_no_prevailing_set_is_not_warned(tmp_path, expect): + """An empty ledger has nothing to be a subset OF. + + A warning here would fire on every first merge, and a warning that fires when + nothing is wrong is one nobody reads by the third time. + """ + empty = _w(tmp_path, "empty.tsv", "") + out, rc = _run("merge", "--ledger", empty, "--date", "2026-09-13", + _one_major_log(tmp_path, "s18.log", "18", "new one")) + expect.num(out.count("WARNING"), 0, "seeding an empty ledger does not warn") + expect.num(rc, 0, "and it still succeeds") + + +def test_a_row_carrying_a_major_the_ledger_has_never_seen_is_not_a_subset( + tmp_path, expect): + """Adding a NEW major is not the defect, and must not be trained out. + + The predicate is STRICT SUBSET, not inequality. A run on a major the ledger has + never carried is how a new major legitimately enters, and warning about it would + make the warning wrong in exactly the case the project wants to encourage. + """ + led = _uniform5(tmp_path, "l.tsv") + out, rc = _run("merge", "--ledger", led, "--date", "2026-09-13", + _one_major_log(tmp_path, "n20.log", "20", "new one")) + expect.num(out.count("WARNING"), 0, + "a row naming an unseen major is not warned about") + expect.num(rc, 0, "and the merge succeeds") + + +def test_the_warning_is_not_a_refusal(tmp_path, expect): + """Reporting, deliberately, not a gate. + + Seeding one major at a time is legitimate -- it is how a contributor without five + installed majors makes progress -- so a hard refusal would block the honest case + to catch the careless one. The gate already refuses later; this only makes the + refusal predictable at the moment it is caused. + """ + led = _uniform5(tmp_path, "l.tsv") + out, rc = _run("merge", "--ledger", led, "--date", "2026-09-13", + _one_major_log(tmp_path, "b18.log", "18", "new one")) + expect.num(rc, 0, "the warning does not fail the merge") + expect.text({r[2]: r[3] for r in _rows(led)}["new one"], "18", + "and the row is written, so the warning is advice rather than a veto") + + +def test_an_existing_rows_widening_is_not_reported_as_a_subset(tmp_path, expect): + """Only rows this merge actually TOUCHED are candidates. + + Every untouched row in a partially-seeded ledger is a subset of the prevailing + set, so a warning computed over the whole file would report the ledger's history + on every merge and drown the one row that matters. + """ + led = _w(tmp_path, "l.tsv", + "demo\tpart1\told one\t15;16;17;18;19\tnever\t-\n" + "demo\tpart1\told two\t15;16;17;18;19\tnever\t-\n" + "demo\tpart1\thistoric\t18\tnever\t-\n") + # Touches only `old one`, which already carries the full set. + out, rc = _run("merge", "--ledger", led, "--date", "2026-09-13", + _one_major_log(tmp_path, "t.log", "15", "old one")) + expect.num(out.count("WARNING"), 0, + "the pre-existing minority row is not re-reported on an unrelated merge") + expect.num(rc, 0, "and the merge succeeds") diff --git a/test/selftest/520-a-merged-row-must-cover-the-majors.sh b/test/selftest/520-a-merged-row-must-cover-the-majors.sh new file mode 100644 index 00000000..1295e9b4 --- /dev/null +++ b/test/selftest/520-a-merged-row-must-cover-the-majors.sh @@ -0,0 +1,172 @@ +# ---- a merged row that covers fewer majors than the ledger must say so ------ +# +# #1071. `pgc_ledger.py merge` writes a row whose `majors` covers only the majors it +# was handed, and nothing warns. A contributor adds checks, runs the suite on ONE +# major, merges that log, and the row lands with `majors = 18`. The gate considers a +# row only where its majors intersect the run's, so: +# +# suites (PG 18) matches the row, `new this run=0`, GREEN +# suites (PG 17) cannot match it, reads it as a check never seen, RED +# +# and the red names the contributor's own checks with `(on major 17)`, which reads as +# though their suite is broken on 17 when it passes there. +# +# FIVE AUTHORS IN A ROW, including the person who wrote the tool, on a PR that was +# itself about ledger hygiene: #1039, #1063, #1065, #1068, #1070, each writing 6-10 +# rows at `18` against a ledger where every other row carried `15;16;17;18;19`. When +# everyone makes the same mistake it is the tool's shape rather than five lapses. +# +# THE TOOL ALREADY KNEW. The distribution it prints for its summary line is computed +# from the same rows, so `merge` could see the new row was a strict subset of what the +# rest of the ledger carries and said nothing. +# +# WHY THE DEFENCES DID NOT FIRE. The recipe the gate prints said ``, singular, so +# following it exactly produces the broken row. A local `harness_selftest.sh` cannot +# catch it, because the gate runs from `run_all_versions.sh`. Every author verified +# locally and was green. +# +# THE PREDICATE IS STRICT SUBSET, not inequality: a row naming a major the ledger has +# never carried is how a new major legitimately enters. And it is REPORTING, not a +# refusal -- seeding one major at a time is how a contributor without five installed +# majors makes progress, so refusing would block the honest case to catch the careless +# one. +# +# This part drives the real tool over files it builds here. It asserts the +# DISCRIMINATION rather than the wording: the same checks merged correctly must not +# warn, or a tool that warned unconditionally would pass and the warning would stop +# being read. +# --------------------------------------------------------------------------- + +_led520="$PGC_TESTDIR/pgc_ledger.py" +_d520="$(mktemp -d)" + +check "premise: the ledger tool this part drives is present" \ + "$([ -f "$_led520" ] && echo yes || echo no)" "yes" + +_mk520_ledger() { # _mk520_ledger PATH -- two rows carrying all five majors + printf 'demo\tpart1\told one\t15;16;17;18;19\tnever\t-\n' >"$1" + printf 'demo\tpart1\told two\t15;16;17;18;19\tnever\t-\n' >>"$1" +} + +_mk520_log() { # _mk520_log PATH MAJOR NAME... + local p="$1" maj="$2"; shift 2 + : >"$p" + local n=0 + for _c in "$@"; do + printf 'RESULT\tdemo\tpart1\t%s\tPASS\t%s\t\n' "$_c" "$maj" >>"$p" + n=$((n + 1)) + done + printf 'checks run: %s\n' "$n" >>"$p" +} + +# THE RC COMES BACK THROUGH A FILE, not through a variable set inside the function. +# The first version of this helper set `_rc520=$?` and was called as `$(_merge520 ...)` +# -- a command substitution is a SUBSHELL, so the assignment died with it and the next +# read aborted the part under `set -u`. Loud rather than silent, which is the only +# reason it was not a green run asserting nothing. +_merge520() { # _merge520 OUTFILE LEDGER LOG... -- output to OUTFILE, rc is the caller's $? + local out="$1" ledger="$2"; shift 2 + python3 "$_led520" merge --ledger "$ledger" --date 2026-09-13 "$@" >"$out" 2>&1 +} + +# ---- the defect: one major merged into a five-major ledger ------------------ + +_mk520_ledger "$_d520/bad.tsv" +_mk520_log "$_d520/b18.log" 18 "new one" "new two" +_merge520 "$_d520/bad.out" "$_d520/bad.tsv" "$_d520/b18.log"; _badrc520=$? +_bad520="$(cat "$_d520/bad.out")" + +check "premise: the single-major merge really did write the minority set" \ + "$(awk -F'\t' '$3=="new one"{print $4}' "$_d520/bad.tsv")" "18" + +check "a row covering fewer majors than the ledger is warned about" \ + "$(printf '%s\n' "$_bad520" | grep -c 'WARNING')" "1" + +# THE MAJORS ARE READ OUT OF THE TEXT. A grep for `15` is also satisfied by the +# prevailing set printed on the same line, so a loose search would pass for a warning +# that named no missing major at all. +check "and it names exactly the majors the gate will redden on" \ + "$(printf '%s\n' "$_bad520" | sed -n 's/.*reddens on \([0-9;]*\).*/\1/p')" "15;16;17;19" + +check "and names the set that was merged, so both sides are visible" \ + "$(printf '%s\n' "$_bad520" | grep -c 'majors=18')" "1" + +# THE OFFENDING CHECKS ARE NAMED, not counted. A count tells the reader something is +# wrong; the names tell them which rows to re-merge. +# A CHARACTER CLASS, not `\t`. `grep -E` does not interpret `\t` as a tab -- it +# matches a literal `t` -- so the first version of this arm counted 0 and read as +# "the tool names nothing" when the tool was naming both rows correctly. +check "and names the checks whose rows are short" \ + "$(printf '%s\n' "$_bad520" | grep -cE '^[[:space:]]+demo[[:space:]]+part1[[:space:]]+new (one|two)$')" "2" + +# REPORTING, NOT A REFUSAL, and the row is still written. A warning that also failed +# the merge would block seeding a major at a time, which is legitimate. +check "the warning does not fail the merge" "$_badrc520" "0" + +check "and the row is written anyway, so the warning is advice not a veto" \ + "$(awk -F'\t' '$3=="new two"{print $4}' "$_d520/bad.tsv")" "18" + +# ---- the control: the same checks, merged correctly ------------------------- +# +# Five LOGS, not one log naming five majors: the same name twice in one log is a +# duplicate sharing a row, which is a different thing and would make this unfaithful. + +_mk520_ledger "$_d520/good.tsv" +for _m520 in 15 16 17 18 19; do + _mk520_log "$_d520/g$_m520.log" "$_m520" "new one" "new two" + _merge520 "$_d520/g$_m520.out" "$_d520/good.tsv" "$_d520/g$_m520.log" +done +_mk520_log "$_d520/gnoop.log" 15 "new one" +_merge520 "$_d520/gnoop.out" "$_d520/good.tsv" "$_d520/gnoop.log" +_good520="$(cat "$_d520/gnoop.out")" + +check "premise: the five-log merge wrote the full major set" \ + "$(awk -F'\t' '$3=="new one"{print $4}' "$_d520/good.tsv")" "15;16;17;18;19" + +check "and a correct merge does not warn at all" \ + "$(printf '%s\n' "$_good520" | grep -c 'WARNING' || true)" "0" + +# ---- the cases that must NOT warn ------------------------------------------ + +# An empty ledger has nothing to be a subset OF. A warning here would fire on every +# first merge, and one that fires when nothing is wrong is not read by the third time. +: >"$_d520/empty.tsv" +_mk520_log "$_d520/s18.log" 18 "new one" +_merge520 "$_d520/seed.out" "$_d520/empty.tsv" "$_d520/s18.log"; _seedrc520=$? +_seed520="$(cat "$_d520/seed.out")" +check "seeding a ledger with no prevailing set is not warned" \ + "$(printf '%s\n' "$_seed520" | grep -c 'WARNING' || true)" "0" +check "and seeding still succeeds" "$_seedrc520" "0" + +# STRICT SUBSET, not inequality. A run on a major the ledger has never carried is how +# a new major legitimately enters, and warning about it would make this wrong in +# exactly the case the project wants to encourage. +_mk520_ledger "$_d520/newmaj.tsv" +_mk520_log "$_d520/n20.log" 20 "new one" +_merge520 "$_d520/new.out" "$_d520/newmaj.tsv" "$_d520/n20.log" +_new520="$(cat "$_d520/new.out")" +check "a row naming a major the ledger has never seen is not a subset" \ + "$(printf '%s\n' "$_new520" | grep -c 'WARNING' || true)" "0" + +# ONLY ROWS THIS MERGE TOUCHED. Every untouched row in a partly-seeded ledger is a +# subset of the prevailing set, so a sweep over the whole file would reprint the +# ledger's history on every merge and bury the one row that matters. +_mk520_ledger "$_d520/hist.tsv" +printf 'demo\tpart1\thistoric\t18\tnever\t-\n' >>"$_d520/hist.tsv" +_mk520_log "$_d520/t15.log" 15 "old one" +_merge520 "$_d520/hist.out" "$_d520/hist.tsv" "$_d520/t15.log" +_hist520="$(cat "$_d520/hist.out")" +check "a pre-existing minority row is not re-reported on an unrelated merge" \ + "$(printf '%s\n' "$_hist520" | grep -c 'WARNING' || true)" "0" + +# ---- the recipe that produced the defect ----------------------------------- +# +# The gate printed `merge --ledger ... --date `, singular. Following it +# exactly writes a row covering one major, which is how at least five PRs got here. +check "the gate's printed recipe names one log per gated major" \ + "$(grep -c 'log-pg15> ' "$_led520")" "1" + +check "and the old singular recipe is gone from the tool" \ + "$(grep -v '^[[:space:]]*#' "$_led520" | grep -c -- '--date "' || true)" "0" + +rm -rf "$_d520" diff --git a/test/selftest/parts.manifest b/test/selftest/parts.manifest index c68ee622..eafc7e7f 100644 --- a/test/selftest/parts.manifest +++ b/test/selftest/parts.manifest @@ -48,3 +48,4 @@ 490-a-one-line-body-ends-at-its-own-brace.sh 500-the-gate-takes-a-prior-the-caller-names.sh 510-a-residual-must-be-counted.sh +520-a-merged-row-must-cover-the-majors.sh From 17d6b635198f68bb04a03c4edbee57c0e313b902 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Thu, 17 Sep 2026 19:57:05 -0600 Subject: [PATCH 2/2] test: ledger rows for the sixteen checks part 520 adds (#1071) Derived from five majors on ONE frozen snapshot, all uniform 15;16;17;18;19: PG15..PG19 rc=0 records=992 namehash=1d21455d575c verdicthash=f4208895d654 each its own major The verdict hash is beside the name hash deliberately. An earlier run of this matrix had identical NAME hashes across five legs while one leg was red, because the tree changed under the loop and only the verdict moved. A name hash alone reports agreement in exactly the case where the legs disagree about what happened. The merge that wrote these rows exercised the warning this branch adds and it stayed SILENT, which is the control the arms cannot provide: a real five-log merge into the real ledger, sixteen uniform rows, no WARNING line. checks_never_observed_red re-derived 1277 -> 1293 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: no suite was added. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NhwXKAgSmYDUjteWkfajHK --- test/check_ledger.tsv | 16 ++++++++++++++++ test/check_ledger_budget.txt | 15 ++++++++++++++- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/test/check_ledger.tsv b/test/check_ledger.tsv index 7cbddafd..18a7a465 100644 --- a/test/check_ledger.tsv +++ b/test/check_ledger.tsv @@ -1210,6 +1210,22 @@ harness_selftest 510-a-residual-must-be-counted the residual count agrees with t harness_selftest 510-a-residual-must-be-counted the skipped-but-accounted suites are named, not folded into the residual 15;16;17;18;19 never - harness_selftest 510-a-residual-must-be-counted the summary prints the residual from the reader, not by subtraction 15;16;17;18;19 never - harness_selftest 510-a-residual-must-be-counted the tally function records the name of every suite that ran 15;16;17;18;19 never - +harness_selftest 520-a-merged-row-must-cover-the-majors a pre-existing minority row is not re-reported on an unrelated merge 15;16;17;18;19 never - +harness_selftest 520-a-merged-row-must-cover-the-majors a row covering fewer majors than the ledger is warned about 15;16;17;18;19 never - +harness_selftest 520-a-merged-row-must-cover-the-majors a row naming a major the ledger has never seen is not a subset 15;16;17;18;19 never - +harness_selftest 520-a-merged-row-must-cover-the-majors and a correct merge does not warn at all 15;16;17;18;19 never - +harness_selftest 520-a-merged-row-must-cover-the-majors and it names exactly the majors the gate will redden on 15;16;17;18;19 never - +harness_selftest 520-a-merged-row-must-cover-the-majors and names the checks whose rows are short 15;16;17;18;19 never - +harness_selftest 520-a-merged-row-must-cover-the-majors and names the set that was merged, so both sides are visible 15;16;17;18;19 never - +harness_selftest 520-a-merged-row-must-cover-the-majors and seeding still succeeds 15;16;17;18;19 never - +harness_selftest 520-a-merged-row-must-cover-the-majors and the old singular recipe is gone from the tool 15;16;17;18;19 never - +harness_selftest 520-a-merged-row-must-cover-the-majors and the row is written anyway, so the warning is advice not a veto 15;16;17;18;19 never - +harness_selftest 520-a-merged-row-must-cover-the-majors premise: the five-log merge wrote the full major set 15;16;17;18;19 never - +harness_selftest 520-a-merged-row-must-cover-the-majors premise: the ledger tool this part drives is present 15;16;17;18;19 never - +harness_selftest 520-a-merged-row-must-cover-the-majors premise: the single-major merge really did write the minority set 15;16;17;18;19 never - +harness_selftest 520-a-merged-row-must-cover-the-majors seeding a ledger with no prevailing set is not warned 15;16;17;18;19 never - +harness_selftest 520-a-merged-row-must-cover-the-majors the gate's printed recipe names one log per gated major 15;16;17;18;19 never - +harness_selftest 520-a-merged-row-must-cover-the-majors the warning does not fail the merge 15;16;17;18;19 never - index_fetch_penalty_crossover index_fetch_penalty_crossover a 50000-row correlated range uses the custom scan, not a fetching index 15;16;17;18;19 never - index_fetch_penalty_crossover index_fetch_penalty_crossover a selective point lookup still uses the index 15;16;17;18;19 never - index_fetch_penalty_crossover index_fetch_penalty_crossover both paths return the same aggregate at 50000 15;16;17;18;19 never - diff --git a/test/check_ledger_budget.txt b/test/check_ledger_budget.txt index ffccce23..533e04c0 100644 --- a/test/check_ledger_budget.txt +++ b/test/check_ledger_budget.txt @@ -65,4 +65,17 @@ suites_not_covered 249 # 1306 -> 1309: three arms pinning the collation of the runner's `comm` readers, added # after @OffgridwithJD found the three helpers unpinned in review. Re-derived by the # command above and by the gate's census in the same run. -checks_never_observed_red 1309 +# 1277 -> 1293 when #1071's sixteen checks merged: selftest part 520, over the warning +# `merge` now prints when it writes a row covering fewer majors than the ledger carries. +# Derived by the command above on this tree, and by the gate's own census in the same +# run, which printed `never observed red=1293` independently. +# +# THE MERGE THAT WROTE THESE ROWS EXERCISED THE NEW WARNING AND IT STAYED SILENT, which +# is the control: five logs, all sixteen rows uniform at 15;16;17;18;19, no WARNING line. +# MERGED with #1110. Both sides moved this key from 1277 -- that branch to 1309 over +# three merges, this one to 1293 -- and neither is the merged truth. The two ledgers +# had NO key in common (28 rows against 16, union 44, zero shared), so the rows are a +# clean union and only the COUNT had to be re-derived. Both files conflicted loudly +# here, which is the safe half: the ledger is always right and only sometimes speaks. +# Re-derived by the command above on the merged tree, never by adding the deltas: +checks_never_observed_red 1325