cal/paper-dynasty-database
1
0
Fork 0
Code Issues 7 Pull Requests 4 Actions Packages Projects Releases Wiki Activity

feat: Refractor Phase 2 foundation — boost functions, schema, tests #176

Merged
cal merged 3 commits from feature/refractor-phase2-foundation into main 2026-03-30 16:11:08 +00:00
Conversation 0 Commits 3 Files Changed 5 +1299 -1
cal commented 2026-03-29 18:39:59 +00:00
Owner
Copy Link

Summary

  • app/services/refractor_boost.py (new, 315 lines) — Pure functions for computing boosted card ratings at tier-up: apply_batter_boost (fixed +0.5 to 4 offensive columns), apply_pitcher_boost (1.5 TB-budget priority algorithm), compute_variant_hash (deterministic variant ID). All use Decimal arithmetic internally and preserve the 108-sum invariant.
  • app/db_engine.py — Add RefractorBoostAudit model (audit trail for tier boosts), Card.variant, BattingCard.image_url, PitchingCard.image_url, RefractorCardState.variant fields.
  • tests/conftest.py — Register RefractorBoostAudit + FK parents in test model list.
  • migrations/2026-03-28_refractor_phase2_boost.sql — Additive DDL: refractor_card_state.variant column + refractor_boost_audit table (JSONB, UNIQUE constraint, transactional).
  • tests/test_refractor_boost.py (new, 26 tests) — Comprehensive coverage: 108-sum invariant, correct deltas, truncation handling, TB accounting, determinism, x-check protection, variant hash behavior, and edge cases.

Review notes

  • 4 PO reviews (pd-database agent) all returned PRODUCTION-READY
  • Should-fix items addressed: on_delete='CASCADE', applied_at default, BEGIN/COMMIT wrapper, TextField/JSONB comment
  • Nice-to-haves addressed: quantize remainder logic with zero-clamp, redundant index removed, 3 edge case tests added
  • All 26 tests passing

Test plan

  • python -m pytest tests/test_refractor_boost.py -v — 26/26 passed
  • Run full test suite (python -m pytest) to verify no regressions
  • Run migration on dev (ssh pd-database)
  • Verify ORM model loads correctly with new fields

🤖 Generated with Claude Code

## Summary - **`app/services/refractor_boost.py`** (new, 315 lines) — Pure functions for computing boosted card ratings at tier-up: `apply_batter_boost` (fixed +0.5 to 4 offensive columns), `apply_pitcher_boost` (1.5 TB-budget priority algorithm), `compute_variant_hash` (deterministic variant ID). All use Decimal arithmetic internally and preserve the 108-sum invariant. - **`app/db_engine.py`** — Add `RefractorBoostAudit` model (audit trail for tier boosts), `Card.variant`, `BattingCard.image_url`, `PitchingCard.image_url`, `RefractorCardState.variant` fields. - **`tests/conftest.py`** — Register `RefractorBoostAudit` + FK parents in test model list. - **`migrations/2026-03-28_refractor_phase2_boost.sql`** — Additive DDL: `refractor_card_state.variant` column + `refractor_boost_audit` table (JSONB, UNIQUE constraint, transactional). - **`tests/test_refractor_boost.py`** (new, 26 tests) — Comprehensive coverage: 108-sum invariant, correct deltas, truncation handling, TB accounting, determinism, x-check protection, variant hash behavior, and edge cases. ## Review notes - 4 PO reviews (pd-database agent) all returned **PRODUCTION-READY** - Should-fix items addressed: `on_delete='CASCADE'`, `applied_at` default, `BEGIN/COMMIT` wrapper, TextField/JSONB comment - Nice-to-haves addressed: quantize remainder logic with zero-clamp, redundant index removed, 3 edge case tests added - All 26 tests passing ## Test plan - [x] `python -m pytest tests/test_refractor_boost.py -v` — 26/26 passed - [ ] Run full test suite (`python -m pytest`) to verify no regressions - [ ] Run migration on dev (`ssh pd-database`) - [ ] Verify ORM model loads correctly with new fields 🤖 Generated with [Claude Code](https://claude.com/claude-code)
cal added 1 commit 2026-03-29 18:40:01 +00:00 
Pure functions for computing boosted card ratings when a player
reaches a new Refractor tier. Batter boost applies fixed +0.5 to
four offensive columns per tier; pitcher boost uses a 1.5 TB-budget
priority algorithm. Both preserve the 108-sum invariant.

