pgc_ledger merge writes a single-major row and never warns, so the next run on another major reddens: five PRs hit it

[OffgridwithJD]

pgc_ledger.py merge writes a row whose majors covers only the majors it was handed, and nothing warns. Four PRs in a row have gone red on it.

The failure

A contributor adds checks, runs the suite on one major, merges that log, commits. The row lands with majors = 18. The gate then considers a row only where its majors intersect the majors the current run observed, so:

• suites (PG 18) matches the row, reports new this run=0, green
• suites (PG 17) cannot match it, reads it as a check the ledger has never seen, red

The failure names the contributor's own checks with (on major 17), so it reads like their suite is broken on PG 17 when the suite passes there.

It is not a one-off

Measured today, on each branch's own tracked ledger:

PR      rows added   majors on them   every other row
#1039        6           18            15;16;17;18;19
#1063        6           18            15;16;17;18;19
#1065        8           18            15;16;17;18;19
#1068       10           18            15;16;17;18;19
#1070        6           18            15;16;17;18;19   (jdatcmd, same trap)

Five authors' worth of the same mistake, including the person who wrote the tool, on a PR that is about ledger hygiene. When everyone makes the same mistake it is the tool's shape, not five lapses.

Why the obvious defences did not fire

• The recipe the gate prints is incomplete. It says merge --ledger ... --date <today> <log> — singular. Following it exactly produces the broken row.
• 3a640b0 established the five-major requirement, and nothing enforces it.
• The merge summary already computes what it needs. It prints majors: uniform, all N rows carry 15;16;17;18;19 when the file is uniform — so it knows what the rest of the ledger carries and says nothing when a new row is a strict subset of it.
• A local run cannot catch it. harness_selftest.sh green does not cover the ledger gate, because the gate runs in run_all_versions.sh. Every author verified locally and was green.

What would fix it

Cheapest, and it needs no new concept: merge warns when it writes a row whose majors is a strict subset of the majors the rest of the ledger carries. The distribution is already computed for the summary line. Something like:

WARNING: 6 row(s) written with majors=18, while the other 1217 rows carry
         15;16;17;18;19. The gate will refuse these on every major not listed.
         Merge a log from each major, or this PR reddens on 17.

Stronger options, if wanted: make the gate's printed recipe name every gated major, or have merge exit non-zero on a strict subset unless a flag says it is deliberate. I would start with the warning, because a hard refusal would block the legitimate case of seeding a major at a time.

What I am not proposing

Not a change to the majors mechanism itself. Per-major rows are right — a check really can exist on one major and not another, and #1010 added the field for that reason. The defect is that the tool knows the new row is anomalous and does not say so.

Evidence

I cleared all five PRs by running each suite on 15/16/17/18/19 and merging five logs, which makes the rows uniform and the gate green. That is the correct fix per PR and it is entirely mechanical — which is the argument for the tool doing the telling.

[OffgridwithJD]

Two days on, this is still the only thing red on four PRs, and it has survived two rounds of the authors pushing other fixes.

PR     head       red on            new rows   majors     rest of the ledger
#1039  a1a322f3   suites (PG 17)        6        18        1217 x 15;16;17;18;19
#1063  00272fc6   suites (PG 17)        6        18        1217 x 15;16;17;18;19
#1065  2be200ff   suites (PG 17)        8        18        1217 x 15;16;17;18;19
#1068  a2a28941   suites (PG 17)       10        18        1217 x 15;16;17;18;19

#1039 was force-pushed and #1068 was pushed to in the meantime, and both came back with majors = 18 again. #1068's two pytest legs went green in that push, so the author is acting on review comments — the ledger field is the one that keeps coming back. That is the strongest evidence I can offer that this is the tool's shape and not inattention: it is the failure that survives the author fixing everything else.

What the fix costs, for calibration

I ran each suite on all five majors and compared the emitted check names against each PR's ledger rows, name by name:

#1039  6 rows   names identical to the five-major run
#1063  6 rows   identical
#1065  8 rows   identical
#1068 10 rows   identical

So the correct value is mechanically derivable in every case, and the edit is one field. Verified on a real copy of #1063's ledger: 6 rows change, row count holds, every row still has six fields, and every row outside the suite is byte-identical (md5 ed258834 before and after).

A change that is always mechanical, always the same, and that five authors in a row have got wrong, is a change the tool should make or at minimum name. The merge summary already prints majors: uniform, all N rows carry 15;16;17;18;19 — it has the comparison in hand and says nothing when the rows it just wrote are a strict subset of it.

Smallest thing that would have prevented all five

The gate's own repair instruction is the place it goes wrong:

Regenerate it with:
  python3 test/pgc_ledger.py merge --ledger <...> --date <today> <log>

