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/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 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