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

fix: consolidate redundant double-query in get_one_play (#14) #52

Merged
cal merged 1 commits from ai/paper-dynasty-database#14 into next-release 2026-03-05 03:36:27 +00:00
Conversation 0 Commits 1 Files Changed 1 +3 -3
cal commented 2026-03-04 03:02:59 +00:00
Owner
Copy Link

Summary

Fixes #14.

get_one_play was making two round-trips to the database for a single record:

  1. get_or_none(...) — to check existence, result discarded
  2. get_by_id(...) — to fetch the same record again

The fix reuses the result from get_or_none, eliminating the redundant second query.

Changes

  • app/routers_v2/stratplays.py: Store get_or_none result and return it directly

Test Results

No test suite — verified by reading back the modified function.

## Summary Fixes #14. `get_one_play` was making two round-trips to the database for a single record: 1. `get_or_none(...)` — to check existence, result discarded 2. `get_by_id(...)` — to fetch the same record again The fix reuses the result from `get_or_none`, eliminating the redundant second query. ## Changes - `app/routers_v2/stratplays.py`: Store `get_or_none` result and return it directly ## Test Results No test suite — verified by reading back the modified function.
cal added 1 commit 2026-03-04 03:03:00 +00:00 
Reuse the result of get_or_none instead of discarding it and calling
get_by_id again, eliminating one unnecessary round-trip per request.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
cal added the
ai-reviewing
 label 2026-03-04 03:16:53 +00:00
cal reviewed 2026-03-04 03:18:08 +00:00
cal left a comment
Author
Owner
Copy Link

AI Code Review

Files Reviewed

  • app/routers_v2/stratplays.py (modified)

Findings

Correctness

  • Fix is correct. get_or_none returns the model instance (or None), so storing it and passing directly to model_to_dict is semantically identical to the original two-query pattern — with one fewer round-trip to the database.
  • The 404 path is unchanged: None check raises the same HTTPException.
  • Removing the intermediate r_play variable is a minor cleanup with no behavioral change.

Security

  • No issues. This is an unauthenticated GET endpoint (same as before). play_id is typed as int, so no injection risk.

Style & Conventions

  • Follows existing project patterns. Clean and minimal change.

Suggestions

  • The patch_play endpoint directly below (~line 1412) has the same double-query pattern (get_or_none result discarded, then a second fetch). Worth a follow-up issue to apply the same fix there for consistency.

Verdict: APPROVED

Clean, correct optimization. Eliminates a redundant DB round-trip with no behavioral change. Exactly what the PR claims.

⚠️ Note: Gitea prevented posting as APPROVED (self-review restriction). This review is COMMENT-typed but the verdict is APPROVED — ready to merge.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `app/routers_v2/stratplays.py` (modified) ### Findings #### Correctness - Fix is correct. `get_or_none` returns the model instance (or `None`), so storing it and passing directly to `model_to_dict` is semantically identical to the original two-query pattern — with one fewer round-trip to the database. - The 404 path is unchanged: `None` check raises the same `HTTPException`. - Removing the intermediate `r_play` variable is a minor cleanup with no behavioral change. #### Security - No issues. This is an unauthenticated GET endpoint (same as before). `play_id` is typed as `int`, so no injection risk. #### Style & Conventions - Follows existing project patterns. Clean and minimal change. #### Suggestions - The `patch_play` endpoint directly below (~line 1412) has the same double-query pattern (`get_or_none` result discarded, then a second fetch). Worth a follow-up issue to apply the same fix there for consistency. ### Verdict: APPROVED Clean, correct optimization. Eliminates a redundant DB round-trip with no behavioral change. Exactly what the PR claims. > ⚠️ *Note: Gitea prevented posting as APPROVED (self-review restriction). This review is COMMENT-typed but the verdict is APPROVED — ready to merge.* --- *Automated review by Claude PR Reviewer*
cal added
ai-reviewed
 and removed
ai-reviewing
 labels 2026-03-04 03:18:39 +00:00
cal force-pushed ai/paper-dynasty-database#14 from d12ce52d65 to f1d289a0e9 2026-03-05 03:36:02 +00:00 Compare
cal merged commit 8283425b84 into next-release 2026-03-05 03:36:27 +00:00
cal deleted branch ai/paper-dynasty-database#14 2026-03-05 03:36:27 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
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
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#52
No description provided.
Delete Branch "ai/paper-dynasty-database#14"

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?