batch(baseball): 22 fixes — single-deploy batch#650
Merged
Conversation
…re' into batch/baseball-fixes
…to batch/baseball-fixes
…ewcount' into batch/baseball-fixes
…ition' into batch/baseball-fixes
… into batch/baseball-fixes
…e' into batch/baseball-fixes
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds BaseballHelm planning and audit docs, the Living Annual UI foundation, and a large batch of related BaseballHelm data, routing, page-flow, and dashboard page migration changes. ChangesDocumentation and Living Annual foundation
Application changes
Estimated code review effort: 5 (Critical) | ~150 minutes Possibly related issues
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5 | ❌ 7❌ Failed checks (7 inconclusive)
✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
PR Summary by QodoBaseball one-deploy batch: pipeline, privacy, lineup, telemetry & routing fixes
AI Description
Diagram
High-Level Assessment
Files changed (22)
|
…og (#434) Verifier gap: the per-game rate cells still used notation IP (row.ip) as the denominator; a 0.2 IP line rendered ERA as 9*er/0.2 instead of /0.667. Divide by ipToInnings(row.ip) to match the corrected footer totals. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017VUauUXsm9aXegShmpJi5n
…-any) The #445 fix reads baseball_events via fromUntyped (requires_rsvp not yet in generated types), which made the map param implicitly any. Annotate the row. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017VUauUXsm9aXegShmpJi5n
njrini99-code
changed the title
Code Review by Qodo
Context used✅ Tickets:
🎫 BaseballHelm: Check saveLineup result before showing roster success toast 🎫 BaseballHelm: Use recruiting ProfileEditor for JUCO on dashboard/profile 🎫 BaseballHelm: Align Staff Settings edit affordance with can_invite_staff server gate +3 more✅ Compliance rules (platform):
93 rules 1. Postgame priorItems DB error ignored
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+369
to
+377
| const result = await removeFromWatchlist(coachId, playerId); | ||
| if (result.success) { | ||
| setWatchlistIds((prev) => prev.filter((id) => id !== playerId)); | ||
| } | ||
| } else { | ||
| await addToWatchlist(coachId, playerId); | ||
| setWatchlistIds((prev) => [...prev, playerId]); | ||
| const result = await addToWatchlist(coachId, playerId); | ||
| if (result.success) { | ||
| setWatchlistIds((prev) => [...prev, playerId]); | ||
| } |
There was a problem hiding this comment.
2. Silent watchlist failure, no toast 🐞 Bug ☼ Reliability
In DiscoverView.tsx, when addToWatchlist or removeFromWatchlist returns { success: false } (e.g.,
recruitability denial, duplicate, permission error), neither the add nor the remove branch shows any
user-facing error toast — the star simply stays in its current state with no explanation. The catch
block only logs to console, so the failure is completely invisible to the coach.
Agent Prompt
## Issue description
When `addToWatchlist` or `removeFromWatchlist` returns `{ success: false, message: '...' }` in `DiscoverView.tsx`, the code silently does nothing — no toast, no user feedback. The coach sees the star icon not flip but gets no explanation.
## Issue Context
The server actions return `{ success: false, message: '...' }` without throwing for expected failures (recruitability denial, duplicate, permission). The current code only handles the success path and the catch block (for thrown errors), leaving the non-throwing failure path silent.
## Fix Focus Areas
- src/components/coach/discover/DiscoverView.tsx[369-377] — add an `else` branch after each `if (result.success)` block that calls a toast with `result.message || 'Failed to update watchlist'`
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+185
to
+193
| const { data: priorItems } = await (supabase as any) | ||
| .from('baseball_postgame_review_items') | ||
| .select('dedupe_key, disposition') | ||
| .eq('review_id', reviewId); | ||
| const priorDisposition = new Map<string, string>( | ||
| ((priorItems ?? []) as { dedupe_key: string; disposition: string }[]).map( | ||
| (r) => [r.dedupe_key, r.disposition], | ||
| ), | ||
| ); |
There was a problem hiding this comment.
3. Postgame prioritems db error ignored 🐞 Bug ☼ Reliability
The new query that fetches prior dispositions to preserve coach actions (converted_to_timeline, dismissed, etc.) destructures only data and silently ignores the error field. If the DB query fails, priorDisposition is an empty Map and the subsequent upsert overwrites all existing dispositions back to 'new', re-opening items the coach already actioned.
Agent Prompt
## Issue description
The query that fetches prior postgame review item dispositions ignores the Supabase error field. If the query fails, all prior coach dispositions are silently lost on the next regeneration.
## Issue Context
The preservation logic was introduced to prevent the upsert from resetting coach-actioned items (dismissed, converted_to_timeline, etc.) back to 'new'. But if the lookup query itself fails, the protection is bypassed silently.
## Fix Focus Areas
- src/app/baseball/actions/postgame.ts[185-193] — destructure `{ data: priorItems, error: priorItemsError }` and if `priorItemsError` is set, either return early with an error or log a warning and skip the upsert (to avoid overwriting dispositions when the lookup is unreliable)
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This was referenced Jul 1, 2026
…floating content in empty voids) All three desktop replicas (Command Center, Stats Center, Decision Room) wrapped their growing section in `flex-1 ... justify-center`, which centers a short row-list inside a much taller bezel well — that's what produced the "floating band in a cream void" look Nick flagged from the captured frames, not a rendering fluke. Root-cause fix: each screen is now a real header/body/footer flex column where the body is top-aligned (never centered), and its last, growing section is a CSS grid with `1fr` rows — that fills the exact available height edge-to-edge regardless of the well's real rendered size, instead of guessing at row counts/heights that may or may not fill it. - CommandCenterScreen: added a Today schedule strip (2 rows) between the stat tiles and the roster; grew the roster from 4 to 7 rows with hairline separators; added a quiet footer strip. - StatsCenterScreen: added a batting-lines table (6 rows: player/AB/R/H/ RBI, right-aligned tabular-nums) between the box score and the At the Plate footer, which used to be ~65% empty. - DecisionRoomScreen: promoted the 3 signals to bordered evidence cards (hairline + a subtle sage bg step) spread edge-to-edge via 1fr grid rows instead of a centered cluster; added a "This Week / 3 Signals" header row and a quiet "Open Decision Room" footer row. Lift Lab phone left unchanged (already acceptable per review). Gate: tsc --noEmit and eslint on the three changed files both clean (0 errors, 0 warnings); lint-ratchet.mjs confirms no repo-wide regression. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fz1ajgmqcEp1yPEzjnXgY5
…rows, dead error fallback csv-utils parseCSV: row objects are Object.create(null) so an uploaded column named __proto__/constructor lands as an own property (js/remote-property-injection, alert #140). player-today: the ack-error branch is mutually exclusive with the schedule-error branch, so the 'error ??' there was provably dead (js/trivial-conditional, alert #465). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fz1ajgmqcEp1yPEzjnXgY5
…tch/baseball-fixes
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| // Null prototype: header text comes from the uploaded file, so a column | ||
| // named __proto__/constructor must land as a plain own property, never | ||
| // walk the prototype chain (CodeQL js/remote-property-injection). | ||
| const row: CSVRow = Object.create(null); |
items-start pinned title/meta/bar to the top of each stretched card, leaving the lower two-thirds empty (pixel review, round-4 frames). items-center balances each card's interior. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fz1ajgmqcEp1yPEzjnXgY5
…tch/baseball-fixes
…/insecure-randomness #477) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fz1ajgmqcEp1yPEzjnXgY5
…baseball + auth only (Nick) Nick 2026-07-02: 'Don't merge anything but the baseball login for now. The rest looks like absolute garbage' + 'Admin merge all the internal baseball changes... one branch, one deploy.' Removed: src/app/page.tsx restored to main's landing; marketing/ first-light moments/scroll/nav/brand/screens/lib + CONTRACTS.md; /join invite page; landing photo assets. Kept: fonts.ts + first-light.css (the only marketing-tree files the baseball auth shell + onboarding + lift-onboarding import — inert token/font definitions); ALL internal baseball product work; the redesigned baseball (auth) surfaces Nick approved. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fz1ajgmqcEp1yPEzjnXgY5
This was referenced Jul 3, 2026
Closed
njrini99-code
pushed a commit
that referenced
this pull request
Jul 3, 2026
…r', not 'Team Roster' The #650 roster rebuild (RosterFairway) changed the visible masthead to 'Roster'; the document title still says 'Team Roster' but the smoke asserts the on-page heading. First honest e2e round confirmed login now works (secrets fix) — this was the only real failure. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fz1ajgmqcEp1yPEzjnXgY5
This was referenced Jul 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
One-merge batch — 22 BaseballHelm fixes, one production build
All fixes folded into a single branch so the whole set lands in one Vercel production deploy. Vercel auto-builds are currently paused at the project level (Ignored Build Step), so this PR did not deploy. Do not merge until instructed.
Deploy sequence (when you say go)
20260701020000_baseball_player_aggregates_slash_line.sqlto prod (addscareer_obp/slg/ops; idempotent). Must land before/with the code or the BaseballHelm: Compute OBP/SLG/OPS in legacy aggregate recalculation #436 upsert errors.Group C fixes (13) — pre-built, rebased clean
#425 recruit→public profile (#552) · #426/#427/#428 pipeline stages+DnD (#550) · #429 discover revert (#557) · #432 message nav (#553) · #459 activate exposure (#566) · #462 JUCO editor (#554) · #467 dead demo link (#570) · #470 privacy columns (#560) · #477 postgame disposition (#569) · #489/#490 lineup swap (#567) · #500 scout-packet view_count (#568) · #505 staff-invite gate (#559) · #511 coach-note scope (#571)
New fixes (9) — authored + adversarially verified
requires_rsvpbodyon player profile (player-profile surface only; the command-center peek needs later read-model wiring — no regression, it shows nothing there today)Verification
Combined
tsc --noEmitclean; 44 stats unit tests pass; each new fix adversarially verified against its issue's acceptance criteria (6 CONFIRMED, 3 PLAUSIBLE with gaps fixed here, 0 rejected).Not included
🤖 Generated with Claude Code