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

fix: remove broken live_update_batting stub endpoint (#10) #54

Merged
cal merged 1 commits from ai/paper-dynasty-database#10 into next-release 2026-03-07 03:22:09 +00:00
Conversation 0 Commits 1 Files Changed 1 +12 -53
cal commented 2026-03-04 04:32:46 +00:00
Owner
Copy Link

Summary

Removes the completely broken POST /api/v2/scouting/live-update/batting endpoint.

Root cause

The endpoint iterated over files.vl_basic which is a str (a filename like "vl-basic.csv"), not parsed CSV data. This caused the loop to iterate over individual characters. The body also contained only pass with TODO comments and no running stats logic — the endpoint would never produce a valid result.

Fix

  • Removed the live_update_batting endpoint entirely
  • Removed the dead commented-out csv_file_to_dataframe helper code
  • Retained BattingFiles model (still used by live_update_pitching)

Files changed

  • app/routers_v2/scouting.py

Test results

No test suite. Verified by reading back the modified file.

Other observations

  • live_update_pitching (POST /api/v2/scouting/live-update/pitching) is also a stub — it accepts BattingFiles (not a pitching model) and simply returns the input unchanged. Not fixing here as it's out of scope.
## Summary Removes the completely broken `POST /api/v2/scouting/live-update/batting` endpoint. ### Root cause The endpoint iterated over `files.vl_basic` which is a `str` (a filename like `"vl-basic.csv"`), not parsed CSV data. This caused the loop to iterate over individual characters. The body also contained only `pass` with TODO comments and no running stats logic — the endpoint would never produce a valid result. ### Fix - Removed the `live_update_batting` endpoint entirely - Removed the dead commented-out `csv_file_to_dataframe` helper code - Retained `BattingFiles` model (still used by `live_update_pitching`) ### Files changed - `app/routers_v2/scouting.py` ### Test results No test suite. Verified by reading back the modified file. ### Other observations - `live_update_pitching` (`POST /api/v2/scouting/live-update/pitching`) is also a stub — it accepts `BattingFiles` (not a pitching model) and simply returns the input unchanged. Not fixing here as it's out of scope.
cal added 1 commit 2026-03-04 04:32:47 +00:00 
The endpoint iterated over `files.vl_basic` (a string, not parsed CSV),
causing it to loop over individual characters. The body contained only
`pass` with TODO comments and no running stats logic. Removed the
endpoint entirely along with the dead commented-out csv helper code.
The `BattingFiles` model is retained as it is still used by
`live_update_pitching`.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
cal added the
ai-reviewing
 label 2026-03-04 05:19:39 +00:00
cal reviewed 2026-03-04 05:20:38 +00:00
cal left a comment
Author
Owner
Copy Link

AI Code Review

Files Reviewed

  • app/routers_v2/scouting.py (modified)

Findings

Correctness

  • The live_update_batting removal is accurate and well-motivated. files.vl_basic is typed as str (a filename), so iterating over it yields individual characters — the loop logic was fundamentally broken. The body was also entirely pass with TODO comments, making this endpoint a no-op.
  • BattingFiles model is correctly retained; it is still referenced by live_update_pitching.
  • The dead csv_file_to_dataframe commented block is cleanly removed.
  • No remaining references to live_update_batting elsewhere in the file.

Security

  • No issues introduced by this PR.
  • live_update_pitching still logs the bearer token in plaintext (logging.warning(f"Bad Token: {token}")), but this is a pre-existing issue being addressed separately in PR #55, and is out of scope here.

Style & Conventions

  • The PR includes out-of-scope quote-style changes throughout the file (single → double quotes on strings, trailing commas added to logging.basicConfig, APIRouter condensed to one line). These are not mentioned in the PR body. The changes are harmless and consistent, but unrelated to the stated fix.

Suggestions

  • The PR body could mention the incidental quote-style reformatting to keep the diff readable for future reviewers.
  • live_update_pitching accepts BattingFiles (a batting model) — worth a follow-up issue to give it a proper pitching model once that endpoint is implemented.

Verdict: APPROVED

The core change is correct: the endpoint was irreparably broken and removal is the right call. Retained model, removed dead code, no regressions introduced. The cosmetic reformatting is harmless.

Automated review by Claude PR Reviewer (self-approval blocked by Gitea — verdict is APPROVED)

## AI Code Review ### Files Reviewed - `app/routers_v2/scouting.py` (modified) ### Findings #### Correctness - The `live_update_batting` removal is accurate and well-motivated. `files.vl_basic` is typed as `str` (a filename), so iterating over it yields individual characters — the loop logic was fundamentally broken. The body was also entirely `pass` with TODO comments, making this endpoint a no-op. - `BattingFiles` model is correctly retained; it is still referenced by `live_update_pitching`. - The dead `csv_file_to_dataframe` commented block is cleanly removed. - No remaining references to `live_update_batting` elsewhere in the file. #### Security - No issues introduced by this PR. - `live_update_pitching` still logs the bearer token in plaintext (`logging.warning(f"Bad Token: {token}")`), but this is a pre-existing issue being addressed separately in PR #55, and is out of scope here. #### Style & Conventions - The PR includes out-of-scope quote-style changes throughout the file (single → double quotes on strings, trailing commas added to `logging.basicConfig`, `APIRouter` condensed to one line). These are not mentioned in the PR body. The changes are harmless and consistent, but unrelated to the stated fix. #### Suggestions - The PR body could mention the incidental quote-style reformatting to keep the diff readable for future reviewers. - `live_update_pitching` accepts `BattingFiles` (a batting model) — worth a follow-up issue to give it a proper pitching model once that endpoint is implemented. ### Verdict: APPROVED The core change is correct: the endpoint was irreparably broken and removal is the right call. Retained model, removed dead code, no regressions introduced. The cosmetic reformatting is harmless. --- *Automated review by Claude PR Reviewer (self-approval blocked by Gitea — verdict is APPROVED)*
cal added
ai-reviewed
 and removed
ai-reviewing
 labels 2026-03-04 05:20:56 +00:00
cal removed the
ai-reviewed
 label 2026-03-05 03:42:03 +00:00
cal added the
ai-reviewing
 label 2026-03-05 03:51:55 +00:00
cal reviewed 2026-03-05 03:53:35 +00:00
cal left a comment
Author
Owner
Copy Link

AI Code Review

Files Reviewed

  • app/routers_v2/scouting.py (modified)

Findings

Correctness

  • The live_update_batting endpoint is cleanly removed. The root cause described in the PR body is accurate: files.vl_basic is a str (filename), so for row in files.vl_basic iterates over individual characters — the endpoint was fundamentally broken and could never produce valid output.
  • The dead commented-out csv_file_to_dataframe block is correctly removed.
  • BattingFiles model is retained — still required by live_update_pitching. Correct.
  • The PR body notes live_update_pitching is also a stub but out of scope. Appropriate scope discipline.

Security

  • Merge conflict concern (not a code defect in this PR, but critical to flag): This PR branch predates the merge of PR #56 (fix: use constant-time comparison for bearer token validation), which also patched live_update_pitching in this file — changing logging.warning(f'Bad Token: {token}') to 'Bad Token: [REDACTED]'. The current next-release has the redacted version; this PR branch does not. The PR is currently unmergeable (mergeable: false) due to this conflict. When resolving the conflict, the security fix from #56 must be preserved — do not reintroduce token logging in live_update_pitching.

Style & Conventions

  • The diff includes ruff-driven reformatting (single → double quotes, minor line wrapping). This is consistent with the project's pre-commit hook behavior, as noted in PR #57 for the same file. No issues.

Suggestions

  • None.

Verdict: COMMENT

The removal is correct and well-justified. The PR cannot merge in its current state due to a conflict with next-release (introduced by #56 touching the same file). Resolve the conflict carefully to preserve the [REDACTED] token logging fix already in next-release.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `app/routers_v2/scouting.py` (modified) ### Findings #### Correctness - The `live_update_batting` endpoint is cleanly removed. The root cause described in the PR body is accurate: `files.vl_basic` is a `str` (filename), so `for row in files.vl_basic` iterates over individual characters — the endpoint was fundamentally broken and could never produce valid output. - The dead commented-out `csv_file_to_dataframe` block is correctly removed. - `BattingFiles` model is retained — still required by `live_update_pitching`. Correct. - The PR body notes `live_update_pitching` is also a stub but out of scope. Appropriate scope discipline. #### Security - **Merge conflict concern (not a code defect in this PR, but critical to flag):** This PR branch predates the merge of PR #56 (`fix: use constant-time comparison for bearer token validation`), which also patched `live_update_pitching` in this file — changing `logging.warning(f'Bad Token: {token}')` to `'Bad Token: [REDACTED]'`. The current `next-release` has the redacted version; this PR branch does not. The PR is currently unmergeable (`mergeable: false`) due to this conflict. **When resolving the conflict, the security fix from #56 must be preserved — do not reintroduce token logging in `live_update_pitching`.** #### Style & Conventions - The diff includes ruff-driven reformatting (single → double quotes, minor line wrapping). This is consistent with the project's pre-commit hook behavior, as noted in PR #57 for the same file. No issues. #### Suggestions - None. ### Verdict: COMMENT The removal is correct and well-justified. The PR cannot merge in its current state due to a conflict with `next-release` (introduced by #56 touching the same file). Resolve the conflict carefully to preserve the `[REDACTED]` token logging fix already in `next-release`. --- *Automated review by Claude PR Reviewer*
cal added the
ai-reviewed
 label 2026-03-05 03:53:42 +00:00
cal force-pushed ai/paper-dynasty-database#10 from 03cea0d96f to 7b494faa99 2026-03-05 21:34:13 +00:00 Compare
cal merged commit be02ba1e3f into next-release 2026-03-07 03:22:09 +00:00
cal deleted branch ai/paper-dynasty-database#10 2026-03-07 03:22:10 +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#54
No description provided.
Delete Branch "ai/paper-dynasty-database#10"

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?