fix(refractor): mask variant hash to 31 bits to fit Postgres INTEGER #217
No reviewers
Labels
No Label
ai-changes-requested
ai-failed
ai-merged
ai-pr-opened
ai-reviewed
ai-reviewing
ai-working
autonomous
bug
enhancement
evolution
performance
phase-0
phase-1a
phase-1b
phase-1c
phase-1d
security
size:M
size:S
tech-debt
todo
type:feature
type:stability
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: cal/paper-dynasty-database#217
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "fix/variant-hash-int32-overflow"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Problem
compute_variant_hashtakes the first 8 hex chars of a SHA-256 digestand casts to int, producing values up to 2^32 - 1. The variant columns
on
Card,BattingCard,PitchingCard, andRefractorCardStatearePeewee
IntegerField→ PostgresINTEGER(signed 32-bit, max 2^31 - 1).Roughly 50% of all player-tier combinations fall in the range
[2^31, 2^32 - 1] and crash the tier-up write:
Discovery
Surfaced during
/dev refractor-test card_id:64460(Charles Nagy,player_id=10795). His tier-1 hash is 2874960417 — above the int32
ceiling. The outer exception handler in
routers_v2/refractor.py(
evaluate_game) caught theDataErrorand logged a warning, sothe dev harness reported
⚠️ No tier-up detectedwhileapply_tier_boostwas actually failing mid-write. Silently droppedtier-ups in production would look like "some cards just never
tier up, others do."
Fix
One-line mask in
compute_variant_hash:Drops 1 bit of entropy (~2.1B distinct values remain, still
astronomically collision-safe) and guarantees every return value
fits in Postgres INTEGER.
Backwards compatibility
All 9 existing
refractor_boost_auditrows and 9 persistednon-zero variants have hashes ≤ 1.88B — their high bit was
already 0. Masking leaves those values unchanged. No migration
required.
Test
New regression test
test_fits_postgres_int32covers 10,000player IDs × 5 tiers = 50,000 combinations, all asserted
≤ 2,147,483,647. Full
test_refractor_boost.py+test_refractor_boost_integration.pysuites pass (72/72).PR #217 Review — fix(refractor): mask variant hash to 31 bits to fit Postgres INTEGER
Self-review restriction applies — posting as COMMENT. No blocking issues. Ready to merge.
Fix correctness ✅
0x7FFFFFFF= 2^31 − 1 = 2,147,483,647. Masking the raw 32-bit SHA-256 slice to 31 bits zeroes the sign bit, so every result is a non-negative value in [0, 2^31 − 1] — exactly the PostgresINTEGERrange. The existing zero-remap (return result if result != 0 else 1) runs after the mask, so the reserved-zero contract is preserved. Arithmetic is correct.Model coverage ✅
Confirmed in
db_engine.py— every column that receives a value fromcompute_variant_hashisIntegerField(PostgresINTEGER):CardvariantIntegerField(null=True)BattingCardvariantIntegerField()PitchingCardvariantIntegerField()RefractorCardStatevariantIntegerField(null=True)RefractorBoostAuditvariant_createdIntegerField()All five are covered by the single-function fix.
CardPosition.variant(alsoIntegerField) is not written to byapply_tier_boost, so it is out of scope and unaffected.Regression test ✅
test_fits_postgres_int32generates 10,000 player IDs × 5 tiers = 50,000 combinations and asserts every result ≤ 2,147,483,647. Coverage is adequate. The known reproducer (player_id=10795, Charles Nagy) falls outside therange(10000)loop, but the mask is unconditional — if 0–9999 all pass, 10795 passes too. Would be a minor nicety to addcompute_variant_hash(10795, 1)as a concrete named example inside the test (documents the original incident), but it is not required.Backwards compatibility ✅
PR body is correct: any existing hash ≤ 1.88B has its high bit already clear.
value & 0x7FFFFFFF == valuefor all such values. No migration required, no data drift.Docstring ✅
Return range updated to
[1, 2^31 - 1]— accurate after the mask.Merge status
mergeable: trueNo blocking issues.
PR #217 Review — fix(refractor): mask variant hash to 31 bits to fit Postgres INTEGER
Self-review restriction applies — posting as COMMENT. No blocking issues. Ready to merge.
Fix correctness
0x7FFFFFFF= 2^31 - 1 = 2,147,483,647. Masking the raw 32-bit SHA-256 slice to 31 bits zeroes the sign bit, so every result is a non-negative value in [0, 2^31 - 1] — exactly the Postgres INTEGER range. The existing zero-remap runs after the mask, so the reserved-zero contract is preserved.Model coverage
Confirmed in db_engine.py — every column that receives a value from
compute_variant_hashis IntegerField:CardPosition.variant is also IntegerField but is not written to by
apply_tier_boost— out of scope.Regression test
test_fits_postgres_int32covers 50,000 combinations (10,000 player IDs x 5 tiers). The known reproducer (player_id=10795) falls outsiderange(10000), but the mask is unconditional — if 0-9999 all pass, 10795 passes too. Minor nicety: could addcompute_variant_hash(10795, 1)as a named concrete example, but not required.Backwards compatibility
Any existing hash <= 1.88B has high bit already clear — masking is a no-op for those values. No migration required.
Docstring
Return range updated to [1, 2^31 - 1] — accurate after the mask.
mergeable: true— no blocking issues.AI Code Review
Files Reviewed
app/services/refractor_boost.py(modified)tests/test_refractor_boost.py(modified)Findings
Correctness
int(raw[:8], 16) & 0x7FFFFFFFclears bit 31, capping the result at 2,147,483,647 — exactly the PostgresINTEGERceiling. The zero-guard (return result if result != 0 else 1) still functions correctly after masking since 0 remains reachable (e.g. when the 8-char hex prefix is00000000) and is properly remapped to 1.[1, 2^32 - 1]to[1, 2^31 - 1]— correct.Security
Style & Conventions
Suggestions
test_fits_postgres_int32runs 50,000 pure-Python iterations — fast in practice, but could be marked with a size marker or reduced to a smaller but still statistically representative sample (e.g. 1,000 × 5) if CI time becomes a concern. Non-blocking.Verdict: COMMENT
Self-review restriction applies (author == cal). No correctness, security, or convention issues found. The one-line fix is correct, well-commented, and backed by a solid regression test. Ready to merge.
Automated review by Claude PR Reviewer