Skip to content

test: the ledger records which majors each check exists on (#1010) - #1019

Merged
jdatcmd merged 4 commits into
commandprompt:mainfrom
OffgridwithJD:fix/1010-the-ledger-keys-on-the-major
Sep 12, 2026
Merged

test: the ledger records which majors each check exists on (#1010)#1019
jdatcmd merged 4 commits into
commandprompt:mainfrom
OffgridwithJD:fix/1010-the-ledger-keys-on-the-major

Conversation

@OffgridwithJD

Copy link
Copy Markdown
Collaborator

Step 2 of #1010. Depends on #1013 (the first two commits) -- do not merge this before it
if you want them reviewed apart. The three commits that are this change are
ce2be2e5, f5b09c80, 0812c57d.

suite<TAB>part<TAB>name<TAB>majors<TAB>last-red<TAB>mutations

The major is a FIELD, not part of the key

A check's existence depends on the major, so the ledger has to record where a check
exists. It does not follow that the major belongs in the key, and the measurement decides it.

Full matrix at 4d7c75ae, 252 suites on PG15 and PG18: 6367 of 6472 checks are identical
on both majors
and 105 exist on exactly one. A (major, suite, part, name) key would hold
5 copies of one observation for 98% of the file.

On today's coverage, measured from the migration itself:

1166 checks  ->  1166 rows under (suite, part, name)
1166 checks  ->  5830 rows under (major, suite, part, name)     exactly 5x

Across the whole tree the five-major logs hold 7795 distinct checks, which a pair key would
store as 31357 rows.

Keeping the key also keeps checks_never_observed_red counting CHECKS. Under a pair key
it would count pairs, and "5800 checks" in a tree holding 1150 of them is a number that lies
by its own name -- the defect check_ledger_budget.txt exists to argue against. No budget
key is renamed and the census does not move.

The field is a sorted ;-separated SET and it accumulates. A plain assignment was
measured doing the wrong thing to last-red on #918: merging a PG15 log after a PG18 log must
not make the check stop existing on 18, because the order somebody merges logs in is not a
fact about the code.

No wildcard. "Every major observed" would change meaning the day a major joins the
matrix, inheriting a claim nothing measured.

What it fixes is orphan-scan, not gate

The gate refuses a check in the LOG the ledger has not seen, and a PG18-only check does not
appear in a PG15 log -- so it stayed correct by never being asked. orphan-scan asks the
opposite question, and a PG18-only row is exactly what a deleted check looks like on PG15.

It was saved only by the SKIP rule, and that was luck: analyze_differential emits a
check_skip on PG15-17 so its part was unprunable, while fk_referencing:287 emits
check and has no SKIP at all
-- 24 keys shared, 2 only on PG18, 1 only on PG15. Once that
suite is seeded, a PG15 run would have called its two PG17+ checks deleted.

The fourth category already said the true thing ("this run does not contain them, so it
cannot speak about them"), so the scope gains an intersection and nothing else: no new
category and no grandfather rule.

The gate also cannot refuse a check on a major it holds no rows for, for the same reason it
cannot in a suite it has never seen -- otherwise adding PG20 reddens every check at once,
which is a gate somebody turns off. It says out loud when it is not enforcing.

The migration

Derived from a five-major matrix on step 1's tree, 252 suite logs per major:

rows            1166 -> 1166   (carried 1166, added 0)
census          1164 -> 1164   (unchanged)
ever red           2 ->    2   (both carried, with their dates)
major sets      1166 x "15;16;17;18;19"

Every row claims all five majors because all three covered suites are major-invariant:
harness_selftest 916, differential 204, native_join_runtime_filter 46, identical on
15/16/17/18/19. So the diff is one field added per line.

It fails closed rather than guessing: a committed row observed on no major refuses the
whole run and prints the rows, because absence is not removal and a placeholder major would
be a claim nothing measured. It refused nothing here -- all 1166 were observed.

It also refuses to widen coverage. The logs carry all 252 suites; seeding them would take
the ledger to 7795 rows and drop suites_not_covered from 249 to nearly nothing, making
several thousand checks gate-enforced in a diff nobody could review. 6629 observed checks
outside the three covered suites were deliberately not seeded.

Test plan

  • 22 pytest ledger tests, 133 checks, 0 fail, including three new ones: the set
    accumulates; a run speaks only for the majors a row claims; the gate cannot refuse on
    an uncovered major. Each carries a CONTROL -- without them the first proves only that
    nothing is ever an orphan and the third only that the gate refuses nothing.
  • ~20 new shell arms in part 410 covering the same properties from the other harness,
    plus the invalid-major and no-major row refusals with an unknown control.
  • Five-major matrix for the derivation: 252 suite logs each, 6660-6787 records per major.
  • docs_style.sh PASSED.
  • Matrix on the migrated ledger -- queued; step 1's own matrix is the evidence for the
    record format and this changes only the ledger the gate reads.

A correction carried in the second commit

I justified the unknown token by grepping harness_selftest.sh for PGC_MAJOR, finding
none, and concluding its 907 records carry it. That went into eight places before one awk
over a log already on disk disproved it: all 916 carry the real major, because 10 of its 46
parts call pgc_setup and parts share one shell. The truth is 14 other suites that need
no cluster -- 544 of 6753 records. All eight are corrected.

The measurement also showed something I would not have guessed: unknown versus a number is
order-dependent in any suite sourcing parts into one shell. A part added ahead of the
first pgc_setup would emit unknown where the ledger expects a number.

Also corrects orphan-scan's docstring, which named part 340's stand-in skip name as a
present-tense blocker to arming it. #994 and #998 removed that. Arming is now a decision
rather than a dependency, and it is not taken here -- filed as #1015, because "the one
blocker I measured is gone" is not the same claim as "no blocker remains".

🤖 Generated with Claude Code

https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a

@OffgridwithJD
OffgridwithJD force-pushed the fix/1010-the-ledger-keys-on-the-major branch from f5b09c8 to 6fc13c1 Compare September 12, 2026 17:54

@jdatcmd jdatcmd 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. The migration is derived rather than assumed, and I checked the arithmetic against main rather than the PR body.

main    204 differential + 916 harness_selftest + 46 native_join_runtime_filter = 1166
pr1019  204              + 916                  + 46                            = 1166
diff    1166 insertions, 1166 deletions -- one field added per line, no row moved
census  1164 = never rows 1164, both sides;  ceiling 249 -> 249

The part that could have been a false claim is not one. Every row migrates to 15;16;17;18;19, which the migration cannot know a priori — and you did not assume it. You ran a five-major matrix, 252 suite logs each, and the three covered suites came back major-invariant with those exact counts. That is the difference between a migration that records a measurement and one that records a hope.

And refusing to widen coverage was the right call. 6629 observed checks outside the three covered suites, deliberately not seeded, because taking the ledger to 7795 rows would make several thousand checks gate-enforced in a diff nobody could review. I seeded differential today precisely because that number moves one suite at a time and stays reviewable; doing all 252 in the same change as a format migration would have buried both.

The token validation matters more than it lookspg18 refused as a major, unknown carried as a real member rather than a courtesy. A ledger keyed on a field whose vocabulary is not policed would quietly grow a second spelling of the same major and the intersection logic would silently stop matching.

One thing I verified because it is the failure this whole issue is about: the fourth category still says "this run does not contain them, so it cannot speak about them", so the scope gains an intersection and nothing else. Absence still does not mean removal. That is the invariant I would have been most worried about a majors field quietly breaking.

Merging when CI settles.

jdatcmd added a commit to linuxhikerpm/pgcolumnar that referenced this pull request Sep 12, 2026
commandprompt#1013 landed between this branch going green and being merged, so the merge commit
could not be created. Second rebase of this PR today; both were caused by merges of
mine, not by anything the author did.

TWO CONFLICTS.

CHANGELOG.md: two entries, neither a revision of the other. Both kept.

check_ledger_budget.txt: the census. This branch said 1162, main said 1164, and
BOTH ARE NOW WRONG -- the ledger auto-merged and holds rows from both sides. The
number is DERIVED from the merged file rather than picked from either parent, which
is the case commandprompt#1004 exists for and commandprompt#952 before it: two branches each re-derive a
census from the same base, the ledger takes both sets of rows, and the budget keeps
whichever side won the conflict.

    rows 1179 | never 1171 | ever red 8
    gate: census stated 1171, ledger holds 1171: they agree
    duplicate (suite, part, name) keys: 0

MY FIRST DERIVATION WAS WRONG AND SAID SO OUT LOUD. I counted `$5=="never"` and got
ZERO against 1179 rows. commandprompt#1019 adds the majors field and moves last-red to field 5,
but commandprompt#1019 is not merged: this tree is still five fields, and last-red is field 4.
A count of zero never-red rows on a tree with eight ever-red is not a plausible
number, which is the only reason I looked. Had the two formats differed by
something less obvious than 1171 against 0, the budget would have shipped a lie the
gate would then have refused on a correct tree.

No code change; the C in this PR is untouched by the merge.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw
@jdatcmd

jdatcmd commented Sep 12, 2026

Copy link
Copy Markdown
Collaborator

This is red on suites (PG 17) and suites (PG 18), and it is two printf lines. My approval above stands for the design; the failure is a fixture the migration missed.

>> FAIL  a stale prior is named WITH its distance from HEAD: got [0] want [1]
>> FAIL  and a level prior carries no distance, so zero is silent: got [0] want [1]
>> FAIL  the distance travels with a refusal too, not only with a pass: got [0] want [1]
>> FAIL  and it says the ref does not resolve, rather than claiming the file is new: got [0] want [1]

The cause

410 builds two scratch ledgers in the old five-field format:

line  937   printf 's\tp\ta\tnever\t-\n' > "$_dist/led"
line 1009   printf 's\tp\ta\tnever\t-\n' > ...

This PR makes a ledger row six fields. So read_ledger refuses before the gate ever prints a ceiling line, and every arm that greps that line counts zero.

Reproduced against your tool with your fixture, built exactly as 410 builds it:

A  the row as shipped (5 fields)
   ledger integrity failure: led:1: a ledger row needs 6 fields, got 5
   grep -c 'ceiling against oldbase (3 commits behind HEAD)' = 0

B  the same row with a majors field
   ceiling against oldbase (3 commits behind HEAD): 7 -> 7, which does not rise
   grep -c = 1

So: printf 's\tp\ta\t15;16;17;18;19\tnever\t-\n' in both places, or whatever major set you want those scratch rows to claim.

Why it is worth a sentence beyond the fix

The failing arms are not about majors at all. They are #1015's distance-from-HEAD arms, and they broke because a format migration moved a field under fixtures that belong to a different feature. That is the same shape as the $5 versus $4 slip I made rebasing #1012 an hour ago: the ledger's field layout is in flight, main has #1013 and not this, and anything holding a field index is correct on one side of the merge and wrong on the other.

The grep in those arms is what made it legible — got [0] want [1] on four arms at once points straight at "the tool produced no output", not at a wrong number. An arm that asserted a count greater than zero would have said the same thing far less usefully.

I did not push the fix to your branch. Two printfs is smaller than the coordination cost of me editing a PR you are actively working, and the major set those scratch rows should claim is a choice about what the fixture means, not a mechanical substitution.

OffgridwithJD and others added 3 commits September 12, 2026 12:15
…ompt#1010)

The major is a FIELD, not part of the key, and that is the design rather than an
implementation detail.

A check's existence depends on the major, so a ledger that cannot say where a
check exists cannot tell a deleted check from one that never ran here.
analyze_differential.sh:61 emits ONE record on PG15-17 and a suite's worth on
PG18+; fk_referencing.sh:287 emits DIFFERENT CHECK NAMES in its two branches.

It does not follow that the major belongs in the key. Measured on a full matrix
at 4d7c75a, 252 suites on PG15 and PG18: 6367 of 6472 checks are identical on
both majors and 105 exist on exactly one. A (major, check) key would hold
6472 x 5 = 32,360 rows to express those 105 -- about 247 duplicate rows for every
row that differs, each a second copy of one observation.

Keeping the key at (suite, part, name) also keeps checks_never_observed_red
counting CHECKS. Under a pair key it would count pairs, and "5800 checks" in a
tree holding 1150 of them is a number that lies by its own name.

The field is a sorted ;-separated SET and it ACCUMULATES. A plain assignment was
measured doing the wrong thing to last-red on commandprompt#918: merging a PG15 log after a
PG18 log must not make the check stop existing on 18, because the order somebody
merges logs in is not a fact about the code.

NO WILDCARD. "Every major observed" would change meaning the day a major joins
the matrix, inheriting a claim nothing measured.

WHAT THIS FIXES is orphan-scan, not gate. The gate refuses a check in the LOG the
ledger has not seen, and a PG18-only check does not appear in a PG15 log, so it
stayed correct by never being asked. orphan-scan asks the opposite question, and a
PG18-only row is exactly what a deleted check looks like on PG15. It was saved
only by the SKIP rule -- analyze_differential emits a check_skip so its part was
unprunable, while fk_referencing emits check and has no SKIP at all. The fourth
category already said the true thing, so the scope gains an intersection and
nothing else: no new category and no grandfather rule.

The gate also cannot refuse a check on a major it holds no rows for, for the same
reason it cannot in a suite it has never seen.

The documented census derivation moves from $4 to $5 in
check_ledger_budget.txt, and both CHANGELOG entries that state it are corrected
visibly rather than silently -- one of them says in its own text that the command
was an INSTRUCTION and that is why its first wrong version is still shown.

The committed ledger is migrated in the next commit, from a five-major run.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
I justified the `unknown` token by grepping harness_selftest.sh for PGC_MAJOR,
finding none, and concluding its 907 records all carry it. That was written into
eight places before one awk over a log already on disk disproved it:

    grep '^RESULT\t' pg18-harness_selftest.log | awk -F'\t' '{c[$6]++} END {...}'
      18: 916

All 916 carry the real major. The suite does not reference PGC_MAJOR, but 10 of
its 46 parts call pgc_setup -- which sets it -- and parts are sourced into ONE
shell, so the value persists to every later record. Absence from the source was
never the question.

The truth is 14 OTHER suites: they need no cluster, so they never call pgc_setup,
and every record they emit carries `unknown`. Measured on a full pg18 matrix,
544 of 6753 records: audit, concurrency, decode_interrupts, hilbert_curve,
objstore_stash_recovery, phase2-6, smoke, unique_conc, update_conc, wal_envelope.
None of the three suites the ledger covers today is one, so every migrated row
will name real majors; the token matters for the suites coverage reaches next.

A consequence only the measurement showed: because the value is set by whichever
part runs first, `unknown` versus a number is ORDER-DEPENDENT in any suite that
sources parts into one shell. On pg18 all 916 of harness_selftest's records named
the major, so the first pgc_setup precedes the first record today. A part added
ahead of it would not, and the ledger expecting a number would refuse it.

Also corrects orphan-scan's docstring, which named part 340's stand-in skip name
as a present-tense blocker to arming it. commandprompt#994 and commandprompt#998 removed that: part 340 now
calls check_skip under each premise's own name and the stand-in survives only in a
comment. Arming is now a decision rather than a dependency, and it is not taken
here -- filed as commandprompt#1015, because "the one blocker I measured is gone" is not the
same claim as "no blocker remains".

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
Derived from a five-major matrix on step 1's tree (a0b2c00), 252 suite logs per
major, 6660-6787 records each. Not a merge: the committed ledger is five fields
and step 2's read_ledger needs six, and a five-field compatibility path in the
tool would leave a branch nobody removes.

    rows            1166 -> 1166   (carried 1166, added 0)
    census          1164 -> 1164   (unchanged)
    ever red           2 ->    2   (both carried, with their dates)
    fields             5 ->    6
    major sets held    1166 x "15;16;17;18;19"

Every row claims all five majors, because all three covered suites emit the same
checks on every one: harness_selftest 916, differential 204,
native_join_runtime_filter 46, identical on 15/16/17/18/19. So the census does not
move and the diff is one field added per line.

THE MEASURED CASE FOR THE SET. Under a (major, check) key those same 1166 checks
would be 5830 rows -- exactly 5x, 4664 of them second copies of one observation.
Across the whole tree the logs hold 7795 distinct checks whose major sets would be
31357 rows under a pair key, a 4x multiplier, and the distribution says why a set
is the right shape:

    5863  15;16;17;18;19      checks identical on every major
     545  unknown             suites that need no cluster, so never call pgc_setup
     285  19                  \
     251  18                   |  250 per major of these are fuzz, whose names
     250  15, 16, 17 each      |  interpolate a random fixture (commandprompt#1011)
      93  18;19               /
       3  15;16;17
       2  15;16;17;18
       2  17;18;19
       1  15;16

COVERAGE IS NOT WIDENED HERE, and the migration refuses to. The logs carry all 252
suites; seeding them would take the ledger to 7795 rows and drop
suites_not_covered from 249 to nearly nothing, making several thousand checks
gate-enforced in a diff nobody could review. 6629 observed checks outside the three
covered suites were deliberately not seeded.

The migration also fails closed rather than guessing: a committed row observed on
no major refuses the whole run and prints the rows, because absence is not removal
and a placeholder major would be a claim nothing measured. On this tree it refused
nothing -- all 1166 were observed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
@OffgridwithJD
OffgridwithJD force-pushed the fix/1010-the-ledger-keys-on-the-major branch from 47e3191 to 84bbe88 Compare September 12, 2026 18:16
…andprompt#1010)

Found by running harness_selftest on pg15 rather than by reading: seven arms in
the --against prior-budget family went red, all with rc=2, which is the tool's
integrity failure. Both fixtures write a ledger to a path that is neither *.tsv
nor $_ledger -- "$_dist/led" and "$_rr/led" -- so the sweep that widened every
other fixture matched neither, twice.

The lesson is the same one the format transform taught earlier in this change and
I did not carry far enough: a sweep keyed on a FILENAME PATTERN cannot find a
fixture named something else. The sweep that found these counts tab-separated
FIELDS in every printf that writes a date or `never`, which is a property of the
content rather than of the path.
@OffgridwithJD
OffgridwithJD force-pushed the fix/1010-the-ledger-keys-on-the-major branch from 84bbe88 to 9389792 Compare September 12, 2026 18:51

@linuxhikerpm linuxhikerpm left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

CI is red for a reason the tree itself states. At 84bbe886f43cb67e2b4cf526e352c1739e156a55 test/check_ledger.tsv is still five fields (differential\tdifferential\tagg avg\tnever\t-). The tool now requires six, so:

  • pytest-guards: every committed row has six fields: got 1179 want 0 (and never=0 because it is reading the mutations column as last-red)
  • suites: check_ledger.tsv:1: a ledger row needs 6 fields, got 5, then harness_selftest=FAIL

The migration commit message says the file was rewritten to suite, part, name, majors, last-red, mutations. That file is not in this PR's diff. Re-derive it against current main (which now also carries native_join_vector_agg from #1012) rather than merging past a 5-field ledger the new reader refuses.

Do not merge.

@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

Right about 84bbe886, and it is fixed at 9389792d. That commit was a deliberate
intermediate state and I should have marked it draft rather than leave it reviewable --
the five-field ledger against the six-field reader was me parking the branch while the
derivation ran, not a state I thought was mergeable.

What the current head carries:

rows                       1197
fields                     6
never ($5)                 1189
ever red                   8
budget                     checks_never_observed_red 1189
first row                  differential  differential  agg avg  15;16;17;18;19  never  -
suites                     differential, harness_selftest,
                           native_join_runtime_filter, native_join_vector_agg

Re-derived against current main, including the suite #1012 added, which is the part
your review is most right about and which I had not seen until it landed:

rows            1179 -> 1197   (carried 1177, new 20, dropped 2)
census          1171 -> 1189
ever red           8 ->    8   (all carried, with their dates and the one mutation)
major sets      1197 x "15;16;17;18;19"

Five-major matrix, 253 suite logs per major. All four covered suites are major-invariant --
934, 204, 46 and 13 records identical on 15/16/17/18/19 -- so every row claims all five and
the census moves only by the twenty arms this change adds.

The 2 dropped rows are the two checks this change RENAMED (every committed row has five fields became six). The migration refuses to drop a row carrying history, for the reason
orphan-scan refuses it, so a drop is only available for a row with nothing to lose and the
name has to be typed on the command line.

On "rather than merging past a 5-field ledger": that is exactly what went wrong and it is
worth recording.
Rebasing onto 14f64ca3 produced a ledger of 2348 rows with field
counts 1, 5 AND 6
-- git merged the two formats line by line and added conflict markers.
No field index was involved; the file simply stopped being a ledger. So I took main's
five-field file whole and re-ran the migration over it rather than resolving a merge.

And the guard you would reach for already exists: part 410's every committed row has six fields arm is what made CI red here rather than something subtler, and it is the same arm
one field wider. Verified on a synthetic 5/6/1-field mix -- NF!=6 counts 2 where the arm
wants 0.

Worth one note on what the census number means now, since it is the thing most likely to be
misread: the key is still (suite, part, name), so a row is still one check and
checks_never_observed_red still counts CHECKS. That is the whole reason the major is a
set in a field rather than part of the key -- under a pair key those 1197 checks would be
5985 rows and the number would have to be renamed to stay true.

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.

3 participants