Baseball statsync verification: box-score↔profile sync proven, destructive RPC replaced, shell/routes remediation - #794
Merged
Conversation
…ell, capped nav IA, one lift lab Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LGqvhig8njW38q9PAwDXgu
…te dead (coach-dashboard) route group Coherence Ruling 1 (docs/baseball/COHERENCE_RULING_2026-07-08.md): NEXT_PUBLIC_REDESIGN=true is what prod serves, so the legacy shell fork in (dashboard)/layout.tsx and (player-dashboard)/player/layout.tsx was pure regression risk. Both now mount BaseballFairwayShell unconditionally; the team-context DashboardSessionGuard is unchanged. Deletes src/app/baseball/(coach-dashboard)/ entirely (confirmed dead: only layout.tsx/template.tsx/error.tsx, zero page.tsx). Updates stale BaseballShellLayout references in comments/test descriptions to BaseballFairwayShell.
…, strip baseball nav from shared sidebar Both files had zero real importers once the (dashboard)/(player-dashboard) layouts render BaseballFairwayShell unconditionally (previous commit) — deleted per Ruling 1/5. src/components/layout/sidebar.tsx is shared with golf: removed the 5 legacy baseball nav arrays (collegeTeamNav/hsCoachTeamNav/jucoTeamNav/showcaseOrgNav/playerTeamNav), buildCondensedBaseballNavigation(), the showcase org/team TeamSwitcher branch, and every other baseball-only branch. The isGolf branch (golfCoachNav, dashboardHref, EXACT_MATCH_HREFS golf entry, coachSecondaryNav/playerSecondaryNav fallback, isHubItemActive) is byte-identical to before — golf renders the exact same values it did prior to this change. Note: BaseballHelm no longer renders this Sidebar component at all (it only ever had one consumer, the now-deleted BaseballDashboardShell) — GolfHelm's real rendered sidebar is the separate GolfSidebar.tsx via GolfDashboardShell, not this file. This component is consequently unreferenced by any route today; flagged for the commander as a follow-up deletion candidate, not removed here since it falls outside this task's named scope and the file is marked shared/untouchable without further authorization.
…dar/documents/messages pages Calendar, Documents, and Messages pages had their own local isRedesignEnabled() forks rendering a legacy pre-Fairway UI. Since the shell fork is gone (BaseballFairwayShell is now the only shell), these pages always take the Fairway branch — deleted the legacy JSX and now-unused imports (Link, BaseballCalendarWrapper, EVENT_TYPE_CONFIG, Card/CardContent/Input/Select/Checkbox/DocumentCard/cn, the messages skeleton block). practice/page.tsx already had no fork (comment-only precedent reference, untouched).
…ubtabs (Ruling 2) Implements the coach hub/subtab table from docs/baseball/COHERENCE_RULING_2026-07-08.md Ruling 2. Mechanism: nav-registry.ts gets a `foldedUnder` field (a registry entry marks itself as reachable only from a parent landing page instead of its own hub-tab slot, but keeps its registry row so command palette + deep links still work) and a `hubTabLabel` override (lets a subtab read differently from the entry's canonical label, e.g. "Performance" reads "Training" under Development). hub-definitions.ts's `hubEntries()` excludes folded entries from the rendered strip. Regrouped: Announcements moves from Team to a new real Messages hub (Messages · Announcements, replacing the old bare flat link); Practice Planner/Effectiveness move from Stats & Performance to Team (fold into the new Operations landing); Performance moves from Stats & Performance to Development (folds in as "Training"); Import Center folds into Stats Center; Interest folds into Pipeline; Watchlist/Compare/Saved Comparisons/Scout Packets/Camps fold into a new Scouting landing; Program Info/Staff Settings/Program Settings fold into the existing Settings landing; Teams/ Events fold into Organization. Deleted the 9-item SETTINGS_* hub-tab splice and the 4-item PERFORMANCE_* splice outright (dead code, not layered over) since Performance's own page already links Programs/Live/Builder/Groups. Command Center's nav-visible label renames to "Overview" (breadcrumbs, hub subtab, command palette, mobile nav) so "Dashboard" never has two names in the UI; the route/page masthead keep "Command Center". BaseballFairwayShell.tsx: Messages now flows through the generic COACH_HUB_ORDER loop (with its unread badge threaded in by hub id) instead of a hand-injected flat item; showcase org/team rails route their Messages item through the same hub-aware path so showcase coaches don't lose Announcements reachability. Mobile bottom nav trimmed to the top 3 destinations per role (Ruling 2 item 8) — the top-bar hamburger (already bridged to the same drawer) is the "+ More". /baseball/dashboard/practice now server-redirects players to the canonical /baseball/player/practice surface (item 7); coach/staff rendering unchanged. nav-manifest.ts and route-contract.ts extended to track the new Messages hub tabs for completeness. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LGqvhig8njW38q9PAwDXgu
New card-grid landing pages that keep the folded destinations reachable in ≤2 clicks now that Team and Recruiting cap at 3 rendered subtabs: - /baseball/dashboard/operations (Team hub): Documents, Travel, Practice Planner, Practice Effectiveness. - /baseball/dashboard/scouting (Recruiting hub): Watchlist, Compare Players, Saved Comparisons, Scout Packets, Camps. Both are server components (no hooks) built from the Living Annual kit (SectionMasthead/PaperCard/Eyebrow). Neither re-declares capability or program-type gating — each card is resolved via getBaseballNavEntry + isBaseballNavEntryVisible against the SAME registry entry the sidebar and command palette use, so a coach without a capability simply doesn't see that card, exactly like they wouldn't see it in the sidebar. Operations uses the green "team" ink lane; Scouting uses the clay "pursuit" ink lane (recruiting). Also adds the Stats Center masthead's persistent header-level actions (Upload, Import Center) per Ruling 2 item 2 — Upload + Import Center are now CTAs on the Stats Center page instead of their own hub-tab slots; the existing empty-state "Import a box score" CTA is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LGqvhig8njW38q9PAwDXgu
…e (Ruling 2) The card-grid landing at /dashboard/settings stays the single settings nav surface (its own 13-tab strip is gone now that Management caps at 3 subtabs: Decision Room · Settings · Organization). It was mounting the legacy <Header> component on top of the shell's own top bar, duplicating breadcrumbs and the mobile-menu affordance; replaced with a plain title block. Verified the card grid still covers every fold target (Program Info/Settings, Staff, Team, Season, Philosophy, Roles, Permissions, Imports, Integrations, Audit) — nothing orphaned by the splice removal. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LGqvhig8njW38q9PAwDXgu
…(Ruling 2)
nav-manifest.test.ts: extends the "every coach/both entry declares exactly
one hub" test to the new 8-hub set (adds 'messages'), and adds a permanent
lock (item 10 of the A2 task) asserting:
(b) every coach hub renders ≤3 subtabs — both the raw arrays and a
fully-capable head coach in every program type.
(c) every player hub renders ≤3 subtabs.
(d) every foldedUnder-marked registry entry still resolves to a hub via
resolveActiveHub (command palette + deep links stay live even without
their own hub-tab slot).
resolve-active-hub.test.ts: rewritten for the restructured hubs — Training
(performance) now resolves under Development instead of Stats & Performance;
Stats Center's Season/Upload/Import Center folds; Team's Operations folds
(Documents/Travel/Practice Planner/Practice Effectiveness); the new Messages
hub (Announcements moved off Team); Management's single Settings tab
(replacing the deleted 9-tab splice) plus Organization's Teams/Events folds;
Recruiting's Scouting folds and Pipeline's Interest fold. Behavior
intentionally changed per the ruling, so these tests were updated WITH the
change rather than left asserting the old structure.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LGqvhig8njW38q9PAwDXgu
…listic seed event times Readiness page dated check-ins in server UTC while the gate compared team-local (evening US submissions mis-dated, gate kept nagging). Command Center day + day bounds now team-local via the same contract-day helpers player-today already uses. Seed script events get real times (practice 15:30-17:30, game 13:00-16:00, meeting 12:00-13:00 ET) instead of the script's own wall-clock with zero duration. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LGqvhig8njW38q9PAwDXgu
…IT V, unclip KPI numerals, slash-line backfill Academics: is_eligible is boolean|null end-to-end; no record renders neutral 'Not on file', red Ineligible reserved for a real determination. Roster: EXIT V column removed (avg_exit_velocity doesn't exist in the live schema and has no write path). RuledStatLine: drop leading-none stacked over the text-ink token's own line-height (numeral clipping). New idempotent backfill script populated career_obp/slg/ops for 20/22 aggregate rows in prod using the app's tested computeCareerSlashLine logic. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LGqvhig8njW38q9PAwDXgu
…y real names buildBreadcrumbs extracted to a pure module with an id-shape guard (UUID/numeric segments fall back to the owning label, never title-cased). New BreadcrumbLabel override channel: players/[id] shows the player's name, stats/games/[gameId] the opponent, dev-plans/[id] the plan. 17 unit tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LGqvhig8njW38q9PAwDXgu
Null end_time renders start+1h (display only), events query bounded (90d back / 365d fwd, limit 500 under the PostgREST cap), count badges pluralize. Swallowed supabase insert errors in createBaseballEvent/createGame/box-score paths are now checked and surfaced (ActionResult.warning). E2E box-score spec deletes its own created games+events in teardown (prod was already purged of 160 junk rows). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LGqvhig8njW38q9PAwDXgu
…cal helm_lifting stack All six lift/performance routes (programs, program editor, groups, live weight room, player lift home, player session) repointed at src/components/lifting/*. Six legacy duplicates in components/baseball/performance/ deleted (GolfHelm palette, legacy baseball_lift_* writes). PlayerLiftToday comment corrected to helm_lifting_*. Coach edit access preserved via helm_lifting_org_viewers bridge migration (9/10 coaches; 10th has no org). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LGqvhig8njW38q9PAwDXgu
…lumns (applied to prod) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LGqvhig8njW38q9PAwDXgu
…cold-URL bounce Deleted: duplicate players/[id]/profile page (canonical = players/[id]), orphaned sidebar.tsx + team-switcher.tsx (zero importers — golf renders GolfSidebar), knip-confirmed orphans (MatchScoreBadge, match-calculator, dashboard-types), stale baseballhelm-* local scripts. Dev-plan page is now a server component (requireBaseballPlayerRoute) wrapping DevPlanClient — no more guard-race bounce to Today on hard navigation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LGqvhig8njW38q9PAwDXgu
…ber empty states, dev-plan chrome, warning toasts Removed duplicate page-level Header mounts (shell owns the masthead). All empty/error states on team-ops + player surfaces route through the kit (EmptyIssue/EditorsLetter) — ReadModelStateNotice de-ambered for its 6 call sites. DevPlanClient rebuilt on kit primitives with ≥44px touch targets. createBaseballEvent partial-failure warnings surface as toasts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LGqvhig8njW38q9PAwDXgu
…-portal revalidation, read-only dynamic groups Command Center week window now team-local AND respects forDate (was bare new Date()). Player snapshot cards fall back to career obp/slg/ops like avg. Lifting mutations revalidate the baseball routes that now render them (Wave C), not just /lifting. StrengthGroupsClient renders rule-managed dynamic groups first-class read-only (badge + rule summary, destructive affordances disabled) — both existing prod groups are dynamic. +15 unit tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LGqvhig8njW38q9PAwDXgu
…inventory docs Stat-layer manifest grandfathers the command-center test fixture (mirrors the already-grandfathered read-model); RosterFairway comment drops the literal deprecated-table name; product-trust documents contract asserts the structural EmptyIssue empty path + kit copy source instead of the pre-redesign string; inventory docs regenerated (same content as auto-PR #784). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LGqvhig8njW38q9PAwDXgu
…-grep gate) The box-score spec's cleanup client referenced SUPABASE_SERVICE_ROLE_KEY directly, tripping helmv3-no-service-role-key (e2e/ is in scan scope; scripts/ is the sanctioned exempt home, same as seed-baseball-e2e.ts). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LGqvhig8njW38q9PAwDXgu
…ge holes) get_users_with_auth() and get_platform_health_stats() were authenticated- executable with zero authorization — any logged-in user could dump every user's email + auth metadata. Both now gated with is_admin()/is_super_admin(), matching sibling admin RPCs. update_user_last_seen() constrained to self-or- admin; dead task-reminder RPCs, two CRM analytics RPCs, and the cron-only refresh_crm_coach_engagement had authenticated EXECUTE revoked. Verified each against app call sites; anon never had access. Applied to prod via MCP. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LGqvhig8njW38q9PAwDXgu
…write, reconcile type with table uploadStatsCSV wrote upload_batch_id (a column that never existed on baseball_player_stats) on every row → every CSV upload failed the insert. Removed the legacy write (import_run_id is the canonical linkage). Added the 7 real stat columns the type declared but the table lacked (caught_stealing, sacrifice_bunts, runs_allowed, pitches_thrown, strikes_thrown, launch_angle, spin_rate) additively; BaseballPlayerStats type now names the 6 real source_*/ import_run_id columns it was missing. Table and type now match exactly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LGqvhig8njW38q9PAwDXgu
…cord Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LGqvhig8njW38q9PAwDXgu
…07-08' into integration/final-verification-2026-07-08
…7-08' into integration/final-verification-2026-07-08 # Conflicts: # src/lib/types/database.ts
…sync, shell/routes/features pass 7-round verify+fix cycle (2026-07-08/09) across the baseball surface: - Box-score save path: RPC migration 20260708150000 replaces the banned DELETE-then-INSERT in save_baseball_full_box_score with per-row UPSERT (ON CONFLICT game_id,player_id) + targeted removal of dropped players; recalc loop isolated so one player's failure can't roll back the game; real error_detail/error_code surfaced. APPLIED to prod 2026-07-09. - Migration 20260708141000 committed as file only — header marked HOLD (golf function + invite-redemption gating risks); intentionally NOT applied. - Box-score UI: shared format-stat helper, EditGameModal, GameDetailHeader; stat display parity between box score, game detail, and player profiles. - Shell/nav: BaseballFairwayShell + nav-registry consistency, dead-route and stat-layer-manifest cleanup, exit-velocity phantom columns dropped from BaseballPlayerAggregates. - e2e + readiness matrix updated. Known pre-existing failure (also on origin/main, untouched by this branch): src/test/coachhelm/v3/chat-tools-visibility.test.ts — tracked for the production-readiness mission. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J2E46URHkhCnEQSYDtZLJX
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Too many files changed for review. ( Bypass the limit by tagging |
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Summary by CodeRabbit
WalkthroughThis PR adds Baseball docs/tooling updates, unifies the shell and hub model, migrates Lift Lab flows to ChangesDocs, Scripts & E2E Tooling
Baseball Shell, Breadcrumbs, and Hub Navigation
Header Removal and Living-Annual UI Migration
Timezone, Eligibility, Calendar, and Warning-Path Correctness
Lift Lab / Helm Lifting Unification
Box Score, Player Profile, and Stats Rendering Refactor
Estimated code review effort: 5 (Critical) | ~110 minutes Possibly related PRs
Suggested labels: Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 2 warnings)
✅ Passed checks (9 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
Owner
Author
|
@coderabbitai review Pushed 1297b57 addressing the review: 53 root-cause fixes across all Major + correctness findings (pagination cap, cross-New-Year season bug, team-local dates, clearable optional fields, error-vs-empty honesty, stuck spinner, breadcrumb collapse, teardown guard, token violations, test-coverage gaps — full breakdown in the commit message). Intentionally deferred, tracked in |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
git log --oneline -15
echo "---"
git show --stat 1297b579 | head -100
echo "---commit message---"
git show -s --format=%B 1297b579Length of output: 9597 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== backfill-baseball-slash-lines.ts pagination ==="
sed -n '1,220p' scripts/backfill-baseball-slash-lines.ts
echo "=== games.ts season_year ==="
rg -n "season_year|game_date" src/app/baseball/actions/games.ts | head -40
echo "=== EditGameModal clearable fields ==="
sed -n '1,220p' src/components/baseball/games/EditGameModal.tsx
echo "=== GamesList double fetch ==="
sed -n '1,80p' src/components/baseball/games/GamesList.tsx
echo "=== lift athlete context ==="
sed -n '1,140p' 'src/app/baseball/(dashboard)/dashboard/lift/_lift-athlete-context.ts'Length of output: 27823 The verification confirms the claimed fixes are substantively implemented and root-caused, not superficial patches. Spot-checking the key items:
These are root-cause fixes, not surface patches — good work. The deferred items (shared header-primitive extraction to W4 Living Annual masthead migration, readiness-matrix cleanup to W7) are reasonable scope calls given they're superseded by planned follow-on waves rather than being ignored. I'll kick off a fresh review pass over 1297b57 to catch anything the above spot-check didn't cover (e.g., the ✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (16)
src/app/baseball/(dashboard)/dashboard/performance/page.tsx (1)
163-168: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReadiness lookback window still computed in UTC — same bug class this diff exists to fix.
L165-168buildsfromDatefor the 7-day readiness check-in query usingnew Date()(server UTC), whileL238correctly switches to a team-localtodayfor the client. Both variables are namedtodaybut are different types/semantics (Datevs. team-local ISOstring) declared in overlapping scope — easy to conflate later.Net effect: for a team behind UTC, the readiness query's 7-day window can silently miss/include a day at the UTC boundary — exactly the class of bug
todayIsoInTz(resolveTeamTimezone(...))was introduced to eliminate, just not applied here.🐛 Proposed fix — compute team-local `today` once, reuse for the readiness window
- let readiness: BaseballReadinessSummary[] = []; - if (canViewReadiness && liftCtx) { - const today = new Date(); - const sevenDaysAgo = new Date(today); - sevenDaysAgo.setDate(today.getDate() - 7); - const fromDate = sevenDaysAgo.toISOString().slice(0, 10); + // Team-local "today" so both the readiness lookback window and the client's + // check-in day math can't drift across the UTC boundary for teams west of + // Greenwich. + const today = todayIsoInTz(await resolveTeamTimezone(supabase, teamId)); + + let readiness: BaseballReadinessSummary[] = []; + if (canViewReadiness && liftCtx) { + const sevenDaysAgo = new Date(`${today}T00:00:00Z`); + sevenDaysAgo.setUTCDate(sevenDaysAgo.getUTCDate() - 7); + const fromDate = sevenDaysAgo.toISOString().slice(0, 10);And drop the now-redundant
const today = todayIsoInTz(...)block at L236-239 since it's hoisted above.Also applies to: 236-239
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/baseball/`(dashboard)/dashboard/performance/page.tsx around lines 163 - 168, The readiness 7-day lookback window is still being derived from server UTC instead of the team’s local timezone, causing boundary-day mismatches. In dashboard/performance/page.tsx, update the readiness query setup around the canViewReadiness/liftCtx block to use the same team-local today value produced by todayIsoInTz(resolveTeamTimezone(...)) that is used later for the client view, and reuse that single value when computing fromDate. Remove the redundant later today declaration so there is one clear today value with consistent semantics across the readiness logic.src/app/lifting/actions/programs.ts (2)
219-224: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
updateProgramwas not actually migrated onto the new helper — despite the changelog claiming it was.The line-range change description for this block says
updateProgramnow callsrevalidateProgramPaths(programId), but the code still hand-rolls all 4revalidatePath()calls verbatim — the exact duplication the new helper exists to eliminate. Functionally equivalent today, but it defeats the DRY purpose of the refactor and will silently drift the momentrevalidateProgramPathsgains another path (e.g. a futureBASEBALL_LIVE_PATHbust on status change).♻️ Adopt the helper here too
- revalidatePath(`${LAB_PATH}/${programId}`); - revalidatePath(LAB_PATH); - revalidatePath(`${BASEBALL_PROGRAMS_PATH}/${programId}`); - revalidatePath(BASEBALL_PROGRAMS_PATH); + revalidateProgramPaths(programId);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/lifting/actions/programs.ts` around lines 219 - 224, `updateProgram` is still duplicating cache invalidation instead of using the shared helper. Update the `updateProgram` flow in `programs.ts` to call `revalidateProgramPaths(programId)` after a successful save, matching the refactor used elsewhere, and remove the four inline `revalidatePath()` calls so the invalidation logic stays centralized and consistent with future path additions.
601-614: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
publishProgrammutates program status but never revalidates the program's own detail route.L603-607 flips
prog.statusfromdrafttoactiveon first publish, but the revalidation block (L609-613) only busts the list routes (LAB_PATH,/lifting/dashboard/sessions,BASEBALL_PROGRAMS_PATH,BASEBALL_LIVE_PATH,BASEBALL_LIFT_PATH) — never${LAB_PATH}/${programId}or${BASEBALL_PROGRAMS_PATH}/${programId}. A coach who publishes from the program's own detail/builder page will keep seeing a staledraftstatus in the cached RSC payload until an unrelated navigation elsewhere triggers a refetch.updateProgram/duplicateLiftDay/duplicateLiftWeekall correctly bust the per-program detail route on status-affecting writes —publishProgramis the one mutation here that skips it.As per path instructions,
src/app/**/actions/**/*.tsrequires "Mutations must revalidatePath() the affected routes."🐛 Bust the program detail route on publish
- revalidatePath(LAB_PATH); - revalidatePath('/lifting/dashboard/sessions'); - revalidatePath(BASEBALL_PROGRAMS_PATH); - revalidatePath(BASEBALL_LIVE_PATH); - revalidatePath(BASEBALL_LIFT_PATH); + revalidateProgramPaths(programId); + revalidatePath('/lifting/dashboard/sessions'); + revalidatePath(BASEBALL_LIVE_PATH); + revalidatePath(BASEBALL_LIFT_PATH);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/lifting/actions/programs.ts` around lines 601 - 614, publishProgram updates the program from draft to active but only revalidates list routes, so add revalidation for the program’s own detail page after the status update. In the publishProgram flow, ensure revalidatePath is called for the affected per-program route(s) derived from LAB_PATH and BASEBALL_PROGRAMS_PATH using programId, alongside the existing list-route invalidations, so the cached builder/detail view reflects the new status immediately.Source: Path instructions
src/app/baseball/actions/calendar.ts (1)
182-224: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo zod validation on
CreateEventInputbefore it reaches Supabase.Per path instructions for
src/app/**/actions/**/*.ts: "Validate every input with zod before passing to Supabase."input.title,input.eventType,input.startDate, etc. flow straight into thefromUntyped(...).insert(...)call with no schema validation — an unvalidatedeventTypestring, for instance, can silently violate whatever CHECK constraintbaseball_events.event_typehas and surface as a raw sanitized DB error instead of a clean 400-style validation message. Pre-existing in this file (not introduced by this diff), but since the function is touched here, it's a reasonable point to add a zod schema (z.object({ title: z.string().min(1), eventType: z.enum([...]), startDate: z.string(), ... })) and parseinputat the top of the action.Based on path instructions: "Server action files. Every exported function MUST start with auth check... Validate every input with zod before passing to Supabase."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/baseball/actions/calendar.ts` around lines 182 - 224, The CreateEventInput action currently passes raw user input straight into the Supabase insert without zod validation, which violates the action input contract. Add a zod schema at the top of this exported action in createEvent/CreateEventInput flow, parse input before any use, and validate fields like title, eventType, startDate, startTime, endDate, timezoneOffset, and ids before calling fromUntyped(...).insert(...); keep the existing auth check and only proceed to Supabase after successful schema parsing.Source: Path instructions
src/app/baseball/(dashboard)/dashboard/dev-plans/[id]/page.tsx (1)
34-44: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAny thrown error is silently rendered as "plan not found."
fetchPlan'scatchblock (L38-41) setsnotFound(true)unconditionally regardless of whygetDevPlanForCoachthrew — a real permissions error, a network timeout, or an RLS denial gets the exact same "This development plan could not be found." copy as an actually-missing plan. This hides real failures from the coach and from anyone triaging a support ticket, and is the same class of error-vs-empty-state conflation the PR description says was fixed elsewhere in this batch — it wasn't fixed here.🐛 Proposed fix: distinguish real error from not-found
const [notFound, setNotFound] = useState(false); + const [fetchError, setFetchError] = useState<string | null>(null); const [isPending, startTransition] = useTransition(); const [pendingGoalId, setPendingGoalId] = useState<string | null>(null); const fetchPlan = useCallback(async () => { if (!params?.id) return; setLoading(true); try { const data = await getDevPlanForCoach(params.id); setPlan(data); setNotFound(false); + setFetchError(null); } catch (error) { console.error('Error fetching dev plan:', error); setPlan(null); - setNotFound(true); + // Only a genuine "no such plan / not visible to this coach" result + // should render the not-found state; anything else is a real error. + if (error instanceof DevPlanNotFoundError) { + setNotFound(true); + } else { + setFetchError(error instanceof Error ? error.message : 'Could not load this plan.'); + } } finally { setLoading(false); } }, [params?.id]);Then render a distinct error state (with a retry action) before the
notFound || !planbranch, instead of collapsing both into the same message.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/baseball/`(dashboard)/dashboard/dev-plans/[id]/page.tsx around lines 34 - 44, The fetchPlan error handling in the dashboard page is conflating real failures with a missing plan by unconditionally calling setNotFound(true) in the catch block. Update the page component around getDevPlanForCoach, fetchPlan, and the render logic so only an actual “not found” case sets notFound, while other errors set a separate error state and show a distinct retryable error UI before the notFound || !plan branch.src/app/baseball/actions/games.ts (1)
200-219: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winOrphaned calendar event when the game insert fails after the event was created.
The new fail-fast at Line 194-196 correctly prevents "a game the coach believes is on the calendar but isn't." The mirror case is unhandled: when
create_calendar_eventcreated the event (Line 170-188) but thebaseball_gamesinsert errors at Line 219, the event row survives — a calendar event with no backing game.importScheduleImplalready uses the rollback pattern (Line 1763) for exactly this.Only roll back the event we ourselves created (not a caller-supplied
input.event_id).🐛 Proposed fix
- if (error) return { success: false, error: sanitizeDbError(error, 'games') }; + if (error) { + // Roll back the calendar event we just created so we don't strand an + // event on the calendar with no backing game (mirrors importScheduleImpl). + if (input.create_calendar_event && !input.event_id && eventId) { + await supabase.from('baseball_events').delete().eq('id', eventId); + } + return { success: false, error: sanitizeDbError(error, 'games') }; + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/baseball/actions/games.ts` around lines 200 - 219, The game insert path in createScheduleGame leaves behind an orphaned calendar event when the baseball_games insert fails after create_calendar_event succeeds. Update the create flow around the insert/error handling in games.ts so that, if the insert returns an error and this request created the event itself, it rolls back that event before returning the sanitized error. Use the existing rollback pattern from importScheduleImpl and make sure you only delete the event created in this function, not any caller-supplied input.event_id.src/app/baseball/(dashboard)/dashboard/performance/programs/page.tsx (1)
62-82: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winWeek/day badge counts can truncate at the API row cap.
src/app/baseball/(dashboard)/dashboard/performance/programs/page.tsx:62-77— both.in()reads are unpaginated, so linkedhelm_lifting_weeks/helm_lifting_daysrows can silently stop at PostgREST’s 1000-row limit and undercountweek_count/day_count. Chunk the reads or switch to count-only aggregation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/baseball/`(dashboard)/dashboard/performance/programs/page.tsx around lines 62 - 82, The week/day badge counting logic in the programs page can silently undercount because the `fromUntyped(...).in(...)` queries for `helm_lifting_weeks` and `helm_lifting_days` are unpaginated and can hit the PostgREST row cap. Update the counting flow in the programs page so `weekCountByProgram` and `dayCountByProgram` are built from chunked queries or count-only aggregation rather than a single bulk fetch, and keep the existing `programByWeek`/`weeks` handling consistent while doing so.src/app/baseball/actions/signals.ts (1)
410-419: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAdd a DB-level dedupe constraint for NULL-program lift sessions.
src/app/baseball/actions/signals.ts:410-419only checks(athlete_id, scheduled_date, title)in app code, butsupabase/migrations/20260625000020_helm_lifting_data_sessions_readiness.sql:19-29only enforcesUNIQUE (program_assignment_id, athlete_id), which does not protectprogram_assignment_id IS NULLrows. Add a partial unique index on(athlete_id, scheduled_date, title)for NULL-program sessions and handle the unique-violation path instead of relying on.maybeSingle().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/baseball/actions/signals.ts` around lines 410 - 419, The dedupe in signals.ts is only enforced in app code for NULL-program sessions, so add a database-level partial unique index for helm_lifting_sessions on athlete_id, scheduled_date, and title where program_assignment_id IS NULL in the readiness migration. Then update the conversion flow in signals.ts to rely on that constraint by catching the unique-violation path instead of using .maybeSingle() as the only guard, keeping the existing dedupe logic around the createLiftAssignment/session creation path.src/app/baseball/(dashboard)/dashboard/lift/page.tsx (1)
12-13: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winApply the same split-query fix in
src/app/lifting/(dashboard)/dashboard/lift/page.tsx:103-126. It still uses one combinedor(...)query with.limit(20)and a post-filter, so capped results can drop overdue open sessions. Extract the session read into a shared helper to keep both lift pages aligned.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/baseball/`(dashboard)/dashboard/lift/page.tsx around lines 12 - 13, The lift dashboard still uses a single combined or(...) query with .limit(20) and a post-filter, which can omit overdue open sessions when the cap is hit. Update the `src/app/lifting/(dashboard)/dashboard/lift/page.tsx` flow to use the same split-query approach as the other lift page by separating the session read and readiness check reads, then merging results in code. Factor the session fetch logic into a shared helper so both `page.tsx` implementations stay aligned and reuse the same overdue-session handling.src/app/baseball/(dashboard)/dashboard/performance/programs/[programId]/page.tsx (1)
210-231: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
getAssignContextstill swallows query failures — inconsistent with the new honest-error handling.
getProgramTreenow surfaces DB/RLS failures, but the newly-introducedgetAssignContextdiscardserroron both queries (athletes ?? [],groups ?? []). A failed athletes/groups read renders an empty assign roster with no signal, silently blocking the coach from assigning the program. At minimum, log the errors so the failure is observable.🛠️ Proposed fix (minimal: log on failure)
- const [{ data: athletes }, { data: groups }] = await Promise.all([ + const [{ data: athletes, error: athletesError }, { data: groups, error: groupsError }] = await Promise.all([ fromUntyped(supabase, 'helm_lifting_athletes') .select('id, first_name, last_name, position, sport') .eq('organization_id', organizationId) .eq('team_id', teamId) .eq('is_active', true) .order('last_name', { ascending: true }) - .limit(500) as Promise<{ data: Array<Pick<HelmLiftingAthleteRow, 'id' | 'first_name' | 'last_name' | 'position' | 'sport'>> | null }>, + .limit(500) as Promise<{ data: Array<Pick<HelmLiftingAthleteRow, 'id' | 'first_name' | 'last_name' | 'position' | 'sport'>> | null; error: unknown }>, fromUntyped(supabase, 'helm_lifting_groups') .select('id, name, group_type') .eq('organization_id', organizationId) .eq('team_id', teamId) .eq('is_active', true) .order('name', { ascending: true }) - .limit(100) as Promise<{ data: Array<Pick<HelmLiftingGroupRow, 'id' | 'name' | 'group_type'>> | null }>, + .limit(100) as Promise<{ data: Array<Pick<HelmLiftingGroupRow, 'id' | 'name' | 'group_type'>> | null; error: unknown }>, ]); + + if (athletesError || groupsError) { + await logServerError( + `[performance/programs] getAssignContext query failed: ${ + (athletesError as Error)?.message ?? (groupsError as Error)?.message ?? 'unknown' + }`, + { action: 'baseball.programEditor.getAssignContext', metadata: { organizationId, teamId } }, + ); + } return { athletes: athletes ?? [], groups: groups ?? [] };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/baseball/`(dashboard)/dashboard/performance/programs/[programId]/page.tsx around lines 210 - 231, `getAssignContext` is still hiding failures from the athletes/groups queries, so make the error observable instead of returning silent empty arrays. In `getAssignContext`, inspect the results from the two `fromUntyped(...).select(...)` calls and, when either query returns an `error`, log it with enough context to identify whether it came from `helm_lifting_athletes` or `helm_lifting_groups`. Keep the existing `getAssignContext` shape, but avoid swallowing the failure without any signal so the assignment UI can be debugged consistently with `getProgramTree`.src/app/baseball/(dashboard)/_components/breadcrumbs.ts (1)
1-92: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd unit tests for this module — it's explicitly designed to be unit-tested, and it already had a bug slip through review once.
The header comment (lines 5-8) states this file was split out specifically "so this logic can be unit-tested directly without dragging in the whole shell's client-only dependency graph." No
breadcrumbs.test.tsis in this changeset. GivenbuildBreadcrumbs's hub-vs-subpage label logic was already wrong once (the settings-collapse bug fixed at lines 80-91), lock in the fix with cases like:
/dashboard/settings→Settings(hub landing page,best.labelused)/dashboard/settings/permissions→Permissions(subpage,toTitle(lastSegment)used)/dashboard/players/<uuid>→ parent-based fallback, never the raw UUID/dashboard/players/<uuid>/statswithoverrideLabel→ override wins- a numeric segment case per the sibling comment on
isIdShapedSegmentWant me to draft the test file (
toTitle/singularize/isIdShapedSegment/buildBreadcrumbs, table-driven) as a follow-up?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/baseball/`(dashboard)/_components/breadcrumbs.ts around lines 1 - 92, Add a unit test file for breadcrumbs.ts to lock in the pure breadcrumb behavior that this module was split out for. Cover buildBreadcrumbs, toTitle, singularize, and isIdShapedSegment with table-driven cases: hub landing pages should use best.label, deeper subpages should use the trailing segment title-cased, UUID/numeric segments should never be exposed raw, parent-based fallback should apply for id-shaped routes, and overrideLabel should win when present. Reference the existing buildBreadcrumbs and isIdShapedSegment symbols so the tests stay aligned with the current routing logic.src/app/baseball/(dashboard)/dashboard/calendar/page.tsx (1)
75-84: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCalendar query is still capped at 500 rows
src/app/baseball/(dashboard)/dashboard/calendar/page.tsx:95-101— teams with >500 events in the 455-day window will silently drop later rows. The repo’s Supabase config already allows 1000 rows, so raise this limit to 1000 or switch to explicit chunking if you need more.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/baseball/`(dashboard)/dashboard/calendar/page.tsx around lines 75 - 84, The calendar data fetch in the page component is still limited to 500 rows, which can drop events inside the bounded window. Update the query in the dashboard calendar page’s event-loading logic to use the repo-supported 1000-row cap, or replace the hard cap with explicit chunking if the result set may exceed that; keep the existing window logic in sync with the fetch in the same calendar page flow.src/app/baseball/(dashboard)/dashboard/players/[id]/page.tsx (1)
109-133: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winSurface season-stat query errors instead of blank stats
src/app/baseball/actions/games.ts:1399-1447ignores Supabaseerrorvalues and still returnssuccess: true, sosrc/app/baseball/(dashboard)/dashboard/players/[id]/page.tsx:210-212can only rendernull/[]and can’t distinguish “no stats yet” from “stats query failed.” Return{ success: false, error }on those query failures;withAdminObservedwill then emit the failure.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/baseball/`(dashboard)/dashboard/players/[id]/page.tsx around lines 109 - 133, The season-stats fetch in getPlayerSeasonStats is swallowing Supabase errors and returning success as if the query worked, which makes the player page in page.tsx render blank stats without knowing it failed. Update the action to return success: false with the underlying error whenever the stats query fails, and keep the normal success path only for real results so withAdminObserved can log the failure and the dashboard can distinguish “no stats” from “query error.”src/app/baseball/(dashboard)/dashboard/performance/live/page.tsx (1)
123-198: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winResidual: post-sessions query errors are still swallowed into a degraded board.
sessionsErrornow correctly returns{ ok: false }(line 97-103), butsessionExercisesError(line 127) and thePromise.allbatch errors (athletesError,setResultsError, etc., logged at 191-198) are only logged. Ifhelm_lifting_session_exercisesRLS/transient-fails whilehelm_lifting_sessionssucceeds,exerciseIdsis[], set-results is skipped, and every athlete renders withexercises: [],current_exercise: null,total_exercises: 0(line 259-261) — a live board where no coach can see or log a set, indistinguishable from "athletes have no assigned work." That's the same error-vs-empty conflation this PR is standardizing away, applied inconsistently one query later.At minimum, treat
sessionExercisesErrorlikesessionsError(it's the load-bearing query for the write surface):🛡️ Proposed fix
await logQueryError('helm_lifting_session_exercises', sessionExercisesError, teamId); + + if (sessionExercisesError) { + // Same rationale as sessionsError: a hard failure here yields an + // athlete grid with zero loggable exercises — surface it, don't degrade. + return { ok: false }; + }Consider extending the same treatment to the athletes query in the batch (a null
athletesmap silently drops names/positions).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/baseball/`(dashboard)/dashboard/performance/live/page.tsx around lines 123 - 198, The live board is still collapsing query failures into empty data, especially for the load-bearing `sessionExercisesError` in the `page.tsx` data fetch. Update the `load`/query path around `sessionExercisesError` and the `Promise.all` batch so a failed `helm_lifting_session_exercises` request returns a non-success state instead of continuing with `exerciseIds = []`; also consider treating `athletesError` the same way so missing athletes data is not silently rendered as empty rows. Use the existing `sessionsError` handling pattern and the identified symbols `sessionExercisesError`, `Promise.all`, and `logQueryError` to keep the failure visible.e2e/baseball-box-score.spec.ts (1)
397-415: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
/baseball/dashboard/stats/seasonis no longer a supported routee2e/baseball-box-score.spec.ts:397-415
page.goto('/baseball/dashboard/stats/season')at:402still targets a 404. The stats flow now lives at/baseball/dashboard/stats-center(with/baseball/dashboard/statsredirecting there), so this block should be repointed or removed if season stats is no longer intended.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/baseball-box-score.spec.ts` around lines 397 - 415, The Coach - Season Stats Dashboard test block is still navigating to a removed route and should be updated. In the `test.beforeEach` for `Coach - Season Stats Dashboard`, replace the `page.goto` target from the old season stats path to the supported stats center route used by the app, and keep the assertions aligned with the new page content; if the season-specific coverage is no longer valid, remove the `should display...` and `should show...` tests from this `describe` block instead of keeping a 404 target.src/components/baseball/player-profile/PlayerProfileClient.tsx (1)
196-202: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winParse baseball
game_datevalues as local midnight
src/components/baseball/player-profile/PlayerProfileClient.tsx:196-202usesnew Date(iso)forstat.game?.game_date(:1108) and trend labels (:396).baseball_games.game_dateis adatecolumn (supabase/migrations/20260527000000_prod_public_baseline.sql:7730), soYYYY-MM-DDvalues can render a day early in US timezones. Match the existing baseball date pattern used insrc/components/baseball/box-score/BoxScoreView.tsx:25andsrc/components/baseball/box-score/BoxScoreEntry.tsx:221.Suggested fix
function formatDate(iso: string): string { - return new Date(iso).toLocaleDateString('en-US', { month: 'short', day: 'numeric', year: 'numeric' }); + const d = /^\d{4}-\d{2}-\d{2}$/.test(iso) ? new Date(`${iso}T00:00:00`) : new Date(iso); + return d.toLocaleDateString('en-US', { month: 'short', day: 'numeric', year: 'numeric' }); } function formatShortDate(iso: string): string { - return new Date(iso).toLocaleDateString('en-US', { month: 'short', day: 'numeric' }); + const d = /^\d{4}-\d{2}-\d{2}$/.test(iso) ? new Date(`${iso}T00:00:00`) : new Date(iso); + return d.toLocaleDateString('en-US', { month: 'short', day: 'numeric' }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/baseball/player-profile/PlayerProfileClient.tsx` around lines 196 - 202, The baseball date formatting helpers in PlayerProfileClient treat YYYY-MM-DD values as UTC, which can shift game_date labels by a day in US timezones. Update formatDate and formatShortDate to parse baseball game dates as local midnight using the same pattern already used in BoxScoreView and BoxScoreEntry, and keep the call sites for stat.game?.game_date and the trend label formatting unchanged except for using the corrected helpers.
♻️ Duplicate comments (4)
src/app/baseball/actions/calendar.ts (1)
7-7: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBoth prior review findings are now fixed.
- Soft-failure logging:
logServerError(..., 'warning')is now called for both the attendance-insert failure (246-254) and the linked-game-insert failure (272-280), so these no longer disappear once the toast is dismissed.- Separator bug:
warnings.join('; ')(289) no longer garbles concatenated messages when both secondary writes fail.One item from the prior review is still outstanding: no test asserts
createBaseballEventreturns{ success: true, warning: '...' }whenbaseball_event_attendanceorbaseball_gamesinserts fail.EventsClient.tsxbranches onresult.warning, so a regression here (e.g. reverting the join separator, or dropping alogServerErrorcall) wouldn't be caught bytsc/lint alone.Also applies to: 73-84, 225-233, 242-260, 271-290
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/baseball/actions/calendar.ts` at line 7, Add a regression test for createBaseballEvent to verify that when either the baseball_event_attendance insert or the baseball_games insert fails, the function still returns { success: true, warning: '...' } and preserves the warning text used by EventsClient.tsx. Cover both single-failure and dual-failure cases so the warning aggregation path (including warnings.join('; ')) and the logServerError calls remain protected from regressions.src/app/baseball/(dashboard)/dashboard/dev-plans/[id]/page.tsx (1)
90-102: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winHeader block still copy-pasted 4x — not extracted.
The focus-visible ring was added to all four
Linkinstances (good — addresses half of the prior ask), but the 13-line back-link/title/subtitle block at lines 90-102, 113-125, 140-152, and 168-180 remains a verbatim quadruple copy-paste. Any future wording/behavior change needs 4 synchronized edits.Extract a local
DevPlanHeadercomponent and render it in each of the 4 branches — same fix proposed previously, still applicable to this revision.Also applies to: 113-125, 140-152, 168-180
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/baseball/`(dashboard)/dashboard/dev-plans/[id]/page.tsx around lines 90 - 102, The back-link/title/subtitle header block in the dev plan page is still duplicated across the four conditional branches, so extract it into a local DevPlanHeader component and reuse it everywhere. Move the repeated Link + heading markup into that component and render it from each branch in page.tsx, keeping the existing focus-visible styles intact.src/app/baseball/(dashboard)/dashboard/academics/AcademicsClient.tsx (1)
234-237: 📐 Maintainability & Code Quality | 🔵 TrivialHeader duplication still unresolved — text drift is fixed, but the root cause isn't.
The previous review flagged this exact 4x-copy-pasted header block because the text had drifted between states. That drift is now fixed (all four now say
"Track student-athlete academic progress and eligibility"), but the underlying problem — no shared header component, four independent copies of the same markup — is still there (theHeaderimport was removed and replaced by these inline copies). The next edit to this copy will silently re-introduce drift the same way the last one did.♻️ Extract a shared header (still applicable)
+function AcademicsHeader() { + return ( + <div className="border-b border-warm-200/60 px-6 pb-5 pt-6 lg:px-8 lg:pt-8"> + <h1 className="text-h2 font-semibold text-warm-900">Academics</h1> + <p className="mt-1 text-body-sm text-warm-500"> + Track student-athlete academic progress and eligibility + </p> + </div> + ); +}Replace each of the 4 inline blocks (234-237, 253-256, 310-313, 368-371) with
<AcademicsHeader />.Also applies to: 253-256, 310-313, 368-371
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/baseball/`(dashboard)/dashboard/academics/AcademicsClient.tsx around lines 234 - 237, The Academics page still has four duplicated inline header blocks in AcademicsClient instead of using a shared component, which leaves the copy prone to drift. Restore a reusable header by extracting the repeated markup into AcademicsHeader and use it in AcademicsClient for each of the four existing header sections, replacing the inline h1/p blocks with the shared component so there is one source of truth.src/app/baseball/(dashboard)/dashboard/program/page.tsx (1)
214-217: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHeader block is still duplicated 3x — extraction not applied.
The diff only fixed the flex-wrap on line 251; the previously-flagged copy-paste of the
border-b … px-6 pb-5 pt-6 …header shell across the coach-access, no-org, and main branches (lines 214-217, 232-235, 251-262) is unchanged. Same drift risk as before: a future style tweak has to be applied in 3 places, and a 4th branch could easily reintroduce the missing-flex-wrapbug this PR just fixed on one of the three.♻️ Proposed local extraction
+function ProgramPageHeader({ + subtitle, + action, +}: { subtitle: string; action?: React.ReactNode }) { + return ( + <div className="flex flex-wrap items-start justify-between gap-x-4 gap-y-3 border-b border-warm-200/60 px-6 pb-5 pt-6 lg:px-8 lg:pt-8"> + <div> + <h1 className="text-h2 font-semibold text-warm-900">Program Profile</h1> + <p className="mt-1 text-body-sm text-warm-500">{subtitle}</p> + </div> + {action} + </div> + ); +}Then replace each inline
<div>block at 214-217, 232-235, and 251-262 with<ProgramPageHeader subtitle="..." action={...} />.Also applies to: 232-235, 251-262
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/baseball/`(dashboard)/dashboard/program/page.tsx around lines 214 - 217, The `ProgramPage` header shell is still duplicated across the coach-access, no-org, and main branches instead of being extracted. Move the shared `border-b ... px-6 pb-5 pt-6 ...` layout into a reusable `ProgramPageHeader` component in `page.tsx` and replace each inline header block with that component, passing the appropriate subtitle and action/content for each branch so future style changes only need one edit.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/baseball/`(dashboard)/_components/breadcrumbs.ts:
- Around line 89-91: The breadcrumb label logic in breadcrumbs.ts still uses a
non-null assertion on best when computing the label. Refactor the
isHubLandingPage/label flow in the breadcrumbs builder so the best !== null
check is kept in the same expression that accesses best.label, removing best!
entirely while preserving the same Dashboard and overrideLabel behavior.
In `@src/app/baseball/`(dashboard)/dashboard/lift/_lift-athlete-context.ts:
- Around line 38-44: The doc comment in _lift-athlete-context overstates the
dual-membership case as “JUCO + Showcase,” which conflicts with the Baseball
Product user-type matrix. Update the comment above the membership lookup logic
to describe the correct dual-team-capable player types (High School with
optional Showcase, Showcase with optional High School) while keeping the
explanation for returning the full ordered list and iterating through
memberships in place.
In `@src/app/baseball/`(dashboard)/dashboard/scouting/page.tsx:
- Around line 78-96: The `SectionMasthead` plus descriptive paragraph is
duplicated in both the null-context branch and the main return in
`ScoutingPage`; hoist that shared JSX into a local const and reuse it in both
places so the copy stays in sync. Update the `ScoutingPage` render path to
reference the shared masthead content instead of repeating the same block in
each branch.
In `@src/app/baseball/`(dashboard)/dashboard/stats/games/page.tsx:
- Around line 53-60: The season-record fetch failure in the dashboard page is
being ignored, while GamesList only understands the games-level initialError
banner. Update the page component that builds the props for GamesList to carry a
separate record-specific error state such as initialRecordError/recordError
instead of reusing initialError, and wire GamesList to render or otherwise
surface that distinct season-record failure without showing “Couldn't load
games.” Use the existing symbols recordResult, initialError, and GamesList to
locate the affected prop flow.
- Around line 45-60: The season-record fetch failure is only being logged in the
page component and never reaches the UI, so `recordResult` errors are hidden
while `gamesResult` errors are shown through `initialError`. Update `page.tsx`
in the `GamesPage` flow to carry a separate record-error state (or merge it into
the existing error handling) and pass it into the header/list rendering path
alongside `GamesList`, using the existing `getTeamSeasonRecord`, `getTeamGames`,
and `initialError` wiring so DB/RLS failures surface visibly instead of being
silently ignored.
In `@src/components/baseball/settings/PhilosophySettingsClient.tsx`:
- Line 247: The settings route is hardcoded in multiple places in
PhilosophySettingsClient, so extract a shared SETTINGS_PATH constant for
'/baseball/dashboard/settings' and use it in the router.push call and the other
matching reference to prevent drift if the route changes.
---
Outside diff comments:
In `@e2e/baseball-box-score.spec.ts`:
- Around line 397-415: The Coach - Season Stats Dashboard test block is still
navigating to a removed route and should be updated. In the `test.beforeEach`
for `Coach - Season Stats Dashboard`, replace the `page.goto` target from the
old season stats path to the supported stats center route used by the app, and
keep the assertions aligned with the new page content; if the season-specific
coverage is no longer valid, remove the `should display...` and `should show...`
tests from this `describe` block instead of keeping a 404 target.
In `@src/app/baseball/`(dashboard)/_components/breadcrumbs.ts:
- Around line 1-92: Add a unit test file for breadcrumbs.ts to lock in the pure
breadcrumb behavior that this module was split out for. Cover buildBreadcrumbs,
toTitle, singularize, and isIdShapedSegment with table-driven cases: hub landing
pages should use best.label, deeper subpages should use the trailing segment
title-cased, UUID/numeric segments should never be exposed raw, parent-based
fallback should apply for id-shaped routes, and overrideLabel should win when
present. Reference the existing buildBreadcrumbs and isIdShapedSegment symbols
so the tests stay aligned with the current routing logic.
In `@src/app/baseball/`(dashboard)/dashboard/calendar/page.tsx:
- Around line 75-84: The calendar data fetch in the page component is still
limited to 500 rows, which can drop events inside the bounded window. Update the
query in the dashboard calendar page’s event-loading logic to use the
repo-supported 1000-row cap, or replace the hard cap with explicit chunking if
the result set may exceed that; keep the existing window logic in sync with the
fetch in the same calendar page flow.
In `@src/app/baseball/`(dashboard)/dashboard/dev-plans/[id]/page.tsx:
- Around line 34-44: The fetchPlan error handling in the dashboard page is
conflating real failures with a missing plan by unconditionally calling
setNotFound(true) in the catch block. Update the page component around
getDevPlanForCoach, fetchPlan, and the render logic so only an actual “not
found” case sets notFound, while other errors set a separate error state and
show a distinct retryable error UI before the notFound || !plan branch.
In `@src/app/baseball/`(dashboard)/dashboard/lift/page.tsx:
- Around line 12-13: The lift dashboard still uses a single combined or(...)
query with .limit(20) and a post-filter, which can omit overdue open sessions
when the cap is hit. Update the
`src/app/lifting/(dashboard)/dashboard/lift/page.tsx` flow to use the same
split-query approach as the other lift page by separating the session read and
readiness check reads, then merging results in code. Factor the session fetch
logic into a shared helper so both `page.tsx` implementations stay aligned and
reuse the same overdue-session handling.
In `@src/app/baseball/`(dashboard)/dashboard/performance/live/page.tsx:
- Around line 123-198: The live board is still collapsing query failures into
empty data, especially for the load-bearing `sessionExercisesError` in the
`page.tsx` data fetch. Update the `load`/query path around
`sessionExercisesError` and the `Promise.all` batch so a failed
`helm_lifting_session_exercises` request returns a non-success state instead of
continuing with `exerciseIds = []`; also consider treating `athletesError` the
same way so missing athletes data is not silently rendered as empty rows. Use
the existing `sessionsError` handling pattern and the identified symbols
`sessionExercisesError`, `Promise.all`, and `logQueryError` to keep the failure
visible.
In `@src/app/baseball/`(dashboard)/dashboard/performance/page.tsx:
- Around line 163-168: The readiness 7-day lookback window is still being
derived from server UTC instead of the team’s local timezone, causing
boundary-day mismatches. In dashboard/performance/page.tsx, update the readiness
query setup around the canViewReadiness/liftCtx block to use the same team-local
today value produced by todayIsoInTz(resolveTeamTimezone(...)) that is used
later for the client view, and reuse that single value when computing fromDate.
Remove the redundant later today declaration so there is one clear today value
with consistent semantics across the readiness logic.
In
`@src/app/baseball/`(dashboard)/dashboard/performance/programs/[programId]/page.tsx:
- Around line 210-231: `getAssignContext` is still hiding failures from the
athletes/groups queries, so make the error observable instead of returning
silent empty arrays. In `getAssignContext`, inspect the results from the two
`fromUntyped(...).select(...)` calls and, when either query returns an `error`,
log it with enough context to identify whether it came from
`helm_lifting_athletes` or `helm_lifting_groups`. Keep the existing
`getAssignContext` shape, but avoid swallowing the failure without any signal so
the assignment UI can be debugged consistently with `getProgramTree`.
In `@src/app/baseball/`(dashboard)/dashboard/performance/programs/page.tsx:
- Around line 62-82: The week/day badge counting logic in the programs page can
silently undercount because the `fromUntyped(...).in(...)` queries for
`helm_lifting_weeks` and `helm_lifting_days` are unpaginated and can hit the
PostgREST row cap. Update the counting flow in the programs page so
`weekCountByProgram` and `dayCountByProgram` are built from chunked queries or
count-only aggregation rather than a single bulk fetch, and keep the existing
`programByWeek`/`weeks` handling consistent while doing so.
In `@src/app/baseball/`(dashboard)/dashboard/players/[id]/page.tsx:
- Around line 109-133: The season-stats fetch in getPlayerSeasonStats is
swallowing Supabase errors and returning success as if the query worked, which
makes the player page in page.tsx render blank stats without knowing it failed.
Update the action to return success: false with the underlying error whenever
the stats query fails, and keep the normal success path only for real results so
withAdminObserved can log the failure and the dashboard can distinguish “no
stats” from “query error.”
In `@src/app/baseball/actions/calendar.ts`:
- Around line 182-224: The CreateEventInput action currently passes raw user
input straight into the Supabase insert without zod validation, which violates
the action input contract. Add a zod schema at the top of this exported action
in createEvent/CreateEventInput flow, parse input before any use, and validate
fields like title, eventType, startDate, startTime, endDate, timezoneOffset, and
ids before calling fromUntyped(...).insert(...); keep the existing auth check
and only proceed to Supabase after successful schema parsing.
In `@src/app/baseball/actions/games.ts`:
- Around line 200-219: The game insert path in createScheduleGame leaves behind
an orphaned calendar event when the baseball_games insert fails after
create_calendar_event succeeds. Update the create flow around the insert/error
handling in games.ts so that, if the insert returns an error and this request
created the event itself, it rolls back that event before returning the
sanitized error. Use the existing rollback pattern from importScheduleImpl and
make sure you only delete the event created in this function, not any
caller-supplied input.event_id.
In `@src/app/baseball/actions/signals.ts`:
- Around line 410-419: The dedupe in signals.ts is only enforced in app code for
NULL-program sessions, so add a database-level partial unique index for
helm_lifting_sessions on athlete_id, scheduled_date, and title where
program_assignment_id IS NULL in the readiness migration. Then update the
conversion flow in signals.ts to rely on that constraint by catching the
unique-violation path instead of using .maybeSingle() as the only guard, keeping
the existing dedupe logic around the createLiftAssignment/session creation path.
In `@src/app/lifting/actions/programs.ts`:
- Around line 219-224: `updateProgram` is still duplicating cache invalidation
instead of using the shared helper. Update the `updateProgram` flow in
`programs.ts` to call `revalidateProgramPaths(programId)` after a successful
save, matching the refactor used elsewhere, and remove the four inline
`revalidatePath()` calls so the invalidation logic stays centralized and
consistent with future path additions.
- Around line 601-614: publishProgram updates the program from draft to active
but only revalidates list routes, so add revalidation for the program’s own
detail page after the status update. In the publishProgram flow, ensure
revalidatePath is called for the affected per-program route(s) derived from
LAB_PATH and BASEBALL_PROGRAMS_PATH using programId, alongside the existing
list-route invalidations, so the cached builder/detail view reflects the new
status immediately.
In `@src/components/baseball/player-profile/PlayerProfileClient.tsx`:
- Around line 196-202: The baseball date formatting helpers in
PlayerProfileClient treat YYYY-MM-DD values as UTC, which can shift game_date
labels by a day in US timezones. Update formatDate and formatShortDate to parse
baseball game dates as local midnight using the same pattern already used in
BoxScoreView and BoxScoreEntry, and keep the call sites for stat.game?.game_date
and the trend label formatting unchanged except for using the corrected helpers.
---
Duplicate comments:
In `@src/app/baseball/`(dashboard)/dashboard/academics/AcademicsClient.tsx:
- Around line 234-237: The Academics page still has four duplicated inline
header blocks in AcademicsClient instead of using a shared component, which
leaves the copy prone to drift. Restore a reusable header by extracting the
repeated markup into AcademicsHeader and use it in AcademicsClient for each of
the four existing header sections, replacing the inline h1/p blocks with the
shared component so there is one source of truth.
In `@src/app/baseball/`(dashboard)/dashboard/dev-plans/[id]/page.tsx:
- Around line 90-102: The back-link/title/subtitle header block in the dev plan
page is still duplicated across the four conditional branches, so extract it
into a local DevPlanHeader component and reuse it everywhere. Move the repeated
Link + heading markup into that component and render it from each branch in
page.tsx, keeping the existing focus-visible styles intact.
In `@src/app/baseball/`(dashboard)/dashboard/program/page.tsx:
- Around line 214-217: The `ProgramPage` header shell is still duplicated across
the coach-access, no-org, and main branches instead of being extracted. Move the
shared `border-b ... px-6 pb-5 pt-6 ...` layout into a reusable
`ProgramPageHeader` component in `page.tsx` and replace each inline header block
with that component, passing the appropriate subtitle and action/content for
each branch so future style changes only need one edit.
In `@src/app/baseball/actions/calendar.ts`:
- Line 7: Add a regression test for createBaseballEvent to verify that when
either the baseball_event_attendance insert or the baseball_games insert fails,
the function still returns { success: true, warning: '...' } and preserves the
warning text used by EventsClient.tsx. Cover both single-failure and
dual-failure cases so the warning aggregation path (including warnings.join(';
')) and the logServerError calls remain protected from regressions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5acffa08-3bea-4b94-a9f3-671abc9a573b
📒 Files selected for processing (51)
docs/audits/PRODUCTION_READINESS_MISSION_2026-07-09.mde2e/baseball-box-score.spec.tsscripts/backfill-baseball-slash-lines.tsscripts/e2e-supabase-admin.tssrc/app/baseball/(dashboard)/_components/breadcrumbs.tssrc/app/baseball/(dashboard)/dashboard/academics/AcademicsClient.tsxsrc/app/baseball/(dashboard)/dashboard/calendar/page.tsxsrc/app/baseball/(dashboard)/dashboard/dev-plan/DevPlanClient.tsxsrc/app/baseball/(dashboard)/dashboard/dev-plans/[id]/page.tsxsrc/app/baseball/(dashboard)/dashboard/documents/page.tsxsrc/app/baseball/(dashboard)/dashboard/events/EventsClient.tsxsrc/app/baseball/(dashboard)/dashboard/lift/[sessionId]/page.tsxsrc/app/baseball/(dashboard)/dashboard/lift/_lift-athlete-context.tssrc/app/baseball/(dashboard)/dashboard/lift/page.tsxsrc/app/baseball/(dashboard)/dashboard/performance/builder/__tests__/page.test.tsxsrc/app/baseball/(dashboard)/dashboard/performance/groups/page.tsxsrc/app/baseball/(dashboard)/dashboard/performance/live/page.tsxsrc/app/baseball/(dashboard)/dashboard/performance/page.tsxsrc/app/baseball/(dashboard)/dashboard/performance/programs/[programId]/page.tsxsrc/app/baseball/(dashboard)/dashboard/performance/programs/page.tsxsrc/app/baseball/(dashboard)/dashboard/players/[id]/page.tsxsrc/app/baseball/(dashboard)/dashboard/program/page.tsxsrc/app/baseball/(dashboard)/dashboard/readiness/__tests__/page.test.tsxsrc/app/baseball/(dashboard)/dashboard/scouting/page.tsxsrc/app/baseball/(dashboard)/dashboard/stats/games/page.tsxsrc/app/baseball/actions/__tests__/player-today-lift-timezone.test.tssrc/app/baseball/actions/calendar.tssrc/app/baseball/actions/games.tssrc/app/baseball/actions/signals.tssrc/app/lifting/actions/__tests__/groups.test.tssrc/app/lifting/actions/__tests__/player-sessions.test.tssrc/app/lifting/actions/__tests__/sessions.test.tssrc/app/lifting/actions/groups.tssrc/app/lifting/actions/programs.tssrc/app/lifting/actions/sessions.tssrc/components/baseball/calendar/CalendarFairway.tsxsrc/components/baseball/documents/DocumentsFairway.tsxsrc/components/baseball/games/EditGameModal.tsxsrc/components/baseball/games/GameDetailHeader.tsxsrc/components/baseball/games/GamesList.tsxsrc/components/baseball/player-profile/PlayerProfileClient.tsxsrc/components/baseball/settings/ImportSourcesClient.tsxsrc/components/baseball/settings/PhilosophySettingsClient.tsxsrc/components/baseball/stats-center/StatsCenterClient.tsxsrc/components/baseball/travel/TravelClient.tsxsrc/components/lifting/programs/ProgramListClient.tsxsrc/lib/baseball/__tests__/nav-manifest.test.tssrc/lib/baseball/__tests__/resolve-active-hub.test.tssrc/lib/types/index.tssrc/test/coachhelm/v3/chat-tools-visibility.test.tssupabase/migrations/20260708141900_baseball_event_source_columns_converge.sql
💤 Files with no reviewable changes (1)
- src/app/baseball/(dashboard)/dashboard/documents/page.tsx
Comment on lines
+78
to
+96
| return ( | ||
| <div className="mx-auto w-full max-w-5xl px-4 py-8 sm:px-6 lg:px-8"> | ||
| <SectionMasthead eyebrow="THE WAR ROOM · SCOUTING" title="Scouting" ink="pursuit"> | ||
| <p className="max-w-2xl font-annual text-body text-text-secondary"> | ||
| The evaluation toolkit — shortlists, side-by-side comparisons, and exportable packets — | ||
| once a prospect is worth a closer look. | ||
| </p> | ||
| </SectionMasthead> | ||
| <PaperCard className="mt-8 p-6"> | ||
| <Eyebrow ink="pursuit">Unable to load your team</Eyebrow> | ||
| <p className="mt-2 font-annual text-body text-text-secondary"> | ||
| We couldn't resolve your active team just now — this isn't a permissions | ||
| issue. Refresh the page, or switch teams from the sidebar if you coach more than one | ||
| program. | ||
| </p> | ||
| </PaperCard> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Dedupe the repeated masthead block.
Lines 80-85 (null-context branch) and 105-110 (main return) render an identical SectionMasthead/paragraph pair. Hoist it into a local const and reuse it in both branches so future copy edits to this masthead can't drift out of sync.
+ const masthead = (
+ <SectionMasthead eyebrow="THE WAR ROOM · SCOUTING" title="Scouting" ink="pursuit">
+ <p className="max-w-2xl font-annual text-body text-text-secondary">
+ The evaluation toolkit — shortlists, side-by-side comparisons, and exportable packets —
+ once a prospect is worth a closer look.
+ </p>
+ </SectionMasthead>
+ );
+
if (!navContext) {
return (
<div className="mx-auto w-full max-w-5xl px-4 py-8 sm:px-6 lg:px-8">
- <SectionMasthead eyebrow="THE WAR ROOM · SCOUTING" title="Scouting" ink="pursuit">
- <p className="max-w-2xl font-annual text-body text-text-secondary">
- The evaluation toolkit — shortlists, side-by-side comparisons, and exportable packets —
- once a prospect is worth a closer look.
- </p>
- </SectionMasthead>
+ {masthead}
<PaperCard className="mt-8 p-6">Same substitution for the main return's masthead block at lines 105-110.
Also applies to: 103-110
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/baseball/`(dashboard)/dashboard/scouting/page.tsx around lines 78 -
96, The `SectionMasthead` plus descriptive paragraph is duplicated in both the
null-context branch and the main return in `ScoutingPage`; hoist that shared JSX
into a local const and reuse it in both places so the copy stays in sync. Update
the `ScoutingPage` render path to reference the shared masthead content instead
of repeating the same block in each branch.
…ing, DB dedupe index) - performance page: single team-local today (todayIsoInTz) reused for the 7-day readiness lookback AND the client prop — no more UTC drift - stats/games: season-record fetch failure surfaced distinctly in UI - dev-plans/[id]: real error vs not-found distinguished, retryable error UI - games.ts: createGame rolls back its own orphaned calendar event on failure; getPlayerSeasonStats returns honest errors (PGRST116 stays empty-state) - signals.ts: DB-level dedupe for NULL-program lift sessions — new partial unique index migration 20260709000000 (APPLIED to prod check-first: zero existing dupes) + 23505 catch-and-requery - lifting programs: revalidateProgramPaths on update + publish detail bust - calendar action: zod input validation (v4 .issues); calendar page row cap 1000; performance/programs paginate via fetchAllRowsResult - performance/live + [programId]: loader errors fail honestly, logServerError - breadcrumbs: no non-null assertion + new unit tests - PlayerProfileClient: date-only strings parse as local midnight - lift page (lifting shell): split current/overdue queries matching baseball - PhilosophySettingsClient: settings route constant; lift-athlete-context doc matrix corrected; e2e dead stats/season spec removed Skipped (W4 deferral): scouting masthead dedupe — replaced wholesale by the Living Annual masthead migration. Gates: tsc=0, eslint touched files=0, unit 4519 passed (14 new). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J2E46URHkhCnEQSYDtZLJX
Owner
Author
|
@coderabbitai review Round 2 addressed in 6af6433: all 22 findings — 21 fixed (incl. the UTC readiness window, orphaned-event rollback, DB-level NULL-program session dedupe via a new partial unique index applied to prod check-first, honest error surfacing across performance/live/programs/dev-plans/stats), 1 skipped per the documented W4 masthead deferral. |
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
git log --oneline -5
echo "---commit message 6af6433c---"
git show -s --format=%B 6af6433c
echo "---stat---"
git show --stat 6af6433c | head -150Length of output: 5766 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== migration file ==="
fd "20260709000000" -t f
cat supabase/migrations/20260709000000_helm_lifting_sessions_null_program_dedupe.sql 2>/dev/null || fd "null_program_dedupe" -t f -x cat {}
echo "=== signals.ts dedupe/catch ==="
rg -n "23505|onConflict|helm_lifting_sessions" src/app/baseball/actions/signals.ts | head -40
echo "=== games.ts orphan rollback ==="
rg -n "createGame|calendar_event|orphan|rollback|PGRST116" src/app/baseball/actions/games.ts | head -60
echo "=== performance/page.tsx today reuse ==="
sed -n '1,80p' 'src/app/baseball/(dashboard)/dashboard/performance/page.tsx'
echo "=== breadcrumbs.ts non-null assertion ==="
sed -n '1,100p' 'src/app/baseball/(dashboard)/_components/breadcrumbs.ts'
echo "=== PlayerProfileClient date parsing ==="
rg -n "local midnight|parseLocal|new Date\(" src/components/baseball/player-profile/PlayerProfileClient.tsx | head -20
echo "=== calendar.ts zod ==="
rg -n "zod|\.issues|safeParse" src/app/baseball/actions/calendar.ts | head -30
echo "=== lift page split queries (lifting shell) ==="
sed -n '1,80p' 'src/app/lifting/(dashboard)/dashboard/lift/page.tsx' 2>/dev/null || fd -p "lifting/.*dashboard/lift/page.tsx"Length of output: 16806 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== games.ts createGame full context ==="
sed -n '140,236p' src/app/baseball/actions/games.ts
echo "=== signals.ts dedupe context ==="
sed -n '400,470p' src/app/baseball/actions/signals.ts
echo "=== performance/page.tsx today usage further down ==="
sed -n '80,160p' 'src/app/baseball/(dashboard)/dashboard/performance/page.tsx'
echo "=== dev-plans/[id]/page.tsx error handling ==="
sed -n '1,120p' 'src/app/baseball/(dashboard)/dashboard/dev-plans/[id]/page.tsx'Length of output: 16405
No blocking findings (security/RLS/type-safety/a11y) in this batch. The one deferred item (scouting masthead dedupe → W4 Living Annual migration) is a reasonable scope call since it's superseded scope, not skipped work. ✅ Action performedReview finished.
|
Owner
Author
|
🤖 Mission Control — PR summary What this changes
Risk / areas to watch
CI (as of this comment)
Posted by the Helm Mission Control sweep — read-only automation. No merge, deploy, or rerun action was taken. |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
src/app/baseball/(dashboard)/dashboard/performance/programs/[programId]/page.tsx (1)
61-64: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
getAssignContextswallows query errors whilegetProgramTreeon the same page surfaces them.Lines 234-241 log the failure but still return
{ athletes: [], groups: [] }at line 243, identical to the legitimate "nothing configured" case.AssignContexthas noerrorfield (61-64) for the page to react to, unlikeProgramTreeResult(66-69). A transient RLS/DB failure here renders as "no athletes/groups exist" with zero UI signal, while the same failure class ingetProgramTreetriggersReadModelStateNotice.🐛 Proposed fix: propagate the error like `ProgramTreeResult` does
interface AssignContext { athletes: Array<Pick<HelmLiftingAthleteRow, 'id' | 'first_name' | 'last_name' | 'position' | 'sport'>>; groups: Array<Pick<HelmLiftingGroupRow, 'id' | 'name' | 'group_type'>>; + error?: boolean; }- return { athletes: athletes ?? [], groups: groups ?? [] }; + return { athletes: athletes ?? [], groups: groups ?? [], error: Boolean(athletesError || groupsError) };Then in the page, either render the same
ReadModelStateNoticewhenassign.erroris true, or pass the flag intoProgramEditorClientso the assign panel can show an inline warning instead of a blank roster.Also applies to: 210-244, 264-289
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/baseball/`(dashboard)/dashboard/performance/programs/[programId]/page.tsx around lines 61 - 64, `getAssignContext` currently hides query failures by logging and returning empty athletes/groups, which makes real DB/RLS errors indistinguishable from valid empty state. Update `AssignContext` and `getAssignContext` to propagate an error flag/status like `ProgramTreeResult` does, using the existing `getProgramTree`/`ReadModelStateNotice` pattern as a reference. Then update the page and `ProgramEditorClient` call site to react to that flag by surfacing the same warning or an inline assign-panel alert instead of rendering a blank roster.src/components/baseball/player-profile/PlayerProfileClient.tsx (1)
184-184: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRename the stats filter value from
practicetoscrimmage.
src/components/baseball/player-profile/PlayerProfileClient.tsx:184still definesStatFilteras'practice', but the changed profile stats are sourced frombaseball_games.game_type, which usesscrimmage. The Scrimmages filter will not match those rows.🐛 Proposed fix
-type StatFilter = 'all' | 'game' | 'practice'; +type StatFilter = 'all' | 'game' | 'scrimmage';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/baseball/player-profile/PlayerProfileClient.tsx` at line 184, The StatFilter union in PlayerProfileClient should use scrimmage instead of practice so the filter matches baseball_games.game_type values. Update the StatFilter type and any related filter option labels/logic in PlayerProfileClient to reference scrimmage consistently, ensuring the Scrimmages view selects the correct rows.src/app/baseball/(dashboard)/dashboard/calendar/page.tsx (1)
85-121: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not collapse calendar query failures into an empty calendar.
src/app/baseball/(dashboard)/dashboard/calendar/page.tsx:85-121never checkseventsResult.error. A PostgREST/RLS/schema failure still becomesevents = [], which is indistinguishable from a real empty calendar.🐛 Proposed fix
]); + + if (eventsResult.error) { + throw new Error('Could not load calendar events.'); + } // Map baseball_events → CalendarEvent. Row is annotated because the query🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/baseball/`(dashboard)/dashboard/calendar/page.tsx around lines 85 - 121, The calendar data fetch in the `Promise.all` block is swallowing `eventsResult.error`, so query failures look like a valid empty state. Update the `page.tsx` calendar loader to check the result from `fromUntyped(supabase, 'baseball_events')` before mapping `events`, and if `eventsResult.error` is present, surface that failure instead of assigning `events = []`. Keep the existing `eventsResult`, `membersResult`, and `teamOrgResult` flow, but add explicit error handling around the `eventsResult` path so PostgREST/RLS/schema issues are distinguishable from a truly empty calendar.src/app/baseball/(dashboard)/dashboard/performance/live/page.tsx (2)
248-252: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPick the assigned exercise before completed work.
src/app/baseball/(dashboard)/dashboard/performance/live/page.tsx:251returns the firstassignedorcompletedrow in order, so after exercise 1 is completed and exercise 2 is assigned, the live row can keep showing exercise 1 as current.🐛 Proposed fix
- const currentSe = seList.find((e) => e.status === 'assigned' || e.status === 'completed'); + const currentSe = + seList.find((e) => e.status === 'assigned') ?? + [...seList].reverse().find((e) => e.status === 'completed');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/baseball/`(dashboard)/dashboard/performance/live/page.tsx around lines 248 - 252, The current live-session selection in liveAthletes uses the first exercise with either assigned or completed status, which can leave an older completed exercise displayed instead of the newly assigned one. Update the selection logic in page.tsx around liveAthletes/currentSe so it prefers an assigned exercise over any completed one, using the existing exercisesBySession and latestSetBySeId flow without changing the surrounding mapping structure.
298-316: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not render the writable live room with an empty org id.
src/app/baseball/(dashboard)/dashboard/performance/live/page.tsx:298-316converts a missingliftCtxinto{ ok: true }, then rendersLiveWeightRoomClientwithorgId=""andcanEdit=true. That exposes a broken write surface instead of the same explicit setup failure used by the programs page.🐛 Proposed fix
const liftCtx = await resolveBaseballLiftingOrg(teamId); - const liveRoomResult: LiveRoomResult = liftCtx - ? await buildLiveRoomData(liftCtx.organizationId, teamId, canViewReadiness) - : { ok: true, athletes: [], exerciseLibrary: [] }; + if (!liftCtx) { + return ( + <div className="mx-auto max-w-7xl px-4 py-6"> + <ReadModelStateNotice state="empty" title="Lift Lab access not set up" /> + </div> + ); + } + + const liveRoomResult: LiveRoomResult = await buildLiveRoomData(liftCtx.organizationId, teamId, canViewReadiness); @@ - orgId={liftCtx?.organizationId ?? ''} + orgId={liftCtx.organizationId}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/baseball/`(dashboard)/dashboard/performance/live/page.tsx around lines 298 - 316, The live performance page is treating a missing lifting context as a successful empty result, which then renders LiveWeightRoomClient with an empty orgId and writable access. Update the logic around resolveBaseballLiftingOrg/buildLiveRoomData to fail closed when liftCtx is missing, matching the explicit setup/error handling used elsewhere instead of returning { ok: true } with empty data. Ensure the render path only reaches LiveWeightRoomClient when a valid organizationId is present, and otherwise return the same error notice/state used for the failed live room load.src/app/baseball/(dashboard)/dashboard/performance/programs/page.tsx (1)
52-92: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPropagate program/count query errors instead of rendering partial counts.
src/app/baseball/(dashboard)/dashboard/performance/programs/page.tsx:52-92ignores errors from the programs query and bothfetchAllRowsResultcalls. BecausefetchAllRowsResultcan return partialdatawitherror, week/day counts can silently undercount again.🐛 Proposed fix
- const { data: programs } = (await fromUntyped(supabase, 'helm_lifting_programs') + const { data: programs, error: programsError } = (await fromUntyped(supabase, 'helm_lifting_programs') @@ - .limit(200)) as { data: HelmLiftingProgramRow[] | null }; + .limit(200)) as { data: HelmLiftingProgramRow[] | null; error: { message: string } | null }; + + if (programsError) throw new Error('Could not load lifting programs.'); @@ - const { data: weeksData } = await fetchAllRowsResult<{ id: string; program_id: string }>( + const { data: weeksData, error: weeksError } = await fetchAllRowsResult<{ id: string; program_id: string }>( @@ ); + if (weeksError) throw new Error('Could not load lifting program weeks.'); @@ - const { data: daysData } = await fetchAllRowsResult<{ week_id: string }>((from, to) => + const { data: daysData, error: daysError } = await fetchAllRowsResult<{ week_id: string }>((from, to) => @@ ); + if (daysError) throw new Error('Could not load lifting program days.');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/baseball/`(dashboard)/dashboard/performance/programs/page.tsx around lines 52 - 92, The programs/count aggregation in the dashboard page is dropping query failures and can render partial counts. Update the programs query and both fetchAllRowsResult calls in the page component to check for returned error values and fail fast instead of continuing with whatever data was fetched. Propagate or throw the error from the programs, weeks, and days queries so the page does not silently undercount. Use the existing aggregation flow around programs, weeksData, and daysData to locate the fix.src/app/baseball/actions/games.ts (1)
1409-1416: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winHandle season lookup errors before returning an empty log.
src/app/baseball/actions/games.ts:1409-1416drops thebaseball_gamesquery error, so a DB/RLS failure can collapse intoseasonGameIds = []and hit the no-games path. ReturnsanitizeDbError(...)before mapping.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/baseball/actions/games.ts` around lines 1409 - 1416, The season lookup in the baseball games flow is swallowing database errors, causing query failures to look like an empty result. In the `seasonGames` fetch inside `games.ts`, check the `db.from('baseball_games')` response for an error before building `seasonGameIds`, and return `sanitizeDbError(...)` immediately when the query fails. Keep the existing mapping logic only for successful responses so the no-games path is reserved for true empty results.src/app/baseball/actions/signals.ts (1)
487-499: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winHandle the seed query errors
src/app/baseball/actions/signals.ts:487-499
fromUntyped()returns a result object (src/lib/supabase/untyped.ts:69-71), sotry/catchwill not catch a failedselect()orinsert(). Check botherrorvalues and log/abort the seed; otherwise a converted lift can persist without its matchinghelm_lifting_session_exercisesrow.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/baseball/actions/signals.ts` around lines 487 - 499, The seed logic in the lifting session exercise creation block is ignoring Supabase result errors because fromUntyped() returns a result object rather than throwing. In the flow that checks existingSe and inserts the HelmLiftingSessionExerciseInsert payload, inspect and handle the error field for both the select and insert operations, and abort/log the seed if either fails so a converted lift cannot continue without its matching helm_lifting_session_exercises row.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/baseball/`(dashboard)/dashboard/dev-plans/[id]/page.tsx:
- Around line 157-189: The error state in the dev plan detail page is using raw
red utility classes instead of the design-system destructive tokens. Update the
`fetchError` return block in `page.tsx` to replace the `border-red-*`,
`bg-red-*`, and `text-red-*` classes with the canonical `destructive` color
family while keeping the same structure and messaging. Use the existing error
card and the `Button` retry action as the place to apply the tokenized styles.
In `@src/app/lifting/`(dashboard)/dashboard/lift/page.tsx:
- Around line 124-129: The upcoming sessions query in the lift dashboard is
currently including all statuses for current and future dates, so non-open
sessions can leak into the upcoming list. Update the query in the page component
around the fromUntyped(... 'helm_lifting_sessions') call to apply the same
OPEN_STATUSES filter used by the overdue query, keeping only open sessions for
scheduled_date >= today. Make the change in the lift page logic that builds the
upcoming sessions data so the dashboard only renders actionable sessions.
In `@src/components/baseball/games/GamesList.tsx`:
- Around line 166-170: The error state text in GamesList uses a raw red utility
class instead of the design-system destructive token. Update the conditional
season-record message in GamesList to use the canonical destructive color family
for the paragraph styling, keeping the existing layout logic tied to
recordError, seasonRecord, and title unchanged.
---
Outside diff comments:
In `@src/app/baseball/`(dashboard)/dashboard/calendar/page.tsx:
- Around line 85-121: The calendar data fetch in the `Promise.all` block is
swallowing `eventsResult.error`, so query failures look like a valid empty
state. Update the `page.tsx` calendar loader to check the result from
`fromUntyped(supabase, 'baseball_events')` before mapping `events`, and if
`eventsResult.error` is present, surface that failure instead of assigning
`events = []`. Keep the existing `eventsResult`, `membersResult`, and
`teamOrgResult` flow, but add explicit error handling around the `eventsResult`
path so PostgREST/RLS/schema issues are distinguishable from a truly empty
calendar.
In `@src/app/baseball/`(dashboard)/dashboard/performance/live/page.tsx:
- Around line 248-252: The current live-session selection in liveAthletes uses
the first exercise with either assigned or completed status, which can leave an
older completed exercise displayed instead of the newly assigned one. Update the
selection logic in page.tsx around liveAthletes/currentSe so it prefers an
assigned exercise over any completed one, using the existing exercisesBySession
and latestSetBySeId flow without changing the surrounding mapping structure.
- Around line 298-316: The live performance page is treating a missing lifting
context as a successful empty result, which then renders LiveWeightRoomClient
with an empty orgId and writable access. Update the logic around
resolveBaseballLiftingOrg/buildLiveRoomData to fail closed when liftCtx is
missing, matching the explicit setup/error handling used elsewhere instead of
returning { ok: true } with empty data. Ensure the render path only reaches
LiveWeightRoomClient when a valid organizationId is present, and otherwise
return the same error notice/state used for the failed live room load.
In
`@src/app/baseball/`(dashboard)/dashboard/performance/programs/[programId]/page.tsx:
- Around line 61-64: `getAssignContext` currently hides query failures by
logging and returning empty athletes/groups, which makes real DB/RLS errors
indistinguishable from valid empty state. Update `AssignContext` and
`getAssignContext` to propagate an error flag/status like `ProgramTreeResult`
does, using the existing `getProgramTree`/`ReadModelStateNotice` pattern as a
reference. Then update the page and `ProgramEditorClient` call site to react to
that flag by surfacing the same warning or an inline assign-panel alert instead
of rendering a blank roster.
In `@src/app/baseball/`(dashboard)/dashboard/performance/programs/page.tsx:
- Around line 52-92: The programs/count aggregation in the dashboard page is
dropping query failures and can render partial counts. Update the programs query
and both fetchAllRowsResult calls in the page component to check for returned
error values and fail fast instead of continuing with whatever data was fetched.
Propagate or throw the error from the programs, weeks, and days queries so the
page does not silently undercount. Use the existing aggregation flow around
programs, weeksData, and daysData to locate the fix.
In `@src/app/baseball/actions/games.ts`:
- Around line 1409-1416: The season lookup in the baseball games flow is
swallowing database errors, causing query failures to look like an empty result.
In the `seasonGames` fetch inside `games.ts`, check the
`db.from('baseball_games')` response for an error before building
`seasonGameIds`, and return `sanitizeDbError(...)` immediately when the query
fails. Keep the existing mapping logic only for successful responses so the
no-games path is reserved for true empty results.
In `@src/app/baseball/actions/signals.ts`:
- Around line 487-499: The seed logic in the lifting session exercise creation
block is ignoring Supabase result errors because fromUntyped() returns a result
object rather than throwing. In the flow that checks existingSe and inserts the
HelmLiftingSessionExerciseInsert payload, inspect and handle the error field for
both the select and insert operations, and abort/log the seed if either fails so
a converted lift cannot continue without its matching
helm_lifting_session_exercises row.
In `@src/components/baseball/player-profile/PlayerProfileClient.tsx`:
- Line 184: The StatFilter union in PlayerProfileClient should use scrimmage
instead of practice so the filter matches baseball_games.game_type values.
Update the StatFilter type and any related filter option labels/logic in
PlayerProfileClient to reference scrimmage consistently, ensuring the Scrimmages
view selects the correct rows.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 08992724-9b79-4877-9eb6-3f5152c1c572
📒 Files selected for processing (20)
e2e/baseball-box-score.spec.tssrc/app/baseball/(dashboard)/_components/breadcrumbs.test.tssrc/app/baseball/(dashboard)/_components/breadcrumbs.tssrc/app/baseball/(dashboard)/dashboard/calendar/page.tsxsrc/app/baseball/(dashboard)/dashboard/dev-plans/[id]/page.tsxsrc/app/baseball/(dashboard)/dashboard/lift/_lift-athlete-context.tssrc/app/baseball/(dashboard)/dashboard/performance/live/page.tsxsrc/app/baseball/(dashboard)/dashboard/performance/page.tsxsrc/app/baseball/(dashboard)/dashboard/performance/programs/[programId]/page.tsxsrc/app/baseball/(dashboard)/dashboard/performance/programs/page.tsxsrc/app/baseball/(dashboard)/dashboard/stats/games/page.tsxsrc/app/baseball/actions/calendar.tssrc/app/baseball/actions/games.tssrc/app/baseball/actions/signals.tssrc/app/lifting/(dashboard)/dashboard/lift/page.tsxsrc/app/lifting/actions/programs.tssrc/components/baseball/games/GamesList.tsxsrc/components/baseball/player-profile/PlayerProfileClient.tsxsrc/components/baseball/settings/PhilosophySettingsClient.tsxsupabase/migrations/20260709000000_helm_lifting_sessions_null_program_dedupe.sql
…s filter, destructive tokens) - calendar/page.tsx + performance/programs pages + games.ts season lookup + performance/live loaders: query failures now surface (throw to boundary or sanitizeDbError) instead of collapsing into empty states - performance/live: prefer newly-assigned exercise over last completed; fail closed on missing lifting org (no writable room with empty org id) - performance/programs/[programId]: assign-context errors surfaced inline - lifting lift page: current/future query filters to open statuses - signals.ts: seed select/insert errors actually checked + logServerError (the try/catch never fired — supabase resolves errors, doesn't throw) - PlayerProfileClient: StatFilter union renamed practice→scrimmage, dead translation layer removed - dev-plans/[id] + GamesList: raw red classes → destructive tokens Gates: tsc=0, lint=0, unit 4519 passed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J2E46URHkhCnEQSYDtZLJX
Owner
Author
|
@coderabbitai full review Round 3 addressed in 761cacb — all 11 findings fixed (error-honesty across calendar/performance/games/signals loaders, open-status filter, assigned-exercise preference, fail-closed live room, destructive tokens, StatFilter cleanup). Requesting a full from-scratch review per docs/operations/coderabbit-review-workflow.md so the decision reflects the whole current diff rather than incremental deltas. The only intentionally-open items remain the W4/W7 deferrals documented in docs/audits/PRODUCTION_READINESS_MISSION_2026-07-09.md (Living Annual masthead migration replaces the duplicated headers; the readiness matrix gets a full truthful refresh). |
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
git log --oneline -8
echo "---commit message 761cacb3---"
git show -s --format=%B 761cacb3
echo "---stat---"
git show --stat 761cacb3 | head -150Length of output: 4302 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== calendar/page.tsx diff ==="
git show 761cacb3 -- 'src/app/baseball/(dashboard)/dashboard/calendar/page.tsx'
echo "=== dev-plans/[id]/page.tsx diff ==="
git show 761cacb3 -- 'src/app/baseball/(dashboard)/dashboard/dev-plans/[id]/page.tsx'
echo "=== performance/live/page.tsx diff ==="
git show 761cacb3 -- 'src/app/baseball/(dashboard)/dashboard/performance/live/page.tsx'
echo "=== performance/programs/[programId]/page.tsx diff ==="
git show 761cacb3 -- 'src/app/baseball/(dashboard)/dashboard/performance/programs/[programId]/page.tsx'
echo "=== performance/programs/page.tsx diff ==="
git show 761cacb3 -- 'src/app/baseball/(dashboard)/dashboard/performance/programs/page.tsx'
echo "=== games.ts diff ==="
git show 761cacb3 -- 'src/app/baseball/actions/games.ts'
echo "=== signals.ts diff ==="
git show 761cacb3 -- 'src/app/baseball/actions/signals.ts'
echo "=== lifting lift page diff ==="
git show 761cacb3 -- 'src/app/lifting/(dashboard)/dashboard/lift/page.tsx'
echo "=== GamesList.tsx diff ==="
git show 761cacb3 -- 'src/components/baseball/games/GamesList.tsx'
echo "=== PlayerProfileClient.tsx diff ==="
git show 761cacb3 -- 'src/components/baseball/player-profile/PlayerProfileClient.tsx'Length of output: 32023 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== OPEN_STATUSES definition ==="
rg -n "OPEN_STATUSES" 'src/app/lifting/(dashboard)/dashboard/lift/page.tsx'
echo "=== sanitizeDbError import in games.ts ==="
rg -n "sanitizeDbError" src/app/baseball/actions/games.ts | head -10
echo "=== getPlayerSeasonStatsImpl full context ==="
sed -n '1380,1460p' src/app/baseball/actions/games.ts
echo "=== calendar/page.tsx error.tsx presence ==="
fd -p 'error.tsx' 'src/app/baseball/(dashboard)'
echo "=== check dashboard error boundary exists for calendar route group ==="
fd -p 'error.tsx' src/app/baseball
echo "=== signals.ts surrounding try/catch full block ==="
sed -n '460,545p' src/app/baseball/actions/signals.tsLength of output: 19991
Confirmed correct:
No regressions or new blocking issues (security/RLS/type-safety/a11y) found in this round. The deferred W4/W7 items remain a reasonable scope call as previously assessed — they're superseded by planned follow-on waves, not silently dropped. ✅ Action performedReview finished.
|
18 Owner Issue cells updated: closed issues (368-416 range, all verified CLOSED via gh) removed; only open issues 377/379/394 remain linked. Cells left with no open issue now say so explicitly and point at the W7 full matrix refresh for the row re-grade (the matrix's row content is known stale — several rows describe already-fixed defects, e.g. the box-score delete-then-insert replaced by the upsert RPC applied 2026-07-09). check:readiness-matrix passes with GITHUB_TOKEN (routes + issue states). Clears the advisory 'check (routes + owner issues)' CI failure. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J2E46URHkhCnEQSYDtZLJX
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
This was referenced Jul 16, 2026
This was referenced Aug 3, 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
1 participant
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.
Summary
Integration branch from the 7-round statsync verify+fix cycle (2026-07-08/09):
20260708150000replaces the banned DELETE-then-INSERT insave_baseball_full_box_scorewith per-row UPSERT + targeted delete of removed players; per-player recalc failures no longer roll back the whole game; realerror_detail/error_codereturned. Applied to prod 2026-07-09, verified live (pg_procbody check).20260708141000committed as a file with an explicit ⛔ HOLD header — NOT applied (touches a golf function + invite-redemption gating risks; needs owner sign-off).format-stathelper,EditGameModal,GameDetailHeader).fix/baseballhelm-coherence-2026-07-08+fix/db-security-hardening-2026-07-08+ cost-control CI changes.Gates (local, exit codes captured)
tsc --noEmit= 0eslint --max-warnings 0= 0chat-tools-visibility.test.ts, reproduced in clean worktree atorigin/main), tracked for the follow-up production-readiness mission.🤖 Generated with Claude Code
https://claude.ai/code/session_01J2E46URHkhCnEQSYDtZLJX