- Create refractor_boost.py with apply_batter_boost, apply_pitcher_boost,
  and compute_variant_hash (Decimal arithmetic, zero-floor truncation)
- Add RefractorBoostAudit model, Card.variant, BattingCard/PitchingCard
  image_url, RefractorCardState.variant fields to db_engine.py
- Add migration SQL for refractor_card_state.variant column and
  refractor_boost_audit table (JSONB, UNIQUE constraint, transactional)
- 26 unit tests covering 108-sum invariant, deltas, truncation, TB
  accounting, determinism, x-check protection, and variant hash behavior

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Claude added the
ai-reviewing
 label 2026-03-29 18:45:47 +00:00
Claude reviewed 2026-03-29 18:49:41 +00:00
Claude left a comment
Collaborator
Copy Link

AI Code Review

Files Reviewed

  • app/services/refractor_boost.py (added, 315 lines)
  • app/db_engine.py (modified — 4 new ORM fields + RefractorBoostAudit model)
  • migrations/2026-03-28_refractor_phase2_boost.sql (added)
  • tests/test_refractor_boost.py (added, 26 tests)
  • tests/conftest.py (modified)

Findings

Correctness

  • Batter boost invariant: Positive deltas (+2.0 total) exactly equal negative deltas (-2.0 total), so the 108-sum is preserved by construction. The truncation path is handled correctly: last_delta = total_actually_reduced - running_sum with a zero-floor ensures the sum of positive applications equals what was actually taken from negative columns. ✓
  • Pitcher boost invariant: The algorithm moves budget from outcome columns into strikeout; net column-sum change is zero, so the 79-sum is preserved by construction. ✓
  • compute_variant_hash zero-remap: variant=0 is reserved and correctly remapped to 1. ✓
  • RefractorBoostAudit FK ordering in _TEST_MODELS: BattingCard and PitchingCard are inserted above RefractorBoostAudit and below their own FK parent (Player). Dependency order is valid. ✓
  • Migration completeness: battingcard.image_url, pitchingcard.image_url, and card.variant were already added to the DB by 2026-03-17_add_evolution_tables.sql. Phase 2 migration correctly adds only the new refractor_card_state.variant column and refractor_boost_audit table. No missing migrations. ✓
  • applied_at = DateTimeField(default=datetime.now): Callable reference (not a called value) — correct. ✓
  • UNIQUE(card_state_id, tier): Prevents double-application of a boost at the same tier on retry. Correct safety guard. ✓

Security

No issues found. Pure functions with no I/O, no SQL construction, no user-controlled input paths. Migration uses parameterless DDL.

Style & Conventions

Naming inconsistency — evolution_tier parameter: The compute_variant_hash function uses evolution_tier as the parameter name, but the entire system was renamed from "Evolution" to "Refractor" in PR #131. New code in this PR should use refractor_tier (or just tier) to stay consistent with the established convention. The docstring compounds the issue by saying evolution_tier: Refractor tier (0–4) — it uses both names simultaneously.

# Current (inconsistent)
def compute_variant_hash(player_id: int, evolution_tier: int, ...) -> int:

# Should be
def compute_variant_hash(player_id: int, refractor_tier: int, ...) -> int:

boost_delta_json multi-line formatting: The parentheses around TextField() are redundant and non-idiomatic:

# Current
boost_delta_json = (
    TextField()
)  # JSONB in PostgreSQL; TextField for SQLite test compat

# Simpler
boost_delta_json = TextField()  # JSONB in PostgreSQL; TextField for SQLite test compat

The comment itself is valuable and should be kept.

Test Coverage

26 tests across 8 test classes. Coverage is thorough:

  • 108-sum invariant across single/multi-tier/multi-fixture scenarios
  • Correct delta values for normal and truncation paths
  • Extra keys (x-check columns) passed through unchanged
  • Pitcher TB accounting with partial-budget exhaustion
  • Hash determinism, tier/player differentiation, cosmetic order-independence, never-zero guarantee

