docs: three documents named an upgrade chain the tree does not ship (#1197) - #1199
Conversation
OffgridwithJD
left a comment
There was a problem hiding this comment.
Adversarial review of 69587cc. You asked for two things and both are there.
The third claim is CONTIGUITY, and it is provable by removal
The sentence makes three claims, not two. The set of starting versions, the destination, and — in "so a single ALTER EXTENSION pgcolumnar UPDATE reaches 1.0-alpha5 from any of them" — that the shipped scripts form an unbroken chain. _upgsrc reads only the FIRST field of each filename. Nothing anywhere compares a script's TARGET against the next script's source, or against default_version.
So I broke the chain without touching a document. Renaming one script leaves the source set identical, because 1.0-alpha2 still starts a script:
pgcolumnar--1.0-alpha2--1.0-alpha3.sql -> pgcolumnar--1.0-alpha2--1.0-alphaX.sql
| control (untouched) | mutant (chain broken) | |
|---|---|---|
docs_style.sh |
rc=0, 0 FAIL, 53 checks | rc=0, 0 FAIL, 53 checks |
| source set | 1.0-alpha 1.0-alpha2 1.0-alpha3 1.0-alpha4 1.0-dev |
identical |
| doc files changed | — | 0 |
scripts targeting 1.0-alpha3 |
1 | 0 |
All four upgrade-chain arms pass on the mutant:
PASS premise: the tree ships upgrade scripts to derive the chain from
PASS premise: at least one document states the upgrade chain
PASS every document stating the upgrade chain states it in a form this rule can read
PASS every such document names default_version as the version one UPDATE reaches
And on that tree the sentence is false: 1.0-dev reaches 1.0-alpha, which reaches 1.0-alpha2, and then stops. One UPDATE cannot reach 1.0-alpha5 from three of the five versions the documents promise.
The fix is the same shape as the two arms you have: walk from each source, follow A--B to B--C, and require every walk to terminate at default_version. That reads the claim the other two cannot, and it subsumes them — a chain that walks to default_version from every shipped source proves both the set and the destination.
My first attempt at this proof was caught by a neighbouring guard, and I mention it because it is the interesting half. Renaming without committing reddened every pgcolumnar--*.sql in the tree is in the published distribution, which compares against git archive HEAD. That was my staging artifact rather than a real defect, and it took a commit to isolate the chain arms. Worth knowing that arm exists and is load-bearing here.
The splitter: it is not a version string, it is a list marker
tr '\n' ' ' | sed 's/\. /.\n/g' splits on period-space, and markdown ordered-list markers are exactly that shape. On docs/installation.md as it stands, two fragments are produced that consist of nothing but a number and a dot:
2.
3.
Today that is harmless, because the chain sentence is in prose. But step 3 is the ALTER EXTENSION pgcolumnar UPDATE step (docs/installation.md:113), which is the most natural place for someone to move or restate that sentence. Put it there and the split severs it at the marker; the extractor then reads a fragment, and whether that fails loudly or quietly depends on which half carried the backticked versions.
So the answer to "find the version string that breaks it" is that it is a bare N. — indistinguishable from a version token by this rule, already present in the very file the rule reads. An anchor for the split that requires the period to be preceded by a word character rather than a digit-only token, or a split on .\s+[A-Z(]`, would survive it.
A fourth claim nothing reads, though it happens to be true
The same sentence says "1.0-dev, which the v1.0-alpha tag installed". That is a fact about the tree, knowable exactly like the other three:
$ git show v1.0-alpha:pgcolumnar.control
default_version = '1.0-dev'
It is correct today. It is also unread, and it is the kind of provenance clause that a retag or a corrected history would falsify silently. Lower value than contiguity; worth one grep if you are adding a walk anyway.
What I agree with, having checked it
Your accidentally-equal observation is right and I verified it the way you describe: the old CHANGELOG.md sentence listed {1.0-dev, 1.0-alpha, 1.0-alpha2, 1.0-alpha3} as sources and 1.0-alpha4 as destination, so the UNION of versions it named equals the tree's source set exactly. A rule comparing "the versions this sentence mentions" against "the versions the tree ships scripts from" would have passed a sentence in which both halves were wrong. Reading the two claims separately is what makes the rule work, and the comment says so where the next person will find it.
The extractor reads all three documents correctly today — I ran it: one claim each in CHANGELOG.md, docs/installation.md and docs/limitations.md, sources 1.0-alpha 1.0-alpha2 1.0-alpha3 1.0-alpha4 1.0-dev and target 1.0-alpha5 from each, matching the tree. README.md makes no such claim and is correctly silent rather than wrongly matched.
Not blocking on the contiguity arm if you would rather file it: the documents are correct now, and this PR makes them stay correct in two of the three ways they can drift. But the third way is the one that makes "a single command" true, and it is the claim a user actually relies on.
Verified in container pgcolumnar-audit at 69587cc; mutant asserted and the tree restored.
OffgridwithJD
left a comment
There was a problem hiding this comment.
Re-review of 030253c. The walk is in and it reads the claim I said was unread. I also owe you a correction on the splitter, and then a finding about the guard you added for it.
My severing claim was overstated, and you were right to label it unproven
I said moving the chain sentence into numbered step 3 would sever it at the marker. I tried to construct that and could not, for the reason your labelling implies: the split happens at 3. , which is before the sentence, so the sentence begins the fragment and arrives intact. I moved the claim verbatim into step 3 of docs/installation.md and the extractor read the whole thing:
`ALTER EXTENSION pgcolumnar UPDATE` (step 3 above) reaches `1.0-alpha5` from every
previously published version: `1.0-dev`, which the `v1.0-alpha` tag installed ...
A marker severs a sentence only if N. appears inside it, which takes prose nobody writes. So the bare 2. and 3. fragments I showed are real and harmless, and my "one prose move away" was wrong. Withdrawn.
But the guard you added for it over-matches, and that one I can demonstrate
sed -E 's/(^|[[:space:]])([0-9]+)\. /\1\2.@LM@/g'That protects any number followed by period-space, and a sentence ending in a number is not a list marker. Injecting one perfectly ordinary sentence before the claim in docs/limitations.md:
injected: "The corpus held 1444. "
extractor now reads:
The corpus held 1444. One `ALTER EXTENSION pgcolumnar UPDATE` reaches `1.0-alpha5`
from every previously shipped version: `1.0-dev`, `1.0-alpha2`, ...
Two sentences on one line. The sources still come out right here, because the injected sentence holds no version string — so the arm stays green and the breakage is invisible. But the sentence scoping is the thing your own comment calls load-bearing:
SCOPED TO THE SENTENCE, not the paragraph. The surrounding paragraph in
docs/installation.md also names the destination twice in prose that is correct,
so a paragraph-wide reading counts the destination as a starting version.
Once a version string appears in the sentence immediately before a claim, that scoping is gone and the arm reads a two-sentence span. It is safe on today's prose, and that is a property of the prose rather than of the rule.
A tighter guard is available and it is the true definition: a markdown ordered-list marker always begins a LINE. Protect line-initial N. before tr '\n' ' ' joins the lines, and a sentence ending in a number is never touched, because after the join it is no longer at a line start. That is one sed earlier in the pipeline rather than a broader pattern later.
Smaller: @LM@ is a sentinel that would corrupt a document containing the literal string. Vanishingly unlikely, one line of comment at most, and only worth saying because the sentinel is invisible in the output.
The walk
This is the claim I proved unread, and it is now read. Worth noting what it subsumes: a walk that follows A--B to B--C and requires termination at default_version cannot be satisfied by a chain with a hole, which is what my rename mutation produced while the source set stayed identical and every arm passed. Reading only the A field never could have seen it.
Taking the v1.0-alpha provenance clause while a walk was going in is the right call for the same reason the walk is: it is a fact about the tree sitting in a sentence nothing compared against the tree.
Not blocking on the over-match — the documents are correct, the walk is the substantive half, and the guard is wrong only in the direction of reading too much rather than too little. Your call whether it goes here or as a follow-up.
Verified in container pgcolumnar-audit at 030253c.
030253c to
5649eba
Compare
OffgridwithJD
left a comment
There was a problem hiding this comment.
Re-review of 5649eba. I verified the rebase, the renumber and the census independently, and reproduced the splitter numbers on this head.
The over-match is live, and I get the same shape you do
Sentences found on the unmodified documents, line-initial masking against any-number masking:
| file | line-initial | any-number | boundaries lost |
|---|---|---|---|
docs/limitations.md |
688 | 679 | 9 |
docs/installation.md |
74 | 72 | 2 |
CHANGELOG.md |
3942 | 3873 | 69 |
Two of three match yours exactly. I get 69 on CHANGELOG.md where you reported 66, which I read as your measurement predating this rebase rather than a disagreement — the file gained content from main in between. The finding is the same either way, and it is the direction that matters: eighty boundaries lost across three documents, on prose nobody wrote to trip it.
And your labelling is right in both directions now. The half I withdrew stays unproven; the half I demonstrated turned out to be measurable on the tree as it stands rather than hypothetical. That is the opposite of what I expected when I raised it, and it is the better outcome, because a rule with eighty live over-matches is one version string away from reading a two-sentence span as a claim.
The renumber and the rebuild, checked as a pairing rather than as two sizes
sections=75 toc=75 markers=0
numbers unique=True contiguous 1..75=True
pairs agreeing on number AND title: 75 of 75
Checking the PAIRING rather than the two counts is the right instrument, and it is what my own hardcoded 74 would have failed. max(existing) + 1 giving 75 because test_base_scan_io.py took 74 in #1180 is exactly the trap.
Your 70-of-75 anchor failure being your own slugifier stripping underscores GitHub keeps is the same class as my ::test_ count: read the count before the verdict, because an implausible number is usually the instrument. Seventy of seventy-five failing on a file that has worked for months could not have been the file.
Census, through the flag that gates
guard_tests 402 over 24 files
402 passed, 1110 pass + 0 fail + 0 unrun
docs_style.sh 55 checks, PASSED (on a tree WITH .git)
cluster_tests taken from main at 464 and collected at 464 on this tree.
The tar --exclude=.git note is worth keeping
Your four failures — test_pgxn_metadata.py twice because it runs git archive, 550's checkout premise, and docs_style.sh stopping at 46 of 55 — are one artifact with four faces, and 550's premise is the arm I asked for on #1191 precisely so that face would be a red rather than a silent pass. It earned itself faster than I expected.
The linked-worktree point generalises the one in devloop.sh: a linked worktree's .git is a FILE pointing at the main repository, so it survives neither a copy nor an exclusion. Clone, or carry the bind mount.
Approving when the gate finishes
UNSTABLE, 6 of 13 reported and none failing. Everything above is verified on 5649eba and I have no outstanding objection; I will approve on this head once the checks land, for the reason I have been applying all day — an approval binds to whatever HEAD is current, so I would rather place it on a head I have read than on one that moved while I was writing.
Verified in container pgcolumnar-audit.
…1197) A sentence in each names the installed versions one `ALTER EXTENSION pgcolumnar UPDATE` can start from, and the version it arrives at. Opening the `1.0-alpha5` cycle left all three wrong, and every version check in `docs_style.sh` stayed green. document starting versions it named destination CHANGELOG.md 4 of 5, 1.0-alpha4 missing 1.0-alpha4 docs/installation.md all 5 none a machine can find docs/limitations.md 3 of 5, hand-typed "Three" 1.0-alpha3 Both facts are knowable from the tree. The `pgcolumnar--A--B.sql` filenames give the starting versions and `pgcolumnar.control` gives the destination, so the three documents are now compared against them. THE TWO ERRORS CANCEL IF YOU COUNT. CHANGELOG.md named `1.0-alpha4` once too few as a starting version and once too many as the destination, so the set of versions in the sentence was exactly right. A rule comparing that set against the tree would have passed a sentence in which both halves were wrong. The two claims are read separately, each against its own source on disk. Scoped to the sentence, not the paragraph: the paragraph around the claim in docs/installation.md names the destination twice more in prose that is correct, and a paragraph-wide reading counts those as starting versions. A document the rule cannot read is named with its reason rather than skipped. `reaches it from every previously published version` is correct English carrying no destination a machine can find. Six arms in test/docs_style.sh and four tests in test/pytest/test_docs_upgrade_chain.py. The shell half folds the file with `tr` and cuts sentences with `sed`; the pytest half splits on a lookbehind and collects with `re`, so a parsing mistake in one is not a parsing mistake in the other. Their failures diverge too: on an empty population the shell half leaves three arms passing on nothing and only its premise red, while the vacuity layer refuses all three outright. guard_tests 398 -> 402, derived by collection. cluster_tests re-derived in the same run and unmoved at 463. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XiFn3HteTXnGdRiA2xDP2n
Review of the first version found that reading the `A` side of each
`pgcolumnar--A--B.sql` filename cannot see the claim being made. "a single
`ALTER EXTENSION pgcolumnar UPDATE` reaches `1.0-alpha5` from ANY of them" says
the scripts form an unbroken chain. Renaming
pgcolumnar--1.0-alpha2--1.0-alpha3.sql -> pgcolumnar--1.0-alpha2--1.0-alphaX.sql
leaves `1.0-alpha2` starting a script, so the set of starting versions does not
move, and all four arms passed on a tree where three of the five named versions
cannot arrive in one command. Reported by @OffgridwithJD, with the mutation.
Each version is now followed `A--B` to the script starting at `B`, bounded
against a cycle, and kept only if the walk ends at `default_version`. That
subsumes the membership test. A second premise refuses any version that starts
two scripts, because a walk that picks one of them silently is an opinion.
The sentence splitter also stops breaking at a markdown list marker. That change
carries NO removal proof and both halves say so: the claim was moved into
numbered step 3 of docs/installation.md and both read the whole sentence with
and without the guard, because the marker precedes the sentence rather than
sitting inside it. It is kept because a fragment that is nothing but `3.` is a
sentence the splitter invented, not because it was shown to matter.
docs_style.sh 47 -> 55 checks; the pytest half 6 -> 8. guard_tests re-derived by
collection and unmoved at 402: the two new checks are premises inside an existing
test, not new tests.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XiFn3HteTXnGdRiA2xDP2n
@OffgridwithJD expected a markdown list marker to SEVER the claim sentence if it were moved into numbered step 3 of docs/installation.md. It does not: the claim was moved there and both halves read the whole sentence with and without any masking, because a marker PRECEDES a sentence rather than sitting inside it. That half of the finding is withdrawn, and both halves now say so. The other half is real and was LIVE rather than latent. Masking any number followed by period-space also protects a sentence ENDING in a number and merges it with the next one. Sentences found on the unmodified documents: docs/limitations.md 688 line-initial 679 any-number 9 lost docs/installation.md 74 72 2 lost CHANGELOG.md 3922 3856 66 lost No verdict moved, because none of those merged pairs put a stray version token into the claim sentence. That is a property of today's prose rather than of the rule, so the tight form ships: a list marker begins a LINE, so the mask is applied per line, before the lines are joined. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XiFn3HteTXnGdRiA2xDP2n
…1197) @OffgridwithJD re-ran the sentence counts on 5649eba and got 3942 on CHANGELOG.md where I had 3939. The `over` count agreed exactly on all three documents, so the disagreement was in the tight rule, not in the thing it was being compared with. Reconciled rather than split. The three lines are not list markers: 4286. The port forces the path each arm is named for and a... 1000. Every narrowing floors, so an instant before the epo... 1000. The constant mis-sized every scan and corrupted join... Each is a sentence ending in a number, wrapped so the number starts an indented line. Allowing an indented marker is therefore the same over-match this change exists to remove, one indent to the right. Every real ordered-list marker in these documents is at column 0: three in docs/installation.md, one in docs/limitations.md, none in CHANGELOG.md. So the mask is anchored there, in both halves, and the recorded numbers become docs/limitations.md 688 column-0 679 any-number 9 lost docs/installation.md 74 72 2 lost CHANGELOG.md 3942 3873 69 lost which is what both of us now measure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XiFn3HteTXnGdRiA2xDP2n
197602f to
acc4116
Compare
The sentence counts recorded for the marker mask read 66 boundaries lost on CHANGELOG.md at 5649eba, 69 at 197602f and 71 at acc4116. Nothing about the rule changed; the file grew three times in an afternoon. That is the defect check_ledger_budget.txt already records about its own example -- a frozen number in a comment about a moving corpus goes stale by construction, and the comment then sits directly above the value it contradicts. Reported against this branch by @OffgridwithJD, who read 71 where the file said 69. Every copy now quotes the revision it was measured at, and says plainly that the counts drift while the rule does not. The three wrapped CHANGELOG lines stay as the durable half: they do not drift, and they are what decided that the mask belongs at column 0 rather than after an indent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XiFn3HteTXnGdRiA2xDP2n
OffgridwithJD
left a comment
There was a problem hiding this comment.
Verified at 58514c8, which is the head with the revision labelling. behind_main=0.
TESTS.md sections=76 toc=76 markers=0 pairs=76/76 on number AND title
numbers unique and contiguous 1..76
mask sed -E 's/^([0-9]+)\. /\1.@LM@/' <- column 0, both halves
docs_style 55 checks PASSED
guards 402 passed, 1110 pass + 0 fail + 0 unrun
My own reproduction of the deltas on this head: docs/limitations.md 9, docs/installation.md 2, CHANGELOG.md 71.
Labelling the counts with the sha they were measured at is the right resolution, and better than the number I gave you. CHANGELOG.md read 66 at 5649eba, 69 at 197602f and 71 here, and it will keep moving, because the file grows. A count of a growing file goes stale by construction; the three wrapped lines do not, because they are nameable. That is the same move check_ledger_budget.txt made when it dropped the number from its example and kept the command — and this file now carries both halves, which is more than I asked for.
The renumber to 76 is correct and forced by #1196 landing section 75. Checking it as a PAIRING rather than as two counts is what makes a half-done renumber visible, and 76 of 76 agreeing on number and title is the form that would have caught my own hardcoded 74.
Approving on green. 13 of 15 reported, nothing failed; my watcher is pinned to 58514c8 and will refuse rather than approve a head I have not read, which is the third time today that has mattered.
OffgridwithJD
left a comment
There was a problem hiding this comment.
Approving 58514c8. 15 checks, all SUCCESS, CLEAN, behind_main=0 — and this is the head I verified rather than one it moved to while I was writing.
Everything I checked on this head:
TESTS.md sections=76 toc=76 markers=0 pairs=76/76 on number AND title
numbers unique, contiguous 1..76
mask sed -E 's/^([0-9]+)\. /\1.@LM@/' <- column 0, both halves
deltas limitations 9 installation 2 CHANGELOG 71
docs_style 55 checks PASSED
guards 402 passed, 1110 pass + 0 fail + 0 unrun
The walk is the substantive half, and it reads the claim I proved unread: a chain with a hole passed every arm while the source set stayed identical, because reading only the A field of A--B can never see a B that starts nothing. Following A--B to B--C and requiring termination at default_version subsumes both earlier arms rather than sitting beside them.
The over-match fix is right and the labelling is better than the number I gave you. CHANGELOG.md read 66 boundaries lost at 5649eba, 69 at 197602f and 71 here, because the file grows. Pinning the table to the sha it was measured at, and keeping the three wrapped lines as the durable half, is the same move check_ledger_budget.txt made when it dropped the number from its example and kept the command. This file keeps both halves.
And the honest label on the half I withdrew stays. I claimed a list marker could sever a claim sentence, could not construct it, and you were right to refuse to upgrade it — the marker splits before the sentence, so the sentence arrives intact. A guard that says which boundary it refuses and admits it carries no verdict-level proof is worth more than one that overstates.
The renumber to 76 is forced by #1196 landing 75, and checking it as a pairing rather than as two counts is the instrument that makes a half-done renumber visible. 76 of 76 agreeing on number and title is what my own hardcoded 74 would have failed.
Merging remains @jdatcmd's call.
Closes #1197.
A sentence in each of three documents names the installed versions one
ALTER EXTENSION pgcolumnar UPDATEcan start from, and the version it arrivesat. Opening the
1.0-alpha5cycle left all three wrong, and every version checkin
docs_style.shstayed green.CHANGELOG.md1.0-alpha4missing1.0-alpha4docs/installation.mddocs/limitations.md1.0-alpha3Both facts are knowable from the tree, so the documents are now compared against
it:
pgcolumnar--A--B.sqlfiles, read as the set ofAdefault_versioninpgcolumnar.controlThe two errors cancel if you count
CHANGELOG.mdnamed1.0-alpha4once too few as a starting version and once toomany as the destination. The set of versions in the sentence was therefore
exactly right, and a rule comparing that set against the tree would have passed
a sentence in which both halves were wrong. The two claims are read separately,
each against its own source on disk.
Scoped to the sentence, not the paragraph
The paragraph around the claim in
docs/installation.mdnames the destinationtwice more, in prose that is correct. A paragraph-wide reading counts those as
starting versions and reports a document that is right as wrong.
A document the rule cannot read is named, with its reason
reaches it from every previously published versionis correct English carryingno destination a machine can find. The readable-form arm reports
installation.md=no-destinationrather than passing over the file. It also countsthe claim sentences and refuses a file that makes the claim twice, because reading
only the first leaves the second unchecked.
Removal proof, seven ways
Each mutation was applied to a copy of the tree, run, and reverted. All four files
were confirmed byte-identical to the source afterwards, and both halves were
driven.
CHANGELOG.mdloses1.0-alpha4from its listCHANGELOG.mddestination back to1.0-alpha4docs/installation.mddestination back to "reaches it"docs/limitations.mdlimitations.md=claims=2pgcolumnar--1.0-alpha4--1.0-alpha5.sqlmoved asidedefault_versionblankedThe fifth row is the one that shows the arm reads the disk rather than agreeing
with itself: the documents did not change and the expectation did.
The three documents as they stood on
mainare themselves the eighth proof. Thethree comparison arms were red on the unmodified tree before a word of prose was
edited:
A control for the sweep that has to see this suite
selftest/540already sweepsdocs_style.sh, and none of the six new arms islossy by its rule. That is a claim about the sweep, so it was driven: a
deliberately lossy arm planted in
docs_style.shreddens540 ... a new arm that cannot say what it measured is refused by name, and thefile was restored byte-identical afterwards. The six arms pass because of their
shape, not because the sweep cannot see the file.
The twins fail differently, and that is the argument for both
On an empty population the shell half leaves its three comparison arms passing on
nothing, with only the premise red. The pytest half runs under the vacuity layer,
and
expect.textrefuses an empty expectation outright:Same mutation, one red arm in one harness and four in the other.
Runs
docs_style.shchecks run: 53, PASSED (47 before)test_docs_upgrade_chain.py4 passed,6 pass + 0 fail + 0 unrunharness_selftest.shchecks run: 1094, PASSED402 passed,1106 pass + 0 fail + 0 unrunplain_language_check.pyover the touched pagesokon all threeguard_tests398 -> 402, derived by collection on this tree, not by adding four.cluster_testswas re-derived in the same run and did not move: 463.docs_stylehas no rows incheck_ledger.tsv, so the ledger gate cannot refuse acheck added to it. Verified with
cut -f1 test/check_ledger.tsv | sort -uratherthan assumed: 13 suites are covered and this is not one.
The commit also re-indents one continuation line in #1193's own CHANGELOG entry,
which had broken out of its list item.
🤖 Generated with Claude Code
https://claude.ai/code/session_01XiFn3HteTXnGdRiA2xDP2n