Skip to content

test/pytest: stop driving lib.sh for properties selftest/340 already holds (#432) - #948

Merged
jdatcmd merged 1 commit into
commandprompt:mainfrom
OffgridwithJD:audit/432-stamp-and-freshness-belong-to-the-shell
Sep 11, 2026
Merged

test/pytest: stop driving lib.sh for properties selftest/340 already holds (#432)#948
jdatcmd merged 1 commit into
commandprompt:mainfrom
OffgridwithJD:audit/432-stamp-and-freshness-belong-to-the-shell

Conversation

@OffgridwithJD

Copy link
Copy Markdown
Collaborator

Closes the python→shell half of the harness-independence debt (#432). With #944 merged, this is the other direction.

test_build_refusal.py drove test/lib.sh for seven calls over pgc_write_source_stamp, pgc_source_stamp_path, pgc_freshness_report and pgc_freshness_verdict. All four are pure shell, so those arms were a second measurement of someone else's subject: they agree with it by construction and can never report it wrong, which is the coupling CONTEXT.md's rule exists to remove.

Five arms removed, and not one needed porting

test/selftest/340 already held every property they asserted, with more arms in each case:

the python arm what 340 already had
the stamp writer reports failure the same, plus a premise that the stamp really was not written
two installations of one major do not share a stamp the same, plus pkglibdir keying and two unreadable pg_configs
the report names each hashed file and states how many the same two arms
an empty manifest says so rather than printing nothing the same arm
a failed digest is unknown, never a false stale the same, over two unreadable files, plus the premise below

340's version of the last one carries a premise the pytest twin never stated: that the unprivileged read AGREES with the privileged one while nothing is denied. Without it the arms measure the user switch rather than the permission denial. That is the better test, and it is on the side whose subject it is.

The first measurement of 340 was wrong, and it nearly cost a duplicate

Enumerating 340's checks with grep -cE '^check "' gave 81. The real number is 89. 340 has indented check calls inside an if and a for, and the eight the sweep missed are exactly the unreadable-source block — the one I had concluded was a genuine gap.

So on that count I wrote a duplicate of it, and proved it discriminates against a mutation, before noticing the duplication. A check sweep has to be anchored at ^[[:space:]]*, not at column 0. This is the same family as never anchoring a check-name regex at $, from the other end of the line.

The tell was a duplicate check name, and my first response to it was wrong. pgc_ledger.py reported "duplicate check name in one run, so one ledger row covers 2", and I renamed my new check to clear it. The right response to a name that already exists is to ask why it exists. Renaming it destroyed the only evidence that the work was unnecessary, and I carried on for two more mutation rounds before the arm's own output showed me 340's arms failing beside mine.

And the first mutation was a no-op, for a reason worth keeping. pgc_source_fingerprint returns empty because the module prints nothing and exits 0 — not because the wrapper's rc != 0 branch fires. Mutating that branch changes nothing on this path, and the run went green while looking like a disproof. The property lives in test/pgc_fingerprint.py; the wrapper's only contribution is that it does not substitute a value for the module's empty answer. Measured:

module, unreadable file, as postgres   rc=0  stdout=[]
wrapper, same                                [] 
wrapper with ${out:-unknown}                 [unknown]   <- the only mutation that reaches it

What remains, named rather than counted

Four calls in two arms:

  • test_the_two_fingerprint_implementations_cover_the_same_inputs — the one permitted cross-reference, because the property is the relationship between the two implementations and cannot be stated from one side.
  • a historical-parity arm whose fixture is its own.

_sh_fp_as is deleted with its last caller. SHELL_REFERENCES keeps test_build_refusal.py — it still reaches across — with a mechanism description that is now true.

Documentation

TESTS.md loses the rows naming the deleted arms, and three prose passages that named them are rewritten to say where the property lives now rather than left pointing at arms that do not exist. A backticked name is a claim that it exists; the reverse sweep enforces that, and it caught this.

CONTEXT.md's inventory goes from "13 calls, seven of them debt" to "four calls, none of them debt", and records the bad enumeration so the next reader does not repeat it.

Gate

pg18a (assert):

harness_selftest.sh     757 checks run, 0 FAIL
pgc_ledger.py gate      rc=0, new this run=0, ceiling 250
driver-free pytest      202 passed   <- 207 minus the five
full pytest corpus      287 passed

No selftest check changed, so check_ledger.tsv and its census are untouched.

Based on main 3d42c682.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a

@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 at 4c36f953, 13/13 green. The risk in a PR that deletes arms is losing a property, so I checked all five against 340 individually rather than taking the summary.

Nothing is lost, and 340 is stronger in every case

removed python arm                                      340 holds it
test_the_stamp_writer_reports_failure                   yes, +2 more assertions
test_two_installations_of_one_major_do_not_share_a_stamp yes
test_a_failed_digest_gives_unverified_and_never_a_false_stale yes, WITH the composition
test_the_fatal_report_can_be_run_rather_than_grepped_for yes
test_an_empty_manifest_is_reported_as_empty_not_as_silence yes

The stamp writer is the clearest illustration of "more". The python arm asserted rc=1 on an unwritable target and rc=0 on a writable one. 340 asserts both of those plus:

premise: and the stamp really was not written, so the arm is not vacuous
control: writing the value it was given

That premise is exactly what the python twin lacked — without it, rc=1 could come from anywhere and the arm would pass on a writer that failed for the wrong reason.

And the composition arm is already on main, which is the one I most expected to be a gap, because your earlier note described 340 as handing pgc_freshness_verdict an empty string directly. It does not:

chmod 000 "$_fp/tree/src/b.c"
check "so the verdict is unknown -- UNVERIFIED -- and never stale" \
    "$(pgc_freshness_verdict "$_fp_base" "$(_fp_as "pgc_source_fingerprint \"$_fp/tree\"")")" "unknown"

A real chmod 000 fed through both functions, with a readable control beside it — the composition, not a stand-in for it. Verified present on origin/main independently of this PR, so the deletion loses nothing. Its premise arm, "the unprivileged read agrees while everything is readable", is the one you noted the python twin never stated, and it is the thing that stops the block measuring the user switch instead of the permission denial.

One number to check, not a blocker

CONTEXT.md goes from 13 calls to FOUR calls. Counting shell-invoking calls by AST — _sh(...) plus subprocess.run with bash:

origin/main   9    {_sh: 7, subprocess bash: 2}
this branch   4    {_sh: 2, subprocess bash: 2}

The 4 is exactly right, and matches your prose description of four calls in two arms — the permitted fingerprint cross-reference and the historical-parity arm. The 13 does not reproduce under the same definition that produces the 4, so the before and after appear to be counted differently. The after-number is the one a future reader will check against the tree, and it is correct; I would just make the two commensurable, since a pair of numbers invites subtraction and 13 - 4 is not the five arms this PR removes.

Merge order

Taking your own measurement as load-bearing: #942 before #948. With #948 first, #942's side of the test_build_refusal.py hunk adds back the five arms this PR deletes — not because it touches them but because it branched earlier, so "take theirs" restores the debt. Your plan to rebase rather than leave that resolution to whoever merges is the right call.

@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.

Approving exact head 4c36f953f69b. MERGEABLE, CLEAN, 13/13 on this SHA.

The five deleted pytest arms were a second measurement of pure-shell functions test/selftest/340 already holds, including the unprivileged-agrees-while-nothing-is-denied premise the pytest copy never stated. Removing them is the independence rule, not a coverage loss. The four remaining lib.sh calls are named: the fingerprint cross-implementation arm (the one permitted cross-reference) and a historical-parity arm. TESTS.md no longer names the deleted tests. Ledger and census are untouched, which is right: no selftest check changed.

Non-blocking: #942 still has to drop _sh(srcdir, …) on the two fingerprint call sites this PR keeps. Landing either first is fine; the other rebases.

@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.

Reviewed exact head 4c36f953f69b. The deletion of the five lib.sh-driving arms is still the right #432 move.

Blocking: mergeable=CONFLICTING against current main (e030c15). #942 landed and edited test_build_refusal.py (dropped _sh(srcdir)). This PR still deletes the old _sh(tmp_path, …) call sites. Please rebase onto current main so the remaining fingerprint _sh(expr) calls and the deleted stamp/freshness arms are the resolved tree CI runs.

…holds (commandprompt#432)

`test_build_refusal.py` drove `test/lib.sh` for seven calls over
`pgc_write_source_stamp`, `pgc_source_stamp_path`, `pgc_freshness_report` and
`pgc_freshness_verdict`. All four are PURE SHELL, so those arms were a second
measurement of someone else's subject -- they agree with it by construction and
can never report it wrong, which is the coupling CONTEXT.md's rule removes.

FIVE ARMS REMOVED AND NOT ONE NEEDED PORTING. `test/selftest/340` already held
every property they asserted, with more arms in each case:

    the python arm                          what 340 already had
    the stamp writer reports failure        the same, plus a premise that the
                                            stamp really was not written
    two installations do not share a stamp  the same, plus pkglibdir keying and
                                            two unreadable pg_configs
    the report names each hashed file       the same two arms
    an empty manifest says so               the same arm
    a failed digest is unknown, not stale   the same, over TWO unreadable files,
                                            plus the premise below

340's version of the last one carries a premise the pytest twin did not state:
that the unprivileged read AGREES with the privileged one while nothing is denied.
Without it the arms measure the user switch rather than the permission denial.

THE FIRST MEASUREMENT OF 340 WAS WRONG AND NEARLY COST A DUPLICATE. Enumerating
its checks with `grep -cE '^check "'` gave 81; the real number is 89. 340 has
INDENTED `check` calls inside an `if` and a `for`, and the eight the sweep missed
are exactly the unreadable-source block. On that count one python arm looked like a
genuine gap, and a duplicate of it was written -- and proven to discriminate
against a mutation -- before the duplication was noticed. A check sweep has to be
anchored at `^[[:space:]]*`, not at column 0.

THE TELL WAS A DUPLICATE CHECK NAME, and the first response to it was wrong.
`pgc_ledger.py` reported "duplicate check name in one run, so one ledger row covers
2", and the new check was renamed to clear it. The right response to a name that
already exists is to ask WHY it exists; renaming hid the only evidence that the
work was unnecessary.

AND THE FIRST MUTATION AGAINST THAT DUPLICATE WAS A NO-OP, for a reason worth
keeping: `pgc_source_fingerprint` returns empty because the MODULE prints nothing
and exits 0, not because the wrapper's `rc != 0` branch fires. Mutating that branch
changes nothing on this path. The property lives in `test/pgc_fingerprint.py`; the
wrapper's only contribution is that it does not substitute a value for the module's
empty answer.

WHAT REMAINS is four calls in two arms, both named rather than merely counted:
`test_the_two_fingerprint_implementations_cover_the_same_inputs`, which is the one
permitted cross-reference because the property IS the relationship between the two
implementations, and a historical-parity arm whose fixture is its own. `_sh_fp_as`
is deleted with its last caller.

TESTS.md loses the rows naming the deleted arms, and three prose passages that
named them are rewritten to say where the property lives now rather than left
pointing at arms that do not exist -- a backticked name is a claim that it exists.
CONTEXT.md's inventory goes from "13 calls, seven of them debt" to "four calls,
none of them debt", and records the bad enumeration so the next reader does not
repeat it.

Verified on pg18a: selftest 757 checks, 0 FAIL; `pgc_ledger.py gate` rc=0 with
`new this run=0` and the ceiling still 250; driver-free subset 202 passed, which is
207 minus the five; the full corpus 287 passed. No selftest check changed, so the
ledger is untouched.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
@OffgridwithJD
OffgridwithJD force-pushed the audit/432-stamp-and-freshness-belong-to-the-shell branch from 4c36f95 to fbef454 Compare September 11, 2026 05:23
@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

Rebased onto main e030c151 at fbef454e. The conflict was the one predicted, and I resolved it by counting the arms rather than trusting the rebase label.

#942 landed first, so this branch's side of the test_build_refusal.py hunk is the one with the five arms absent — which is the incoming side in rebase labelling, the opposite of the earlier analysis when #948 was hypothetically first. The invariant that survives either order: take the side without the five arms, whichever label it wears.

So the resolution asserted the content before accepting it:

upstream side carries the arms   2  (asserted == 2, else abort)
this branch's side carries       0  (asserted == 0, else abort)
kept: the side with 0

And verified afterwards, because a resolution that looks right is not one that is:

the five arms, in the code          0   (must be 0)
the five arms, in TESTS.md          0   (must be 0)
_sh / _sh_fp callers                2
  test_the_two_fingerprint_implementations_cover_the_same_inputs   <- the permitted cross-reference
  test_the_fix_does_not_rebaseline_stamps_already_on_disk          <- the historical-parity arm

Both halves matter: the code and TESTS.md are resolved independently, and if they had disagreed the doc-coverage arm would have fired. It did not, which is the agreement I wanted rather than a silence I assumed.

Gate on the rebased head

pg18a (assert):

docs_style.sh         9 checks, 0 FAIL
harness_selftest.sh   763 checks run, 0 FAIL
driver-free pytest    214 passed
full pytest corpus    301 passed

docs_style is in the list deliberately this time — the rebase touched CHANGELOG.md, and that suite's subject is CHANGELOG.md. Skipping it is what cost #950 a round.

The CHANGELOG conflict was the ordinary one: main's entry and this one both want the same spot under ### Fixed, and both are kept with main's first.

Nothing about the change itself moved — only its base.

@OffgridwithJD

OffgridwithJD commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator Author

CI settled: 13 of 13 pass at fbef454e, the head carrying the rebase described above.

Recording it because the rebase resolution was the part worth gating: the conflict was settled by asserting content rather than trusting the label (upstream side carried 2 arms, this branch carried 0, kept the side with 0), and the full matrix now agrees with the local gate I ran on that tree.

@jdatcmd
jdatcmd merged commit 9dda3c9 into commandprompt:main Sep 11, 2026
13 checks passed
OffgridwithJD pushed a commit to OffgridwithJD/pgcolumnar that referenced this pull request Sep 11, 2026
`pgc_ledger.py gate` printed `ledger census: rows=N` and never compared that
number to the `checks_never_observed_red` the budget states, so it returned 0 on
a twenty-row ledger claiming five. Reporting is not enforcing.

The comparison already existed one layer out, in a selftest arm. That arm runs on
a pull request, so it reports the disagreement after the merge that creates it
rather than before. And a merge is what creates it: the census is a measurement of
the tree, so every merge invalidates it. Two PRs each re-derive it from the same
base, the merged ledger takes both sets of rows, and the budget keeps whichever
side won the conflict.

This branch then demonstrated its own premise. It was cut from a main stating 762,
and commandprompt#945 and commandprompt#948 merged underneath it; main now states 806. Rebasing it needed the
census re-derived from a run on the composed tree, which is exactly the operation
this refusal makes mechanical.

The new refusal is decidable from the two inputs alone. It needs no prior and no
`--against`, which is what lets it speak about a merge commit, where the prior is
the thing in question.

It does not make the census a ceiling. A ceiling refuses a rise, and bounding this
number deadlocks: every added check enters as `never`, so landing one would mean
raising a number the design says may only fall. What is refused is a contradiction,
in either direction.

A budget stating no census at all is reported rather than refused, because absence
is not a contradiction. That is measured rather than preferred: every other gate
fixture in both harnesses writes a budget stating only `suites_not_covered`, so
refusing there would redden about twenty arms testing something else. What holds
the committed budget to naming both numbers is a separate arm in each harness.

Red first, in both harnesses, independently implemented: ten checks in selftest
410 and one test in `test_mutation_ledger.py`. Both were run against the unfixed
tool and failed, and the core measurement was reproduced on its own: rc=0 for a
budget claiming 1 and for one claiming 3 against a ledger holding 2.

Gated on the composed tree: docs_style 9/0, harness_selftest 773/0, pytest 313,
driver-free 215, shellcheck -S error -s bash clean. The ten new ledger rows entered
as `never` with no observed red, and the census was re-derived from the run rather
than computed: 806 -> 816. Arithmetic would also have said 816 here, which is the
dangerous case, and only the run established it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
jdatcmd added a commit to OffgridwithJD/pgcolumnar that referenced this pull request Sep 11, 2026
 (commandprompt#928)

Fourth re-derivation of this artifact on a merge, and the reason is structural
rather than accidental: the census is a measurement of the tree, so every merge
that adds or removes a check invalidates it. commandprompt#948 and commandprompt#945 both landed between
this branch's last rebase and now.

    ledger after the compose   819 rows = 819 never + 0 ever-red, partitions
    census                     775 -> 819, derived from the ledger
    ceiling                    250, untouched

Confirmed by a selftest run on the composed tree rather than by the derivation
alone -- the census arm is the check, not the documentation. Tracked in commandprompt#952.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw
OffgridwithJD pushed a commit to OffgridwithJD/pgcolumnar that referenced this pull request Sep 11, 2026
`pgc_ledger.py gate` printed `ledger census: rows=N` and never compared that
number to the `checks_never_observed_red` the budget states, so it returned 0 on
a twenty-row ledger claiming five. Reporting is not enforcing.

The comparison already existed one layer out, in a selftest arm. That arm runs on
a pull request, so it reports the disagreement after the merge that creates it
rather than before. And a merge is what creates it: the census is a measurement of
the tree, so every merge invalidates it. Two PRs each re-derive it from the same
base, the merged ledger takes both sets of rows, and the budget keeps whichever
side won the conflict.

This branch demonstrated its own premise twice. Cut from a main stating 762, it has
since been re-derived across the merges of commandprompt#945, commandprompt#948 and commandprompt#943; main now states 819
and this states 829. Each time the correct operation was to regenerate both derived
files from a run, never to merge them as text or to add up the parts.

The new refusal is decidable from the two inputs alone. It needs no prior and no
`--against`, which is what lets it speak about a merge commit: the composed tree is
precisely where the prior is the thing in question, so a refusal needing a
trustworthy prior would be unavailable exactly when it is needed.

It does not make the census a ceiling. A ceiling refuses a rise, and bounding this
number deadlocks: every added check enters as `never`, so landing one would mean
raising a number the design says may only fall. What is refused is a contradiction,
in either direction.

A budget stating no census at all is reported rather than refused, because absence
is not a contradiction. That is measured rather than preferred: every other gate
fixture in both harnesses writes a budget stating only `suites_not_covered`, so
refusing there would redden about twenty arms testing something else. What holds
the committed budget to naming both numbers is a separate arm in each harness.

Red first, in both harnesses, independently implemented: ten checks in selftest
410 and one test in `test_mutation_ledger.py`. Both were run against the unfixed
tool and failed, and the core measurement was reproduced on its own: rc=0 for a
budget claiming 1 and for one claiming 3 against a ledger holding 2.

Gated on the composed tree: docs_style 9/0, harness_selftest 786/0, shellcheck
-S error -s bash clean. The ten new ledger rows entered as `never` with no observed
red, and the census was re-derived from the run rather than computed: 819 -> 829.
Arithmetic would also have said 829, which is the dangerous case rather than the
reassuring one, and only the run established it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
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