Pool printing-consensus votes across md5 identity groups - #482
Merged
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…gents Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… pins for md5 group pooling Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Author
|
OWNER RATIFICATION (2026-07-25, via session): docs/theory.md §4 item 3 as of commit 59eff92 is read and approved by the owner as written. The STATUS-line update recording this review lands with the guard-removal rebase commit (post-#484-merge), not as its own round. Merge order unchanged: #484 first, then guard-removal rebase here, then this PR's merge poll. |
…d) into md5-group-pooling
…rs it Per issue #473's own coordination note ("PR-3 removes PR-2's interim Stage D guard"): the join-key/fallback calculators no longer exclude transferred-evidence cards outright, since pool_group_votes now dedupes a transferred row's vote against its source at the group tally level instead. TRANSFERRED_INTERIM_GUARD_SKIP_REASON stays defined for historical CardScanLog rows only. Also fixes three comments (models.py, evidence_transfer.py, image_evidence.py) that mislocated the guard at _eligible_cards_queryset - it lived in the two calculators' own loop bodies. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 25, 2026
Closed
18 tasks
WilfordGrimley
added a commit
that referenced
this pull request
Aug 4, 2026
…ist/tag exclusions (#686) * fix(question_feed): phase-C not-official-art routing + md5-expand artist/tag exclusions Two independent routing gaps in the question feed (MPCAutofill/cardpicker/question_feed.py), diagnosed against reason_tags.py's WTC phase B partition and issue #473's md5 identity groups: 1. reason_tags.py: promotes the WTC phase B "not-official-printing" vs. "not-official-art" partition from prose in the module docstring into two real, checked frozensets (NOT_OFFICIAL_PRINTING_REASON_TAGS / NOT_OFFICIAL_ART_REASON_TAGS), mirroring the frontend's NoMatchReasonStrip.tsx NO_MATCH_REASON_TAG_GROUPS. test_reason_tags.py asserts they are exhaustive and disjoint over NO_MATCH_REASON_TAGS. 2. question_feed.py: phase C routing. A card carrying a positive (VotePolarity.APPLY) CardTagVote for a not-official-art tag has had its artwork question declared unanswerable by a human, so the feed now excludes it from artist-shaped questions in both _tier_2_contested and _tier_4_fresh (_not_official_art_card_ids, md5-group-widened, computed once per feed request in get_next_question_feed_item). The printing question is unaffected. Judgment call: the exclusion requires a HUMAN-BACKED source (vote_consensus. is_human_backed_source), not merely "any positive vote". Reason tags are cast by a human through NoMatchReasonStrip today, but nothing in the schema stops a future machine caster from writing one, and this routing signal is meant to represent an actual human declaration that the artwork question is meaningless for the card - a machine-cast source earning the same trust would need its own explicit decision, not a silent inclusion via this change. 3. question_feed.py: _tier_2_contested's artist and tag own-vote exclusions are now md5-group- widened (_voter_answered_artist_card_ids / _voter_answered_tag_card_ids_by_tag), so a voter who answered one member of a byte-identical group is not re-asked the identical artist/tag question under a sibling's identifier - the same convention _voter_answered_printing_card_ids already established for the printing tiers. The tag widening is scoped to the CARD axis only (never the tag axis), preserving _tier_2_contested's existing per-tag granularity, and costs one query total (not one per review pair) plus one md5 expansion per distinct tag the voter has touched. Scoped to _tier_2_contested only, per the engineering brief - _tier_4_fresh's own artist/tag own-vote exclusions keep their pre-existing, unwidened form. Observation (out of scope, no implementation): local_illustration. _eligible_illustration_cards_queryset (the machine illustration calculator) has no awareness of NOT_OFFICIAL_ART_REASON_TAGS either, and plausibly warrants the same exclusion for the same reason phase C does - left as a follow-up. New tests: test_reason_tags.py (partition exhaustiveness/disjointness), test_question_feed.py (TestPhaseCNotOfficialArtRouting - positive not-official-art excludes, not-official-printing doesn't, negative vote doesn't, machine-cast vote doesn't), test_md5_group_pooling.py (TestPhaseCAndTierTwoMd5Expansion - artist/tag md5 widening, sibling non-re-serving, the per-tag-axis regression the existing _tier_2_contested comment warns against, and the not-official-art group-wide exclusion; plus a computed-once-per-feed-request pin for all three new exclusions, extending the existing 2026-07-25 PR #482 condition f1 test). * fix(ci): scope constant-rename gate to base-minus-head, not symmetric diff scope_modules() computed the affected-name set as a symmetric difference between base and head declarations, so a constant ADDED at head with nothing removed anywhere also pulled its module into the whole-module equivalence comparison. Since base never declared the new name, that module can never normalise identically to base - any PR merely adding a constant matching the pattern (SKIP_REASON|ANONYMOUS_ID|_VERSION|_WEIGHT| _THRESHOLD|_PREFIX|_REASON) failed this gate permanently. PR #686's own two new reason_tags.py constants tripped exactly this. Fix: use base - head (names that disappeared from base) instead of the symmetric difference, matching the function's own docstring ('modules that touch a constant whose DECLARED NAME changed'). Renames and removals are unaffected - the old name is still base-only either way, so the same module still gets pulled in and still gets the same whole-module comparison it got before. The removed/head-only reporting split at the print site is dead in its 'head only' branch under the new scope (moved is now always base-only), so it's replaced with a plain removed-name list plus a separate, purely-informational head-only listing an operator can use to see both halves of a rename without it affecting scope. One real tradeoff, surfaced and pinned rather than hidden: an ISOLATED literal-to-constant extraction (no companion rename/removal in the same module) is no longer auto-checked for a wrong extracted value, because it is provably indistinguishable, by name-diffing alone, from a harmless new constant backing brand-new logic - the exact shape that caused PR #686's false positive. Verified against real history (PR #567's LANDS_PHASH_SKIP_REASON_PREFIX) that this capability was only ever exercised when a companion rename existed in the same module, which remains fully covered; the isolated case is now pinned as a documented limitation in test_an_isolated_extraction_with_a_wrong_value_is_a_known_scope_blind_spot rather than silently regressing. Adds three tests distinguishing the fix (pure addition -> nothing to prove, rename -> still scoped and still catches a real diff, removal -> still scoped) plus updates to the extraction-folding tests above. Verified: gate script exits 0 against this PR's actual base/head (was 4); test_constant_rename_equivalence.py alone (49 passed) and the full .github/scripts/tests/ suite together (214 passed, no order-dependence per issue #679); py_compile and pre-commit both clean.
This was referenced Aug 6, 2026
WilfordGrimley
added a commit
that referenced
this pull request
Aug 6, 2026
…eed request (#729) get_contested_card_ids() and get_contested_artist_card_ids() were each called separately by every tier that consulted them (_tier_2_contested, _tier_4_fresh), recomputing an identical answer within one get_next_question_feed_item() call - the same duplication shape #482 already fixed for the voter-exclusion sets. Both are now resolved once, right before the tier-2 fallthrough (so a request that resolves via the likely-resolve pool or tier 1 never pays this cost), and threaded through as optional parameters, matching the existing convention for answered_card_ids et al. The tiers keep the parameter optional (default None, computed lazily if absent) so a direct caller - a test or a shell - can still invoke one tier by anonymous_id alone.
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
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.
Description
Issue #473, PR-3 of 3 — group-level vote pooling for md5 identity groups (PROTECTED CORE)
plus the
docs/theory.mdindependence amendment that ruling required to ship in the same PR.No
Closeskeyword: #473 stays open until PR-1 and PR-2 land and the Bug-A tail drain resumes.A set of
Cards sharing a non-nullCard.md5_checksumindexes the same image file and isone identification target. This makes printing consensus tally that target once, instead of
once per catalog row.
Gate status: NO-GO verdict of 2026-07-25 addressed in
ec18ecd8— see "Gate conditions"below for the per-condition mapping. A delta re-gate is expected before merge.
What it does
build_group_printing_vote_tupleskeys everyvote in a multi-member group on its caster's
anonymous_id— human-backed included — andvote_consensus.pool_group_votesapplies two rules: one agent's agreeing votes acrossmembers collapse to one, and an agent whose votes across members disagree is withheld from
the tally entirely (it has contradicted itself about byte-identical bytes, so it is evidence
for neither side — the same withhold-never-manufacture logic
g₄'s cross-checks use).Distinct agents still sum: two different people voting on two members are two votes, which is
the intended multiplier of treating the set as one question.
resolve_weighted_consensusunchanged — implicit cap, D1 no-machine-tipping, D4 machine-dissent-never-de-resolves, the
human-backed gate, and the deductive-backfill zero-weight override all run exactly as before,
with no group-awareness of their own. Human dissent inside a group is one visible contest on
one target, decided by that same matrix (ruling 2).
resolve_and_persist_printingwritesinferred_canonical_card+printing_tag_statusto every member (in pk order, so concurrent votes on two membersqueue rather than deadlock), reindexing only the members whose effective indexed printing
actually changed.
path is not merely equivalent, it is the same code:
group_printing_votesreturnscard.printing_tags.all()(a caller'sprefetch_relatedstill honoured, zero extra queries),no vote is keyed,
pool_group_votesis never called, and persistence performs the one writeit always did through the caller's own instance. 841 pre-existing tests pass UNMODIFIED.
consensus_recomputeiterates groups once (never members N times) while still countingand reporting a transition for every member;
question_feed.is_likely_resolve_printingevaluates the group tally, and every printing tier excludes the full identity group of
every card the voter has already answered, so the feed serves at most one member per group.
docs/theory.md§4 gains item 3 (the mechanism list goes from two structural properties tothree), stating three claims separated by strength: the machine-alone bound is preserved
exactly (
g₅untouched, pooling cannot manufacture a human-backed vote); quorum'sindependence assumption is restored, not newly guaranteed (per-agent dedupe is what makes
min_weighta count of distinct agents once one person can answer n siblings and one machineobservation can be transferred to n siblings by PR-2); and it explicitly does not claim
that pooling reaches no new resolutions — it reaches some, by design.
Coordination (two facts a reviewer/merger needs)
md5-checksum-substrate) forCard.md5_checksum. This branch is cut frommasterbefore that field exists and adds no migration. Every read of the column funnelsthrough exactly three functions (
_card_md5_checksum— agetattrwith aNonedefault;_md5_checksums_for_card_idsand_card_ids_with_md5_checksums— both guarded, and the formershort-circuits to zero queries while the column is absent), so on today's
masterschema everycard is a group of one and every path here degenerates, provably, to its pre-md5 identity groups: checksum substrate, evidence transfer, and group-level vote pooling #473 behavior. The
branch is therefore mergeable in either order and inert until PR-1 populates the column. Merge
order should still be PR-1 → PR-2 → PR-3.
a guard excluding transferred-evidence cards from Stage D machine voting for the window between
its deploy and this one. It does not exist on the branch point, so there is nothing to delete in
this diff. Removing that guard is a separate, post-PR-2-merge rebase commit on this branch,
orchestrator-coordinated — this PR must not be merged as "the guard is gone" without it.
Gate conditions (2026-07-25 NO-GO verdict →
ec18ecd8)printing_consensusseam:dedupe_key=vote.anonymous_id if pool else None, for every voterather than for non-human-backed votes only.
anonymous_ididentifies the VOTER, so theoriginal form let ONE person reach a 2.0 quorum by answering the same image under two of its
identifiers — a resolution neither card could reach alone. The gate's scenario A now returns
Nonewhere it returnedX; shipped asTestGroupTally::test_one_human_answering_two_siblings_is_one_vote, withtest_a_third_distinct_human_still_resolves_that_grouppinning that dedupe does not make agroup unresolvable, only un-inflatable.
disagree now contributes nothing for that group; the keep-the-max/first-on-tie collapse is
gone. The gate's scenario B returns
Noneat every k (it returnedXfor k=2..4). Shipped astest_a_self_contradicting_machine_agent_contributes_nothing(the k=2 case) andtest_self_contradiction_handling_is_independent_of_card_id_order+TestPoolGroupVotes::test_pooling_is_order_independent— the explicit order-independence pair,since the defect was that equal weights resolved the contradiction by input order, i.e. by
card_id, manufacturing correlated agreement.docs/theory.md§4 item 3 rewrittenper the split above; the false "pooling only removes weight, so no new resolution is reachable"
sentence is replaced and explicitly retracted in place rather than quietly dropped. "A class
of inconsistency unreachable by construction" → consistency is a write-path property
(members cannot diverge while the shared path is the only writer; a MEMBERSHIP change needs a
consensus_recomputepass).docs/features/printing-tags.md's bullet gets the samecorrections: the "real, independent people" phrasing is replaced by what the code now enforces
(per-agent dedupe, human included; withhold on self-contradiction; distinct agents sum).
_voter_answered_printing_card_idsis now resolved ONCE per feedrequest in
get_next_question_feed_itemand threaded to every printing tier (each tier keeps anoptional parameter, so a direct/test caller can still pass an
anonymous_idalone).md5_group_expanded_card_idsreads the checksum column with.values_listinstead of hydratinga
Cardinstance per previously-voted card, and short-circuits to zero queries while thecolumn doesn't exist.
_likely_resolve_printing_card's docstring nowstates the post-PR-1 per-scanned-card group-membership query, why it is accepted (it rides the
same bounded stop-at-first-match scan the docstring already accepts as a v1 cost, and it buys
the pooling that stops the pool serving n copies of one question), and what the fix would be
if it ever profiles hot (the materialized likely-resolve index already deferred there, not
un-grouping the tally).
consensus_impact_report(f4, follow-up), and everything else marked non-blocking.Checklist
pre-commitand installed the hooks withpre-commit installbefore creating any commits.cardpicker/tests/test_md5_group_pooling.py— 35 tests: purepool_group_votesproperties (agreement collapse, max-weight, withhold-on-self-contradiction, withholding
scoped to the offending agent, order-independence, "never increases total weight"); singleton
degeneration (group of one, no pooling key, prefetch honoured, zero extra queries); machine
dedupe (3 siblings' OCR votes from one agent = one 0.5 event, and the same shape denying a
lone human a fabricated quorum); the two gate scenarios above; human sum across distinct
agents; group-level contest; machine dissent cannot tip a group contest; zero machine-only
group resolutions (4 distinct agents, 2.0 of machine weight, still
None); propagation andde-propagation to every member; per-member reindex gating; recompute visits a 3-member group
once while reporting all 3, and is idempotent; likely-resolve on the group tally; feed
serves one member per group.
keep-first-on-tie assertion is now a withhold assertion). No pre-existing test was modified.
coordination item 1), because the column ships in PR-1 — every other line under test is the
real one. These keep passing unchanged after PR-1 merges and can then be supplemented with
column-backed equivalents.
pytest cardpicker/tests/test_md5_group_pooling.py→ 35 passed.test_printing_consensus test_vote_consensus test_question_feed test_consensus_recompute test_consensus_impact_report test_implicit_votes→ 228 passed (incl. the 35 above);
test_vote_queue_views test_tag_votes test_moderation_views test_purge_machine_votes test_deductive_backfill test_retract_stage_d_by_run_id test_card_serialise test_search_functions test_views test_review_cluster_views test_local_identify_printing_tags test_reparse_collector_evidence→ 648 passed (116 snapshots). 841 pre-existing tests green, unmodified. Not a full-suite
run — this box shares testcontainer ports with concurrent worktree sessions
(
docs/troubleshooting.md); CI is the full-suite signal.vote_consensus.pywith theshipped construction line: scenario A
None(wasX), scenario BNoneat k=2/3/4/5 (wasXat k=2/3/4) — i.e. shipped behavior now equals option (iii) at every k.pre-commit run --files ...(ruff / isort / black / mypy / prettier) → all Passed..github/scripts/docs_lint.py→ clean.docs/theory.md§4 — item 3, rewritten per gate condition 3 (edited in place, no datedsection appended).
docs/features/printing-tags.md—md5-identity-group-poolingbullet, same corrections.docs/identification-pipeline.md— checked, deliberately unchanged: itsg5prose("A match casts one machine CardPrintingTag vote… Resolution requires total weight ≥ 2
including at least one human-backed vote — structurally, the machine can never resolve a card
alone") never states the tally SCOPE, only the resolution requirement, and every sentence in it
stays true verbatim under group pooling.
pool_group_voteslives inside the vote system itself and the group helpers depend on the fork's
Cardmodeland printing consensus, so neither is a dependency-free primitive.
Open items (merge blockers first)
ec18ecd8against the four conditions above, and the owner reviews thetheory.md text personally. Neither has happened yet.
merges. Do not merge this PR as-is if PR-2 is already deployed.
gate specified option (iii) for "an agent", and scoping it to machines would have left a human
who votes two different ways across siblings resolved by input order, the exact defect
condition 2 closes. It can only remove weight. Flagged because the gate's wording discussed it
under the machine heading.
get_contested_card_ids(a cheapqueue-ordering proxy — a group-level-only contest won't surface in the contested tier),
get_vote_tally(display),get_remaining_estimate's counts (a many-member group counts onceper member), and
consensus_impact_report's per-card iteration (gate item f4, follow-up).None affect resolution.
questions in the feed; resolving one member resolves its siblings) cannot occur until PR-1
populates checksums — worth revisiting as a merge-time item after PR-1 deploys, not now.
Round: delta-gate wording + test pins (2026-07-25, commit 59eff92)
Addressed the 2026-07-25 delta-gate follow-up conditions on top of
ec18ecd8:docs/theory.md§4 item 3: "Human disagreement inside a group is notspecial-cased" now reads "Human disagreement BETWEEN DISTINCT PEOPLE inside
a group is not special-cased", and the withholding-rule sentence now
explicitly says "any agent, human included, whose votes across the group
disagree with each other contributes nothing."
min_weighta count of distinct agents again" is now "a sum overdistinct agents rather than over rows" in both
docs/theory.mdandvote_consensus.pool_group_votes's docstring (a lone admin at weight 5.0still clears the sum - the old phrasing implied otherwise).
docs/features/printing-tags.md: "serves at most one member of a group pervoter" is now "never serves a second member of a group a voter has already
answered."
cardpicker/tests/test_md5_group_pooling.pygained:test_a_self_contradicting_human_alone_does_not_resolve_the_group: a DB-level HUMAN (
source=user) self-contradiction across two md5 siblings iswithheld, the group does not resolve on it alone.
test_self_contradicting_human_withheld_not_latest_wins(issue Unify one-agent contradiction policy: latest-wins vs both-count vs withhold across per-card and group tallies #483): h1votes X on sibling1 then Y on sibling2 (withheld), h2 votes Y once - group
result is
None, pinning withhold-entirely over latest-wins semantics.TestGroupExpansionQueryCostWithoutTheChecksumColumn: the REAL (non-monkeypatched) group-expansion helpers issue zero queries while
Card.md5_checksumdoesn't exist on this branch.TestAnsweredSetComputedOncePerFeedRequest:_voter_answered_printing_card_idsis called exactly once per
get_next_question_feed_iteminvocation.Verification:
test_md5_group_pooling.py(49 tests) plustest_vote_consensus.py/test_printing_consensus.py/test_question_feed.py/test_consensus_recompute.py/test_consensus_impact_report.py(209 total)all pass against
/tmp/mpcvenv.pre-commit(ruff/isort/black/mypy/prettier)and
docs_lint.pyboth clean. Zero code-semantics changes - docs/docstringwording and new tests only.
Round: merge master (PR #484) + retire PR-2's interim Stage D guard (2026-07-25, commit 6981a59)
origin/master(a plain merge commite488eb92, no rebase, no force-push) —picked up PR Add Stage C evidence transfer, decoupled fetch-ahead, and echo suppression #484 (Stage C evidence transfer + decoupled fetch-ahead + echo suppression),
which itself carries issue md5 identity groups: checksum substrate, evidence transfer, and group-level vote pooling #473 PR-2's
TRANSFERRED_INTERIM_GUARD_SKIP_REASONinterim guardand its own coordination note: "Removal is PR-3's own business." The merge was clean - no
conflicts, since master's changes (evidence_transfer.py, stage_e_dispatch.py, etc.) and this
branch's group-pooling code are disjoint, and
docs/theory.mdwas not touched by master.run_join_key_calculator/run_fallback_calculatorno longerexclude a card whose CURRENT evidence is
transferred=True- both calculators now cast avote on it exactly as they would for a real extraction.
TRANSFERRED_INTERIM_GUARD_SKIP_REASONstays defined and stays a member of both
RESCANNABLE_SKIP_REASONSsets, purely so ahistorical
CardScanLogrow from a pre-PR-3 run still reads sensibly - no code path writes anew one.
_eligible_cards_queryset(Tron's LOW)models.py(ImageEvidence.transferredfield docstring),evidence_transfer.py(
transfer_evidencedocstring), andimage_evidence.py(persist_evidencedocstring). Alsoupdated
docs/features/stage-e-operations.md's own "Interim Stage D guard" section (itadditionally had a pre-existing "excluded from all three calculators" / "slow-path NOT
guarded" self-contradiction, fixed in the same edit) and added one
docs/theory.mdSTATUSline recording §4 item 3 owner-reviewed-and-approved 2026-07-25.
test_local_calculate_verdicts.py::TestTransferredEvidenceIsEligible(renamed fromTestTransferredInterimGuard) - the mirror-image of the old skip-assertions: a transferredcard now casts a vote in both machine-voting calculators, identically to a real-extraction
card; the slow-path calculator's behavior is unchanged (it was never guarded either way).
test_md5_group_pooling.py::TestTransferredEvidencePoolsWithItsSource(new) - runs the REALrun_join_key_calculatoragainst an md5-sibling pair (one real extraction, one transferredcopy of it) and pins the actual mechanism that makes the guard's removal safe: both votes
are cast under the SAME calculator's fixed
anonymous_id, sopool_group_votescollapsesthem to ONE event rather than manufacturing a fabricated second confirmation - and a second,
contrast test confirms a genuinely independent second agent's vote can still resolve a group
containing a transferred member (pooling isn't dead weight, just correct dedup).
Card.md5_checksumisnow a REAL column on this branch (via PR Add Card.md5_checksum substrate and a listings-rewalk backfill command #477,
md5-checksum-substrate, merged to masterand picked up in this round's merge) -
TestGroupExpansionQueryCostWithoutTheChecksumColumnpreviously asserted ZERO queries for
md5_group_expanded_card_idson that premise; nowcorrectly asserts the real bound (at most ONE query, never the second lookup) that actually
holds once the column exists but carries no checksums yet.
Verification:
test_md5_group_pooling.py(42 tests) +test_local_calculate_verdicts.py+test_evidence_transfer.py+test_image_evidence.py+test_vote_consensus.py+test_printing_consensus.py+test_question_feed.py+test_consensus_recompute.py+test_consensus_impact_report.py+test_stage_e_dispatch.py+test_stage_e_signals.py+test_run_image_evidence_cohort.py(552 total) all pass against/tmp/mpcvenv.pre-commit(ruff/isort/black/mypy/prettier) and
docs_lint.pyboth clean. No model/migration changes -every edit here is calculator logic, comments, docs, and tests.