test: derive sentinel-sweep exclusions from the signature (#938)

[linuxhikerpm]

Fixes #938.

_comparisons() selected on the first two parameter names, so wrote(cur, want, name) sat outside because its first parameter is not called got. That is the right answer for a cursor, and it would also have been the answer for a comparison whose first parameter was left.

Exclusion is now a positive match on the kind of the left operand (cur, result, plan, exc, reason), not a list of method names. A new method whose first parameter is cur is excluded for the same reason wrote is. inputs == selected + excluded fails when a method matches neither rule.

Red first on the unfixed tree: 18 public methods, 10 selected, 0 excluded. After the map: 18 == 10 + 8. Dropping cur reddens the partition at inputs 18 == selected 10 + excluded 7.

No shell twin. The subject is Expect in pgc_vacuity.py. A shell part that greps or inspects that module is the coupling selftest 350 and 360 deleted; the behaviour is pinned where it can be observed, in test_failed_query_sentinel.py.

Made with Cursor

[linuxhikerpm]

pytest-guards failed at 8e52cf4c9d357bacd9988e7088c7c36c92de7d72 on test_no_helper_in_this_corpus_takes_a_parameter_it_never_reads. The Probe class in the residue arm had pass bodies, so the unused-parameter sweep saw got/want/name. Those methods now return their arguments.

Head is 0fe198e0028ed937c633374f2851ed56848ad3b3. The previous APPROVE, if any, was not on this SHA.

[OffgridwithJD]

Reviewed at 0fe198e0. Every number in the body checks out, and the acceptance test is the best thing in it. One residual to record, one latent fragility, no blockers.

This closes a gap I raised, so to be clear about interest: I have none in this one beyond wanting it right.

Your claims, re-derived rather than read

public=18  selected=10  excluded=8  residue=0        body claims 18 == 10 + 8   MATCHES
selected: at_least differ hash num ordered_rows ordering_observable row_set rowcount rows text
excluded: cannot_run outcomes plan_marker plan_node refusal run_failed sqlstate wrote

Red-first reproduced by deleting cur from the map, and it reddens both arms, not just the one you quote:

without "cur":  selected=10  excluded=7  residue=1
  partition arm:  17 == 18  -> RED
  residue arm:     1 == 0   -> RED

The acceptance test is the right shape

test_a_caller_supplied_value_not_named_got_fails_the_partition builds a throwaway Probe class with eq(left, want, name) and asserts it lands in residue. That proves the discriminating property without mutating the real Expect, and it is the arm that makes this a mechanism rather than a convention. It is also the arm a future reader needs in order to believe the partition, so thank you for writing it rather than asserting the property in a comment.

And the no-shell-twin call is right. The subject is Expect in pgc_vacuity.py; a shell part that greps or inspects that module is exactly the coupling selftest 350 and 360 were cut down to remove. Pinning it where it is observable is the rule, not an exception to it.

Residual worth recording: the map can still capture a real comparison silently

The exclusion is keyed on the first parameter's name, so a genuine comparison whose left operand happens to be named like an excluded kind is excluded without becoming residue. Verified with your own _partition:

class Probe:
    def reason_matches(self, reason, want, name): ...   -> excluded
    def plan_matches(self,   plan,   want, name): ...   -> excluded
    def eq(self,             left,   want, name): ...   -> residue

plan is a realistic name for a caller-supplied value — plan_node and plan_marker already take one — so a future plan_text(plan, want, name) would be silently out, which is the shape of #938 relocated rather than removed.

I do not think this blocks. The hole went from "every name that is not got" to "five specific names", and every unknown name now fails loudly. That is a large, correct narrowing and the direction is fail-closed. It deserves a recorded residual, the way _WRITE_TAGS carries its four escaping statement shapes.

And I checked the obvious strengthening before suggesting it, because it does not work. "First parameter is an excluded kind but second is want/floor, therefore residue" would wrongly flag two real exclusions:

sqlstate (exc, want, name)
wrote    (cur, want, name)

wrote is the very method #938 is about, so that rule would redden the case this PR exists to classify. With no annotations in this layer, I cannot see a signature-only discriminator that separates them. Recording the residual looks better than reaching for a rule that misfires.

Latent fragility, one line

_params() does list(signature(fn).parameters)[1:], dropping the first as self. inspect.getmembers(cls, isfunction) also returns staticmethods as plain functions, whose first parameter is real. I verified there are no staticmethods on Expect today, so nothing is misread now — but a staticmethod added later would have its left operand silently shifted. A guard is cheap if you want one; it is not a defect today.

Gate

test_failed_query_sentinel.py   13 passed
full corpus on your tree       315 passed   (main's 312 + your 3)
driver-free                    217 passed   (the file is declared NO_CLUSTER, and runs there)
CI                             11/13, the two suites jobs still running

CHANGELOG and TESTS.md are both present, so it carries its docs.

I will post a formal approve once the two suites jobs are green — I do not approve into a pending rollup, and I will re-check the head has not moved first.

One disclosure, because it nearly made me wrong about you

My first full-corpus run on your branch showed 10 failures in test_join_runtime_filter.py. They were not yours and not main's: my own instruction-counter experiment had installed a pre-#945 .so into the shared /usr/local/pg18a prefix, and the pytest layer's already-built path reused it, so main's tests for #945's feature were running against a binary that predates #945. Rebuilding from main took that file from 10 failed to 11 passed.

Flagging it in case you see the same thing locally: -- build: already-built from <dir> does not mean the installed .so was built from that dir, and the line reads as though it does.

[OffgridwithJD]

Approving at 0fe198e0 on full green CI (13 of 13), the same head I reviewed and ran everything against.

Summary of the review above, for the merge record:

• Every number in the body re-derived rather than read: public=18, selected=10, excluded=8, residue=0, and the red-first claim reproduced by deleting cur from the map, which reddens both the partition arm (17 == 18) and the residue arm (1 == 0).
• The acceptance test is what makes this a mechanism. test_a_caller_supplied_value_not_named_got_fails_the_partition proves the discriminating property on a throwaway Probe class rather than asserting it in a comment or mutating the real Expect.
• No shell twin, correctly. The subject is Expect in pgc_vacuity.py; a shell part inspecting that module is the coupling selftest 350 and 360 were cut down to remove.
• Gate on the tree: 13 file-local arms, 315 corpus (main's 312 plus your 3), 217 driver-free, CHANGELOG and TESTS.md both present.

Two things I flagged that are not blockers and should not hold the merge:

1. A recorded residual: the map keys on the first parameter's name, so a real comparison whose left operand is named like an excluded kind (plan_matches(plan, want, name)) is excluded without becoming residue. The hole narrowed from "every name that is not got" to five specific names, and unknown names now fail loudly, so the direction is fail-closed. I checked the obvious strengthening and it misfires on sqlstate(exc, want, name) and wrote(cur, want, name) — wrote being the method #938 is about — so recording it beats reaching for that rule.
2. _params() drops the first parameter as self, and getmembers(cls, isfunction) returns staticmethods as plain functions. There are none on Expect today, verified, so nothing is misread now.

The exclusion is a property of the signature rather than a list of method names, a method matching neither rule goes red instead of into a third bucket, and the case that motivated #938 is now classified for its actual reason. That is the shape the issue asked for.