refactor(testing): name the lease-token guard; fill contract coverage#144
Merged
Conversation
Candidate #4 from the architecture review, scoped by research: the SQL-vs-Python "twins" (eligibility predicate + lease-token guard) are already co-verified across both adapters by tests/test_client_contract.py -- the co-verify pattern the review recommended already exists. So the residual is small: - Extract _lease_token_matches(row_token, claim_token) for the NULL=NULL guard, triplicated inline across delete_with_lease / mark_pending (now share it) and delete_batch (set-based; comment names the same rule). The guard's parity with the real client's SQL is pinned by the contract suite, not by mirror comments. - Contract suite: add the missing mark_pending unleased-row (NULL=NULL) scenario that the delete path already had, plus an empty-queues symmetry check. Both run against the fake and real Postgres. The lease-cutoff stays deliberately divergent (server-side make_interval vs Python timedelta) -- the clock-skew invariant -- and is not touched. Behavior-preserving. Full suite green (603 passed, coverage 100%), lint + ty clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Candidate #4 from the architecture review, scoped by research.
Research finding: the report framed the SQL-vs-Python "twins" (the fetch eligibility predicate and the lease-token guard) as "duplicated prose-in-two-languages, held in sync only by comments." On investigation,
tests/test_client_contract.pyalready co-verifies both twins across both adapters, branch by branch (unleased/future/expired/fresh eligibility; token match/mismatch/unleased guard) — which is the "co-verify, don't share" pattern the report recommended as the fix. So the residual is small.Changes
_lease_token_matches(row_token, claim_token)— the NULL=NULL guard (token is not None and row_token == token) was written inline three times.delete_with_leaseandmark_pending_with_leasenow share the helper;delete_batch_with_leasekeeps its set-based form (O(1) lookup) with a comment naming the same rule. The helper docstring notes parity is pinned by the contract suite, not by mirror comments.test_mark_pending_noop_on_unleased_row(the NULL=NULL guard on the retry path — the delete path already had its twin, mark_pending didn't) andtest_fetch_empty_queues_returns_empty. Both run against the fake and real Postgres.The lease-cutoff stays deliberately divergent (server-side
make_intervalvs Pythontimedelta— the clock-skew invariant) and is untouched. The exactly-nowboundary is intentionally not added: equal timestamps can't be manufactured deterministically across both substrates, so such a test would be flaky.Verification
just test— 603 passed (+4: two scenarios × two adapters), coverage 100.00%just lint— ruff + ty clean🤖 Generated with Claude Code