No issues found.

Verdict: COMMENT

Clean, well-tested implementation. Core algorithms are correct and the migration is complete. The only notable finding is the evolution_tier parameter name in compute_variant_hash ��� it contradicts the refractor naming convention established by PR #131. Since this is a pure function signature that callers will import and use, fixing the name now avoids a later rename ripple. All other findings are cosmetic.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `app/services/refractor_boost.py` (added, 315 lines) - `app/db_engine.py` (modified — 4 new ORM fields + `RefractorBoostAudit` model) - `migrations/2026-03-28_refractor_phase2_boost.sql` (added) - `tests/test_refractor_boost.py` (added, 26 tests) - `tests/conftest.py` (modified) ### Findings #### Correctness - **Batter boost invariant**: Positive deltas (+2.0 total) exactly equal negative deltas (-2.0 total), so the 108-sum is preserved by construction. The truncation path is handled correctly: `last_delta = total_actually_reduced - running_sum` with a zero-floor ensures the sum of positive applications equals what was actually taken from negative columns. ✓ - **Pitcher boost invariant**: The algorithm moves budget from outcome columns into strikeout; net column-sum change is zero, so the 79-sum is preserved by construction. ✓ - **`compute_variant_hash` zero-remap**: `variant=0` is reserved and correctly remapped to 1. ✓ - **`RefractorBoostAudit` FK ordering in `_TEST_MODELS`**: `BattingCard` and `PitchingCard` are inserted above `RefractorBoostAudit` and below their own FK parent (`Player`). Dependency order is valid. ✓ - **Migration completeness**: `battingcard.image_url`, `pitchingcard.image_url`, and `card.variant` were already added to the DB by `2026-03-17_add_evolution_tables.sql`. Phase 2 migration correctly adds only the new `refractor_card_state.variant` column and `refractor_boost_audit` table. No missing migrations. ✓ - **`applied_at = DateTimeField(default=datetime.now)`**: Callable reference (not a called value) — correct. ✓ - **`UNIQUE(card_state_id, tier)`**: Prevents double-application of a boost at the same tier on retry. Correct safety guard. ✓ #### Security No issues found. Pure functions with no I/O, no SQL construction, no user-controlled input paths. Migration uses parameterless DDL. #### Style & Conventions **Naming inconsistency — `evolution_tier` parameter**: The `compute_variant_hash` function uses `evolution_tier` as the parameter name, but the entire system was renamed from "Evolution" to "Refractor" in PR #131. New code in this PR should use `refractor_tier` (or just `tier`) to stay consistent with the established convention. The docstring compounds the issue by saying `evolution_tier: Refractor tier (0–4)` — it uses both names simultaneously. ```python # Current (inconsistent) def compute_variant_hash(player_id: int, evolution_tier: int, ...) -> int: # Should be def compute_variant_hash(player_id: int, refractor_tier: int, ...) -> int: ``` **`boost_delta_json` multi-line formatting**: The parentheses around `TextField()` are redundant and non-idiomatic: ```python # Current boost_delta_json = ( TextField() ) # JSONB in PostgreSQL; TextField for SQLite test compat # Simpler boost_delta_json = TextField() # JSONB in PostgreSQL; TextField for SQLite test compat ``` The comment itself is valuable and should be kept. #### Test Coverage 26 tests across 8 test classes. Coverage is thorough: - 108-sum invariant across single/multi-tier/multi-fixture scenarios - Correct delta values for normal and truncation paths - Extra keys (x-check columns) passed through unchanged - Pitcher TB accounting with partial-budget exhaustion - Hash determinism, tier/player differentiation, cosmetic order-independence, never-zero guarantee No issues found. ### Verdict: COMMENT Clean, well-tested implementation. Core algorithms are correct and the migration is complete. The only notable finding is the `evolution_tier` parameter name in `compute_variant_hash` ��� it contradicts the refractor naming convention established by PR #131. Since this is a pure function signature that callers will import and use, fixing the name now avoids a later rename ripple. All other findings are cosmetic. --- *Automated review by Claude PR Reviewer*
Claude added the
ai-reviewed
 label 2026-03-29 18:50:06 +00:00
