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. |
PR Summary by QodoBaseball statsync: safe box-score UPSERT RPC, one shell, nav IA rework
AI Description
Diagram
High-Level Assessment
Files changed (46)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
97 rules 1. Anon last_seen guard bypass
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+43
to
+45
| import { config as loadEnv } from 'dotenv'; | ||
| import { createClient, type SupabaseClient } from '@supabase/supabase-js'; | ||
| import { computeCareerSlashLine, type SlashLineStatRow } from '../src/lib/baseball/aggregates/career-slash-line'; |
There was a problem hiding this comment.
1. @supabase/supabase-js server client 📘 Rule violation ⌂ Architecture
New scripts create Supabase clients by importing createClient directly from @supabase/supabase-js instead of using the shared server-side factory. This bypasses centralized configuration/guardrails and violates the requirement for standardized server-side Supabase access.
Agent Prompt
## Issue description
Server-side scripts are importing `createClient` from `@supabase/supabase-js` directly, violating the requirement to use the shared server-side Supabase client factory.
## Issue Context
These scripts are Node-only, but the compliance rule still requires all server-side Supabase access to go through `@/lib/supabase/server` to centralize configuration and hardening.
## Fix Focus Areas
- scripts/backfill-baseball-slash-lines.ts[43-83]
- scripts/e2e-supabase-admin.ts[19-26]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+22
to
+24
| function makeQueryBuilder(finalResult: FinalResult) { | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| const builder: any = {}; |
There was a problem hiding this comment.
2. builder: any in test 📘 Rule violation ⚙ Maintainability
The new lifting groups action test introduces an explicit any type (const builder: any = {}).
This weakens type safety and violates the rule disallowing any in TypeScript.
Agent Prompt
## Issue description
A new test file uses an explicit `any` (`const builder: any = {}`), which violates the no-`any` TypeScript compliance rule.
## Issue Context
This is a mocked Supabase query builder. It can be typed with an index signature or a small explicit mock interface/type so the test remains flexible without `any`.
## Fix Focus Areas
- src/app/lifting/actions/__tests__/groups.test.ts[17-47]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
264
to
268
| <!-- AUTOGEN:routes:start --> | ||
| <!-- DO NOT EDIT — regenerated by scripts/regen-docs.mjs --> | ||
|
|
||
| **231 routes** (source: `src/app/**/page.tsx`). | ||
| **224 routes** (source: `src/app/**/page.tsx`). | ||
|
|
There was a problem hiding this comment.
5. Edited autogen routes block 📘 Rule violation ⚙ Maintainability
The PR changes content inside an <!-- AUTOGEN:routes:start --> / <!-- AUTOGEN:routes:end --> region. AUTOGEN-marked regions must not be manually modified.
Agent Prompt
## Issue description
Content inside an AUTOGEN-marked documentation region was modified.
## Issue Context
AUTOGEN regions must remain byte-for-byte identical; updates must come from the generator workflow rather than hand edits.
## Fix Focus Areas
- memory/projects/golfhelm.md[264-292]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| {/* Filter toggle */} | ||
| <div className="flex items-center gap-2"> | ||
| <div className="flex bg-cream-100/75 backdrop-blur-sm border border-warm-200/45 rounded-xl p-1 gap-1 shadow-sm" role="group" aria-label="Filter sessions by type"> | ||
| <div className="flex bg-cream-100/75 backdrop-blur-sm border border-warm-200/45 rounded-xl p-1 gap-1 shadow-sm" role="group" aria-label="Filter games by type"> |
There was a problem hiding this comment.
6. Raw glass classes used 📘 Rule violation ⚙ Maintainability
A modified UI element uses raw Tailwind glass styling (backdrop-blur-* with translucent background classes) instead of an approved .glass-* utility class. This increases style drift and violates the glass utility enforcement rule.
Agent Prompt
## Issue description
A changed element applies glass effects using raw Tailwind classes (`backdrop-blur-*` + translucent background) instead of the approved `.glass-subtle` / `.glass-standard` / `.glass-prominent` utilities.
## Issue Context
Standardizing glass surfaces prevents variant drift and ensures consistent theming.
## Fix Focus Areas
- src/components/baseball/player-profile/PlayerProfileClient.tsx[1014-1032]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+15
to
+17
| IF target_user_id <> auth.uid() AND NOT public.is_admin() THEN | ||
| RAISE EXCEPTION 'Forbidden' USING ERRCODE = '42501'; | ||
| END IF; |
There was a problem hiding this comment.
7. Anon last_seen guard bypass 🐞 Bug ⛨ Security
update_user_last_seen() uses target_user_id <> auth.uid() which evaluates to NULL when auth.uid() is NULL, so the IF does not fire and unauthenticated callers can update arbitrary users’ last_seen. This is exploitable because the baseline grants this SECURITY DEFINER function to anon and the migration does not revoke that privilege.
Agent Prompt
## Issue description
`public.update_user_last_seen(target_user_id uuid)` has a fail-open authorization check when `auth.uid()` is NULL, and the function remains callable by `anon`.
## Issue Context
Because SQL boolean expressions can evaluate to NULL, `target_user_id <> auth.uid()` does not behave like a strict inequality when `auth.uid()` is NULL.
## Fix Focus Areas
- supabase/migrations/20260708021000_gate_ungated_definer_rpcs.sql[8-24]
- supabase/migrations/20260527000000_prod_public_baseline.sql[20911-20913]
## What to change
1. Make the guard NULL-safe, e.g.:
- `IF auth.uid() IS NULL OR (target_user_id IS DISTINCT FROM auth.uid() AND NOT public.is_admin()) THEN ...`
- or `IF target_user_id IS DISTINCT FROM auth.uid() AND NOT public.is_admin() THEN ...` plus an explicit `auth.uid() IS NULL` clause.
2. Revoke EXECUTE/ALL from `anon` (and PUBLIC if applicable) and grant only to `authenticated` (and `service_role` if needed).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+1
to
+17
| -- ============================================================================ | ||
| -- ⛔ HOLD — DO NOT APPLY AS-IS. Caller-audit (2026-07-08) found two break risks: | ||
| -- 1. recompute_golf_round_totals is invoked BY A TRIGGER on golf_shots and is | ||
| -- GRANTed to service_role. The owner/coach-only guard here has NO | ||
| -- service_role/backend allowance, so any service_role or seed/import write | ||
| -- that fires the trigger (auth.uid() = NULL) would hit RAISE 'Forbidden' | ||
| -- and BREAK legitimate golf writes. Before applying, add | ||
| -- `auth.role() = 'service_role'` (backend/trigger context) to the allowed | ||
| -- conditions, keeping owner/coach for direct authenticated/anon .rpc(). | ||
| -- 2. try_redeem/release_baseball_team_invitation gate on "owns a | ||
| -- baseball_players row" — a first-time user redeeming their initial invite | ||
| -- may not yet hold one, so the guard could block onboarding. Fix needs the | ||
| -- invite CODE threaded through (a signature change), per W10's own note. | ||
| -- This file touches a GOLF function (owner rule: golf untouchable) and is P2 | ||
| -- security hardening, orthogonal to the routing/shell/UI/half-built pass. Left | ||
| -- unapplied on purpose; apply only after the two fixes above + owner sign-off. | ||
| -- ============================================================================ |
There was a problem hiding this comment.
8. Hold migration still runnable 🐞 Bug ☼ Reliability
A migration labeled “⛔ HOLD — DO NOT APPLY” is committed under supabase/migrations/, so it can be executed by any workflow/tooling that initializes/applies the migrations directory, despite the warning comment. The SQL itself acknowledges it can break legitimate writes (e.g., trigger/service contexts where auth.uid() is NULL) and also touches Golf surfaces on the shared DB.
Agent Prompt
## Issue description
A “DO NOT APPLY” migration is in the canonical migrations directory, which makes accidental execution likely and undermines deterministic environment setup.
## Issue Context
CI starts a local Supabase stack, and the repo contains tooling that treats every `supabase/migrations/*.sql` file as part of the migrations set. A header comment cannot prevent execution.
## Fix Focus Areas
- supabase/migrations/20260708141000_gate_secdef_ownership_and_redemption.sql[1-24]
- .github/workflows/ci.yml[238-243]
- scripts/check-migration-versions.mjs[96-104]
## What to change
Choose one:
1. **Remove the file from `supabase/migrations/`** and move it to a non-applied location (e.g. `supabase/investigations/` or `supabase/migrations_archive/`) until it is safe.
2. If you need it tracked but not applied, introduce an explicit repo-level mechanism to exclude HOLD migrations from apply/lint/start flows (and update CI accordingly).
3. Alternatively, split it into safe-to-apply pieces and ship only the safe subset as real migrations.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 65
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
docs/operations/BASEBALLHELM_FEATURE_READINESS_MATRIX.md (1)
1-112: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winBlocking: CI check
check:readiness-matrixis red — 22 closed issues still linked.Per this file's own Maintenance rule 6 ("Resolved owner issues must be removed or replaced, not left linked"), the pipeline failure lists 22 stale closed-issue URLs still cited across the matrix (416, 392, 371, 412, 368, 369, 411, 399, 381, 415, 407, 370, 400, 408, 384, 397, 393, 375, 402, 396, 405, 406). This PR must update each affected row's Owner Issue column to either remove the closed link or replace it with the next real open gap before merge, per the doc's advisory-but-must-fix-when-flagged contract.
🤖 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 `@docs/operations/BASEBALLHELM_FEATURE_READINESS_MATRIX.md` around lines 1 - 112, The readiness-matrix CI check is failing because several rows still reference closed Owner Issue URLs. Update the affected rows in BASEBALLHELM_FEATURE_READINESS_MATRIX so each stale issue link is either removed or replaced with the next real open issue, following the Maintenance rule that resolved issues must not remain linked. Use the row feature names and issue groupings in the matrix to locate each stale Owner Issue cell, and verify the updated table still satisfies the check: only open, valid issue URLs or intentional N/A entries should remain.Source: Pipeline failures
src/app/baseball/(dashboard)/dashboard/academics/AcademicsClient.tsx (1)
161-172: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve
is_eligible = nullduring edit flows —src/app/baseball/(dashboard)/dashboard/academics/AcademicsClient.tsx:161-172seedseditValues.is_eligiblewithtruewhen the record is unset, andsrc/app/baseball/actions/academics.ts:716then writesvalidated.is_eligible ?? trueback to the DB. Editing GPA/credits on a record with no eligibility value will silently turn it intotrue. Keepnullthrough the edit flow or add an explicit “not on file” option.🤖 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 161 - 172, The edit flow is converting an unset eligibility value into true, which causes records with no eligibility on file to be overwritten during unrelated edits. Update startEditing in AcademicsClient to preserve student.is_eligible as null instead of defaulting it to true, and make sure the submit path in upsertPlayerAcademics keeps null intact rather than applying a fallback. If the UI needs a default, add an explicit “not on file” choice instead of silently coercing the value.src/components/baseball/documents/DocumentsFairway.tsx (1)
116-132: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated upload-CTA JSX — extract to avoid drift.
The upload
<Button>at 121-131 (masthead) and 193-204 (empty-state action) are near-identical: sameonClick={onUpload}, samebusy={isUploadingDocument}, same icon — only the label text differs ("Upload" vs "Upload document"). If one call site's props change (e.g., a disabled/loading variant) the other can silently drift out of sync.+ const uploadButton = (label: string) => + isCoach ? ( + <Button + variant="primary" + size="sm" + busy={isUploadingDocument} + leftIcon={<Upload className="h-4 w-4" />} + onClick={onUpload} + > + {label} + </Button> + ) : undefined;Then use
uploadButton('Upload')anduploadButton('Upload document')at the two call sites.Also applies to: 188-205
🤖 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/documents/DocumentsFairway.tsx` around lines 116 - 132, The upload CTA JSX is duplicated between the masthead and empty-state actions in DocumentsFairway, which risks the two buttons drifting apart. Extract the shared Button config/markup into a helper such as uploadButton in DocumentsFairway and reuse it at both call sites, passing only the label text ("Upload" and "Upload document") while keeping onClick={onUpload}, busy={isUploadingDocument}, and the Upload icon consistent.src/components/baseball/settings/TeamSettingsClient.tsx (1)
3-16: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStale doc comment still lists
Headeras a reused primitive.Line 13 says "Reuses GolfHelm UI primitives verbatim (Card / Button / Header)" —
Headeris no longer imported or used after this change (lines 18-26). Update the comment so it doesn't mislead the next person touching this file about what's actually reused.📝 Suggested fix
-// Reuses GolfHelm UI primitives verbatim (Card / Button / Header)+ cream/green +// Reuses GolfHelm UI primitives verbatim (Card / Button) + cream/green🤖 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/settings/TeamSettingsClient.tsx` around lines 3 - 16, The top-of-file comment in TeamSettingsClient is stale because it still mentions Header as a reused primitive even though the component no longer imports or uses it. Update that doc comment to match the actual primitives used in TeamSettingsClient, keeping the wording aligned with the current imports and JSX so it doesn’t mislead future changes.src/app/baseball/(dashboard)/dashboard/calendar/page.tsx (1)
75-101: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
ascendingorder + hard.limit(500)biases toward past events — future events get silently dropped, not the past ones.
eventsWindowStart/eventsWindowEndspan 455 days (90 back, 365 forward). Once a team's row count in that span exceeds 500 — plausible for a full-season program (daily practices + games + travel + meetings, and if recurring events materialize as individual rows), the query returns the earliest 500 chronologically. That means the oldest rows in the window win and the most-future events — the ones a coach checking/dashboard/calendaractually cares about — are what silently disappears. There's no count check or truncation signal anywhere downstream, so this fails invisibly.Prioritize future events so past-event volume can never crowd them out of the cap:
🐛 Proposed fix — split past/future so the future half is never truncated by the past half
+ const nowIso = new Date().toISOString(); + const EVENT_COLUMNS = 'id, team_id, title, event_type, start_time, end_time, location, description, is_mandatory, max_attendees, rsvp_deadline, all_day, status, recurring, created_by'; - const [eventsResult, membersResult, teamOrgResult] = await Promise.all([ - fromUntyped(supabase, 'baseball_events') - .select('id, team_id, title, event_type, start_time, end_time, location, description, is_mandatory, max_attendees, rsvp_deadline, all_day, status, recurring, created_by') - .eq('team_id', teamId) - .gte('start_time', eventsWindowStart.toISOString()) - .lte('start_time', eventsWindowEnd.toISOString()) - .order('start_time', { ascending: true }) - .limit(500), + const [futureEventsResult, pastEventsResult, membersResult, teamOrgResult] = await Promise.all([ + // Future events are the ones a coach opens this page to see — never let + // trailing past-event volume push these out of the cap. + fromUntyped(supabase, 'baseball_events') + .select(EVENT_COLUMNS) + .eq('team_id', teamId) + .gte('start_time', nowIso) + .lte('start_time', eventsWindowEnd.toISOString()) + .order('start_time', { ascending: true }) + .limit(500), + fromUntyped(supabase, 'baseball_events') + .select(EVENT_COLUMNS) + .eq('team_id', teamId) + .gte('start_time', eventsWindowStart.toISOString()) + .lt('start_time', nowIso) + .order('start_time', { ascending: false }) + .limit(200), supabase .from('baseball_team_members') .select('player_id, baseball_players!inner(id, first_name, last_name, avatar_url)') .eq('team_id', teamId) .limit(100), supabase .from('baseball_teams') .select('organization_id') .eq('id', teamId) .maybeSingle(), ]); + const eventsResult = { + data: [...(pastEventsResult.data || []).reverse(), ...(futureEventsResult.data || [])], + };At minimum, log/track when either half hits its cap so silent truncation stops being invisible.
🤖 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 - 101, The calendar query in the dashboard page is capping results with `order('start_time', { ascending: true })` and `.limit(500)`, which can silently drop the most-future events when the 455-day window is crowded. Update the `eventsResult` fetch to prioritize future events over older past entries, ideally by splitting the range or ordering so upcoming `start_time` rows are guaranteed to survive the cap. If a cap is still kept, add a visible warning/metric when the query truncates results so this loss is not silent.src/app/lifting/actions/programs.ts (1)
118-155: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDRY up the 9x-repeated
LAB_PATH+BASEBALL_PROGRAMS_PATHrevalidation pair;addLiftWeekis missing a free per-program bust.Six functions (createProgram:152, deleteProgram:279, addLiftWeek:727, addLiftDay:756, addLiftSection:784, addLiftPrescription:823) each hand-roll
revalidatePath(LAB_PATH); revalidatePath(BASEBALL_PROGRAMS_PATH);. That's the same two lines copy-pasted 6 times, and it's already grown to a third path (BASEBALL_LIVE_PATH/BASEBALL_LIFT_PATHinpublishProgram) that's not part of the pair. If a golf-side lift portal ever needs the same treatment, someone has to remember to touch all 6+ call sites individually.Separately:
addLiftWeek(L708-730) already hasinput.programIdin scope, yet — unlikeupdateProgram/duplicateLiftDay/duplicateLiftWeek— it never busts${LAB_PATH}/${input.programId}or${BASEBALL_PROGRAMS_PATH}/${input.programId}. If the per-program builder page for either portal does any RSC-level caching, a coach adding week 1 to a brand-new program won't see it reflected on that detail route until an unrelated revalidation happens elsewhere.♻️ Proposed helper + wiring
const BASEBALL_PROGRAMS_PATH = '/baseball/dashboard/performance/programs'; const BASEBALL_LIVE_PATH = '/baseball/dashboard/performance/live'; const BASEBALL_LIFT_PATH = '/baseball/dashboard/lift'; + +// Single choke point for lifting↔baseball cross-portal cache invalidation. +// Any future portal (e.g. golf) only needs one new constant + one call here. +function revalidateProgramPaths(programId?: string): void { + revalidatePath(LAB_PATH); + revalidatePath(BASEBALL_PROGRAMS_PATH); + if (programId) { + revalidatePath(`${LAB_PATH}/${programId}`); + revalidatePath(`${BASEBALL_PROGRAMS_PATH}/${programId}`); + } +}Then, e.g. for
addLiftWeek:- revalidatePath(LAB_PATH); - revalidatePath(BASEBALL_PROGRAMS_PATH); + revalidateProgramPaths(input.programId);And for
createProgram/deleteProgram(no programId at hand for delete, but createProgram has the new id post-insert):- revalidatePath(LAB_PATH); - revalidatePath(BASEBALL_PROGRAMS_PATH); + revalidateProgramPaths((data as { id: string }).id);Also applies to: 216-282, 708-730, 736-759, 765-787, 793-826
🤖 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 118 - 155, Extract the repeated revalidation pair into a shared helper used by createProgram, deleteProgram, addLiftWeek, addLiftDay, addLiftSection, and addLiftPrescription so the LAB_PATH and BASEBALL_PROGRAMS_PATH invalidation stays consistent in one place. Also update addLiftWeek to revalidate the specific program routes using input.programId, matching the per-program cache busting already done in updateProgram, duplicateLiftDay, and duplicateLiftWeek. Use the existing symbols addLiftWeek, createProgram, deleteProgram, and the path constants to wire this in without duplicating the calls.src/app/baseball/(dashboard)/dashboard/performance/programs/page.tsx (1)
40-93: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAdd a lifting-access fallback for this page
src/app/baseball/(dashboard)/dashboard/performance/programs/page.tsx:43-63,84-93still depends onpublic.helm_lifting_can_view_org()for the read path, and that helper only allows an activehelm_lifting_coachesrow or ahelm_lifting_org_viewersrow (supabase/migrations/20260625000000_helm_lifting_identity.sql:162-199). A coach can pass the BaseballHelmcan_manage_liftinggate and still get[]here if they were never onboarded into the lifting org, so the Programs screen goes blank without explanation. Surface an onboarding/linked-org state or fail fast before querying.🤖 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 40 - 93, The Programs page currently queries lifting data even when the coach has BaseballHelm manage access but is not linked to the lifting org, which can cause an empty list with no explanation. In ProgramsPage and getPrograms, add a lifting-access fallback before querying: if resolveBaseballLiftingOrg(teamId) returns no org or the user is not onboarded, surface an onboarding/linked-org state or redirect/fail fast instead of returning []. Keep the check aligned with the existing gate around resolveBaseballCapabilities and the lifting-org lookup used by ProgramsPage.src/app/lifting/actions/player-sessions.ts (1)
10-22: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMove the
BASEBALL_LIFT_PATHconst out of the import block.Line 22's
import type {...}is sandwiched after a non-import statement (lines 14-21). Not a syntax error, but it breaks import-grouping conventions and can tripimport/first-style ESLint rules depending on config. Move the constant below all imports.🔧 Suggested reorder
import { revalidatePath } from 'next/cache'; import { createClient } from '`@/lib/supabase/server`'; import { fromUntyped } from '`@/lib/supabase/untyped`'; import { resolveMyBaseballAthleteId } from '`@/lib/lifting/resolve-baseball-context`'; - -// Baseball Wave C renders these same helm_lifting_sessions / -// helm_lifting_readiness_checkins reads at /baseball/dashboard/lift and -// /baseball/dashboard/lift/[sessionId] (see src/app/baseball/(dashboard)/ -// dashboard/lift/page.tsx + [sessionId]/page.tsx) — bust both alongside -// '/lifting/dashboard/lift' below so a baseball athlete's own lift portal -// reflects a checkin/set/status mutation immediately. -const BASEBALL_LIFT_PATH = '/baseball/dashboard/lift'; import type { HelmLiftingSessionRow, HelmLiftingSessionStatus, HelmLiftingReadinessCheckinRow, HelmLiftingSetResultInsert, } from '`@/lib/types`'; + +// Baseball Wave C renders these same helm_lifting_sessions / +// helm_lifting_readiness_checkins reads at /baseball/dashboard/lift and +// /baseball/dashboard/lift/[sessionId] (see src/app/baseball/(dashboard)/ +// dashboard/lift/page.tsx + [sessionId]/page.tsx) — bust both alongside +// '/lifting/dashboard/lift' below so a baseball athlete's own lift portal +// reflects a checkin/set/status mutation immediately. +const BASEBALL_LIFT_PATH = '/baseball/dashboard/lift';🤖 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/player-sessions.ts` around lines 10 - 22, The BASEBALL_LIFT_PATH constant is placed between import statements, breaking import grouping and potentially triggering import/first lint rules. Move BASEBALL_LIFT_PATH below all imports in player-sessions.ts, keeping the existing symbols like revalidatePath, createClient, fromUntyped, and resolveMyBaseballAthleteId together at the top before any non-import declarations.
🤖 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 `@e2e/baseball-box-score.spec.ts`:
- Around line 84-145: The cleanup logic in the baseball box score spec can still
fail the run because the Supabase delete/select calls in the beforeAll and
afterAll hooks are not guarded. Update the teardown in test.beforeAll and
test.afterAll to wrap the shared delete flow in try/catch so transient failures
are swallowed or logged without failing an otherwise-passing run, and extract
the repeated select→collect ids→delete sequence into a helper used by both
hooks. Keep the existing filtering differences (ilike for leftovers,
createdOpponents for owned rows) but centralize the delete behavior around the
existing createdOpponents tracking and getServiceRoleClient usage.
In `@scripts/backfill-baseball-slash-lines.ts`:
- Around line 81-84: The backfill in backfill-baseball-slash-lines.ts is only
fetching the first page of baseball_player_aggregates, so it silently stops at
Supabase’s default 1000-row limit. Update the aggregate scan in the main
backfill loop to paginate deterministically by adding explicit ordering and a
ranged fetch loop until no more rows are returned, and apply the same paging
pattern to the baseball_player_stats lookup if needed. Use the existing backfill
routine and its fetchError/rows handling so the script processes every aggregate
row before printing completion.
In `@scripts/e2e-supabase-admin.ts`:
- Around line 21-26: getE2eAdminClient currently reads only process.env, so it
can return null during a normal Playwright run when the env vars are only
present in .env.local. Update the bootstrap in scripts/e2e-supabase-admin.ts to
load .env.local before getE2eAdminClient reads NEXT_PUBLIC_SUPABASE_URL and
SUPABASE_SERVICE_ROLE_KEY, or reuse the same dotenv initialization pattern used
by the other scripts utilities. Keep the fix localized around getE2eAdminClient
and the module’s startup setup so the client is created consistently without
manual env exports.
In `@src/app/baseball/`(dashboard)/_components/breadcrumbs.ts:
- Around line 52-81: The breadcrumb builder in buildBreadcrumbs() is collapsing
all /dashboard/settings/* pages to the parent Settings label because those
subpages do not provide a page-specific override. Add a distinct breadcrumb
label for each settings subroute, either by mounting BreadcrumbLabel in the
permissions and roles page components or by passing an overrideLabel from those
routes, so the breadcrumb can show the actual current page name instead of just
the hub label.
In `@src/app/baseball/`(dashboard)/BaseballFairwayShell.tsx:
- Around line 542-561: The showcase link handler in BaseballFairwayShell’s
shellLink callback drops the click event when isShowcaseCoach is true and href
matches SHOWCASE_ORG_HOME_HREF. Update the Link onClick branch to accept and
forward the MouseEvent to onClick after calling setSelectedTeamId(null),
matching the fallback path so consumers preserve modifier-key and
default-prevented behavior.
In `@src/app/baseball/`(dashboard)/dashboard/academics/AcademicsClient.tsx:
- Around line 234-237: The Academics page header copy is duplicated across
multiple render states and has already drifted in wording. Extract the repeated
header block from AcademicsClient into a shared component like AcademicsHeader,
then replace each inline header instance in the different states with that
component so the title/subtitle stay consistent in one place.
In `@src/app/baseball/`(dashboard)/dashboard/calendar/page.tsx:
- Around line 251-262: Add a distinct empty state for the no-team case before
rendering CalendarFairway in the calendar page. Right now the page only handles
the college-coach branch, so when teamId is null for non-college coaches or
players it still falls through to the calendar shell; update the page-level
logic around the teamId/isCollegeCoach checks to return a “no team assigned”
state whenever teamId === null. Also review BaseballCalendarWrapper so its
null-to-undefined normalization does not mask this branch, and keep the fallback
separate from the existing calendar rendering path.
In `@src/app/baseball/`(dashboard)/dashboard/camps/[id]/page.tsx:
- Around line 224-226: The inline section header markup is duplicated multiple
times in this page, including the simple and detail variants, so extract it into
a shared reusable component and replace the repeated blocks with that component.
Use a primitive like InlineSectionHeader in this file’s header sections to
centralize the border/padding/title styling and support optional
subtitle/actions for the detail variant, keeping the Camp Details and related
header blocks consistent across the page.
In `@src/app/baseball/`(dashboard)/dashboard/compare/CompareClient.tsx:
- Around line 133-138: The CompareClient header JSX is duplicated across
loading, error, main, and Suspense fallback states, so extract it into a local
CompareHeader component and reuse it at each call site. Keep the shared title
and layout inside CompareHeader, and pass only the subtitle as a prop so the
main render can still vary the message while loading/error/fallback use the same
static subtitle.
In `@src/app/baseball/`(dashboard)/dashboard/dev-plan/DevPlanClient.tsx:
- Around line 539-546: The plan header is formatting date-only fields with new
Date, which bypasses the local-date handling already added for DATE strings.
Update the plan start/end rendering in DevPlanClient to use parseLocalDate for
plan.start_date and plan.end_date before calling toLocaleDateString, matching
how getDaysUntil and the categorization logic handle date-only values. Keep
completed_at unchanged since it is a full timestamp.
- Around line 333-351: `DevPlanClient` can stay stuck on `DevPlanSkeleton` when
`player?.id` is missing because `fetchPlan` returns before clearing loading
state. Update the `fetchPlan` callback to handle the no-player path by setting
`isLoading(false)` before returning, or adjust the render/useEffect flow so the
skeleton is only shown when `player?.id` exists. Use the `fetchPlan`,
`useEffect`, and `useAuth` loading/player state as the key points to fix this.
In `@src/app/baseball/`(dashboard)/dashboard/dev-plans/[id]/page.tsx:
- Around line 90-102: The repeated Development Plan header block in the page
component should be extracted into a local reusable component so the back link,
icon, and static title/subtitle are defined once and reused at each occurrence.
Update the component used for this header in the page to replace all four inline
copies, and add a visible focus-visible ring to the back Link’s className so
keyboard users can see focus on the chevron button.
In `@src/app/baseball/`(dashboard)/dashboard/discover/DiscoverClient.tsx:
- Around line 337-342: The DiscoverClient header markup is duplicated across the
coach-access and authenticated render paths, both using the same wrapper and
inner structure. Extract a small local helper in DiscoverClient so the shared
header shell has one source of truth and future spacing/token changes only need
to be made once. While doing so, remove the unnecessary flex items-center
justify-between gap-4 from the outer wrapper since it only contains one child
and the justify-between behavior has no effect.
In `@src/app/baseball/`(dashboard)/dashboard/documents/page.tsx:
- Line 29: The `EditorsLetter` usage in the dashboard documents page is passing
`ink="team"` even though `EditorsLetter` already defaults `ink` to `team`;
remove this explicit prop from the `EditorsLetter` JSX so the call site stays
clean and relies on the component default.
In `@src/app/baseball/`(dashboard)/dashboard/events/EventsClient.tsx:
- Around line 392-414: The empty-state branch in EventsClient is not gated by
fetchError, so the `EditorsLetter` error surface and the `EmptyIssue` “Create
Your First Event” CTA can render together on load failure. Update the
conditional around the `filteredEvents.length === 0`/`EmptyIssue` render path so
it only shows when there is no `fetchError`, keeping the error state and empty
state mutually exclusive. Use the existing `fetchError`, `loading`,
`filteredEvents`, and `EmptyIssue`/`EditorsLetter` symbols in `EventsClient` to
place the guard cleanly.
In `@src/app/baseball/`(dashboard)/dashboard/journey/page.tsx:
- Around line 204-209: The journey page duplicates the same header markup in
both the loading and loaded branches, so refactor the shared wrapper and <h1>
into a single header render in page.tsx and branch only on the subtitle/action
area. Use the existing journey page header structure around the current loading
and loaded sections, preserving the “Add Schools” action only in the loaded
state and keeping the conditional wrapper closed where the loaded body already
ends.
In `@src/app/baseball/`(dashboard)/dashboard/lift/_lift-athlete-context.ts:
- Around line 54-56: The handled error paths in resolvePlayerTeamId and
hasReadinessCheckinToday are logging to console.error instead of the paged
server error logger. Replace those console.error calls with logServerError so
both branches still degrade to null/false but also report the incident through
the proper observability channel, keeping the existing function behavior intact.
- Around line 34-83: The current team resolution in resolvePlayerTeamId() only
returns the earliest active baseball_team_members row, which can cause
resolvePlayerLiftAthleteContext() to fail even when another active team would
map to a valid Lift Lab athlete. Update the resolution flow so it tries all
active team_id values for the player, then for each candidate continues through
resolveBaseballLiftingOrg() and resolveMyBaseballAthleteId() until the first
successful organization/athlete pair is found, returning null only after all
active memberships are exhausted.
In `@src/app/baseball/`(dashboard)/dashboard/lift/[sessionId]/page.tsx:
- Around line 60-106: The session/exercises/sets failure branches in
fetchSessionWithExercises currently only use console.error, so handled lift-load
errors are not recorded in the shared server error pipeline. Update those
session query, exercises query, and sets query error paths in page.tsx to call
logServerError with a clear message and the caught error before returning the
error state, so these failures flow into Sentry, error_logs, and admin_events.
In `@src/app/baseball/`(dashboard)/dashboard/lift/page.tsx:
- Around line 62-80: The handled query failures in fetchPlayerSessions are only
sent to console.error, so they should be routed through logServerError instead.
Update the upcomingError and recentError branches in lift/page.tsx to call
logServerError with the same contextual message and error details, keeping the
structured { error: true } return path unchanged. Use the fetchPlayerSessions
logic in the page component to locate both failure branches and replace the
direct console.error logging with the server error logger.
- Around line 55-68: The current upcoming sessions read in the lift dashboard is
applying a single .limit(20) across both future/today rows and overdue-open
rows, which can let overdue items crowd out today’s workout. Update the shared
fetch path in the lift page logic around the upcoming query and filtering so
current/future sessions are prioritized before any cap, either by splitting the
overdue-open and current/future reads into separate bounded queries or by
ordering the query to fetch current/future rows first. Make the same adjustment
in the mirrored lift home path so both `dashboard/lift/page.tsx` implementations
stay consistent.
In `@src/app/baseball/`(dashboard)/dashboard/performance/builder/page.tsx:
- Around line 26-29: Add regression coverage for the team-local date anchoring
in this dashboard page: the current fix in the page component is correct, but
there is no test proving that `todayIsoInTz` and the downstream
`mondayOf()`/`weekOf` logic stay tied to the team timezone instead of server
UTC. Mirror the readiness page test pattern by mocking `resolveTeamTimezone` to
return a timezone where the local date differs from `new Date().toISOString()`,
then assert the page’s `today` and `weekOf` values used by
`getGroupSorenessFlags` and `getGroupAvailability` reflect the team-local date.
In `@src/app/baseball/`(dashboard)/dashboard/performance/groups/page.tsx:
- Around line 45-91: The Supabase loaders are swallowing query failures, so
`getGroupsWithMembers` and `getAthletes` can return empty results on RLS or SQL
errors just like a true no-data case. Update both functions to destructure the
`error` from the `fromUntyped(...).select(...)` calls, and in each path either
throw the error or pass it to the page error handling so failures surface
instead of falling back to `[]`/`data ?? []`.
In `@src/app/baseball/`(dashboard)/dashboard/performance/live/page.tsx:
- Around line 83-101: The `buildLiveRoomData` flow is swallowing `sessionsError`
and returning an empty data set, which makes a DB failure look identical to a
legitimately empty live board. Update `buildLiveRoomData` to return a
failure/notice state instead of `{ athletes: [], exerciseLibrary }` when the
`helm_lifting_sessions` query fails, and have `page.tsx` render the existing
read-model state notice pattern rather than passing empty `initialAthletes` to
`LiveWeightRoomClient`. Use the existing `logQueryError`, `buildLiveRoomData`,
and `LiveWeightRoomClient` entry points to propagate the error state cleanly.
In
`@src/app/baseball/`(dashboard)/dashboard/performance/programs/[programId]/page.tsx:
- Around line 79-115: The prescriptions tree load is unbounded and can truncate
at PostgREST’s row cap, so update the data fetch in the program page loader to
paginate `helm_lifting_prescriptions` using the existing `fetchAllRowsResult` +
`.range()` pattern, and apply the same fix in the mirrored lifting program page.
While touching this loader, stop discarding query failures from `program`,
`weeks`, `days`, `sections`, `prescriptions`, and `exRows`; handle the `error`
values explicitly in the page data function so DB/RLS issues surface instead of
returning empty trees or 404s. Use the existing loader symbols like
`fromUntyped`, `weekList`, `dayList`, `sectionList`, `prescList`, and
`exerciseNameMap` to keep the fix localized.
In `@src/app/baseball/`(dashboard)/dashboard/players/[id]/page.tsx:
- Around line 109-120: The player profile is fetching pitching log data but
`PlayerProfileClient` only consumes `seasonStats` and `battingLog`, so the
pitching history is never shown. Update the profile flow by either threading
`pitchingLog` from the `page.tsx` fetch through `PlayerProfileClient` and using
it where per-game trends are rendered, or remove `pitchingLog` from
`getPlayerSeasonStats()` for this batting-only surface. Use the
`PlayerProfileClient` component and the `getPlayerSeasonStats()` call in the
dashboard player page to locate the affected data flow.
In `@src/app/baseball/`(dashboard)/dashboard/program/page.tsx:
- Around line 214-217: The Program page header shell is duplicated across the
three conditional branches, which will cause the shared layout to drift over
time. Extract the repeated header wrapper from the branch rendering in
program/page.tsx into a reusable local component or helper used by the Program
page, and pass only the varying subtitle/action content into it. Keep the unique
header content in the existing Program page branches, but centralize the shared
border-b/px-6/pb-5/pt-6 header structure so future styling changes are made
once.
- Around line 251-262: The Program Profile header row can overlap on narrow
screens because the wrapper in the dashboard page uses a non-wrapping flex
layout. Update the header container in the Program Profile section to allow
wrapping and better vertical alignment, similar to the shared Header component
behavior, so the title/subtitle block and the “View Public Page” action can
stack cleanly on mobile. Use the existing header markup around the title,
subtitle, and Link/Button to locate the fix.
In `@src/app/baseball/`(dashboard)/dashboard/readiness/__tests__/page.test.tsx:
- Around line 44-59: Replace the bespoke vi.mock for '`@/lib/supabase/server`' in
page.test.tsx with the shared createFakeSupabase fixture from
src/test/fixtures/fake-supabase.ts. The test only needs the
from(...).select().eq().maybeSingle() behavior on the baseball_teams table, so
configure that fixture instead of hand-rolling the chain. Keep the existing
createClient symbol mocked, but delegate its return value to the shared fake to
stay consistent with the rest of the suite.
- Around line 90-97: The readiness page test is asserting against the raw React
element tree from PlayerReadinessPage, which makes it brittle to JSX shape
changes; update the test to verify user-visible behavior indirectly by spying on
PlayerReadinessClient’s props instead of unwrapping element.props.children. Use
the existing PlayerReadinessPage and mocked PlayerReadinessClient setup, make
the mock capture its received props, render the page output with the test
renderer you already use, and assert the captured checkDate matches the expected
team-local date without depending on a single wrapping <div>.
In `@src/app/baseball/`(dashboard)/dashboard/scouting/page.tsx:
- Around line 39-122: Both ScoutingPage and the Operations landing page are
duplicating the same nav-card rendering and empty-state logic, so extract that
shared behavior into a reusable NavCardGrid component/helper. Move the
getBaseballNavEntry and isBaseballNavEntryVisible filtering plus the
PaperCard/Link grid and empty-state handling into the shared abstraction, then
have ScoutingPage and the Operations page pass their card definitions, masthead
text, and empty-state copy into it.
- Around line 66-73: The fallback in the scouting dashboard page is fabricating
a partial BaseballNavContext when getBaseballNavContext() returns null, which
hides a real unresolved-team state and can make the empty-state copy misleading.
Update the logic in the scouting page component so it does not cast a synthetic
{ role: 'coach', capabilities: {} } object to BaseballNavContext; instead either
fail loudly after requireRecruitingCoachRoute() if context is unexpectedly
missing, or render a dedicated no-active-team state. Keep the real resolver
shape intact, including programType, and ensure SCOUTING_CARD_IDS filtering via
isBaseballNavEntryVisible only runs with a valid resolved context.
In `@src/app/baseball/`(dashboard)/dashboard/settings/page.tsx:
- Around line 222-228: Extract the repeated page header markup into a shared
page-header primitive and replace the inline block in Settings with that
component. Create or reuse a small component such as PageHeaderBar that owns the
border-b wrapper, h1 title, and p subtitle styling so the same 3-node structure
is centralized across this PR. Update the Settings page and other duplicated
page-header usages to pass title and subtitle props instead of hand-rolling the
markup.
In `@src/app/baseball/`(dashboard)/dashboard/settings/roles/page.tsx:
- Around line 47-52: The inline page header block is duplicated across many
sibling dashboard pages, so replace this repeated `div`/`h1`/`p` pattern with a
shared header primitive instead of keeping it local to this `Roles` page. Update
the `page.tsx` implementation to use the existing reusable `Header` (or extract
a new shared inline-header component if needed) so the common border, spacing,
title, and subtitle styling live in one place and future changes only require a
single edit.
In `@src/app/baseball/`(dashboard)/dashboard/stats/games/[gameId]/page.tsx:
- Around line 84-96: The page currently makes an extra database round-trip by
calling hasBaseballCapability(team.id, 'can_manage_stats') after already loading
coach/team/member data. Update the game page component to reuse or cache the
existing getBaseballStaff(team.id) result when determining canManageStats, so
the capability check in the dashboard game page does not trigger a redundant
fetch while keeping the BreadcrumbLabel and GameDetailHeader behavior unchanged.
In `@src/app/baseball/`(dashboard)/dashboard/stats/games/page.tsx:
- Around line 42-48: The fetch results in the dashboard page are being silently
collapsed into empty values, so failed `gamesResult` or `recordResult` loads
look identical to a real empty season. Update the `page.tsx` server component to
handle both failure branches explicitly in the `GamesList` data-loading path:
call `logServerError` with a clear message and metadata when either result has
`success === false`, and pass an error indicator into `GamesList` so it can
render a true failure state instead of defaulting to
`initialGames`/`initialRecord`.
In `@src/app/baseball/actions/calendar.ts`:
- Around line 262-273: The warning composition in createBaseballEvent is joining
multiple warning messages without a separator, so if both RSVP invite and linked
game creation fail the returned warning becomes unreadable; update the warning
aggregation so each message is separated cleanly before returning the result
object. Also add a unit test around createBaseballEvent that covers secondary
insert failures (baseball_event_attendance and baseball_games) and asserts the
returned warning field is populated and formatted correctly, since
EventsClient.tsx now branches on result.warning.
- Around line 225-232: The soft-warning path in `calendar.ts` only surfaces
`attendanceError` and `gameError` to the client, so repeated sync or schema
failures are not logged server-side. Update the `warnings` handling in the
calendar action flow to pass each warning through `logServerError` as soon as it
is pushed, while still returning the existing `warning` field on success. Use
the existing `logServerError` helper from `src/lib/server-error-logger.ts` and
apply it around the `attendanceError`/`gameError` branches in the calendar
action logic.
In `@src/app/baseball/actions/games.ts`:
- Around line 720-724: The season-year calculation in the baseball games flow is
using date parsing that is timezone-sensitive and can reintroduce the
cross-New-Year bug. Update the logic in the `games.ts` season bucketing path
(the `seasonYear` derivation near `getTeamGamesImpl` /
`getPlayerSeasonStatsImpl` behavior) so it reads the year directly from the
`game.game_date` string instead of constructing a `Date`, keeping it consistent
with the file’s string-based season filters and preventing mismatches between
the game row and the season row.
In `@src/app/baseball/actions/player-today-lift.ts`:
- Around line 107-109: Add dedicated unit coverage for player-today-lift.ts to
lock in the new team-local date behavior in the action that computes today via
resolveTeamTimezone and todayIsoInTz. Mock resolveTeamTimezone/todayIsoInTz (or
use a negative-UTC-offset team timezone fixture) and assert the
scheduled_date/checkin_date filtering in the player-today-lift action uses the
team-local calendar day rather than server UTC or new Date().toISOString().
In `@src/app/baseball/actions/signals.ts`:
- Around line 401-414: The scheduled_date used in the dedupe query is still
based on UTC via new Date().toISOString().slice(0, 10), which can diverge from
the team-local “Today” logic. Update the write path in signals.ts to use the
same team-local date source already used elsewhere, specifically
todayIsoInTz(resolveTeamTimezone(...)), so the helm_lifting_sessions lookup and
insert key stay aligned with player Today/readiness behavior. Keep the existing
(athlete_id, scheduled_date, title) dedupe in place, but feed it the team-local
date computed from the relevant team timezone.
In `@src/app/lifting/actions/__tests__/groups.test.ts`:
- Around line 4-12: The guard comment in groups.test.ts overstates coverage: the
suite only tests createGroup, updateGroup, and deleteGroup, but not the
member-list mutators addGroupMember and removeGroupMember that match the
stale-member-list regression. Add regression tests in the groups.test.ts
describe block for addGroupMember and removeGroupMember using the real
signatures from groups.ts, and verify both portals’ caches are revalidated; if
those functions are not meant to revalidate both caches, tighten the header
comment so it only claims the functions actually covered.
In `@src/app/lifting/actions/__tests__/player-sessions.test.ts`:
- Around line 112-129: The regression tests in player-sessions.test.ts are only
checking revalidation calls and can pass even if the action fails, so add an
explicit assertion that the action result succeeded before inspecting
revalidatePath. Update the relevant logMySetResult test cases to assert
res.success is true first, then keep the existing portal path checks; use the
logMySetResult call site and revalidatePath assertions as the anchors for both
affected tests.
In `@src/app/lifting/actions/__tests__/sessions.test.ts`:
- Around line 145-174: The Wave C revalidation test in logSetResult should
assert the full revalidation contract, not just the [sessionId] page call.
Update the expectations in sessions.test.ts to verify the second argument passed
to revalidatePath for /baseball/dashboard/lift,
/baseball/dashboard/performance/live, and /lifting/dashboard/sessions/live (and
keep the existing page assertion for /baseball/dashboard/lift/[sessionId]) so
accidental type swaps like layout/page changes are caught. Also remove the
duplicated resetTables() in beforeEach if you touch that setup block.
In `@src/app/lifting/actions/groups.ts`:
- Around line 186-191: The dynamic-group guard in groups.ts is duplicated across
addGroupMember, removeGroupMember, updateGroup, and deleteGroup, and the shared
error text is wrong for the rename/delete call sites. Extract the
`group.group_type === 'dynamic'` check into a single helper (or shared guard)
and pass an action-specific message from each caller: keep the membership
wording only for member-edit actions, and use rename/delete wording for
`updateGroup` and `deleteGroup`. Also verify whether `deleteGroup` should allow
a retirement path for dynamic groups; if not, keep the block but make the
message reflect deletion rather than membership.
In `@src/app/lifting/actions/player-sessions.ts`:
- Around line 14-21: The lifting actions module is hardcoding a sport-specific
portal path, which will keep duplicating revalidation logic as more sports are
added. Refactor the mutation flow in player-sessions.ts by centralizing the
extra portal paths into a small helper or shared array, and have
submitLiftReadiness, startMySession, logMySetResult, and completeMySession call
that helper instead of directly referencing BASEBALL_LIFT_PATH. Keep the generic
lifting path plus any sport-specific paths (like baseball and golf) in one place
so future portals only need a single update.
In `@src/app/lifting/actions/sessions.ts`:
- Around line 237-239: The repeated cache invalidation triple should be
centralized so the same revalidation logic is not duplicated across multiple
mutation paths. Extract the shared revalidatePath calls for BASEBALL_LIVE_PATH,
BASEBALL_LIFT_PATH, and BASEBALL_LIFT_SESSION_PATH into a helper in sessions.ts,
then replace each call site with that helper to keep the behavior consistent and
easier to maintain.
In `@src/components/baseball/calendar/CalendarFairway.tsx`:
- Around line 44-48: The LiveDot label is duplicating pluralization logic
instead of using the shared pluralizeEventLabel helper. Update the label
construction in CalendarFairway so the upcoming-events text is built through
pluralizeEventLabel, matching the existing count-badge formatting and keeping
all singular/plural rules centralized in one place.
In `@src/components/baseball/documents/DocumentsFairway.tsx`:
- Around line 206-213: The filtered-empty branch in DocumentsFairway currently
shows only the EmptyState with no way to recover from a zero-results
search/category view. Update the empty-state handling in DocumentsFairway so the
“No results” branch includes a “Clear filters” CTA, and wire it to reset the
active query and category via onSearchChange('') and onCategoryChange('all').
Keep the existing EmptyState variant="search" copy, but add the new action in
the same conditional branch that renders the filtered-to-zero state.
In `@src/components/baseball/games/EditGameModal.tsx`:
- Around line 74-96: The game type toggle and error banner in EditGameModal are
using non-canonical palette classes, so update the selected scrimmage state in
the game type button group to use the app’s approved design-system token instead
of purple-* and change the error banner styling to use the destructive token
rather than red-*. Keep the rest of the logic in EditGameModal unchanged and
align the className branches for the game type selector and validation message
with the repo’s permitted color tokens.
- Around line 74-96: The game-type and home/away button rows in EditGameModal
lack accessible selection semantics, so update each mutually exclusive group to
expose its state to assistive tech. In the game-type and home/away mappings, add
aria-pressed based on the current selection for each Button, and wrap each
button set in a role="group" container with aria-labelledby referencing the
existing label text (or give the label an id and reference it). Keep the visual
styling as-is, but make sure the selected option is announced and the group
relationship is clear.
- Around line 50-58: The optional baseball game fields are being dropped instead
of cleared because EditGameModal’s update payload converts empty values to
undefined, and updateGame skips undefined keys before calling the database
update. Update the EditGameModal submission logic so empty opponentName,
location, notes, and weather are sent as null values, and verify updateGame
preserves those nulls when building the update payload for baseball_games.
In `@src/components/baseball/games/GameDetailHeader.tsx`:
- Around line 47-49: The conditional render around EditGameModal is redundantly
gating on editOpen, which unmounts the modal immediately and prevents any close
animation from playing. Update GameDetailHeader so EditGameModal stays mounted
and let its own open prop control visibility/transition behavior, using the
component’s existing open/onClose contract rather than wrapping it in an extra
editOpen && check.
In `@src/components/baseball/games/GamesList.tsx`:
- Around line 36-46: The mount-time fetch in GamesList is still running on first
render and overwriting seeded initialGames/initialRecord, causing a duplicate
client fetch and loading flash. Update the existing useEffect that calls
fetchGames so it skips the initial mount when initialGames is already present,
while keeping the later prop-sync effects in place. Use the GamesList
component’s fetchGames callback and the mount effect near the existing useEffect
block to apply the guard cleanly.
In `@src/components/baseball/living-annual/RuledStatLine.tsx`:
- Line 132: The className change on RuledStatLine’s numeral wrapper removed
leading-none, which may break the intended baseline/rule alignment described by
the component’s header comment and the rule-rendering markup. Verify the numeral
still visually rests on the baseline rule in RuledStatLine, especially for the
hero size and the numeralSize-driven text classes, and restore leading-none if
the default line-height adds unwanted vertical space or detaches the glyph from
the rule.
In `@src/components/baseball/performance/PerformanceDashboardClient.tsx`:
- Around line 284-287: The “Checked In Today” count in
PerformanceDashboardClient still falls back to client time because the page does
not pass the team-local today value. Update the server page component to compute
todayIso there and pass it into PerformanceDashboardClient, then rely on the
today prop inside the reportedToday useMemo instead of new
Date().toLocaleDateString('en-CA').
In `@src/components/baseball/settings/ImportSourcesClient.tsx`:
- Around line 383-394: The inline title block in ImportSourcesClient is already
functionally correct, but it can be cleaned up for semantics and readability:
change the outer wrapper used for the page title region to the appropriate
Header landmark, and simplify the teamName/source registry text in the h1
section by removing the unnecessary template-literal wrapper while keeping the
same JSX interpolation.
In `@src/components/baseball/settings/PhilosophySettingsClient.tsx`:
- Around line 238-248: The back control is currently implemented as a Button
nested inside a Link, which creates invalid interactive nesting; update the
shared pattern in PhilosophySettingsClient by making the Link itself the single
interactive element styled like the existing button, or switch to a plain Button
that calls router.push for navigation. Apply the same fix to both
back-navigation instances in PhilosophySettingsClient so only one focusable
control remains, and do not rely on Button asChild since
src/components/ui/button.tsx does not support it.
In `@src/components/baseball/settings/TeamSettingsClient.tsx`:
- Around line 130-135: The Team Settings header JSX is duplicated in both
branches of TeamSettingsClient, so extract the shared `border-b`/`h1`/`p` block
into a reusable header component or helper and pass the subtitle text as a prop.
Update both render paths to use that shared symbol so the title styling stays in
one place and the duplicated markup does not drift. While doing so, remove the
unnecessary `flex items-center justify-between gap-4` wrapper behavior if the
extracted header still has only one child.
In `@src/components/baseball/stats-center/StatsCenterClient.tsx`:
- Around line 546-566: The two adjacent action buttons in StatsCenterClient use
the same upload icon, making Upload and Import Center visually
indistinguishable. Update the Import Center button in the StatsCenterClient
header actions to use a different import-related icon from the existing icon set
(for example a folder/open/import symbol), while keeping Upload on IconUpload.
Use the button JSX in StatsCenterClient as the target location and ensure the
two CTA icons are distinct.
In `@src/components/baseball/travel/TravelClient.tsx`:
- Around line 142-155: The empty-state “Create First Trip” CTA in TravelClient
should mirror the masthead “Create Trip” behavior by clearing editingItinerary
before opening the modal. Update the EmptyIssue action handler in TravelClient
so it calls setEditingItinerary(null) and then setShowCreateModal(true),
matching the existing Create Trip button logic for consistency and future
safety.
In `@src/components/lifting/programs/ProgramListClient.tsx`:
- Around line 45-48: The program link construction in ProgramListClient.tsx is
vulnerable to double-slash routes when basePath includes a trailing slash.
Normalize basePath once in the component (for example in ProgramListClient and
any shared helper) to remove any trailing slash, then use that normalized value
for the “New program” navigation and the per-card links currently built from
basePath and result.id/program.id.
In `@src/lib/baseball/__tests__/nav-manifest.test.ts`:
- Around line 312-336: The hardcoded PROGRAM_TYPES array in nav-manifest.test.ts
is not exhaustively tied to BaseballProgramType, so it can drift silently when
the union changes. Update the test near the PROGRAM_TYPES declaration to derive
the program-type list from a compile-time exhaustive source, using a
Record<BaseballProgramType, true> or equivalent pattern, so any new or removed
BaseballProgramType forces a compile error. Keep the rest of the loop over
COACH_HUB_ORDER, filterHubTabsByCapabilities, and filterHubTabsByProgramType
unchanged.
- Around line 353-378: The folded-route test in resolveActiveHub only checks for
any non-null hub, which can miss a wrong-hub match. Update the assertion in the
folded destination case to verify the resolved hub is the expected owner by
checking resolved?.tabs includes entry.foldedUnder or that resolved?.id matches
the owning hub, using BASEBALL_NAV_REGISTRY, foldedUnder, and resolveActiveHub
as the key symbols.
In `@src/lib/baseball/__tests__/resolve-active-hub.test.ts`:
- Around line 211-220: The cap test in resolve-active-hub.test.ts is incomplete
because it only covers four coach hub tab constants and misses the other coach
hubs exercised in this file. Update the existing describe/it.each block that
references COACH_STATS_TABS, COACH_DEVELOPMENT_TABS, COACH_MANAGEMENT_TABS, and
COACH_MESSAGES_TABS to also include the missing coach tab constants for the
team, recruiting, and academics hubs (the same COACH_*_TABS symbols used by the
hub-specific cases in this test file) so the “every coach hub caps at 3 rendered
subtabs” invariant is actually enforced everywhere.
---
Outside diff comments:
In `@docs/operations/BASEBALLHELM_FEATURE_READINESS_MATRIX.md`:
- Around line 1-112: The readiness-matrix CI check is failing because several
rows still reference closed Owner Issue URLs. Update the affected rows in
BASEBALLHELM_FEATURE_READINESS_MATRIX so each stale issue link is either removed
or replaced with the next real open issue, following the Maintenance rule that
resolved issues must not remain linked. Use the row feature names and issue
groupings in the matrix to locate each stale Owner Issue cell, and verify the
updated table still satisfies the check: only open, valid issue URLs or
intentional N/A entries should remain.
In `@src/app/baseball/`(dashboard)/dashboard/academics/AcademicsClient.tsx:
- Around line 161-172: The edit flow is converting an unset eligibility value
into true, which causes records with no eligibility on file to be overwritten
during unrelated edits. Update startEditing in AcademicsClient to preserve
student.is_eligible as null instead of defaulting it to true, and make sure the
submit path in upsertPlayerAcademics keeps null intact rather than applying a
fallback. If the UI needs a default, add an explicit “not on file” choice
instead of silently coercing the value.
In `@src/app/baseball/`(dashboard)/dashboard/calendar/page.tsx:
- Around line 75-101: The calendar query in the dashboard page is capping
results with `order('start_time', { ascending: true })` and `.limit(500)`, which
can silently drop the most-future events when the 455-day window is crowded.
Update the `eventsResult` fetch to prioritize future events over older past
entries, ideally by splitting the range or ordering so upcoming `start_time`
rows are guaranteed to survive the cap. If a cap is still kept, add a visible
warning/metric when the query truncates results so this loss is not silent.
In `@src/app/baseball/`(dashboard)/dashboard/performance/programs/page.tsx:
- Around line 40-93: The Programs page currently queries lifting data even when
the coach has BaseballHelm manage access but is not linked to the lifting org,
which can cause an empty list with no explanation. In ProgramsPage and
getPrograms, add a lifting-access fallback before querying: if
resolveBaseballLiftingOrg(teamId) returns no org or the user is not onboarded,
surface an onboarding/linked-org state or redirect/fail fast instead of
returning []. Keep the check aligned with the existing gate around
resolveBaseballCapabilities and the lifting-org lookup used by ProgramsPage.
In `@src/app/lifting/actions/player-sessions.ts`:
- Around line 10-22: The BASEBALL_LIFT_PATH constant is placed between import
statements, breaking import grouping and potentially triggering import/first
lint rules. Move BASEBALL_LIFT_PATH below all imports in player-sessions.ts,
keeping the existing symbols like revalidatePath, createClient, fromUntyped, and
resolveMyBaseballAthleteId together at the top before any non-import
declarations.
In `@src/app/lifting/actions/programs.ts`:
- Around line 118-155: Extract the repeated revalidation pair into a shared
helper used by createProgram, deleteProgram, addLiftWeek, addLiftDay,
addLiftSection, and addLiftPrescription so the LAB_PATH and
BASEBALL_PROGRAMS_PATH invalidation stays consistent in one place. Also update
addLiftWeek to revalidate the specific program routes using input.programId,
matching the per-program cache busting already done in updateProgram,
duplicateLiftDay, and duplicateLiftWeek. Use the existing symbols addLiftWeek,
createProgram, deleteProgram, and the path constants to wire this in without
duplicating the calls.
In `@src/components/baseball/documents/DocumentsFairway.tsx`:
- Around line 116-132: The upload CTA JSX is duplicated between the masthead and
empty-state actions in DocumentsFairway, which risks the two buttons drifting
apart. Extract the shared Button config/markup into a helper such as
uploadButton in DocumentsFairway and reuse it at both call sites, passing only
the label text ("Upload" and "Upload document") while keeping
onClick={onUpload}, busy={isUploadingDocument}, and the Upload icon consistent.
In `@src/components/baseball/settings/TeamSettingsClient.tsx`:
- Around line 3-16: The top-of-file comment in TeamSettingsClient is stale
because it still mentions Header as a reused primitive even though the component
no longer imports or uses it. Update that doc comment to match the actual
primitives used in TeamSettingsClient, keeping the wording aligned with the
current imports and JSX so it doesn’t mislead future changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Comment on lines
+542
to
+561
| const shellLink: ShellLinkComponent = useCallback( | ||
| ({ href, children, onClick, ...rest }) => ( | ||
| <Link | ||
| href={href} | ||
| prefetch | ||
| onClick={ | ||
| isShowcaseCoach && href === SHOWCASE_ORG_HOME_HREF | ||
| ? () => { | ||
| setSelectedTeamId(null); | ||
| onClick?.(); | ||
| } | ||
| : onClick | ||
| } | ||
| {...rest} | ||
| > | ||
| {children} | ||
| </Link> | ||
| ), | ||
| [isShowcaseCoach, setSelectedTeamId], | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== File outline =="
ast-grep outline "src/app/baseball/(dashboard)/BaseballFairwayShell.tsx" --view expanded || true
echo
echo "== Relevant lines around shellLink =="
sed -n '520,575p' "src/app/baseball/(dashboard)/BaseballFairwayShell.tsx" | cat -n
echo
echo "== Locate useTeams / usePlayerTeams definitions =="
rg -n "function useTeams|const useTeams|export .*useTeams|function usePlayerTeams|const usePlayerTeams|export .*usePlayerTeams" src/app src/hooks src/lib -g '!**/node_modules/**' || true
echo
echo "== File locations for those hooks =="
fd -a "useTeams.*|usePlayerTeams.*" src/app src/hooks src/lib || trueRepository: njrini99-code/helmv3
Length of output: 4145
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== use-teams.ts =="
sed -n '1,220p' src/hooks/use-teams.ts | cat -n
echo
echo "== use-player-teams.ts =="
sed -n '1,220p' src/hooks/use-player-teams.ts | cat -nRepository: njrini99-code/helmv3
Length of output: 6115
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== team store definition =="
sed -n '1,260p' src/stores/team-store.ts | cat -n
echo
echo "== any custom equality or re-created actions =="
rg -n "setSelectedTeamId|setTeams|setIsLoading|getSelectedTeam|create\\(" src/stores/team-store.ts src/stores -g '!**/node_modules/**' || trueRepository: njrini99-code/helmv3
Length of output: 3233
Forward the click event in the showcase link handler
src/app/baseball/(dashboard)/BaseballFairwayShell.tsx:542-561 — the showcase branch calls onClick?.() without the MouseEvent, while the fallback path forwards it. Pass e through so link consumers keep modifier-key/default-prevented behavior intact.
🤖 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)/BaseballFairwayShell.tsx around lines 542 -
561, The showcase link handler in BaseballFairwayShell’s shellLink callback
drops the click event when isShowcaseCoach is true and href matches
SHOWCASE_ORG_HOME_HREF. Update the Link onClick branch to accept and forward the
MouseEvent to onClick after calling setSelectedTeamId(null), matching the
fallback path so consumers preserve modifier-key and default-prevented behavior.
Comment on lines
+14
to
+21
|
|
||
| // Baseball Wave C renders these same helm_lifting_sessions / | ||
| // helm_lifting_readiness_checkins reads at /baseball/dashboard/lift and | ||
| // /baseball/dashboard/lift/[sessionId] (see src/app/baseball/(dashboard)/ | ||
| // dashboard/lift/page.tsx + [sessionId]/page.tsx) — bust both alongside | ||
| // '/lifting/dashboard/lift' below so a baseball athlete's own lift portal | ||
| // reflects a checkin/set/status mutation immediately. | ||
| const BASEBALL_LIFT_PATH = '/baseball/dashboard/lift'; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Sport-specific path baked into the generic lifting actions module.
This file also resolves golf athletes (golf_players branch in getMyLiftToday, lines 60-85), but only a baseball consumer path gets a constant + revalidation call. Every future consuming portal (golf's own lift view, or a third one) means another hardcoded *_LIFT_PATH const and another 1-2 lines duplicated across submitLiftReadiness, startMySession, logMySetResult, completeMySession. Consider centralizing "extra portal paths to bust on mutation" as a small array/helper (e.g. revalidateLiftingPortals(sessionId?: string)) so adding a sport doesn't require touching every mutation function again.
🤖 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/player-sessions.ts` around lines 14 - 21, The lifting
actions module is hardcoding a sport-specific portal path, which will keep
duplicating revalidation logic as more sports are added. Refactor the mutation
flow in player-sessions.ts by centralizing the extra portal paths into a small
helper or shared array, and have submitLiftReadiness, startMySession,
logMySetResult, and completeMySession call that helper instead of directly
referencing BASEBALL_LIFT_PATH. Keep the generic lifting path plus any
sport-specific paths (like baseball and golf) in one place so future portals
only need a single update.
…s across 5 clusters)
Root-cause fixes for every Major + correctness finding:
- backfill-baseball-slash-lines: paginate past the PostgREST 1000-row cap
(both the aggregates scan and per-player stats fetch)
- games.ts: cross-New-Year season_year bug — parse year from the date string
- signals.ts + performance page/client: team-local "today"
(todayIsoInTz/resolveTeamTimezone) instead of server-UTC
- EditGameModal: optional fields clearable (null not undefined); off-palette
purple-*/red-* → canonical tokens
- GamesList: honor seeded initialGames/initialRecord (no double fetch)
- lift/_lift-athlete-context: try all active memberships before null;
errors → logServerError
- stats/games + performance/{groups,live}: DB errors no longer masquerade
as empty states
- DevPlanClient: stuck-spinner no-player path resolved; parseLocalDate for
header dates
- scouting: honest null-context failure state (no fabricated NavContext)
- breadcrumbs: settings subpages no longer collapse to hub label
- EventsClient: error state and first-event CTA now mutually exclusive
- e2e box-score: teardown guarded per its own invariant + deduped helper
- lifting actions: accurate guard messages, shared revalidation helper,
missing member add/remove revalidation tests, trailing-slash guard
- nav tests: exhaustive PROGRAM_TYPES from canonical array; folded-route
hub assertions; full coach-hub cap coverage
- + a11y/copy/token nits per review
Deferred by design (mission plan W4): shared header-primitive extractions —
those headers are replaced wholesale by the Living Annual masthead migration
(docs/audits/PRODUCTION_READINESS_MISSION_2026-07-09.md). Readiness-matrix
closed-issue cleanup lands with the W7 full matrix refresh.
Also adds docs/audits/PRODUCTION_READINESS_MISSION_2026-07-09.md (mission
plan: target 8/8/8/8 nav IA + waves W0-W10 from the 30-agent discovery).
Gates: tsc=0, eslint --max-warnings 0 =0, unit 4505 passed / 0 failed.
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. |
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.
|
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 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