<log>, singular. Following that line exactly is what produces the broken row. Naming every gated major there would have cost nothing and caught this before merge ever ran.

[OffgridwithJD]

The cause is now confirmed from the contributor's own words, and it is the printed recipe.

linuxhikerpm's #1077 (opened today, sixth occurrence) states this in its test plan, checked off as complete:

• Ledger seeded from a PG18 run (5 never rows, major 18 only)

Verified at that head: 1228 rows at 15;16;17;18;19, 5 at 18, census 1225 == budget 1225.

They are not failing to produce five majors. They are producing one on purpose, and recording it as done. Every other number in that PR is the careful version — suites_not_covered held at 249, cluster_tests re-derived by collection, census matching budget. This is not inattention.

That changes what the fix has to do. Both of the things reviewers have been posting — "here is the correct value" and "here is where in your sequence it goes" — assume the author is trying to write five majors and missing. Neither reaches someone who believes one is complete.

The gate teaches the wrong thing

Regenerate it with:
  python3 test/pgc_ledger.py merge --ledger <...> --date <today> <log>

<log>, singular. A careful person following that line exactly gets a single-major row and has no reason to doubt it. Six PRs, five authors' worth, including the tool's own author on a PR about ledger hygiene.

So the priority I would now put on the two halves of the fix is reversed from how I filed it: the printed recipe is the bug, and the warning is the backstop. A warning fires after the mistake; the recipe is what created it.

A one-line check that works today, before any code change

awk -F'\t' '{print $4}' test/check_ledger.tsv | sort -u

One line out on a healthy file. Two means a row disagrees with the rest, and that row is the one the gate will refuse.

This costs nothing, needs no change to pgc_ledger.py, and catches the class rather than the instance. Putting it in the gate's failure message beside the merge command would make the recipe ship its own verification — which is the smallest change that would have prevented all six.

Suggested wording for the gate's message

Regenerate it with a log from EVERY gated major, not one:
  python3 test/pgc_ledger.py merge --ledger <...> --date <today> <log> [<log> ...]
The majors field accumulates, so merging one major at a time also works.
Check before committing -- this must print exactly one line:
  awk -F'\t' '{print $4}' test/check_ledger.tsv | sort -u

[OffgridwithJD]

The trap is worse than a bad default: it punishes optional work, and doing nothing is green.

New fact, verified against cmd_gate and main's ledger. The refusal is scoped:

covered_suites = {k[0] for k in rows}
...
for key in records:
    if key[0] not in covered_suites or key[3] not in covered_majors:
        continue

A suite is "covered" if and only if it HAS ROWS. And main's ledger covers exactly four suites:

differential   harness_selftest   native_join_runtime_filter   native_join_vector_agg

Every new suite in every red PR is uncovered in main:

native_chunk_length_bound       0 rows in main   -> gate is silent about it
parallel_scan_cost              0 rows
parallel_am_scan                0 rows
index_fetch_penalty_crossover   0 rows
native_fetch_coalesce           0 rows

So the rows are what made them red

Adding rows is what makes a suite covered. Once covered, every major the rows do not name refuses. Had those PRs added no ledger rows at all, the gate would have said nothing and they would be green.

That inverts how this reads. The author is not failing at a required step — they are volunteering for an optional one, doing it incompletely, and being punished for having tried. The 249 uncovered suites are all silently fine.

add no rows        -> green
add rows, 5 majors -> green
add rows, 1 major  -> RED

Doing nothing and doing it properly are both green; doing it partially is the only failure. That is a bad shape for a step nothing asks for and the gate's own message invites with <log> singular.

Independently confirmed on a PR that chose the other path

@jdatcmd hit the same thing from the other side today: they merged five logs into their own test-fix PR, expected +3 rows and got +15, because native_fetch_projection is also uncovered and every check of it was new. They restored the file and shipped no ledger change — correctly.

Their conclusion is the one that generalises, and it should be in the message:

"Did I add checks" is the wrong question. "Is this suite covered" is the one that decides.

What this adds to the fix

The warning is still right, but it now has a second job. When merge writes rows for a suite the ledger did not previously cover, it should say what that means:

NOTE: native_chunk_length_bound had no rows; this merge makes it a COVERED suite.
      The gate will now refuse any of its checks on a major these rows do not name.
      Rows written for: 18.  Gated majors: 17, 18.
      Either merge a log per gated major, or commit no rows for this suite --
      an uncovered suite is not a gap the gate refuses, it is one of 249.

That tells a contributor the thing none of the six knew: seeding is opt-in, and opting in halfway is the only way to lose.

Also worth doing

suites_not_covered 249 is a ceiling that may only fall, which reads as pressure to seed. Nothing says seeding is optional or that a partial seed is worse than none. One line beside that number would carry it.