cal added 2 commits 2026-03-30 16:07:26 +00:00 
- Rename `evolution_tier` parameter to `refractor_tier` in compute_variant_hash()
  to match the refractor naming convention established in PR #131
- Update hash input dict key accordingly (safe: function is new, no stored hashes)
- Update test docstrings referencing the old parameter name
- Remove redundant parentheses on boost_delta_json TextField declaration

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
cal reviewed 2026-03-30 16:08:04 +00:00
cal left a comment
Author
Owner
Copy Link

AI Code Review — Re-review (APPROVED)

Previous findings addressed

Both findings from the initial review (commit 4a1251a) have been resolved:

  1. evolution_tier renamed to refractor_tier — Function signature, docstring, hash input dict key, and test docstrings all updated. Since compute_variant_hash is new in this PR and no variant hashes exist in the database yet, changing the dict key (which changes the hash output) is safe.

  2. Redundant parentheses on boost_delta_json — Simplified to single-line TextField() declaration. Comment preserved.

Test results

  • 26/26 refractor boost tests passing
  • Full suite: 172 passed, 15 skipped, 1 pre-existing failure (test_list_cards_evaluated_only_false_includes_unevaluated — isoformat() bug in refractor.py:55, exists on main)

Verdict: APPROVED

All review findings addressed. Core algorithms remain correct, 108-sum and 79-sum invariants preserved, naming now consistent with refractor conventions. Ready to merge.

Automated re-review by Claude PR Reviewer

## AI Code Review — Re-review (APPROVED) ### Previous findings addressed Both findings from the initial review (commit 4a1251a) have been resolved: 1. **`evolution_tier` renamed to `refractor_tier`** — Function signature, docstring, hash input dict key, and test docstrings all updated. Since `compute_variant_hash` is new in this PR and no variant hashes exist in the database yet, changing the dict key (which changes the hash output) is safe. 2. **Redundant parentheses on `boost_delta_json`** — Simplified to single-line `TextField()` declaration. Comment preserved. ### Test results - 26/26 refractor boost tests passing - Full suite: 172 passed, 15 skipped, 1 pre-existing failure (`test_list_cards_evaluated_only_false_includes_unevaluated` — `isoformat()` bug in `refractor.py:55`, exists on `main`) ### Verdict: APPROVED All review findings addressed. Core algorithms remain correct, 108-sum and 79-sum invariants preserved, naming now consistent with refractor conventions. Ready to merge. --- *Automated re-review by Claude PR Reviewer*
Claude approved these changes 2026-03-30 16:11:01 +00:00
Dismissed
Claude left a comment
Collaborator
Copy Link

Approved via pd-pr

Approved via pd-pr
Claude approved these changes 2026-03-30 16:11:07 +00:00
Claude left a comment
Collaborator
Copy Link

Approved via pd-pr

Approved via pd-pr
cal merged commit 70f984392d into main 2026-03-30 16:11:08 +00:00
cal deleted branch feature/refractor-phase2-foundation 2026-03-30 16:11:09 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Claude
Labels
Clear labels   
ai-changes-requested

   
ai-failed

   
ai-merged

PR merged by AI ops agent

  
ai-pr-opened

   
ai-reviewed

   
ai-reviewing

   
ai-reviewing

   
ai-working

   
bug

   
enhancement

   
evolution

Card Evolution system

  
performance

   
phase-0

Render Pipeline Optimization

  
phase-1a

Schema & Data Foundation

  
phase-1b

API & Formula Engine

  
phase-1c

Bot Integration

  
phase-1d

Deployment

  
security

   
tech-debt

   
todo
No Label
ai-changes-requested
ai-failed
ai-merged
ai-pr-opened
ai-reviewed
ai-reviewing
ai-reviewing
ai-working
bug
enhancement
evolution
performance
phase-0
phase-1a
phase-1b
phase-1c
phase-1d
security
tech-debt
todo
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: cal/paper-dynasty-database#176
No description provided.
Delete Branch "feature/refractor-phase2-foundation"

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?