fix: use Field(default_factory) for offense_col random default (#24) #38

cal · 2026-03-03T20:05:39Z

cal commented

2026-03-03 20:05:39 +00:00

Summary

Fixes #24 — offense_col in PlayerModel was using a bare random.randint(1, 3) as the field default. Pydantic evaluates bare defaults at class definition time (once, on import), so every PlayerModel instance received the same random integer for offense_col throughout the lifetime of the process.

Replaced with pydantic.Field(default_factory=lambda: random.randint(1, 3)) so a fresh random value is generated on each model instantiation.

Files Changed

app/routers_v2/mlbplayers.py — line 40: offense_col default changed to use default_factory

Test Results

No test suite in this repo. Change verified by reading back the modified file — import of pydantic already present; pydantic.Field usage is consistent with the existing module-level import pattern.

## Summary Fixes #24 — `offense_col` in `PlayerModel` was using a bare `random.randint(1, 3)` as the field default. Pydantic evaluates bare defaults at class definition time (once, on import), so every `PlayerModel` instance received the same random integer for `offense_col` throughout the lifetime of the process. Replaced with `pydantic.Field(default_factory=lambda: random.randint(1, 3))` so a fresh random value is generated on each model instantiation. ## Files Changed - `app/routers_v2/mlbplayers.py` — line 40: `offense_col` default changed to use `default_factory` ## Test Results No test suite in this repo. Change verified by reading back the modified file — import of `pydantic` already present; `pydantic.Field` usage is consistent with the existing module-level import pattern.

cal added 1 commit 2026-03-03 20:05:40 +00:00

fix: use Field(default_factory) for offense_col random default (#24 )

Build Docker Image / build (pull_request) Successful in 3m10s

Details

d26bc0dc7b

Pydantic evaluates bare `random.randint(1, 3)` once at class definition
time, so every PlayerModel instance shared the same value. Replaced with
`pydantic.Field(default_factory=...)` so a new random value is generated
per instance.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

cal referenced this pull request

2026-03-03 20:05:55 +00:00

Move random.randint default out of Pydantic model field #24

cal reviewed 2026-03-03 20:32:56 +00:00

cal left a comment

AI Code Review

Files Reviewed

app/routers_v2/mlbplayers.py (modified)

Findings

Correctness

Fix is correct. In Python, a bare random.randint(1, 3) as a class-level default is evaluated once at module import time, making all PlayerModel instances share the same offense_col value for the lifetime of the process. Field(default_factory=lambda: random.randint(1, 3)) defers evaluation to instantiation time, which is the intended behavior.
Both random (line 2) and pydantic (line 7) are already imported — no new dependencies added.
The randint(1, 3) range is preserved (inclusive on both ends: 1, 2, or 3).
The qualified pydantic.Field(...) form is consistent with the rest of the file's use of pydantic.BaseModel.

Security

No issues found.

Style & Conventions

No issues found. Code matches existing patterns in the file.

Suggestions

None. The fix is minimal and correct.

Verdict: APPROVED

Correct, minimal fix for a classic Python evaluated-at-definition-time default pitfall. Field(default_factory=...) is the idiomatic Pydantic v1 solution. No concerns — ready to merge.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `app/routers_v2/mlbplayers.py` (modified) ### Findings #### Correctness - Fix is correct. In Python, a bare `random.randint(1, 3)` as a class-level default is evaluated once at module import time, making all `PlayerModel` instances share the same `offense_col` value for the lifetime of the process. `Field(default_factory=lambda: random.randint(1, 3))` defers evaluation to instantiation time, which is the intended behavior. - Both `random` (line 2) and `pydantic` (line 7) are already imported — no new dependencies added. - The `randint(1, 3)` range is preserved (inclusive on both ends: 1, 2, or 3). - The qualified `pydantic.Field(...)` form is consistent with the rest of the file's use of `pydantic.BaseModel`. #### Security - No issues found. #### Style & Conventions - No issues found. Code matches existing patterns in the file. #### Suggestions - None. The fix is minimal and correct. ### Verdict: APPROVED Correct, minimal fix for a classic Python evaluated-at-definition-time default pitfall. `Field(default_factory=...)` is the idiomatic Pydantic v1 solution. No concerns — ready to merge. --- *Automated review by Claude PR Reviewer*

cal added the

ai-reviewed

label 2026-03-03 20:33:21 +00:00

cal changed target branch from main to next-release

2026-03-03 21:45:48 +00:00

cal force-pushed ai/paper-dynasty-database-24 from d26bc0dc7b to 6130eb993f

2026-03-03 21:46:06 +00:00

Compare

cal merged commit fe5635ef0e into next-release

2026-03-03 21:46:47 +00:00

cal deleted branch ai/paper-dynasty-database-24

2026-03-03 21:46:47 +00:00

cal referenced this issue from a commit

2026-03-03 21:46:49 +00:00

Merge pull request 'fix: use Field(default_factory) for offense_col random default (#24)' (#38) from ai/paper-dynasty-database-24 into next-release

Sign in to join this conversation.

No reviewers

No Milestone

No project

No Assignees

1 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#38