[codex] Prepare benchmark leaderboard implementation plan - #562
Open
zacjones93 wants to merge 62 commits into
Open
[codex] Prepare benchmark leaderboard implementation plan#562zacjones93 wants to merge 62 commits into
zacjones93 wants to merge 62 commits into
Conversation
Contributor
WalkthroughBenchmark support is implemented across competition capabilities, perpetual date handling, absolute-tier scoring, benchmark submissions, leaderboard aggregation, organizer configuration, discovery and stats routes, seed data, tests, and related documentation. ChangesBenchmark leaderboard
Estimated code review effort: 5 (Critical) | ~150 minutes Sequence Diagram(s)sequenceDiagram
participant Athlete
participant submitVideoFn
participant BenchmarkSubmissions
participant AbsoluteTierAlgorithm
participant Database
Athlete->>submitVideoFn: submit score or video
submitVideoFn->>BenchmarkSubmissions: load benchmark context
BenchmarkSubmissions->>AbsoluteTierAlgorithm: calculate candidate tier
AbsoluteTierAlgorithm-->>BenchmarkSubmissions: return tier
BenchmarkSubmissions->>Database: compare and persist best score
BenchmarkSubmissions-->>submitVideoFn: return submission result
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…rker [codex] Add benchmark competition capabilities
…tives [codex] Add benchmark schema primitives
[codex] Seed benchmark training guide battery
Add benchmark absolute-tier scoring
…etest [codex] Add benchmark submission retest flow
…t/hillerfit-plan
…stats M4: benchmark leaderboard stats
Resolve migration numbering collision: renumber crew-billing-state-audit to 0004 and crew-volunteer-intelligence-schema to 0005 so they chain after 0003_benchmark-battery. Fix snapshot prevId chain and journal accordingly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Convert wiki links pointing to plain markdown doc files (not lat sections) to backtick path references so lat check passes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
|
This PR is large and would use a significant portion of your monthly review quota. Comment |
Contributor
Author
|
@cubic-dev-ai review this |
Contributor
@zacjones93 I have started the AI code review. It will take a few minutes to complete. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6b2b0035ae
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| label: "Benchmark", | ||
| createPickerDescription: "Perpetual benchmark board with video submissions", | ||
| leaderboardVariant: "online", | ||
| selectableOnCreate: true, |
There was a problem hiding this comment.
When an organizer selects this new Benchmark option, the create flow only writes the competition record; I searched the repo and the only benchmark_batteries inserts are in seed code, while the benchmark scoring page just shows setupRequired when the battery is missing. Since benchmark submissions and tier setup require a battery/Open-division context, newly created benchmark competitions become a dead end unless creation also bootstraps the battery (and required Open division) or this option stays hidden.
Useful? React with 👍 / 👎.
| @@ -128,6 +130,10 @@ export const scoresTable = mysqlTable( | |||
| table.userId, | |||
| table.scalingLevelId, | |||
There was a problem hiding this comment.
Adding nullable scalingLevelId to the MySQL unique key means scores with scalingLevelId = NULL are no longer unique, because MySQL treats NULL values in unique indexes as distinct. For any competition/registration without a division, the existing onDuplicateKeyUpdate submission paths that set scalingLevelId: registration.divisionId will insert another score on each resubmission instead of updating the prior row, leaving leaderboard/read paths to pick an arbitrary stale duplicate. Use a non-null sentinel/generated key for the open division case or require a non-null division before relying on this upsert key.
Useful? React with 👍 / 👎.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
5 issues found across 147 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/wodsmith-start/src/routes/api/compete/video/submit.ts">
<violation number="1" location="apps/wodsmith-start/src/routes/api/compete/video/submit.ts:94">
P3: The new perpetual-window branch is currently dead in this route: benchmark competitions are rejected earlier, and benchmark is the only registered perpetual type. Keeping unreachable logic here can mislead future edits about what code paths actually execute.</violation>
</file>
<file name="packages/wodsmith-db/src/schemas/benchmarks.ts">
<violation number="1" location="packages/wodsmith-db/src/schemas/benchmarks.ts:70">
P2: This constraint currently blocks creating a second benchmark battery for the same owner even with a different slug because uniqueness is enforced on `ownerKey` alone. Using a composite unique index on `(ownerKey, slug)` preserves the intended slug uniqueness behavior without forcing one-battery-per-owner.</violation>
</file>
<file name="apps/wodsmith-start/src/server/benchmark-open-join-rate-limit.ts">
<violation number="1" location="apps/wodsmith-start/src/server/benchmark-open-join-rate-limit.ts:15">
P2: Rate limiting is currently process-local, so enforcement becomes inconsistent across instances and after restarts. Using a shared backing store (DB/KV/Redis) for the counter window would keep join-attempt limits effective in production topologies.</violation>
</file>
<file name="ai/research/hillerfit-benchmark-leaderboard-guide.md">
<violation number="1" location="ai/research/hillerfit-benchmark-leaderboard-guide.md:29">
P1: Create-picker selectability of benchmark is contradictory between planning docs. The guide's §5.2 registry entry sets `selectableOnCreate: false` and tasks.md M0a says "Benchmark is not selectable in the generic create picker", but `lat.md/competition-type-capabilities.md` states "Benchmark is registered and selectable in the generic create picker" and `lat.md/organizer-dashboard.md` says the create form uses "registry-backed in-person, online, and benchmark type options." This is a fork in the implementation contract: either the create picker exposes benchmark or it doesn't, and the docs must agree before M1a/M1b tasks execute against the wrong selectability.</violation>
<violation number="2" location="ai/research/hillerfit-benchmark-leaderboard-guide.md:936">
P2: Contradictory `isOpenJoin` seed default between planning docs and the LAT. The guide's M1 milestone, tasks.md M1b, and assumptions-and-decisions.md all specify `isOpenJoin: true` as the seed default. But `lat.md/domain.md` states "The seed keeps `isOpenJoin=false`, so registration remains explicit." This is a concrete behavioral difference: with `isOpenJoin: true` athletes auto-register on first submit via guarded open-join; with `isOpenJoin: false` they must use the standard registration flow before scoring. The M3 submission acceptance criteria in tasks.md even specify "Guarded `isOpenJoin` is transactional, idempotent, published/visible only, waiver/profile gated, and rate-limited" — implying open-join is expected. The two stances must be reconciled before the seed and submission path diverge.</violation>
</file>
Tip: cubic used a learning from your PR history. Let your coding agent read cubic learnings directly with the cubic MCP.
Re-trigger cubic
| | **D-B** (§12.1) | Sex axis | **Per-athlete `variant` → one unified Overall/100 board** (not Men/Women divisions) | The single unified scale *is* the product — but it's the biggest engine change (`EventScoreInput.variant`, gender threading), not a free win. | | ||
| | **D-C** (§12.5) | Windowing + retest history | **Defer windowing + per-attempt history to v2; v1 is _best-to-date_ only** (live `scores` row, kept monotonic via **keep-best-on-write**, §8.1) | `score_attempts`/`promoteBest` is the riskiest piece (sortKey needs round-level data). v1 is honestly "best-to-date," **not** "all-time": a keep-best guard makes the single live row monotonic, but without history an `invalid` action can't restore a prior best (that's v2). | | ||
| | **D-D** (§12.12) | Product gate | **Confirm HillerFit partnership + 1,160 thresholds are licensed** — else ship "post your bench" first | Whole schema is sized around 58×2×10; if the data isn't locked, M1's shape is speculative. | | ||
| | **D-A** (§12.2) ✅ fixed | Competition type | **Distinct `competitionType:"benchmark"` behind a capability registry** (not reuse `"online"`), **split into M0a chokepoints + M0b deferred cleanup** | Keeps the standalone online product clean; a mechanical, behavior-preserving refactor of the **~129** `=== "online"`/`"in-person"` checks (verified: `competitionType` 60 / `isOnline` 55 / `isInPerson` 14 across **61 files**) → capability lookups. v1 refactors **only the chokepoints benchmark needs (M0a)**; the rest is deferred (M0b). *(Reversed per owner steer; scoped down per critique #6.)* | |
Contributor
There was a problem hiding this comment.
P1: Create-picker selectability of benchmark is contradictory between planning docs. The guide's §5.2 registry entry sets selectableOnCreate: false and tasks.md M0a says "Benchmark is not selectable in the generic create picker", but lat.md/competition-type-capabilities.md states "Benchmark is registered and selectable in the generic create picker" and lat.md/organizer-dashboard.md says the create form uses "registry-backed in-person, online, and benchmark type options." This is a fork in the implementation contract: either the create picker exposes benchmark or it doesn't, and the docs must agree before M1a/M1b tasks execute against the wrong selectability.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At ai/research/hillerfit-benchmark-leaderboard-guide.md, line 29:
<comment>Create-picker selectability of benchmark is contradictory between planning docs. The guide's §5.2 registry entry sets `selectableOnCreate: false` and tasks.md M0a says "Benchmark is not selectable in the generic create picker", but `lat.md/competition-type-capabilities.md` states "Benchmark is registered and selectable in the generic create picker" and `lat.md/organizer-dashboard.md` says the create form uses "registry-backed in-person, online, and benchmark type options." This is a fork in the implementation contract: either the create picker exposes benchmark or it doesn't, and the docs must agree before M1a/M1b tasks execute against the wrong selectability.</comment>
<file context>
@@ -1,37 +1,38 @@
-| **D-B** (§12.1) | Sex axis | **Per-athlete `variant` → one unified Overall/100 board** (not Men/Women divisions) | The single unified scale *is* the product — but it's the biggest engine change (`EventScoreInput.variant`, gender threading), not a free win. |
-| **D-C** (§12.5) | Windowing + retest history | **Defer windowing + per-attempt history to v2; v1 is _best-to-date_ only** (live `scores` row, kept monotonic via **keep-best-on-write**, §8.1) | `score_attempts`/`promoteBest` is the riskiest piece (sortKey needs round-level data). v1 is honestly "best-to-date," **not** "all-time": a keep-best guard makes the single live row monotonic, but without history an `invalid` action can't restore a prior best (that's v2). |
-| **D-D** (§12.12) | Product gate | **Confirm HillerFit partnership + 1,160 thresholds are licensed** — else ship "post your bench" first | Whole schema is sized around 58×2×10; if the data isn't locked, M1's shape is speculative. |
+| **D-A** (§12.2) ✅ fixed | Competition type | **Distinct `competitionType:"benchmark"` behind a capability registry** (not reuse `"online"`), **split into M0a chokepoints + M0b deferred cleanup** | Keeps the standalone online product clean; a mechanical, behavior-preserving refactor of the **~129** `=== "online"`/`"in-person"` checks (verified: `competitionType` 60 / `isOnline` 55 / `isInPerson` 14 across **61 files**) → capability lookups. v1 refactors **only the chokepoints benchmark needs (M0a)**; the rest is deferred (M0b). *(Reversed per owner steer; scoped down per critique #6.)* |
+| **D-B** (§12.1) ✅ fixed | Sex axis | **Per-athlete `variant` → one unified Overall/100 board** (not Men/Women divisions) | The single unified scale *is* the product — but it's the biggest engine change (`EventScoreInput.variant`, gender threading), not a free win. |
+| **D-C** (§12.5) ✅ fixed | Windowing + retest history | **Defer windowing + per-attempt history to v2; v1 is _best-to-date_ only** (live `scores` row, kept monotonic via **keep-best-on-write**, §8.1) | `score_attempts`/`promoteBest` is the riskiest piece (sortKey needs round-level data). v1 is honestly "best-to-date," **not** "all-time": a keep-best guard makes the single live row monotonic, but without history an `invalid` action can't restore a prior best (that's v2). |
</file context>
| } | ||
| } | ||
|
|
||
| if (competitionCan(competition.competitionType, "perpetual")) { |
Contributor
There was a problem hiding this comment.
P3: The new perpetual-window branch is currently dead in this route: benchmark competitions are rejected earlier, and benchmark is the only registered perpetual type. Keeping unreachable logic here can mislead future edits about what code paths actually execute.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/wodsmith-start/src/routes/api/compete/video/submit.ts, line 94:
<comment>The new perpetual-window branch is currently dead in this route: benchmark competitions are rejected earlier, and benchmark is the only registered perpetual type. Keeping unreachable logic here can mislead future edits about what code paths actually execute.</comment>
<file context>
@@ -85,6 +91,16 @@ async function checkVideoSubmissionWindow(
}
}
+ if (competitionCan(competition.competitionType, "perpetual")) {
+ if (perpetualSubmissionsClosed(competition)) {
+ return {
</file context>
…s upsert MySQL treats NULLs in unique indexes as distinct, so every null-division resubmission inserted a new row instead of upserting. Adds a stored generated scalingKey = COALESCE(scaling_level_id, '') column and rekeys idx_scores_competition_user_unique on it. Also documents the ownerKey teamId:slug convention on benchmark batteries. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…change deleteBenchmarkTest now unlinks track_workouts rows in the same transaction; updateBenchmarkTest propagates categoryKey changes to linked events. Server-fn auth/not-found/capability errors now use the coded NOT_AUTHORIZED/NOT_FOUND/FORBIDDEN convention. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ts to competition saveCompetitionEventFn writes now run in one db.transaction so a failed benchmark-link check rolls back workout/movement edits. resolveBenchmarkTestLink takes the tx handle and uses SELECT ... FOR UPDATE, closing the check/update race, and the conflict query joins programming tracks to enforce competition-wide (not per-track) test uniqueness. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The isBenchmarkCompetition check ran before each handler's try/catch, so a DB failure escaped the JSON/CORS error shape. Moved inside the try block; behavior otherwise unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…-limit entries doesBenchmarkScoreImprove now falls through to tiebreak comparison when tier/primary/secondary tie, matching leaderboard ordering. Open-join rate limiter sweeps expired entries once the map hits 1000 keys so per-isolate memory stays bounded. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…filtered views Review summaries are computed from the full submission set before invalid videos are filtered out of display. Benchmark context builds from unfiltered track workouts so division-filtered views keep tier/category metadata while validation still fails closed. competitionType prop narrowed back to CompetitionTypeId. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Non-hybrid ascending comparison treated a cap time as a finish time, awarding tiers to capped time-with-cap submissions. Threshold checks now require scored status. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Batch event submission fetch loaded benchmark context (3 queries) per event inside the loop; now prefetched with Promise.all into a map. Also removes a dead ternary branch in submitVideoFn. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
No organizer battery-setup flow exists yet, so a picker-created benchmark competition dead-ends at the setupRequired tiers page. Sets selectableOnCreate: false per the plan spec (guide 5.2, tasks M0a), updates the registry truth-table and create-form tests, and aligns lat.md capability/organizer docs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Stats: division picker stays visible on empty divisions; stale division query params normalize against known divisions. Stat line: in-review rows no longer show Untested; points fallback drops the /100 unit. Tiers editor: unsaved threshold edits survive other-section saves, category rows track test-count changes, rating band rows lock while saving, hybrid flip tier rejects fractional values, videoPolicy validated before submit. Athlete panel refresh guards stale responses; results header respects auto-publish default. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ds coverage Negative testCount case no longer hides behind an invalid key; adds min<=max and duplicate-key validation tests the docstring promised; branding boundary now covers _auth, index, privacy, terms, and maintenance routes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…te generated gitnexus skills Planning docs claimed isOpenJoin: true but the seed and submit flow require explicit registration; docs now say false with open join deferred to M3. AGENTS/CLAUDE gitnexus CLI tables note the skill files are generated locally and gitignored. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The full registration flow can exceed Playwright's default 30s test timeout on cold CI runners — the first attempt registered successfully server-side but timed out before the redirect assertion, and retries then failed because the seeded user was already registered. Bump the test budget to 60s and treat the "You're Registered!" card as success on retry so the test is idempotent. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
2 issues found across 39 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/wodsmith-start/src/server-fns/video-submission-fns.ts">
<violation number="1" location="apps/wodsmith-start/src/server-fns/video-submission-fns.ts:1272">
P2: Large benchmark batches can spike DB load and slow or fail this endpoint because every requested event now launches `getBenchmarkSubmissionContext` concurrently. Capping concurrency (and deduping IDs) would keep latency improvements without creating unbounded query fan-out.</violation>
</file>
<file name="apps/wodsmith-start/src/components/compete/athlete-score-submission-panel.tsx">
<violation number="1" location="apps/wodsmith-start/src/components/compete/athlete-score-submission-panel.tsx:209">
P2: Division switching during an in-flight submit can leave this panel stuck in loading and allow late old-division refreshes to win, because refreshSubmissions advances the same sequence token used by the effect’s loading lifecycle. Consider isolating refresh staleness checks from the effect token (or validating current registration/division before applying refresh results) so refreshes cannot invalidate the active effect completion.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Comment on lines
+1272
to
+1279
| const contexts = await Promise.all( | ||
| data.trackWorkoutIds.map((twId) => | ||
| getBenchmarkSubmissionContext(data.competitionId, twId), | ||
| ), | ||
| ) | ||
| data.trackWorkoutIds.forEach((twId, index) => { | ||
| benchmarkContextMap.set(twId, contexts[index]) | ||
| }) |
Contributor
There was a problem hiding this comment.
P2: Large benchmark batches can spike DB load and slow or fail this endpoint because every requested event now launches getBenchmarkSubmissionContext concurrently. Capping concurrency (and deduping IDs) would keep latency improvements without creating unbounded query fan-out.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/wodsmith-start/src/server-fns/video-submission-fns.ts, line 1272:
<comment>Large benchmark batches can spike DB load and slow or fail this endpoint because every requested event now launches `getBenchmarkSubmissionContext` concurrently. Capping concurrency (and deduping IDs) would keep latency improvements without creating unbounded query fan-out.</comment>
<file context>
@@ -1262,6 +1262,23 @@ export const getBatchEventVideoSubmissionsFn = createServerFn({
+ BenchmarkSubmissionContext | null
+ >()
+ if (competition?.competitionType === "benchmark") {
+ const contexts = await Promise.all(
+ data.trackWorkoutIds.map((twId) =>
+ getBenchmarkSubmissionContext(data.competitionId, twId),
</file context>
Suggested change
| const contexts = await Promise.all( | |
| data.trackWorkoutIds.map((twId) => | |
| getBenchmarkSubmissionContext(data.competitionId, twId), | |
| ), | |
| ) | |
| data.trackWorkoutIds.forEach((twId, index) => { | |
| benchmarkContextMap.set(twId, contexts[index]) | |
| }) | |
| const uniqueTrackWorkoutIds = [...new Set(data.trackWorkoutIds)] | |
| const contexts: Array<BenchmarkSubmissionContext | null> = [] | |
| const batchSize = 10 | |
| for (let i = 0; i < uniqueTrackWorkoutIds.length; i += batchSize) { | |
| const batchIds = uniqueTrackWorkoutIds.slice(i, i + batchSize) | |
| contexts.push( | |
| ...(await Promise.all( | |
| batchIds.map((twId) => | |
| getBenchmarkSubmissionContext(data.competitionId, twId), | |
| ), | |
| )), | |
| ) | |
| } | |
| uniqueTrackWorkoutIds.forEach((twId, index) => { | |
| benchmarkContextMap.set(twId, contexts[index] ?? null) | |
| }) |
| } | ||
|
|
||
| let cancelled = false | ||
| const seq = ++fetchSeqRef.current |
Contributor
There was a problem hiding this comment.
P2: Division switching during an in-flight submit can leave this panel stuck in loading and allow late old-division refreshes to win, because refreshSubmissions advances the same sequence token used by the effect’s loading lifecycle. Consider isolating refresh staleness checks from the effect token (or validating current registration/division before applying refresh results) so refreshes cannot invalidate the active effect completion.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/wodsmith-start/src/components/compete/athlete-score-submission-panel.tsx, line 209:
<comment>Division switching during an in-flight submit can leave this panel stuck in loading and allow late old-division refreshes to win, because refreshSubmissions advances the same sequence token used by the effect’s loading lifecycle. Consider isolating refresh staleness checks from the effect token (or validating current registration/division before applying refresh results) so refreshes cannot invalidate the active effect completion.</comment>
<file context>
@@ -193,15 +193,20 @@ export function AthleteScoreSubmissionPanel({
}
- let cancelled = false
+ const seq = ++fetchSeqRef.current
setLoading(true)
setFetchError(false)
</file context>
Flip the benchmark registry entry to selectableOnCreate, which flows through the create picker, form schema, and server schema via the shared selectable-type guards. Selecting Benchmark switches the form into perpetual mode (start date + optional end date, no multi-day toggle). Stored benchmark competitions still cannot be switched to another type in the edit form. Also lands the public discovery split: a dedicated /benchmarks index of perpetual boards, nav links on desktop and mobile, and the home index filtered to non-perpetual competitions. Verified end-to-end in the browser: created a benchmark competition via the organizer form; its dashboard exposes Benchmark scoring (tiers) and Submissions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
2 issues found across 13 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/wodsmith-start/src/routes/benchmarks.tsx">
<violation number="1" location="apps/wodsmith-start/src/routes/benchmarks.tsx:123">
P2: Benchmark status can flip to `Completed` early for non-UTC competitions. This route already evaluates start dates in `comp.timezone`, but `perpetualSubmissionsClosed(comp)` uses a UTC end-of-day check; consider using the competition timezone for the end-date check too.</violation>
<violation number="2" location="apps/wodsmith-start/src/routes/benchmarks.tsx:255">
P2: Empty state "Clear search" button desyncs the search input from the URL. Clicking it navigates away from the search params, so all benchmarks are shown, but the search input still displays the old query because `localSearch` in `BenchmarksPage` is not updated. The user sees a stale query in the search box that does not match the unfiltered results. Pass `onClearSearch` as a prop to `EmptyState` and call `handleSearchChange("")` instead of navigating directly, or add a `useEffect` in `BenchmarksPage` that syncs `localSearch` with `search.q` whenever the URL search changes.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| variant="ghost" | ||
| size="sm" | ||
| className="mt-4 text-muted-foreground" | ||
| onClick={() => navigate({ search: {} })} |
Contributor
There was a problem hiding this comment.
P2: Empty state "Clear search" button desyncs the search input from the URL. Clicking it navigates away from the search params, so all benchmarks are shown, but the search input still displays the old query because localSearch in BenchmarksPage is not updated. The user sees a stale query in the search box that does not match the unfiltered results. Pass onClearSearch as a prop to EmptyState and call handleSearchChange("") instead of navigating directly, or add a useEffect in BenchmarksPage that syncs localSearch with search.q whenever the URL search changes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/wodsmith-start/src/routes/benchmarks.tsx, line 255:
<comment>Empty state "Clear search" button desyncs the search input from the URL. Clicking it navigates away from the search params, so all benchmarks are shown, but the search input still displays the old query because `localSearch` in `BenchmarksPage` is not updated. The user sees a stale query in the search box that does not match the unfiltered results. Pass `onClearSearch` as a prop to `EmptyState` and call `handleSearchChange("")` instead of navigating directly, or add a `useEffect` in `BenchmarksPage` that syncs `localSearch` with `search.q` whenever the URL search changes.</comment>
<file context>
@@ -0,0 +1,262 @@
+ variant="ghost"
+ size="sm"
+ className="mt-4 text-muted-foreground"
+ onClick={() => navigate({ search: {} })}
+ >
+ Clear search
</file context>
| }) | ||
| .map((comp) => ({ | ||
| ...comp, | ||
| _status: perpetualSubmissionsClosed(comp) |
Contributor
There was a problem hiding this comment.
P2: Benchmark status can flip to Completed early for non-UTC competitions. This route already evaluates start dates in comp.timezone, but perpetualSubmissionsClosed(comp) uses a UTC end-of-day check; consider using the competition timezone for the end-date check too.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/wodsmith-start/src/routes/benchmarks.tsx, line 123:
<comment>Benchmark status can flip to `Completed` early for non-UTC competitions. This route already evaluates start dates in `comp.timezone`, but `perpetualSubmissionsClosed(comp)` uses a UTC end-of-day check; consider using the competition timezone for the end-date check too.</comment>
<file context>
@@ -0,0 +1,262 @@
+ })
+ .map((comp) => ({
+ ...comp,
+ _status: perpetualSubmissionsClosed(comp)
+ ? ("past" as const)
+ : ("active" as const),
</file context>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (15)
apps/wodsmith-start/src/server/benchmark-scoring-tiers.ts (1)
629-651: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider batching the threshold upsert.
upsertBenchmarkTierThresholdRowsissues one round-trip per row inside a transaction. For a full battery (tests × variants × maxTier) this can be dozens of sequential statements, extending the transaction's lock hold time. Drizzle MySQL supports a single multi-rowinsert(...).values([...]).onDuplicateKeyUpdate(...), which collapses this into one statement.♻️ Sketch
await tx .insert(benchmarkTierThresholdsTable) .values(rows.map((row) => ({ testId: row.testId, variant: row.variant, tier: row.tier, rawValue: row.rawValue, thresholdValue: row.thresholdValue, }))) .onDuplicateKeyUpdate({ set: { rawValue: sql`values(${benchmarkTierThresholdsTable.rawValue})`, thresholdValue: sql`values(${benchmarkTierThresholdsTable.thresholdValue})`, updatedAt: new Date(), }, })🤖 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 `@apps/wodsmith-start/src/server/benchmark-scoring-tiers.ts` around lines 629 - 651, The upsert helper is doing one insert per threshold row inside a transaction, which creates many sequential round-trips; update upsertBenchmarkTierThresholdRows to batch all rows into a single multi-row insert with onDuplicateKeyUpdate so the transaction only executes one statement. Use the existing benchmarkTierThresholdsTable insert flow in upsertBenchmarkTierThresholdRows, map the incoming rows array to the values payload, and keep the same per-row fields and updatedAt refresh in the duplicate-key update path.apps/wodsmith-start/test/components/benchmark-rating-bands-editor.test.tsx (1)
1-243: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing
@lat:reference for these tests.Same as the other new benchmark test files — as per coding guidelines, each test covering a spec section should have an
@lat:comment placed next to it; none are present here.🤖 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 `@apps/wodsmith-start/test/components/benchmark-rating-bands-editor.test.tsx` around lines 1 - 243, The new RatingBandsEditor test file is missing the required `@lat`: annotations for each spec section. Add the appropriate `@lat`: comment next to every describe/it group in benchmark-rating-bands-editor.test.tsx so the tests are traceable to the correct benchmark spec sections, following the pattern used in the other benchmark test files.Source: Coding guidelines
apps/wodsmith-start/scripts/seed/seeders/23-benchmark.ts (1)
8-41: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueConsider wrapping the multi-table seed inserts in a transaction.
seed()performs 14 sequentialbatchInsertcalls across many FK-dependent tables (teams → memberships → scaling → competitions → workouts → benchmark tables → athlete rows) without a surrounding transaction. If a later insert throws mid-sequence (e.g., an encoding error frombuildBenchmarkAthleteSeedRows, or a network blip), earlier tables are left partially populated. Since inserts useINSERT IGNOREand IDs are deterministic, a full re-run is idempotent, so this is low risk for a dev-only seed script, but wrapping inclient.beginTransaction()/commit()/rollback()would make partial failures cleanly recoverable.🤖 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 `@apps/wodsmith-start/scripts/seed/seeders/23-benchmark.ts` around lines 8 - 41, The seed() function in 23-benchmark.ts performs many dependent batchInsert calls without atomicity, so a mid-sequence failure can leave partially seeded data behind. Wrap the full insert sequence in a transaction using the Connection client’s begin/commit/rollback flow, and ensure any error rolls back before rethrowing; locate the work inside seed() around the rows, athleteRows, and batchInsert calls.apps/wodsmith-start/test/components/benchmark-categories-manager.test.tsx (1)
1-164: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing
@lat:reference for these tests.As per coding guidelines,
apps/wodsmith-start/test/**/*.{ts,tsx,js,jsx}requires an@lat:comment next to each test covering a spec section, "and do not duplicate refs." None of theit(...)blocks in this suite carry such an annotation.🤖 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 `@apps/wodsmith-start/test/components/benchmark-categories-manager.test.tsx` around lines 1 - 164, Add the required `@lat`: annotation to each it(...) test in CategoriesManager so the suite maps to the relevant spec section, and ensure each reference is unique with no duplicates. Update the tests in benchmark-categories-manager.test.tsx near the existing describe("CategoriesManager") blocks, keeping the comments adjacent to each case and using the appropriate spec identifiers for the behaviors covered.Source: Coding guidelines
apps/wodsmith-start/test/components/benchmark-branding-boundary.test.ts (2)
9-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBranding-boundary scope excludes server-side strings that could surface to users.
CUSTOMER_FACING_PATHScoverssrc/componentsand public route files, but notsrc/serverorsrc/server-fns, where error messages or descriptions returned to the client could also leak "HillerFit" text (e.g., via thrown errors rendered in toasts). Given the PR's explicit branding-boundary requirement, consider whether server-side user-facing strings should also be scanned.🤖 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 `@apps/wodsmith-start/test/components/benchmark-branding-boundary.test.ts` around lines 9 - 18, The branding-boundary test scope in CUSTOMER_FACING_PATHS is too narrow and misses server-side strings that can still reach users. Update benchmark-branding-boundary.test.ts to include user-facing server locations such as src/server and src/server-fns, alongside the existing component and route paths, so the boundary check also scans server-returned error/description text that may surface in the UI.
1-41: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing
@lat:reference for this test.As per coding guidelines,
apps/wodsmith-start/test/**/*.{ts,tsx,js,jsx}requires "Place one@lat:comment next to each test that covers a spec section... Put the@lat:comment next to the relevant test rather than at the top of the file." This new branding-boundary test (and itsitblock at Line 30) has no@lat:annotation, so if this test is intended to satisfy a traceability leaf-section requirement inlat.md/, that link is missing.🤖 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 `@apps/wodsmith-start/test/components/benchmark-branding-boundary.test.ts` around lines 1 - 41, Add the missing `@lat`: traceability comment next to the benchmark customer-facing branding boundary test in benchmark-branding-boundary.test.ts, specifically beside the it("does not add HillerFit-branded route or component copy") block. Keep the annotation local to that test and reference the relevant spec section it covers so this test satisfies the apps/wodsmith-start/test traceability requirement.Source: Coding guidelines
apps/crew/src/server/crew-judge-rotations.server.ts (1)
54-55: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueType derivation is correct; consider centralizing later.
Same schema-derived approach as
competition-server-logic.ts, just re-declared locally since this is a separate app. If drift becomes a concern, exportingStoredCompetitionType(or similar) from the shared DB package would avoid maintaining two independent derivations of the same column type.Also applies to: 79-79
🤖 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 `@apps/crew/src/server/crew-judge-rotations.server.ts` around lines 54 - 55, Centralize the schema-derived competition type instead of re-declaring StoredCompetitionType locally in crew-judge-rotations.server.ts. Export the shared type from the DB package (the same source used by competition-server-logic.ts) and update crewJudgeRotations logic to import and use that single definition so both apps stay in sync if the competitionType column changes.apps/wodsmith-start/src/lib/scoring/algorithms/absolute-tier.ts (1)
39-43: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffPositional parameters vs. named-object convention.
calculateAbsoluteTier(score, table, scheme)uses positional args. As per coding guidelines,apps/wodsmith-start/src/**/*.{ts,tsx}should "Use named object parameters for functions with more than one parameter." This function is already consumed positionally inbenchmark-submissions.tsat multiple call sites, so converting now touches several files for a mostly stylistic gain.🤖 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 `@apps/wodsmith-start/src/lib/scoring/algorithms/absolute-tier.ts` around lines 39 - 43, Keep calculateAbsoluteTier as a positional-parameter function for now; the review note indicates the named-object convention would require updating multiple existing call sites in benchmark-submissions.ts without much benefit. Do not change the signature or its current callers unless you are planning a broader coordinated refactor across all usages of calculateAbsoluteTier, EventScoreInput, AbsoluteTierEventTable, and WorkoutScheme.Source: Coding guidelines
apps/wodsmith-start/src/routes/compete/organizer/$competitionId/-components/judges/judge-scheduling-container.tsx (1)
124-124: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
CompetitionTypeIdinstead ofstringforcompetitionType.
Import the shared type from@/lib/competitions/capabilitiesso this container stays aligned with the competition-type registry and doesn’t accept unsupported values.🤖 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 `@apps/wodsmith-start/src/routes/compete/organizer/`$competitionId/-components/judges/judge-scheduling-container.tsx at line 124, The judge scheduling container is typing competitionType as a plain string instead of the shared CompetitionTypeId, which allows unsupported values. Update the type annotation in judge-scheduling-container.tsx to use CompetitionTypeId, and add the import from `@/lib/competitions/capabilities` so the container stays aligned with the competition-type registry.apps/wodsmith-start/src/components/benchmark-tiers/categories-manager.tsx (2)
194-213: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse named object parameters for multi-parameter helpers.
moveRow(index, direction)andupdateRow(rowId, patch)each take multiple positional parameters, diverging from the repo convention of named object parameters for functions with more than one parameter.As per coding guidelines, "Use named object parameters for functions with more than one parameter."
♻️ Example refactor
- const updateRow = (rowId: string, patch: Partial<CategoryRow>) => { + const updateRow = ({ rowId, patch }: { rowId: string; patch: Partial<CategoryRow> }) => { setRows((current) => current.map((row) => (row.rowId === rowId ? { ...row, ...patch } : row)), ) } - const moveRow = (index: number, direction: -1 | 1) => { + const moveRow = ({ index, direction }: { index: number; direction: -1 | 1 }) => { setRows((current) => { const target = index + direction ...🤖 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 `@apps/wodsmith-start/src/components/benchmark-tiers/categories-manager.tsx` around lines 194 - 213, The helpers in categories-manager.tsx are using multiple positional parameters instead of the repo’s named-object convention. Refactor updateRow and moveRow to accept a single object argument with named fields, and update their call sites accordingly; keep deleteRow as-is since it already uses one parameter. Use the existing identifiers updateRow, moveRow, and CategoryRow to locate and adjust the affected logic.Source: Coding guidelines
108-240: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffCustom form state instead of React Hook Form + Zod.
This component implements manual
useState-based validation (label/weight/uniqueness) rather than React Hook Form with Zod, which the repo's coding guidelines mandate for forms in this app. The current implementation is functionally correct, but diverges from the project's standard form pattern (harder to reuse shared validation/error-display conventions).As per coding guidelines, "Use React Hook Form with Zod validation for forms."
🤖 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 `@apps/wodsmith-start/src/components/benchmark-tiers/categories-manager.tsx` around lines 108 - 240, The CategoriesManager form is using custom useState-based validation instead of the required React Hook Form + Zod pattern. Refactor CategoriesManager to manage rows and validation through react-hook-form with a Zod schema for label, weight, and uniqueness rules, and wire save/dirty/error state through the form APIs. Keep the existing behavior in the CategoriesManager component, but replace the manual validation logic and save gating with the standard form pattern used in the app.Source: Coding guidelines
apps/wodsmith-start/src/server-fns/video-submission-fns.ts (2)
401-538: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftScore-encoding/cap/sort-key logic duplicated between benchmark and non-benchmark paths.
The score parsing, time-cap derivation, tiebreak handling, and
computeSortKeyconstruction insubmitBenchmarkVideoScore(lines 428-538) closely mirrors the inline logic later insubmitVideoFn(lines ~1746-1915). Any future fix to cap/tiebreak/sort-key semantics needs to be applied in two places, risking silent divergence between benchmark and non-benchmark scoring behavior.Consider extracting a shared
encodeSubmissionScore({ workout, data })helper returning{ encodedValue, encodedRounds, status, secondaryValue, tiebreakValue, sortKey, roundStatuses, cappedRoundCount }used by both call sites.Also applies to: 1746-1915
🤖 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 `@apps/wodsmith-start/src/server-fns/video-submission-fns.ts` around lines 401 - 538, The score parsing, cap handling, tiebreak encoding, and computeSortKey assembly in submitBenchmarkVideoScore are duplicated in submitVideoFn and should be centralized. Extract a shared helper such as encodeSubmissionScore that takes the workout and submission data and returns the encoded values, statuses, round metadata, and sortKey, then have both submitBenchmarkVideoScore and the non-benchmark path call it so future scoring changes stay consistent.
128-239: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftPerpetual submission-window check is re-implemented near-identically in 5 files.
The same
competitionCan(type, "perpetual")→perpetualSubmissionsClosed(competition)branch pattern is copy-pasted acrosscheckSubmissionWindowhere,athlete-score-fns.ts,api/compete/scores/submit.ts,api/compete/scores/window-status.ts, andapi/compete/video/submit.ts(including the batch-loop replica at lines 1296-1307). Extracting a shared helper (e.g.evaluateSubmissionWindow(competition, event)) insrc/lib/competitions/would reduce the risk of these diverging as perpetual/benchmark rules evolve.Also applies to: 1296-1307
🤖 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 `@apps/wodsmith-start/src/server-fns/video-submission-fns.ts` around lines 128 - 239, The perpetual submission-window logic in checkSubmissionWindow is duplicated across multiple call sites and should be centralized. Extract the shared competitionCan(type, "perpetual") plus perpetualSubmissionsClosed(competition) decision branch into a reusable helper such as evaluateSubmissionWindow(competition, event) under src/lib/competitions/, then update checkSubmissionWindow and the matching branches in athlete-score-fns.ts, api/compete/scores/submit.ts, api/compete/scores/window-status.ts, api/compete/video/submit.ts, and the batch-loop replica to call it so the rules stay consistent as they evolve.apps/wodsmith-start/src/server-fns/competition-workouts-fns.ts (1)
1737-1804: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoffDivision-description upsert loop duplicates
updateWorkoutDivisionDescriptionsFnand does N+1 round-trips.This manual select→insert/update-per-division logic is nearly identical to the existing loop in
updateWorkoutDivisionDescriptionsFn(Line 1962 onward), and does 2 sequential DB round-trips per division. Given the unique indexworkout_scaling_desc_unique_idxon(workoutId, scalingLevelId), Drizzle's MySQLonDuplicateKeyUpdate()would collapse this into a single batched statement per delete/upsert group and remove the duplication withupdateWorkoutDivisionDescriptionsFn.Consider extracting a shared
upsertDivisionDescriptions(tx, workoutId, descriptions)helper (persrc/server/guidance for shared business logic) used by both functions.Based on learnings: "this project migrated from D1 to PlanetScale (MySQL)... leverage native MySQL features: avoid chunking for inArray-like operations, and use db.transaction for atomic multi-step operations."
🤖 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 `@apps/wodsmith-start/src/server-fns/competition-workouts-fns.ts` around lines 1737 - 1804, The division-description write path in `updateWorkoutDivisionDescriptionsFn` duplicates the same manual select-then-update/insert logic used elsewhere and causes N+1 round-trips per division. Refactor this block into a shared helper such as `upsertDivisionDescriptions(tx, workoutId, descriptions)` and use Drizzle’s MySQL `onDuplicateKeyUpdate()` to batch the upserts against the `workoutScalingDescriptionsTable` unique key instead of looping with per-row existence checks. Keep the explicit delete handling for null descriptions, but collapse the non-null path into one batched statement so both call sites share the same atomic implementation.Source: Learnings
apps/wodsmith-start/src/server-fns/athlete-score-fns.ts (1)
129-170: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPerpetual submission-window logic duplicated across four call sites.
The same "select competitionType/startDate/endDate →
competitionCan(..., "perpetual")→perpetualSubmissionsClosed(...)" pattern is now re-implemented independently in this file,routes/api/compete/scores/submit.ts,routes/api/compete/scores/window-status.ts, androutes/api/compete/video/submit.ts. The duplication already produced a wording inconsistency insubmit.ts(see comment there). Consider extracting a shared helper (e.g.resolveSubmissionWindowStatus(competition, event?)) in@/lib/competitions/perpetual-dates.tsor a new server module to keep behavior/messages consistent.🤖 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 `@apps/wodsmith-start/src/server-fns/athlete-score-fns.ts` around lines 129 - 170, The perpetual submission-window checks are duplicated across checkSubmissionWindow and the related submit/window-status call sites, which is already causing inconsistent messaging. Extract the shared competitionType/startDate/endDate lookup plus competitionCan(..., "perpetual") and perpetualSubmissionsClosed(...) logic into one helper such as resolveSubmissionWindowStatus in a shared server module, then update checkSubmissionWindow and the other callers to use it so the status shape and reason text stay consistent.
🤖 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 `@ai/research/hillerfit-benchmark-leaderboard/reviewer-alignment.md`:
- Line 42: The referenced seed description still hardcodes a machine-specific
PDF location, making the packet non-portable; replace the
/Users/zacjones/Downloads/HillerFit_Training_Guide.pdf literal in
reviewer-alignment.md with a neutral, repo-relative source-artifact reference,
and make sure any other occurrences of that same path string are updated
consistently.
In `@apps/wodsmith-start/src/components/benchmark-stat-line.tsx`:
- Around line 149-156: The benchmark score display is assuming a fixed 100-point
scale instead of using the battery’s configurable max score. Update
benchmark-stat-line.tsx so the overall suffix and the category bar math both use
the battery score scale from the existing data/model (for example the battery’s
scoreMax) rather than hardcoded 100 values. In the BenchmarkStatLine rendering
logic and any helper that clamps category scores, replace the fixed clamp/label
behavior with logic keyed off the battery scale so the overall label and bars
stay correct when scoreMax differs from 100.
In `@apps/wodsmith-start/src/components/benchmark-tiers/test-editor-dialogs.tsx`:
- Around line 683-694: The draft reset effect in the dialog is tied to the
unstable test object, so it reruns on parent re-renders and wipes unsaved edits.
Update the useEffect in test-editor-dialogs.tsx to depend on a stable identifier
such as test.id (or another fixed snapshot field) along with open, while keeping
the existing field-population logic for the selected test. This ensures the form
only resets when opening the dialog for a different test, not when
toEditableTest returns a new object.
In `@apps/wodsmith-start/src/components/online-competition-leaderboard-table.tsx`:
- Around line 183-190: The benchmark overall formatter is hardcoding a `/100`
scale, which breaks configurable batteries where the maximum score differs.
Update `formatBenchmarkOverall` in `online-competition-leaderboard-table` to use
the battery’s `scoreMax` instead of a fixed 100, either by passing `scoreMax`
into the formatter from the leaderboard data or by normalizing the value before
rendering. Keep the fallback for `benchmarkOverallScore === null` unchanged and
make sure the displayed denominator matches the actual score scale.
In `@apps/wodsmith-start/src/routes/api/compete/scores/submit.ts`:
- Around line 77-82: The missing-competition branch in submitScores is returning
the wrong reason string, which is misleading because there is no competition
type to inspect. Update the `if (!competition)` path in `submitScores` to return
the same `"Competition not found"` reason used by the equivalent checks in
`athlete-score-fns` and `window-status`, keeping the rest of the response shape
unchanged.
In
`@apps/wodsmith-start/src/routes/compete/organizer/`$competitionId/scoring/index.tsx:
- Around line 10-16: `loader` in the scoring route should match the sibling
`scoring.tsx` and `scoring/tiers.tsx` error handling by using `notFound()` when
`parentMatch.loaderData` is missing instead of throwing a plain `Error`. Update
the `loader` branch after awaiting `parentMatchPromise` so the missing-data path
returns the 404 flow consistently and preserves the expected UX.
In `@apps/wodsmith-start/src/schemas/benchmark.schema.ts`:
- Around line 49-51: The benchmarkRatingBandsSchema currently only enforces a
minimum array length and misses the duplicate-key validation that
benchmarkCategoriesSchema already performs. Update benchmarkRatingBandsSchema to
use a superRefine duplicate check on each band’s key, mirroring the existing
logic in benchmarkCategoriesSchema and using benchmarkRatingBandSchema as the
source of items. Add a validation issue when duplicate keys are found so
collisions are caught before downstream keyed lookups use the band data.
In `@apps/wodsmith-start/src/server-fns/athlete-score-fns.ts`:
- Around line 155-170: The `submitAthleteScoreFn` flow still allows benchmark
competitions to reach the normal `scoresTable` upsert, so add an early guard in
that function before any write logic. Use the existing
`isBenchmarkCompetition(data.competitionId)` check and mirror the sibling API
routes by returning a 422 response for benchmark competitions, ensuring the
function exits before the regular score insert path and the perpetual branch
remains unchanged.
In `@apps/wodsmith-start/src/server-fns/competition-workouts-fns.ts`:
- Around line 1717-1730: Both workout row updates are currently keyed only by
raw ids, so the handler can mutate records outside the caller’s team scope. In
competition-workouts-fns.ts, add an ownership check tied to data.teamId before
updating workouts and trackWorkoutsTable, using the existing update flow around
requireTeamPermission() and resolveBenchmarkTestLink() to verify that
data.workoutId and data.trackWorkoutId belong to the same team/competition
context. If needed, derive the competition track from data.teamId first, then
use that scoped lookup for both mutations instead of relying on the unscoped ids
alone.
- Around line 331-355: The conflict check in the workout assignment flow can
race because it only locks the matching track workout rows, not the benchmark
test row itself. Update the logic in the benchmark-linking path around the
conflict query to lock benchmarkTestsTable up front before evaluating existing
links, or enforce a unique constraint on trackWorkoutsTable.benchmarkTestId so
concurrent assignments serialize correctly. Use the surrounding
competition-workouts-fns.ts workflow and the existing conflict check block to
place the fix without changing the error handling behavior.
In `@apps/wodsmith-start/src/server-fns/division-results-fns.ts`:
- Around line 692-759: The competition settings update in
setResultsAutoPublishFn is racing with publishDivisionResultsFn and
publishAllDivisionResultsFn because each reads and rewrites the full settings
blob. Fix this by making the settings update atomic in the shared competition
row, either by using a transaction/row lock around the read-modify-write in
setResultsAutoPublishFn (and the other two handlers) or by moving
resultsAutoPublish to its own column so it no longer shares the JSON blob.
In `@apps/wodsmith-start/src/server/benchmark-scoring-tiers.ts`:
- Around line 1115-1186: Position assignment in createBenchmarkTest can still
collide under concurrent inserts because nextPosition is computed before the
transaction. Move the existingTests/max-position read into the db.transaction
block in benchmark-scoring-tiers.ts, use that transaction to derive the next
position, and add a retry or locking approach around the insert/select flow so
the unique (batteryId, position) constraint does not fail when multiple creates
run at once.
In `@apps/wodsmith-start/src/server/benchmark-submissions.ts`:
- Around line 320-357: The rejoin path in benchmark-submissions.ts leaves
existing registrations in removed state because the duplicate-key branch of the
insert only updates updatedAt. Update the onDuplicateKeyUpdate behavior in the
registration creation flow so a rejoin via competitionRegistrationsTable
reactivates the existing row by resetting status from
REGISTRATION_STATUS.REMOVED to the active registration state, then keep the
existing lookup and createdRegistration return logic unchanged.
In `@apps/wodsmith-start/test/components/benchmark-settings-form.test.tsx`:
- Around line 21-97: Add the missing `@lat`: spec-section reference comments to
this BenchmarkSettingsForm test file so each test maps to exactly one leaf
section in lat.md. Place one unique `@lat`: comment next to each relevant it block
in benchmark-settings-form.test.tsx, using the existing test names like
BenchmarkSettingsForm, onSave, and toast.error to anchor the references, and
ensure no spec section is referenced more than once.
In `@apps/wodsmith-start/test/components/benchmark-stat-line.test.tsx`:
- Around line 196-234: Add the missing `@lat`: spec-section reference comments for
each BenchmarkStatLine test in this suite, placing one next to the test that
covers the overall rendering and one next to the test covering
deferred/read-only submission behavior. Use the existing test names in
BenchmarkStatLine to locate the right spots, and ensure each spec-section
reference is added only once without duplicating refs across the two tests.
---
Nitpick comments:
In `@apps/crew/src/server/crew-judge-rotations.server.ts`:
- Around line 54-55: Centralize the schema-derived competition type instead of
re-declaring StoredCompetitionType locally in crew-judge-rotations.server.ts.
Export the shared type from the DB package (the same source used by
competition-server-logic.ts) and update crewJudgeRotations logic to import and
use that single definition so both apps stay in sync if the competitionType
column changes.
In `@apps/wodsmith-start/scripts/seed/seeders/23-benchmark.ts`:
- Around line 8-41: The seed() function in 23-benchmark.ts performs many
dependent batchInsert calls without atomicity, so a mid-sequence failure can
leave partially seeded data behind. Wrap the full insert sequence in a
transaction using the Connection client’s begin/commit/rollback flow, and ensure
any error rolls back before rethrowing; locate the work inside seed() around the
rows, athleteRows, and batchInsert calls.
In `@apps/wodsmith-start/src/components/benchmark-tiers/categories-manager.tsx`:
- Around line 194-213: The helpers in categories-manager.tsx are using multiple
positional parameters instead of the repo’s named-object convention. Refactor
updateRow and moveRow to accept a single object argument with named fields, and
update their call sites accordingly; keep deleteRow as-is since it already uses
one parameter. Use the existing identifiers updateRow, moveRow, and CategoryRow
to locate and adjust the affected logic.
- Around line 108-240: The CategoriesManager form is using custom useState-based
validation instead of the required React Hook Form + Zod pattern. Refactor
CategoriesManager to manage rows and validation through react-hook-form with a
Zod schema for label, weight, and uniqueness rules, and wire save/dirty/error
state through the form APIs. Keep the existing behavior in the CategoriesManager
component, but replace the manual validation logic and save gating with the
standard form pattern used in the app.
In `@apps/wodsmith-start/src/lib/scoring/algorithms/absolute-tier.ts`:
- Around line 39-43: Keep calculateAbsoluteTier as a positional-parameter
function for now; the review note indicates the named-object convention would
require updating multiple existing call sites in benchmark-submissions.ts
without much benefit. Do not change the signature or its current callers unless
you are planning a broader coordinated refactor across all usages of
calculateAbsoluteTier, EventScoreInput, AbsoluteTierEventTable, and
WorkoutScheme.
In
`@apps/wodsmith-start/src/routes/compete/organizer/`$competitionId/-components/judges/judge-scheduling-container.tsx:
- Line 124: The judge scheduling container is typing competitionType as a plain
string instead of the shared CompetitionTypeId, which allows unsupported values.
Update the type annotation in judge-scheduling-container.tsx to use
CompetitionTypeId, and add the import from `@/lib/competitions/capabilities` so
the container stays aligned with the competition-type registry.
In `@apps/wodsmith-start/src/server-fns/athlete-score-fns.ts`:
- Around line 129-170: The perpetual submission-window checks are duplicated
across checkSubmissionWindow and the related submit/window-status call sites,
which is already causing inconsistent messaging. Extract the shared
competitionType/startDate/endDate lookup plus competitionCan(..., "perpetual")
and perpetualSubmissionsClosed(...) logic into one helper such as
resolveSubmissionWindowStatus in a shared server module, then update
checkSubmissionWindow and the other callers to use it so the status shape and
reason text stay consistent.
In `@apps/wodsmith-start/src/server-fns/competition-workouts-fns.ts`:
- Around line 1737-1804: The division-description write path in
`updateWorkoutDivisionDescriptionsFn` duplicates the same manual
select-then-update/insert logic used elsewhere and causes N+1 round-trips per
division. Refactor this block into a shared helper such as
`upsertDivisionDescriptions(tx, workoutId, descriptions)` and use Drizzle’s
MySQL `onDuplicateKeyUpdate()` to batch the upserts against the
`workoutScalingDescriptionsTable` unique key instead of looping with per-row
existence checks. Keep the explicit delete handling for null descriptions, but
collapse the non-null path into one batched statement so both call sites share
the same atomic implementation.
In `@apps/wodsmith-start/src/server-fns/video-submission-fns.ts`:
- Around line 401-538: The score parsing, cap handling, tiebreak encoding, and
computeSortKey assembly in submitBenchmarkVideoScore are duplicated in
submitVideoFn and should be centralized. Extract a shared helper such as
encodeSubmissionScore that takes the workout and submission data and returns the
encoded values, statuses, round metadata, and sortKey, then have both
submitBenchmarkVideoScore and the non-benchmark path call it so future scoring
changes stay consistent.
- Around line 128-239: The perpetual submission-window logic in
checkSubmissionWindow is duplicated across multiple call sites and should be
centralized. Extract the shared competitionCan(type, "perpetual") plus
perpetualSubmissionsClosed(competition) decision branch into a reusable helper
such as evaluateSubmissionWindow(competition, event) under
src/lib/competitions/, then update checkSubmissionWindow and the matching
branches in athlete-score-fns.ts, api/compete/scores/submit.ts,
api/compete/scores/window-status.ts, api/compete/video/submit.ts, and the
batch-loop replica to call it so the rules stay consistent as they evolve.
In `@apps/wodsmith-start/src/server/benchmark-scoring-tiers.ts`:
- Around line 629-651: The upsert helper is doing one insert per threshold row
inside a transaction, which creates many sequential round-trips; update
upsertBenchmarkTierThresholdRows to batch all rows into a single multi-row
insert with onDuplicateKeyUpdate so the transaction only executes one statement.
Use the existing benchmarkTierThresholdsTable insert flow in
upsertBenchmarkTierThresholdRows, map the incoming rows array to the values
payload, and keep the same per-row fields and updatedAt refresh in the
duplicate-key update path.
In `@apps/wodsmith-start/test/components/benchmark-branding-boundary.test.ts`:
- Around line 9-18: The branding-boundary test scope in CUSTOMER_FACING_PATHS is
too narrow and misses server-side strings that can still reach users. Update
benchmark-branding-boundary.test.ts to include user-facing server locations such
as src/server and src/server-fns, alongside the existing component and route
paths, so the boundary check also scans server-returned error/description text
that may surface in the UI.
- Around line 1-41: Add the missing `@lat`: traceability comment next to the
benchmark customer-facing branding boundary test in
benchmark-branding-boundary.test.ts, specifically beside the it("does not add
HillerFit-branded route or component copy") block. Keep the annotation local to
that test and reference the relevant spec section it covers so this test
satisfies the apps/wodsmith-start/test traceability requirement.
In `@apps/wodsmith-start/test/components/benchmark-categories-manager.test.tsx`:
- Around line 1-164: Add the required `@lat`: annotation to each it(...) test in
CategoriesManager so the suite maps to the relevant spec section, and ensure
each reference is unique with no duplicates. Update the tests in
benchmark-categories-manager.test.tsx near the existing
describe("CategoriesManager") blocks, keeping the comments adjacent to each case
and using the appropriate spec identifiers for the behaviors covered.
In `@apps/wodsmith-start/test/components/benchmark-rating-bands-editor.test.tsx`:
- Around line 1-243: The new RatingBandsEditor test file is missing the required
`@lat`: annotations for each spec section. Add the appropriate `@lat`: comment next
to every describe/it group in benchmark-rating-bands-editor.test.tsx so the
tests are traceable to the correct benchmark spec sections, following the
pattern used in the other benchmark test files.
🪄 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: CHILL
Plan: Pro
Run ID: 74a8b229-09d3-4556-ace8-64f46c3d042c
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (147)
AGENTS.mdCLAUDE.mdai/research/hillerfit-benchmark-leaderboard-guide.mdai/research/hillerfit-benchmark-leaderboard/README.mdai/research/hillerfit-benchmark-leaderboard/architecture-walkthrough.htmlai/research/hillerfit-benchmark-leaderboard/assumptions-and-decisions.mdai/research/hillerfit-benchmark-leaderboard/requirements.mdai/research/hillerfit-benchmark-leaderboard/reviewer-alignment.mdai/research/hillerfit-benchmark-leaderboard/tasks.mdai/research/hillerfit-benchmark-leaderboard/technical-design.mdai/research/hillerfit-benchmark-leaderboard/test-strategy.mdai/research/hillerfit-benchmark-leaderboard/traceability.mdai/research/m0-competition-type-capability-registry.mdapps/crew/package.jsonapps/crew/src/server/crew-judge-rotations.server.tsapps/wodsmith-gameday/package.jsonapps/wodsmith-start/e2e/competition-registration.spec.tsapps/wodsmith-start/package.jsonapps/wodsmith-start/scripts/seed/cleanup.tsapps/wodsmith-start/scripts/seed/data/benchmark-training-guide.tsapps/wodsmith-start/scripts/seed/index.tsapps/wodsmith-start/scripts/seed/seeders/23-benchmark.tsapps/wodsmith-start/src/components/benchmark-stat-line.tsxapps/wodsmith-start/src/components/benchmark-tiers/benchmark-settings-form.tsxapps/wodsmith-start/src/components/benchmark-tiers/categories-manager.tsxapps/wodsmith-start/src/components/benchmark-tiers/rating-bands-editor.tsxapps/wodsmith-start/src/components/benchmark-tiers/test-editor-dialogs.tsxapps/wodsmith-start/src/components/cohost-sidebar.tsxapps/wodsmith-start/src/components/compete/athlete-score-submission-panel.tsxapps/wodsmith-start/src/components/compete/video-submission-form.tsxapps/wodsmith-start/src/components/competition-card.tsxapps/wodsmith-start/src/components/competition-hero.tsxapps/wodsmith-start/src/components/competition-leaderboard-table.tsxapps/wodsmith-start/src/components/competition-sidebar.tsxapps/wodsmith-start/src/components/competition-tabs.tsxapps/wodsmith-start/src/components/events/competition-event-row.tsxapps/wodsmith-start/src/components/events/event-details-form.tsxapps/wodsmith-start/src/components/events/organizer-event-manager.tsxapps/wodsmith-start/src/components/leaderboard-page-content.tsxapps/wodsmith-start/src/components/online-competition-leaderboard-table.tsxapps/wodsmith-start/src/components/organizer-competition-form.tsxapps/wodsmith-start/src/components/registration/registration-sections.tsxapps/wodsmith-start/src/db/schemas/benchmarks.tsapps/wodsmith-start/src/lib/competitions/capabilities.tsapps/wodsmith-start/src/lib/competitions/perpetual-dates.tsapps/wodsmith-start/src/lib/scoring/algorithms/absolute-tier.tsapps/wodsmith-start/src/lib/scoring/algorithms/index.tsapps/wodsmith-start/src/lib/scoring/category-aggregation.tsapps/wodsmith-start/src/lib/scoring/format/index.tsapps/wodsmith-start/src/lib/scoring/format/points.tsapps/wodsmith-start/src/lib/scoring/index.tsapps/wodsmith-start/src/routeTree.gen.tsapps/wodsmith-start/src/routes/__root.tsxapps/wodsmith-start/src/routes/api/compete/scores/submit.tsapps/wodsmith-start/src/routes/api/compete/scores/window-status.tsapps/wodsmith-start/src/routes/api/compete/video/submit.tsapps/wodsmith-start/src/routes/compete/$slug.tsxapps/wodsmith-start/src/routes/compete/$slug/announcements.tsxapps/wodsmith-start/src/routes/compete/$slug/index.tsxapps/wodsmith-start/src/routes/compete/$slug/leaderboard.tsxapps/wodsmith-start/src/routes/compete/$slug/register.tsxapps/wodsmith-start/src/routes/compete/$slug/registered.tsxapps/wodsmith-start/src/routes/compete/$slug/review/$eventId/index.tsxapps/wodsmith-start/src/routes/compete/$slug/schedule.tsxapps/wodsmith-start/src/routes/compete/$slug/stats.tsxapps/wodsmith-start/src/routes/compete/$slug/workouts/$eventId.tsxapps/wodsmith-start/src/routes/compete/$slug/workouts/index.tsxapps/wodsmith-start/src/routes/compete/cohost/$competitionId/events/$eventId/submissions/index.tsxapps/wodsmith-start/src/routes/compete/cohost/$competitionId/leaderboard-preview.tsxapps/wodsmith-start/src/routes/compete/organizer/$competitionId.tsxapps/wodsmith-start/src/routes/compete/organizer/$competitionId/-components/judges/judge-scheduling-container.tsxapps/wodsmith-start/src/routes/compete/organizer/$competitionId/-components/organizer-competition-edit-form.tsxapps/wodsmith-start/src/routes/compete/organizer/$competitionId/-components/quick-actions-division-results.tsxapps/wodsmith-start/src/routes/compete/organizer/$competitionId/events/$eventId.tsxapps/wodsmith-start/src/routes/compete/organizer/$competitionId/events/$eventId/index.tsxapps/wodsmith-start/src/routes/compete/organizer/$competitionId/events/$eventId/submissions/index.tsxapps/wodsmith-start/src/routes/compete/organizer/$competitionId/events/index.tsxapps/wodsmith-start/src/routes/compete/organizer/$competitionId/leaderboard-preview.tsxapps/wodsmith-start/src/routes/compete/organizer/$competitionId/scoring.tsxapps/wodsmith-start/src/routes/compete/organizer/$competitionId/scoring/index.tsxapps/wodsmith-start/src/routes/compete/organizer/$competitionId/scoring/tiers.tsxapps/wodsmith-start/src/schemas/benchmark.schema.tsapps/wodsmith-start/src/server-fns/athlete-score-fns.tsapps/wodsmith-start/src/server-fns/benchmark-scoring-tier-fns.tsapps/wodsmith-start/src/server-fns/check-in-fns.tsapps/wodsmith-start/src/server-fns/competition-detail-fns.tsapps/wodsmith-start/src/server-fns/competition-server-logic.tsapps/wodsmith-start/src/server-fns/competition-workouts-fns.tsapps/wodsmith-start/src/server-fns/division-results-fns.tsapps/wodsmith-start/src/server-fns/video-submission-fns.tsapps/wodsmith-start/src/server/benchmark-leaderboard.tsapps/wodsmith-start/src/server/benchmark-open-join-rate-limit.tsapps/wodsmith-start/src/server/benchmark-scoring-tiers.tsapps/wodsmith-start/src/server/benchmark-submissions.tsapps/wodsmith-start/src/server/competition-leaderboard.tsapps/wodsmith-start/src/server/series-leaderboard.tsapps/wodsmith-start/src/types/competitions.tsapps/wodsmith-start/src/utils/registration-window.tsapps/wodsmith-start/test/components/benchmark-branding-boundary.test.tsapps/wodsmith-start/test/components/benchmark-categories-manager.test.tsxapps/wodsmith-start/test/components/benchmark-rating-bands-editor.test.tsxapps/wodsmith-start/test/components/benchmark-settings-form.test.tsxapps/wodsmith-start/test/components/benchmark-stat-line.test.tsxapps/wodsmith-start/test/components/benchmark-test-editor-dialogs.test.tsxapps/wodsmith-start/test/components/competition-sidebar-capability-gates.test.tsapps/wodsmith-start/test/components/competition-tabs.test.tsxapps/wodsmith-start/test/components/leaderboard-page-content.test.tsxapps/wodsmith-start/test/components/online-competition-leaderboard-table.test.tsxapps/wodsmith-start/test/components/organizer-competition-edit-form.test.tsxapps/wodsmith-start/test/components/organizer-competition-form.test.tsxapps/wodsmith-start/test/components/video-submission-form.test.tsxapps/wodsmith-start/test/lib/competitions/capabilities.test.tsapps/wodsmith-start/test/lib/competitions/perpetual-dates.test.tsapps/wodsmith-start/test/lib/scoring/absolute-tier.test.tsapps/wodsmith-start/test/lib/scoring/category-aggregation.test.tsapps/wodsmith-start/test/lib/scoring/factory.test.tsapps/wodsmith-start/test/lib/scoring/format-points.test.tsapps/wodsmith-start/test/lib/scoring/online.test.tsapps/wodsmith-start/test/routes/api/compete/scores/window-status.test.tsapps/wodsmith-start/test/routes/api/compete/submission-gates.test.tsapps/wodsmith-start/test/routes/compete/benchmark-stats-route.test.tsxapps/wodsmith-start/test/routes/compete/video-submission-route-gates.test.tsapps/wodsmith-start/test/schemas/benchmark.test.tsapps/wodsmith-start/test/schemas/scoring.test.tsapps/wodsmith-start/test/scripts/benchmark-seed-data.test.tsapps/wodsmith-start/test/server-fns/athlete-score-fns.test.tsapps/wodsmith-start/test/server-fns/benchmark-submission-m3.test.tsapps/wodsmith-start/test/server-fns/video-submission-fns.test.tsapps/wodsmith-start/test/server/benchmark-leaderboard.test.tsapps/wodsmith-start/test/server/benchmark-scoring-tiers.test.tsapps/wodsmith-start/test/server/competition-leaderboard-capability-gates.test.tsapps/wodsmith-start/test/utils/registration-window.test.tsapps/wodsmith-start/vite.config.tslat.md/architecture.mdlat.md/competition-type-capabilities.mdlat.md/domain.mdlat.md/organizer-dashboard.mdlat.md/registration.mdpackages/wodsmith-db/mysql-migrations/0000_benchmark-battery.sqlpackages/wodsmith-db/mysql-migrations/meta/0000_snapshot.jsonpackages/wodsmith-db/mysql-migrations/meta/_journal.jsonpackages/wodsmith-db/src/schema.tspackages/wodsmith-db/src/schemas/benchmarks.tspackages/wodsmith-db/src/schemas/common.tspackages/wodsmith-db/src/schemas/competitions.tspackages/wodsmith-db/src/schemas/programming.tspackages/wodsmith-db/src/schemas/scores.ts
| - M1 schema/seed must provide complete category caches, test rows, one-to-one event mappings, and threshold rows. | ||
| - M2 absolute-tier scoring supplies the `0 / 0.5 / 1..10` tier semantics. | ||
| - M3 submission supplies profile-variant snapshots and best-to-date writes. | ||
| - The first seed derives source data from `/Users/zacjones/Downloads/HillerFit_Training_Guide.pdf`. |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Replace the machine-specific PDF path.
Hardcoding /Users/zacjones/Downloads/HillerFit_Training_Guide.pdf makes the packet non-portable and leaks a local filesystem location. Please switch to a neutral/repo-relative source-artifact reference everywhere this literal appears.
Suggested wording
- The first seed derives source data from `/Users/zacjones/Downloads/HillerFit_Training_Guide.pdf`.
+ The first seed derives source data from the `HillerFit_Training_Guide.pdf` source artifact.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - The first seed derives source data from `/Users/zacjones/Downloads/HillerFit_Training_Guide.pdf`. | |
| - The first seed derives source data from the `HillerFit_Training_Guide.pdf` source artifact. |
🤖 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 `@ai/research/hillerfit-benchmark-leaderboard/reviewer-alignment.md` at line
42, The referenced seed description still hardcodes a machine-specific PDF
location, making the packet non-portable; replace the
/Users/zacjones/Downloads/HillerFit_Training_Guide.pdf literal in
reviewer-alignment.md with a neutral, repo-relative source-artifact reference,
and make sure any other occurrences of that same path string are updated
consistently.
Comment on lines
+149
to
+156
| <div className="mt-2 flex items-end gap-2"> | ||
| <span className="text-5xl font-semibold tracking-normal tabular-nums"> | ||
| {formatBenchmarkNumber(overall)} | ||
| </span> | ||
| <span className="pb-2 text-sm text-muted-foreground"> | ||
| {entry.benchmarkOverallScore === null ? "pts" : "/100"} | ||
| </span> | ||
| </div> |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the CompetitionLeaderboardEntry type / benchmark leaderboard builder for scale normalization
rg -n "benchmarkOverallScore|benchmarkCategoryScores|scoreMax" apps/wodsmith-start/src/server-fns/leaderboard-fns.ts apps/wodsmith-start/src/server/competition-leaderboard.ts apps/wodsmith-start/src/server/benchmark-leaderboard.ts 2>/dev/null | head -80Repository: wodsmith/thewodapp
Length of output: 1419
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant files, then inspect the leaderboard types and aggregation logic
ast-grep outline apps/wodsmith-start/src/server/competition-leaderboard.ts --view expanded || true
ast-grep outline apps/wodsmith-start/src/server/benchmark-leaderboard.ts --view expanded || true
echo '--- competition-leaderboard.ts (type definitions + aggregation) ---'
sed -n '80,140p' apps/wodsmith-start/src/server/competition-leaderboard.ts
echo '---'
sed -n '1680,1760p' apps/wodsmith-start/src/server/competition-leaderboard.ts
echo '--- benchmark-leaderboard.ts (scoreMax / score scale) ---'
sed -n '1,120p' apps/wodsmith-start/src/server/benchmark-leaderboard.ts
echo '---'
sed -n '200,280p' apps/wodsmith-start/src/server/benchmark-leaderboard.ts
echo '--- component lines around the flagged UI ---'
sed -n '140,200p' apps/wodsmith-start/src/components/benchmark-stat-line.tsxRepository: wodsmith/thewodapp
Length of output: 17706
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- benchmarkStatLine props / data flow ---'
ast-grep outline apps/wodsmith-start/src/components/benchmark-stat-line.tsx --view expanded || true
sed -n '1,140p' apps/wodsmith-start/src/components/benchmark-stat-line.tsx
echo '--- benchmark score aggregation ---'
ast-grep run --pattern 'export function aggregateBenchmarkScores($_) { $$$ }' --lang typescript apps/wodsmith-start/src/server/benchmark-leaderboard.ts || true
rg -n "function aggregateBenchmarkScores|aggregateBenchmarkScores\(" apps/wodsmith-start/src/server/benchmark-leaderboard.ts apps/wodsmith-start/src/server/competition-leaderboard.ts
sed -n '120,220p' apps/wodsmith-start/src/server/benchmark-leaderboard.tsRepository: wodsmith/thewodapp
Length of output: 7909
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- aggregateBenchmarkScores implementation ---'
rg -n "function aggregateBenchmarkScores|overallScore|category.score|scoreMax" apps/wodsmith-start/src/server/benchmark-leaderboard.ts
sed -n '520,760p' apps/wodsmith-start/src/server/benchmark-leaderboard.ts
echo '--- any UI or docs describing category score scale ---'
rg -n "BenchmarkLeaderboardCategoryScore|category.score|/100|scoreMax" apps/wodsmith-start/src/components apps/wodsmith-start/src/server -g '!**/*.map'Repository: wodsmith/thewodapp
Length of output: 6381
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- score field assignment sites ---'
rg -n "benchmarkCategoryScores\s*=|categoryScores|score:\s*.*scoreMax|overallScore|category\.score" apps/wodsmith-start/src/server/competition-leaderboard.ts apps/wodsmith-start/src/server/benchmark-leaderboard.ts apps/wodsmith-start/src/server/benchmark-scoring-tiers.ts
echo '--- likely aggregator / score scaling implementation ---'
rg -n "overallScore|categories.*score|tierSum|weight|scoreMax" apps/wodsmith-start/src/server/benchmark-scoring-tiers.ts apps/wodsmith-start/src/server/benchmark-leaderboard.ts
echo '--- surrounding leaderboard type docs ---'
sed -n '90,120p' apps/wodsmith-start/src/server/competition-leaderboard.ts
sed -n '1,120p' apps/wodsmith-start/src/server/benchmark-scoring-tiers.tsRepository: wodsmith/thewodapp
Length of output: 6829
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- aggregate / scoring math around category score calculation ---'
sed -n '760,860p' apps/wodsmith-start/src/server/benchmark-scoring-tiers.ts
echo '--- any direct normalization to scoreMax in scoring tiers ---'
rg -n "scoreMax|overallScore|category.*score|weight:" apps/wodsmith-start/src/server/benchmark-scoring-tiers.ts
echo '--- benchmark stat line current display logic ---'
sed -n '146,188p' apps/wodsmith-start/src/components/benchmark-stat-line.tsxRepository: wodsmith/thewodapp
Length of output: 5709
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- locate benchmark aggregation function / score field calculation ---'
rg -n "overallScore|categories: .*score|score =|weight" apps/wodsmith-start/src/server -g '*.ts' | head -200
echo '--- search for BenchmarkLeaderboardCategoryScore.score assignment in all server files ---'
rg -n "BenchmarkLeaderboardCategoryScore|score:" apps/wodsmith-start/src/server -g '*.ts' | head -200
echo '--- inspect the public leaderboard table if it uses the same benchmark scale ---'
sed -n '170,220p' apps/wodsmith-start/src/components/online-competition-leaderboard-table.tsx
sed -n '250,290p' apps/wodsmith-start/src/components/online-competition-leaderboard-table.tsxRepository: wodsmith/thewodapp
Length of output: 4545
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- benchmark-scoring-tiers functions related to scoring output ---'
ast-grep outline apps/wodsmith-start/src/server/benchmark-scoring-tiers.ts --view expanded || true
echo '--- search for overall/category score calculation in benchmark-scoring-tiers.ts ---'
rg -n "overallScore|category.*score|tierSum|weight|scoreMax" apps/wodsmith-start/src/server/benchmark-scoring-tiers.ts
echo '--- focused slices around likely aggregation helpers ---'
sed -n '720,860p' apps/wodsmith-start/src/server/benchmark-scoring-tiers.ts
sed -n '860,980p' apps/wodsmith-start/src/server/benchmark-scoring-tiers.tsRepository: wodsmith/thewodapp
Length of output: 13358
Use the battery score scale here instead of hardcoding 100. At apps/wodsmith-start/src/components/benchmark-stat-line.tsx:149-190, /100 and the Math.min(100, category.score) clamp assume a fixed 100-point benchmark, but leaderboard scores are defined on the battery’s configurable scoreMax scale. This will misstate both the overall label and the category bars when scoreMax !== 100.
🤖 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 `@apps/wodsmith-start/src/components/benchmark-stat-line.tsx` around lines 149
- 156, The benchmark score display is assuming a fixed 100-point scale instead
of using the battery’s configurable max score. Update benchmark-stat-line.tsx so
the overall suffix and the category bar math both use the battery score scale
from the existing data/model (for example the battery’s scoreMax) rather than
hardcoded 100 values. In the BenchmarkStatLine rendering logic and any helper
that clamps category scores, replace the fixed clamp/label behavior with logic
keyed off the battery scale so the overall label and bars stay correct when
scoreMax differs from 100.
Comment on lines
+683
to
+694
| useEffect(() => { | ||
| if (!open) return | ||
| setName(test.name) | ||
| setCategoryKey(test.categoryKey) | ||
| setScheme(test.scheme) | ||
| setScoreType(test.scoreType) | ||
| setInputUnit(test.inputUnit) | ||
| setIncludedInScoring(test.includedInScoring) | ||
| setScoreModel(test.scoreModel ?? "standard") | ||
| setHybridFlipTier(test.hybridFlipTier ?? null) | ||
| setThresholds({}) | ||
| }, [open, test]) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== File outline =="
ast-grep outline apps/wodsmith-start/src/components/benchmark-tiers/test-editor-dialogs.tsx --view expanded || true
echo
echo "== Relevant lines around the effect =="
sed -n '640,730p' apps/wodsmith-start/src/components/benchmark-tiers/test-editor-dialogs.tsx
echo
echo "== Search for parent usage of EditTestDialog / test prop flow =="
rg -n "EditTestDialog|test-editor-dialogs|<EditTestDialog|test={test}|test\\.id" apps/wodsmith-start/src -g '!**/*.map'
echo
echo "== Nearby reset pattern in rating-bands-editor.tsx =="
fd -a rating-bands-editor.tsx apps/wodsmith-start/src && trueRepository: wodsmith/thewodapp
Length of output: 5551
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== EditTestDialog usages =="
rg -n --fixed-strings "EditTestDialog" apps/wodsmith-start/src
echo
echo "== benchmark tiers parent file candidates =="
fd -a "scoring/tiers.tsx" apps/wodsmith-start/src || true
fd -a "benchmark-tiers" apps/wodsmith-start/src || true
echo
echo "== rating-bands-editor reset pattern =="
fd -a "rating-bands-editor.tsx" apps/wodsmith-start/src && trueRepository: wodsmith/thewodapp
Length of output: 1118
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== tiers.tsx outline =="
ast-grep outline 'apps/wodsmith-start/src/routes/compete/organizer/$competitionId/scoring/tiers.tsx' --view expanded
echo
echo "== EditTestDialog call sites in tiers.tsx =="
sed -n '450,690p' 'apps/wodsmith-start/src/routes/compete/organizer/$competitionId/scoring/tiers.tsx'
echo
echo "== rating-bands-editor.tsx outline =="
ast-grep outline apps/wodsmith-start/src/components/benchmark-tiers/rating-bands-editor.tsx --view expanded
echo
echo "== reset effect in rating-bands-editor.tsx =="
rg -n "useEffect|reset|serialize|memo|bands" apps/wodsmith-start/src/components/benchmark-tiers/rating-bands-editor.tsx
sed -n '1,260p' apps/wodsmith-start/src/components/benchmark-tiers/rating-bands-editor.tsxRepository: wodsmith/thewodapp
Length of output: 18400
Reset this effect on test.id, not the test object
toEditableTest(test, maxTier) creates a fresh object on every parent render, so [open, test] will re-run while the dialog is open and clear unsaved form state. Key it on test.id (or another stable snapshot) so only switching to a different test resets the draft.
🤖 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 `@apps/wodsmith-start/src/components/benchmark-tiers/test-editor-dialogs.tsx`
around lines 683 - 694, The draft reset effect in the dialog is tied to the
unstable test object, so it reruns on parent re-renders and wipes unsaved edits.
Update the useEffect in test-editor-dialogs.tsx to depend on a stable identifier
such as test.id (or another fixed snapshot field) along with open, while keeping
the existing field-population logic for the selected test. This ensures the form
only resets when opening the dialog for a different test, not when
toEditableTest returns a new object.
Comment on lines
+183
to
+190
| function formatBenchmarkOverall(entry: CompetitionLeaderboardEntry): string { | ||
| // Under online ranking the benchmark score is additive context — an entry | ||
| // with no tiered scores has nothing to show, so fall back to its points. | ||
| if (entry.benchmarkOverallScore === null) { | ||
| return `${entry.totalPoints} pts` | ||
| } | ||
| return `${formatBenchmarkNumber(entry.benchmarkOverallScore)}/100` | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## Files mentioning CompetitionLeaderboardEntry / benchmarkOverallScore / scoreMax\n'
rg -n "CompetitionLeaderboardEntry|benchmarkOverallScore|scoreMax|updateBenchmarkBatterySettings" apps/wodsmith-start/src -S
printf '\n## Locate leaderboard table and related helpers\n'
fd -a "online-competition-leaderboard-table.tsx|competition-leaderboard|benchmark" apps/wodsmith-start/src
printf '\n## Quick tree around likely files\n'
for f in $(fd -a "online-competition-leaderboard-table.tsx|competition-leaderboard|benchmark" apps/wodsmith-start/src); do
echo "---- $f"
ast-grep outline "$f" --view compact || true
doneRepository: wodsmith/thewodapp
Length of output: 19231
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '## competition leaderboard interface + assignment'
sed -n '80,120p' apps/wodsmith-start/src/server/competition-leaderboard.ts
echo
sed -n '1700,1745p' apps/wodsmith-start/src/server/competition-leaderboard.ts
echo
echo '## benchmark leaderboard shape'
sed -n '40,95p' apps/wodsmith-start/src/server/benchmark-leaderboard.ts
echo
sed -n '210,265p' apps/wodsmith-start/src/server/benchmark-leaderboard.ts
echo
echo '## benchmark stat line formatter'
sed -n '120,170p' apps/wodsmith-start/src/components/benchmark-stat-line.tsxRepository: wodsmith/thewodapp
Length of output: 7699
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '## category aggregation'
sed -n '1,220p' apps/wodsmith-start/src/lib/scoring/category-aggregation.ts
echo
echo '## benchmark scoring tiers settings update'
sed -n '1380,1495p' apps/wodsmith-start/src/server/benchmark-scoring-tiers.ts
echo
echo '## server fn for updating benchmark battery settings'
sed -n '1,120p' apps/wodsmith-start/src/server-fns/benchmark-scoring-tier-fns.ts
echo
sed -n '260,320p' apps/wodsmith-start/src/server-fns/benchmark-scoring-tier-fns.tsRepository: wodsmith/thewodapp
Length of output: 10489
Use scoreMax instead of /100 for benchmark overall
benchmarkOverallScore is on the battery’s 0..scoreMax scale, and scoreMax is configurable. Hardcoding /100 will mislabel non-default batteries; pass scoreMax through to this formatter or normalize the score server-side.
🤖 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 `@apps/wodsmith-start/src/components/online-competition-leaderboard-table.tsx`
around lines 183 - 190, The benchmark overall formatter is hardcoding a `/100`
scale, which breaks configurable batteries where the maximum score differs.
Update `formatBenchmarkOverall` in `online-competition-leaderboard-table` to use
the battery’s `scoreMax` instead of a fixed 100, either by passing `scoreMax`
into the formatter from the leaderboard data or by normalizing the value before
rendering. Keep the fallback for `benchmarkOverallScore === null` unchanged and
make sure the displayed denominator matches the actual score scale.
Comment on lines
+77
to
+82
| if (!competition) { | ||
| return { | ||
| isOpen: false, | ||
| reason: "Submission windows are not available for this competition type", | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Misleading reason for a missing competition.
This new if (!competition) branch returns "Submission windows are not available for this competition type", but the competition itself doesn't exist here — there's no type to evaluate. The equivalent checks in athlete-score-fns.ts and window-status.ts correctly return "Competition not found".
💬 Proposed fix
if (!competition) {
return {
isOpen: false,
- reason: "Submission windows are not available for this competition type",
+ reason: "Competition not found",
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (!competition) { | |
| return { | |
| isOpen: false, | |
| reason: "Submission windows are not available for this competition type", | |
| } | |
| } | |
| if (!competition) { | |
| return { | |
| isOpen: false, | |
| reason: "Competition not found", | |
| } | |
| } |
🤖 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 `@apps/wodsmith-start/src/routes/api/compete/scores/submit.ts` around lines 77
- 82, The missing-competition branch in submitScores is returning the wrong
reason string, which is misleading because there is no competition type to
inspect. Update the `if (!competition)` path in `submitScores` to return the
same `"Competition not found"` reason used by the equivalent checks in
`athlete-score-fns` and `window-status`, keeping the rest of the response shape
unchanged.
Comment on lines
+692
to
+759
|
|
||
| /** | ||
| * Enable or disable pre-publishing of division results. | ||
| * | ||
| * When enabled, results appear on the public leaderboard as soon as scores | ||
| * come in — no manual publish step. Only available for perpetual (benchmark) | ||
| * competitions; other types keep the per-division publish gate. | ||
| */ | ||
| export const setResultsAutoPublishFn = createServerFn({ method: "POST" }) | ||
| .inputValidator((data: unknown) => | ||
| setResultsAutoPublishInputSchema.parse(data), | ||
| ) | ||
| .handler( | ||
| async ({ | ||
| data, | ||
| }): Promise<{ success: boolean; resultsAutoPublish: boolean }> => { | ||
| // Verify authentication | ||
| const session = await getSessionFromCookie() | ||
| if (!session?.userId) { | ||
| throw new Error("Not authenticated") | ||
| } | ||
|
|
||
| // Check permission (site admins bypass) | ||
| const isSiteAdmin = session.user?.role === ROLES_ENUM.ADMIN | ||
| const team = session.teams?.find((t) => t.id === data.organizingTeamId) | ||
| if ( | ||
| !isSiteAdmin && | ||
| !team?.permissions.includes(TEAM_PERMISSIONS.MANAGE_PROGRAMMING) | ||
| ) { | ||
| throw new Error("Missing required permission") | ||
| } | ||
|
|
||
| const db = getDb() | ||
|
|
||
| // Get competition with settings | ||
| const [competition] = await db | ||
| .select() | ||
| .from(competitionsTable) | ||
| .where(eq(competitionsTable.id, data.competitionId)) | ||
|
|
||
| if (!competition) { | ||
| throw new Error("Competition not found") | ||
| } | ||
|
|
||
| if (competition.organizingTeamId !== data.organizingTeamId) { | ||
| throw new Error("Competition does not belong to this team") | ||
| } | ||
|
|
||
| if (!competitionCan(competition.competitionType, "perpetual")) { | ||
| throw new Error( | ||
| "Pre-publishing results is only available for benchmark competitions", | ||
| ) | ||
| } | ||
|
|
||
| const settings = parseCompetitionSettings(competition.settings) ?? {} | ||
| const newSettings = stringifyCompetitionSettings({ | ||
| ...settings, | ||
| resultsAutoPublish: data.autoPublish, | ||
| }) | ||
|
|
||
| await db | ||
| .update(competitionsTable) | ||
| .set({ settings: newSettings, updatedAt: new Date() }) | ||
| .where(eq(competitionsTable.id, data.competitionId)) | ||
|
|
||
| return { success: true, resultsAutoPublish: data.autoPublish } | ||
| }, | ||
| ) |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the target file and nearby functions.
ast-grep outline apps/wodsmith-start/src/server-fns/division-results-fns.ts --view expanded
# Inspect the relevant range around the auto-publish handler and the sibling publish handlers.
sed -n '1,220p' apps/wodsmith-start/src/server-fns/division-results-fns.ts
printf '\n--- SNIP ---\n'
sed -n '220,420p' apps/wodsmith-start/src/server-fns/division-results-fns.ts
printf '\n--- SNIP ---\n'
sed -n '420,820p' apps/wodsmith-start/src/server-fns/division-results-fns.ts
# Find any transaction or row-lock usage in this file and related settings updates.
rg -n "transaction\(|for\\(\"update\"\\)|settings\\)|stringifyCompetitionSettings|parseCompetitionSettings|update\\(competitionsTable\\)" apps/wodsmith-start/src/server-fns/division-results-fns.tsRepository: wodsmith/thewodapp
Length of output: 26345
Race on the shared competition settings blob
publishDivisionResultsFn, publishAllDivisionResultsFn, and setResultsAutoPublishFn all read the full settings JSON, mutate one field, then লিখ back the whole blob. Concurrent requests can silently overwrite each other’s changes. Wrap these updates in a transaction/row lock, or move resultsAutoPublish out of the shared settings column.
🤖 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 `@apps/wodsmith-start/src/server-fns/division-results-fns.ts` around lines 692
- 759, The competition settings update in setResultsAutoPublishFn is racing with
publishDivisionResultsFn and publishAllDivisionResultsFn because each reads and
rewrites the full settings blob. Fix this by making the settings update atomic
in the shared competition row, either by using a transaction/row lock around the
read-modify-write in setResultsAutoPublishFn (and the other two handlers) or by
moving resultsAutoPublish to its own column so it no longer shares the JSON
blob.
Source: Learnings
Comment on lines
+1115
to
+1186
| const existingTests = await db | ||
| .select({ position: benchmarkTestsTable.position }) | ||
| .from(benchmarkTestsTable) | ||
| .where(eq(benchmarkTestsTable.batteryId, battery.id)) | ||
| const nextPosition = | ||
| existingTests.reduce((max, row) => Math.max(max, row.position), 0) + 1 | ||
|
|
||
| const scoreModelConfig = resolveBenchmarkTestScoreModel({ | ||
| scheme: test.scheme, | ||
| scoreModel: test.scoreModel, | ||
| hybridFlipTier: test.hybridFlipTier, | ||
| maxTier: battery.maxTier, | ||
| }) | ||
|
|
||
| let thresholdRows: EncodedBenchmarkTestThresholdRow[] = [] | ||
| if (test.includedInScoring) { | ||
| if (!test.thresholds) { | ||
| throw new BenchmarkConfigError( | ||
| "Included benchmark tests require a complete threshold table", | ||
| ) | ||
| } | ||
| assertBenchmarkVariantThresholdsComplete({ | ||
| maxTier: battery.maxTier, | ||
| thresholds: test.thresholds, | ||
| }) | ||
| thresholdRows = encodeBenchmarkTestThresholds({ | ||
| scheme: test.scheme as WorkoutScheme, | ||
| inputUnit: test.inputUnit, | ||
| scoreModel: scoreModelConfig.scoreModel, | ||
| hybridFlipTier: scoreModelConfig.hybridFlipTier, | ||
| thresholds: test.thresholds, | ||
| }) | ||
| } else if (test.thresholds) { | ||
| thresholdRows = encodeBenchmarkTestThresholds({ | ||
| scheme: test.scheme as WorkoutScheme, | ||
| inputUnit: test.inputUnit, | ||
| scoreModel: scoreModelConfig.scoreModel, | ||
| hybridFlipTier: scoreModelConfig.hybridFlipTier, | ||
| thresholds: test.thresholds, | ||
| }) | ||
| } | ||
|
|
||
| let createdTestId = "" | ||
| await db.transaction(async (tx) => { | ||
| await tx.insert(benchmarkTestsTable).values({ | ||
| batteryId: battery.id, | ||
| categoryKey: test.categoryKey, | ||
| name: test.name, | ||
| position: nextPosition, | ||
| scheme: test.scheme, | ||
| scoreType: test.scoreType, | ||
| inputUnit: test.inputUnit, | ||
| includedInScoring: test.includedInScoring, | ||
| scoreModel: scoreModelConfig.scoreModel as "standard" | "hybrid", | ||
| hybridFlipTier: scoreModelConfig.hybridFlipTier, | ||
| }) | ||
|
|
||
| const [created] = await tx | ||
| .select({ id: benchmarkTestsTable.id }) | ||
| .from(benchmarkTestsTable) | ||
| .where( | ||
| and( | ||
| eq(benchmarkTestsTable.batteryId, battery.id), | ||
| eq(benchmarkTestsTable.position, nextPosition), | ||
| ), | ||
| ) | ||
| .limit(1) | ||
|
|
||
| if (!created) { | ||
| throw new BenchmarkConfigError("Failed to create benchmark test") | ||
| } | ||
| createdTestId = created.id |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== benchmark-scoring-tiers.ts excerpt ==\n'
sed -n '1080,1215p' apps/wodsmith-start/src/server/benchmark-scoring-tiers.ts
printf '\n== benchmarkTestsTable definition search ==\n'
rg -n "benchmarkTestsTable|sqliteTable|pgTable|unique\\(|primaryKey\\(" apps/wodsmith-start/src/server apps/wodsmith-start/src -g '!**/node_modules/**'
printf '\n== candidate schema files ==\n'
fd -a "benchmark" apps/wodsmith-start/srcRepository: wodsmith/thewodapp
Length of output: 22460
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== benchmarks schema excerpt ==\n'
wc -l apps/wodsmith-start/src/db/schemas/benchmarks.ts
sed -n '1,260p' apps/wodsmith-start/src/db/schemas/benchmarks.ts
printf '\n== returning()/insert patterns in benchmark server code ==\n'
rg -n "\.returning\(|insert\(benchmarkTestsTable\)|nextPosition|position" apps/wodsmith-start/src/server apps/wodsmith-start/src/server-fns apps/wodsmith-start/src/db/schemas/benchmarks.tsRepository: wodsmith/thewodapp
Length of output: 19535
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== locate shared benchmarks schema ==\n'
fd -a "benchmarks.ts" .
fd -a "benchmarks" .
printf '\n== search for benchmarkTestsTable definition and constraints ==\n'
rg -n "benchmarkTestsTable|uniqueIndex|unique\\(|index\\(|primaryKey\\(" . -g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**'Repository: wodsmith/thewodapp
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== packages/wodsmith-db/src/schemas/benchmarks.ts ==\n'
wc -l packages/wodsmith-db/src/schemas/benchmarks.ts
sed -n '1,260p' packages/wodsmith-db/src/schemas/benchmarks.ts
printf '\n== benchmarkTestsTable insert-return patterns ==\n'
rg -n "benchmarkTestsTable.*returning|returning\\(.*benchmarkTestsTable|insert\\(benchmarkTestsTable\\)" apps/wodsmith-start/src packages/wodsmith-db/srcRepository: wodsmith/thewodapp
Length of output: 6255
Position assignment can still race under concurrent creates.
Two concurrent createBenchmarkTest calls can derive the same nextPosition; the unique index on (batteryId, position) will make one transaction fail. Move the max-position read under the transaction and add a retry/locking strategy if concurrent test creation is possible.
🤖 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 `@apps/wodsmith-start/src/server/benchmark-scoring-tiers.ts` around lines 1115
- 1186, Position assignment in createBenchmarkTest can still collide under
concurrent inserts because nextPosition is computed before the transaction. Move
the existingTests/max-position read into the db.transaction block in
benchmark-scoring-tiers.ts, use that transaction to derive the next position,
and add a retry or locking approach around the insert/select flow so the unique
(batteryId, position) constraint does not fail when multiple creates run at
once.
Comment on lines
+320
to
+357
| await tx | ||
| .insert(competitionRegistrationsTable) | ||
| .values({ | ||
| id: createCompetitionRegistrationId(), | ||
| eventId: context.competitionId, | ||
| userId, | ||
| teamMemberId, | ||
| divisionId: context.openDivisionId, | ||
| registeredAt: now, | ||
| captainUserId: userId, | ||
| athleteTeamId: null, | ||
| metadata: JSON.stringify({ benchmarkOpenJoin: true }), | ||
| }) | ||
| .onDuplicateKeyUpdate({ set: { updatedAt: now } }) | ||
|
|
||
| const [createdRegistration] = await tx | ||
| .select({ | ||
| id: competitionRegistrationsTable.id, | ||
| divisionId: competitionRegistrationsTable.divisionId, | ||
| captainUserId: competitionRegistrationsTable.captainUserId, | ||
| athleteTeamId: competitionRegistrationsTable.athleteTeamId, | ||
| }) | ||
| .from(competitionRegistrationsTable) | ||
| .where( | ||
| and( | ||
| eq(competitionRegistrationsTable.eventId, context.competitionId), | ||
| eq(competitionRegistrationsTable.userId, userId), | ||
| eq(competitionRegistrationsTable.divisionId, context.openDivisionId), | ||
| ne(competitionRegistrationsTable.status, REGISTRATION_STATUS.REMOVED), | ||
| ), | ||
| ) | ||
| .limit(1) | ||
|
|
||
| if (!createdRegistration) { | ||
| throw new Error("Failed to join benchmark board") | ||
| } | ||
|
|
||
| return { ...createdRegistration, isCaptain: true } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== relevant file slice =="
sed -n '280,380p' apps/wodsmith-start/src/server/benchmark-submissions.ts
echo
echo "== competitionRegistrationsTable definition =="
rg -n "competitionRegistrationsTable|uniqueKey|unique.*registration|status.*REMOVED|REGISTRATION_STATUS" apps/wodsmith-start/src -S
echo
echo "== schema files mentioning registrations table =="
fd -a "competition-registrations" apps/wodsmith-start/src || true
fd -a "*registration*" apps/wodsmith-start/src/server apps/wodsmith-start/src | head -n 200Repository: wodsmith/thewodapp
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== find table declaration =="
rg -n "competitionRegistrationsTable\s*=|pgTable\(|mysqlTable\(|sqliteTable\(" apps/wodsmith-start/src -S | head -n 50
echo
echo "== around table declaration files =="
fd -a "schema" apps/wodsmith-start/src || true
fd -a "tables" apps/wodsmith-start/src || trueRepository: wodsmith/thewodapp
Length of output: 185
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== benchmark-submissions around the open-join logic =="
sed -n '240,360p' apps/wodsmith-start/src/server/benchmark-submissions.ts
echo
echo "== locate registrations schema file =="
fd -a "competitions.ts" apps/wodsmith-start/src/db apps/wodsmith-start/src | head -n 20
fd -a "*competitions*" apps/wodsmith-start/src/db apps/wodsmith-start/src | head -n 50
echo
echo "== narrow search for competitionRegistrationsTable declaration =="
rg -n "competitionRegistrationsTable\s*=" apps/wodsmith-start/src/db apps/wodsmith-start/src/server apps/wodsmith-start/src/server-fns -S --max-count 20
echo
echo "== narrow search for unique indexes/constraints on registrations =="
rg -n "unique|uniqueIndex|index\(|primaryKey|constraint" apps/wodsmith-start/src/db/schemas apps/wodsmith-start/src/db -S --max-count 80Repository: wodsmith/thewodapp
Length of output: 4476
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== benchmark-submissions around the open-join logic =="
sed -n '240,360p' apps/wodsmith-start/src/server/benchmark-submissions.ts
echo
echo "== registrations schema candidates =="
fd -a "competitions.ts" apps/wodsmith-start/src/db apps/wodsmith-start/src | head -n 20
fd -a "*competition*" apps/wodsmith-start/src/db apps/wodsmith-start/src | head -n 50
echo
echo "== declaration search =="
rg -n "competitionRegistrationsTable\s*=" apps/wodsmith-start/src/db apps/wodsmith-start/src/server apps/wodsmith-start/src/server-fns -S --max-count 20
echo
echo "== unique/constraint search in schema area =="
rg -n "uniqueIndex|uniqueKey|unique\(|primaryKey|index\(|constraint" apps/wodsmith-start/src/db -S --max-count 120Repository: wodsmith/thewodapp
Length of output: 4474
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file=apps/wodsmith-start/src/db/schemas/competitions.ts
echo "== file size =="
wc -l "$file"
echo
echo "== table declaration and constraints =="
rg -n "competitionRegistrationsTable|uniqueIndex|unique\(|primaryKey|index\(|status|REMOVED" "$file" -S
echo
echo "== surrounding lines for table declaration =="
sed -n '1,260p' "$file"
echo
echo "== later lines if declaration continues =="
sed -n '260,520p' "$file"Repository: wodsmith/thewodapp
Length of output: 266
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== competitions.ts re-export =="
cat -n apps/wodsmith-start/src/db/schemas/competitions.ts
echo
echo "== locate source files imported by the re-export =="
rg -n "from \"\.\/|from './|from \"`@/db/schemas/`|export \* from" apps/wodsmith-start/src/db/schemas -S
echo
echo "== inspect likely underlying schema files =="
for f in $(rg -l "competitionRegistrationsTable" apps/wodsmith-start/src/db/schemas apps/wodsmith-start/src/db -S); do
echo "--- $f"
wc -l "$f"
rg -n "competitionRegistrationsTable|uniqueIndex|unique\(|primaryKey|index\(|status|REMOVED" "$f" -S
doneRepository: wodsmith/thewodapp
Length of output: 432
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate competitions schema package =="
fd -a "competitions.ts" . | sed -n '1,80p'
echo
echo "== search for competitionRegistrationsTable across repo =="
rg -n --fixed-strings "competitionRegistrationsTable" . --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' | sed -n '1,200p'
echo
echo "== search for schema package path markers =="
rg -n --fixed-strings "`@repo/wodsmith-db/schemas/competitions`" . --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**'Repository: wodsmith/thewodapp
Length of output: 27313
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file=packages/wodsmith-db/src/schemas/competitions.ts
echo "== file size =="
wc -l "$file"
echo
echo "== registration table and indexes =="
rg -n "competitionRegistrationsTable|uniqueIndex|unique\(|primaryKey|index\(|status|REMOVED|eventId|userId|divisionId|id:" "$file" -S
echo
echo "== registration table slice =="
sed -n '140,240p' "$file"
echo
echo "== indexes/relations slice =="
sed -n '540,620p' "$file"Repository: wodsmith/thewodapp
Length of output: 12104
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path("packages/wodsmith-db/src/schemas/competitions.ts")
text = p.read_text()
for needle in [
"competitionRegistrationsTable",
"uniqueIndex",
"primaryKey",
"eventId",
"userId",
"divisionId",
"status",
]:
print(f"== {needle} ==")
for i, line in enumerate(text.splitlines(), 1):
if needle in line:
print(f"{i}: {line}")
print()
PYRepository: wodsmith/thewodapp
Length of output: 3411
Re-activate removed benchmark registrations on rejoin. The upsert only updates updatedAt, so a unique-key hit on competition_registrations_event_user_division_idx leaves status = removed. The follow-up lookup filters out removed rows, so a user who left and tries to rejoin still ends up with "Failed to join benchmark board". Reset status in the duplicate-key branch.
🤖 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 `@apps/wodsmith-start/src/server/benchmark-submissions.ts` around lines 320 -
357, The rejoin path in benchmark-submissions.ts leaves existing registrations
in removed state because the duplicate-key branch of the insert only updates
updatedAt. Update the onDuplicateKeyUpdate behavior in the registration creation
flow so a rejoin via competitionRegistrationsTable reactivates the existing row
by resetting status from REGISTRATION_STATUS.REMOVED to the active registration
state, then keep the existing lookup and createdRegistration return logic
unchanged.
Comment on lines
+21
to
+97
| describe("BenchmarkSettingsForm", () => { | ||
| let onSave: ReturnType<typeof vi.fn> | ||
|
|
||
| beforeEach(() => { | ||
| onSave = vi.fn().mockResolvedValue(undefined) | ||
| vi.clearAllMocks() | ||
| }) | ||
|
|
||
| it("renders inline as benchmark settings without battery terminology", () => { | ||
| render(<BenchmarkSettingsForm settings={createSettings()} onSave={onSave} />) | ||
|
|
||
| expect(screen.getByText("Benchmark settings")).toBeInTheDocument() | ||
| expect(screen.getByLabelText("Name")).toHaveValue( | ||
| "Training Guide Benchmark", | ||
| ) | ||
| expect(screen.getByLabelText("Number of tiers")).toHaveValue(10) | ||
| expect(screen.getByLabelText("Score scale")).toHaveValue(100) | ||
| expect(screen.queryByText(/battery/i)).not.toBeInTheDocument() | ||
| }) | ||
|
|
||
| it("keeps save disabled until something changes", () => { | ||
| render(<BenchmarkSettingsForm settings={createSettings()} onSave={onSave} />) | ||
|
|
||
| const save = screen.getByRole("button", { name: /save settings/i }) | ||
| expect(save).toBeDisabled() | ||
|
|
||
| fireEvent.change(screen.getByLabelText("Name"), { | ||
| target: { value: "Gym Benchmark" }, | ||
| }) | ||
| expect(save).not.toBeDisabled() | ||
| }) | ||
|
|
||
| it("warns before destructive tier-count reductions", () => { | ||
| render(<BenchmarkSettingsForm settings={createSettings()} onSave={onSave} />) | ||
|
|
||
| fireEvent.change(screen.getByLabelText("Number of tiers"), { | ||
| target: { value: "8" }, | ||
| }) | ||
|
|
||
| expect( | ||
| screen.getByText(/threshold columns above the new tier count/i), | ||
| ).toBeInTheDocument() | ||
| }) | ||
|
|
||
| it("submits the parsed draft on save", async () => { | ||
| render(<BenchmarkSettingsForm settings={createSettings()} onSave={onSave} />) | ||
|
|
||
| fireEvent.change(screen.getByLabelText("Number of tiers"), { | ||
| target: { value: "12" }, | ||
| }) | ||
| fireEvent.change(screen.getByLabelText("Description"), { | ||
| target: { value: " Annual fitness check " }, | ||
| }) | ||
| fireEvent.click(screen.getByRole("button", { name: /save settings/i })) | ||
|
|
||
| await waitFor(() => expect(onSave).toHaveBeenCalledTimes(1)) | ||
| expect(onSave).toHaveBeenCalledWith({ | ||
| name: "Training Guide Benchmark", | ||
| description: "Annual fitness check", | ||
| scoreMax: 100, | ||
| maxTier: 12, | ||
| videoPolicy: "for_top_scores", | ||
| }) | ||
| }) | ||
|
|
||
| it("surfaces a toast when onSave rejects", async () => { | ||
| onSave.mockRejectedValueOnce(new Error("nope")) | ||
| render(<BenchmarkSettingsForm settings={createSettings()} onSave={onSave} />) | ||
|
|
||
| fireEvent.change(screen.getByLabelText("Name"), { | ||
| target: { value: "Renamed" }, | ||
| }) | ||
| fireEvent.click(screen.getByRole("button", { name: /save settings/i })) | ||
|
|
||
| await waitFor(() => expect(toast.error).toHaveBeenCalledWith("nope")) | ||
| }) | ||
| }) |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Missing @lat: spec-section reference.
No test in this file has a @lat: comment tying it to a test-spec leaf section, despite this being a new test file under apps/wodsmith-start/test/.
As per coding guidelines, "Place one @lat: comment next to each test that covers a spec section, and do not duplicate refs" and "Ensure every test-spec leaf section in lat.md/ has exactly one code reference in the test code."
🤖 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 `@apps/wodsmith-start/test/components/benchmark-settings-form.test.tsx` around
lines 21 - 97, Add the missing `@lat`: spec-section reference comments to this
BenchmarkSettingsForm test file so each test maps to exactly one leaf section in
lat.md. Place one unique `@lat`: comment next to each relevant it block in
benchmark-settings-form.test.tsx, using the existing test names like
BenchmarkSettingsForm, onSave, and toast.error to anchor the references, and
ensure no spec section is referenced more than once.
Source: Coding guidelines
Comment on lines
+196
to
+234
| describe("BenchmarkStatLine", () => { | ||
| it("renders Overall/100, category scores, per-test tiers, and states", () => { | ||
| render(<BenchmarkStatLine entry={createEntry()} />) | ||
|
|
||
| expect(screen.getByText("72.5")).toBeInTheDocument() | ||
| expect(screen.getByText("/100")).toBeInTheDocument() | ||
| expect(screen.getByText("Regional")).toBeInTheDocument() | ||
| expect(screen.getAllByText("Strength").length).toBeGreaterThan(0) | ||
| expect(screen.getAllByText("Engine").length).toBeGreaterThan(0) | ||
| expect(screen.getByText("Strict Press")).toBeInTheDocument() | ||
| expect(screen.getByText("Mile Run")).toBeInTheDocument() | ||
| expect(screen.getByText("Weighted Pull-Up")).toBeInTheDocument() | ||
| expect(screen.getAllByText("8").length).toBeGreaterThan(0) | ||
| expect(screen.getAllByText("6.5").length).toBeGreaterThan(0) | ||
| expect(screen.getByText("Verified")).toBeInTheDocument() | ||
| expect(screen.getByText("Adjusted")).toBeInTheDocument() | ||
| expect(screen.getByText("Unavailable")).toBeInTheDocument() | ||
| expect(screen.getByText("Untested")).toBeInTheDocument() | ||
| expect(screen.getByText("Tier 0")).toBeInTheDocument() | ||
| expect(screen.getByText("Pending")).toBeInTheDocument() | ||
| expect(screen.getByText("Excluded")).toBeInTheDocument() | ||
| }) | ||
|
|
||
| it("keeps unavailable (deferred) tests read-only even with submission context", () => { | ||
| render( | ||
| <BenchmarkStatLine entry={createEntry()} submission={submissionContext} />, | ||
| ) | ||
|
|
||
| // Scorable tests expand into a submission form. | ||
| expect( | ||
| screen.getByRole("button", { name: /Strict Press/ }), | ||
| ).toBeInTheDocument() | ||
| // Deferred tests reject submissions server-side, so no trigger renders. | ||
| expect( | ||
| screen.queryByRole("button", { name: /Weighted Pull-Up/ }), | ||
| ).not.toBeInTheDocument() | ||
| expect(screen.getByText("Weighted Pull-Up")).toBeInTheDocument() | ||
| }) | ||
| }) |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Missing @lat: spec-section reference.
Same as the settings-form test — neither test here has a @lat: comment linking it to a spec section.
As per coding guidelines, "Place one @lat: comment next to each test that covers a spec section, and do not duplicate refs."
🤖 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 `@apps/wodsmith-start/test/components/benchmark-stat-line.test.tsx` around
lines 196 - 234, Add the missing `@lat`: spec-section reference comments for each
BenchmarkStatLine test in this suite, placing one next to the test that covers
the overall rendering and one next to the test covering deferred/read-only
submission behavior. Use the existing test names in BenchmarkStatLine to locate
the right spots, and ensure each spec-section reference is added only once
without duplicating refs across the two tests.
Source: Coding guidelines
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
ai/research/hillerfit-benchmark-leaderboard/benchmark-scoring-walkthrough.html (1)
8-8: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winPin or bundle the Mermaid dependency.
mermaid@11resolves a floating 11.x release and requires network/CDN availability at render time. Pin an exact approved version and provide a bundled or no-JavaScript fallback for reproducible/offline viewing.🤖 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 `@ai/research/hillerfit-benchmark-leaderboard/benchmark-scoring-walkthrough.html` at line 8, Update the Mermaid import in benchmark-scoring-walkthrough.html to use an approved exact version instead of the floating mermaid@11 range, and provide a bundled or no-JavaScript fallback so the walkthrough remains viewable offline without CDN availability.apps/wodsmith-start/src/server/benchmark-creation-access.ts (1)
5-23: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd
@lat:comment linking to the benchmark rollout gates concept.The test files reference
[[competition-type-capabilities#Benchmark Rollout Gates#...]]lat.md sections, but this source file — which directly implements the rollout gate — has no@lat:comment. As per coding guidelines, use@lat:code-reference comments to link source code to lat.md concepts.♻️ Proposed addition
+ // `@lat`: [[competition-type-capabilities#Benchmark Rollout Gates]] export async function assertBenchmarkCreationAccess({🤖 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 `@apps/wodsmith-start/src/server/benchmark-creation-access.ts` around lines 5 - 23, Add an `@lat`: code-reference comment to assertBenchmarkCreationAccess, linking it to the lat.md “competition-type-capabilities” Benchmark Rollout Gates concept. Keep the existing access-check behavior unchanged and place the reference alongside the function implementation.Source: Coding guidelines
apps/wodsmith-start/src/routes/benchmarks.tsx (2)
71-84: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider server-side filtering for perpetual competitions.
Both
/and/benchmarksroutes callgetPublicCompetitionsFn({ data: {} })fetching all public competitions, then filter client-side. Adding a filter parameter (e.g.,{ data: { type: "perpetual" } }) would reduce payload size and avoid loading non-benchmark data on the benchmarks page. ThestaleTime: 30_000mitigates double-fetch on navigation, but the initial load still transfers all records.Also applies to: 113-128
🤖 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 `@apps/wodsmith-start/src/routes/benchmarks.tsx` around lines 71 - 84, Update the benchmarks route loader’s getPublicCompetitionsFn call to request only perpetual competitions via its supported type filter, while preserving the existing session and organizer-application loading and response shape.
90-230: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract shared patterns between benchmarks.tsx and index.tsx.
Both route files duplicate the
hasMountedanimation pattern, search input UI, card grid rendering, and footer. TheEmptyStatecomponents are also near-identical. Extracting these into shared components/hooks would reduce maintenance burden as both pages evolve.♻️ Suggested extractions
1. Shared animation hook (
src/hooks/use-mounted.ts):import { useEffect, useState } from "react" export function useMounted() { const [hasMounted, setHasMounted] = useState(false) useEffect(() => setHasMounted(true), []) return hasMounted }2. Shared search input component (
src/components/competition-search-input.tsx):import { SearchIcon, X } from "lucide-react" import { Button } from "`@/components/ui/button`" import { Input } from "`@/components/ui/input`" export function CompetitionSearchInput({ value, onChange, ariaLabel, placeholder = "Search…", }: { value: string onChange: (value: string) => void ariaLabel: string placeholder?: string }) { return ( <div className="relative w-full sm:w-56"> <SearchIcon className="absolute left-3 top-1/2 h-3.5 w-3.5 -translate-y-1/2 text-muted-foreground/60" aria-hidden="true" /> <Input type="search" aria-label={ariaLabel} placeholder={placeholder} value={value} onChange={(e) => onChange(e.target.value)} autoComplete="off" spellCheck={false} className="pl-9 pr-8 h-9 text-sm bg-secondary/50 border-transparent focus:bg-card focus:border-input" /> {value && ( <Button variant="ghost" size="icon" className="absolute right-0.5 top-1/2 h-7 w-7 -translate-y-1/2" onClick={() => onChange("")}> <X className="h-3.5 w-3.5" /> <span className="sr-only">Clear search</span> </Button> )} </div> ) }🤖 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 `@apps/wodsmith-start/src/routes/benchmarks.tsx` around lines 90 - 230, Extract the duplicated mounted-state logic, competition search input, card-grid rendering, footer, and near-identical EmptyState UI shared by BenchmarksPage and the corresponding index route into reusable components/hooks. Update BenchmarksPage to consume those shared symbols while preserving its current search behavior, filtering, card status/index props, animation timing, accessibility labels, and layout.apps/wodsmith-start/src/components/compete-nav.tsx (1)
35-40: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract shared nav link class helper.
benchmarksLinkClassandcompetitionsLinkClassare identical ternary expressions with different boolean conditions. The same duplication exists incompete-mobile-nav.tsx(lines 79-84). A small helper would eliminate the repetition.♻️ Suggested refactor
+function navLinkClass(isActive: boolean): string { + return isActive + ? "font-bold text-foreground uppercase underline decoration-primary decoration-2 underline-offset-4 dark:text-dark-foreground" + : "font-bold text-foreground uppercase hover:underline dark:text-dark-foreground" +} + const competitionsLinkClass = isCompetitionIndex - ? "font-bold text-foreground uppercase underline decoration-primary decoration-2 underline-offset-4 dark:text-dark-foreground" - : "font-bold text-foreground uppercase hover:underline dark:text-dark-foreground" + ? navLinkClass(true) + : navLinkClass(false) const benchmarksLinkClass = isBenchmarksIndex - ? "font-bold text-foreground uppercase underline decoration-primary decoration-2 underline-offset-4 dark:text-dark-foreground" - : "font-bold text-foreground uppercase hover:underline dark:text-dark-foreground" + ? navLinkClass(true) + : navLinkClass(false)Or more concisely:
-const competitionsLinkClass = isCompetitionIndex - ? "font-bold text-foreground uppercase underline decoration-primary decoration-2 underline-offset-4 dark:text-dark-foreground" - : "font-bold text-foreground uppercase hover:underline dark:text-dark-foreground" -const benchmarksLinkClass = isBenchmarksIndex - ? "font-bold text-foreground uppercase underline decoration-primary decoration-2 underline-offset-4 dark:text-dark-foreground" - : "font-bold text-foreground uppercase hover:underline dark:text-dark-foreground" +const competitionsLinkClass = navLinkClass(isCompetitionIndex) +const benchmarksLinkClass = navLinkClass(isBenchmarksIndex)🤖 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 `@apps/wodsmith-start/src/components/compete-nav.tsx` around lines 35 - 40, Extract a shared navigation-link class helper for the duplicated ternary expressions used by competitionsLinkClass and benchmarksLinkClass, then call it with each respective active-state boolean. Apply the same helper in compete-mobile-nav.tsx to replace its duplicated class logic while preserving the existing active and inactive class strings.
🤖 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
`@ai/research/hillerfit-benchmark-leaderboard/benchmark-scoring-walkthrough.html`:
- Around line 90-115: Add accessible textual summaries for every Mermaid diagram
in the document, including the diagrams near the referenced ranges. Update each
diagram’s surrounding markup to provide a concise description via visually
hidden content or a details section that remains useful when Mermaid or its CDN
fails, while preserving the existing rendered diagrams.
In `@apps/wodsmith-start/src/server/benchmark-creation-access.ts`:
- Around line 18-21: Update the access-denial branch in benchmark creation to
throw the app’s established AppError with the "FORBIDDEN" error type instead of
a generic Error, preserving the existing denial message. Keep the change scoped
to the canCreateBenchmarks check so createCompetitionFn recognizes the expected
authorization failure.
---
Nitpick comments:
In
`@ai/research/hillerfit-benchmark-leaderboard/benchmark-scoring-walkthrough.html`:
- Line 8: Update the Mermaid import in benchmark-scoring-walkthrough.html to use
an approved exact version instead of the floating mermaid@11 range, and provide
a bundled or no-JavaScript fallback so the walkthrough remains viewable offline
without CDN availability.
In `@apps/wodsmith-start/src/components/compete-nav.tsx`:
- Around line 35-40: Extract a shared navigation-link class helper for the
duplicated ternary expressions used by competitionsLinkClass and
benchmarksLinkClass, then call it with each respective active-state boolean.
Apply the same helper in compete-mobile-nav.tsx to replace its duplicated class
logic while preserving the existing active and inactive class strings.
In `@apps/wodsmith-start/src/routes/benchmarks.tsx`:
- Around line 71-84: Update the benchmarks route loader’s
getPublicCompetitionsFn call to request only perpetual competitions via its
supported type filter, while preserving the existing session and
organizer-application loading and response shape.
- Around line 90-230: Extract the duplicated mounted-state logic, competition
search input, card-grid rendering, footer, and near-identical EmptyState UI
shared by BenchmarksPage and the corresponding index route into reusable
components/hooks. Update BenchmarksPage to consume those shared symbols while
preserving its current search behavior, filtering, card status/index props,
animation timing, accessibility labels, and layout.
In `@apps/wodsmith-start/src/server/benchmark-creation-access.ts`:
- Around line 5-23: Add an `@lat`: code-reference comment to
assertBenchmarkCreationAccess, linking it to the lat.md
“competition-type-capabilities” Benchmark Rollout Gates concept. Keep the
existing access-check behavior unchanged and place the reference alongside the
function implementation.
🪄 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: CHILL
Plan: Pro
Run ID: 6485805a-355f-42f4-8e5e-65ae6a12cf2f
📒 Files selected for processing (25)
AGENTS.mdCLAUDE.mdai/research/hillerfit-benchmark-leaderboard/benchmark-scoring-walkthrough.htmlapps/wodsmith-start/scripts/seed/seeders/02-billing.tsapps/wodsmith-start/scripts/seed/seeders/06-team-entitlements.tsapps/wodsmith-start/src/components/compete-mobile-nav.tsxapps/wodsmith-start/src/components/compete-nav.tsxapps/wodsmith-start/src/components/organizer-competition-form.tsxapps/wodsmith-start/src/config/features.tsapps/wodsmith-start/src/lib/competitions/capabilities.tsapps/wodsmith-start/src/lib/posthog/hooks.tsapps/wodsmith-start/src/lib/posthog/index.tsapps/wodsmith-start/src/routeTree.gen.tsapps/wodsmith-start/src/routes/__root.tsxapps/wodsmith-start/src/routes/benchmarks.tsxapps/wodsmith-start/src/routes/index.tsxapps/wodsmith-start/src/server-fns/competition-fns.tsapps/wodsmith-start/src/server-fns/team-fns.tsapps/wodsmith-start/src/server/benchmark-creation-access.tsapps/wodsmith-start/test/components/compete-nav-benchmark-flag.test.tsxapps/wodsmith-start/test/components/organizer-competition-form.test.tsxapps/wodsmith-start/test/lib/competitions/capabilities.test.tsapps/wodsmith-start/test/scripts/benchmark-entitlement-seed.test.tsapps/wodsmith-start/test/server/benchmark-creation-access.test.tslat.md/competition-type-capabilities.md
✅ Files skipped from review due to trivial changes (5)
- apps/wodsmith-start/test/server/benchmark-creation-access.test.ts
- AGENTS.md
- lat.md/competition-type-capabilities.md
- CLAUDE.md
- apps/wodsmith-start/src/routeTree.gen.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/wodsmith-start/src/routes/__root.tsx
- apps/wodsmith-start/src/lib/competitions/capabilities.ts
- apps/wodsmith-start/src/components/organizer-competition-form.tsx
Comment on lines
+90
to
+115
| <div class="card"> | ||
| <pre class="mermaid"> | ||
| flowchart LR | ||
| subgraph SETUP["Organizer setup"] | ||
| A["tiers.tsx<br/>threshold editor"] --> B["saveBenchmarkScoringTiersFn"] | ||
| B --> C["encodeBenchmarkThresholdValue()<br/>'19:30' → 1170000 ms<br/>'315' → 315 reps/lbs"] | ||
| C --> D[("benchmark_tier_thresholds<br/>testId × variant × tier")] | ||
| end | ||
|
|
||
| subgraph ATHLETE["Athlete"] | ||
| E["Submits video + score"] --> F["scores row<br/>+ benchmarkVariant<br/>(from profile gender)"] | ||
| end | ||
|
|
||
| subgraph LB["getCompetitionLeaderboard()"] | ||
| G["loadBenchmarkLeaderboardContext()"] --> H["calculateAbsoluteTier()<br/>per score, per event"] | ||
| F2[("scores")] --> H | ||
| H --> I["aggregateBenchmarkScores()<br/>tiers → category scores → overall"] | ||
| I --> J["findBenchmarkRatingBand()"] | ||
| K["online algorithm<br/>place = points"] --> L["totalPoints → overallRank"] | ||
| end | ||
|
|
||
| D --> G | ||
| F --> F2 | ||
| J --> M["Leaderboard UI<br/>87.5/100 · Elite ·<br/>Tier badges per event"] | ||
| L --> M | ||
| </pre> |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add accessible fallbacks for the Mermaid diagrams.
If Mermaid or the CDN fails, these <pre> blocks expose Mermaid DSL rather than a usable textual description. Add concise accessible summaries—such as visually hidden text or <details> descriptions—for each diagram.
Also applies to: 121-154, 160-174, 200-227, 249-281
🧰 Tools
🪛 HTMLHint (1.9.2)
[error] 94-94: Special characters must be escaped : [ > ].
(spec-char-escape)
[error] 95-95: Special characters must be escaped : [ > ].
(spec-char-escape)
[error] 96-96: Special characters must be escaped : [ > ].
(spec-char-escape)
[error] 100-100: Special characters must be escaped : [ > ].
(spec-char-escape)
[error] 104-104: Special characters must be escaped : [ > ].
(spec-char-escape)
[error] 105-105: Special characters must be escaped : [ > ].
(spec-char-escape)
[error] 106-106: Special characters must be escaped : [ > ].
(spec-char-escape)
[error] 107-107: Special characters must be escaped : [ > ].
(spec-char-escape)
[error] 108-108: Special characters must be escaped : [ > ].
(spec-char-escape)
[error] 111-111: Special characters must be escaped : [ > ].
(spec-char-escape)
[error] 112-112: Special characters must be escaped : [ > ].
(spec-char-escape)
[error] 113-113: Special characters must be escaped : [ > ].
(spec-char-escape)
[error] 114-114: Special characters must be escaped : [ > ].
(spec-char-escape)
🤖 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
`@ai/research/hillerfit-benchmark-leaderboard/benchmark-scoring-walkthrough.html`
around lines 90 - 115, Add accessible textual summaries for every Mermaid
diagram in the document, including the diagrams near the referenced ranges.
Update each diagram’s surrounding markup to provide a concise description via
visually hidden content or a details section that remains useful when Mermaid or
its CDN fails, while preserving the existing rendered diagrams.
Comment on lines
+18
to
+21
| if (!canCreateBenchmarks) { | ||
| throw new Error( | ||
| "Your team does not have access to create benchmark competitions", | ||
| ) |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for HTTP error patterns used in server functions
rg -nP 'throw new (Error|HttpError|APIError|ServerError|UnauthorizedError|ForbiddenError)' --type=ts apps/wodsmith-start/src/server-fns/ -C3
# Check if there's a custom error class for HTTP status handling
rg -nP 'class.*Error|extends.*Error' --type=ts apps/wodsmith-start/src/ -C3Repository: wodsmith/thewodapp
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== benchmark-creation-access.ts =="
cat -n apps/wodsmith-start/src/server/benchmark-creation-access.ts
echo
echo "== createCompetitionFn references =="
rg -n "createCompetitionFn|Failed to create competition|benchmark creation" apps/wodsmith-start/src -C 3
echo
echo "== HTTP/status error patterns in src =="
rg -n "throw new Response|Response\\(|status: 403|statusCode|Forbidden|Unauthorized|HTTPException|createError|TRPCError|ApplicationError|extends .*Error" apps/wodsmith-start/src -C 2Repository: wodsmith/thewodapp
Length of output: 48106
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== utils/errors.ts =="
cat -n apps/wodsmith-start/src/utils/errors.ts
echo
echo "== AppError usage =="
rg -n "AppError|ERROR_CODES|errorCode|code:" apps/wodsmith-start/src -C 2
echo
echo "== server-fn error handling / mapping =="
rg -n "instanceof AppError|error instanceof|status.*403|Forbidden|Unauthorized|Response\\(" apps/wodsmith-start/src/server-fns apps/wodsmith-start/src/server -C 2Repository: wodsmith/thewodapp
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== createCompetitionFn section =="
sed -n '593,690p' apps/wodsmith-start/src/server-fns/competition-fns.ts
echo
echo "== AppError handling in server-fns =="
rg -n "instanceof AppError|throw new AppError|catch \\(error\\)|Failed to create competition|NOT_AUTHORIZED|FORBIDDEN" apps/wodsmith-start/src/server-fns -C 2
echo
echo "== shared server error mapping/wrappers =="
rg -n "AppError|ERROR_CODES|message.*FORBIDDEN|status.*403|throw.*status|createServerFn\\(|serverFn" apps/wodsmith-start/src -C 2 | sed -n '1,240p'Repository: wodsmith/thewodapp
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== AppError references outside server-fns =="
rg -n "AppError|ERROR_CODES|NOT_AUTHORIZED|FORBIDDEN" apps/wodsmith-start/src -g '!apps/wodsmith-start/src/server-fns/**' -C 2
echo
echo "== message-prefix error parsing in client/routes =="
rg -n "startsWith\\(\"(FORBIDDEN|NOT_AUTHORIZED|NOT_FOUND|CONFLICT|ERROR):\"|errorCode|parse error code|message prefix" apps/wodsmith-start/src -C 2
echo
echo "== createServerFn error handling helpers =="
rg -n "createServerFn\\(|ServerFn|server function|error boundary|status.*500|status.*403|throw new AppError|throw new Error\\(\"FORBIDDEN" apps/wodsmith-start/src -C 2 | sed -n '1,220p'Repository: wodsmith/thewodapp
Length of output: 40569
Use the app’s FORBIDDEN error type here. AppError("FORBIDDEN", ...) is already the convention for access denials, and this keeps the expected rejection from being logged as a generic create-failure in createCompetitionFn.
🤖 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 `@apps/wodsmith-start/src/server/benchmark-creation-access.ts` around lines 18
- 21, Update the access-denial branch in benchmark creation to throw the app’s
established AppError with the "FORBIDDEN" error type instead of a generic Error,
preserving the existing denial message. Keep the change scoped to the
canCreateBenchmarks check so createCompetitionFn recognizes the expected
authorization failure.
Contributor
There was a problem hiding this comment.
3 issues found across 16 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/wodsmith-start/src/server-fns/competition-fns.ts">
<violation number="1" location="apps/wodsmith-start/src/server-fns/competition-fns.ts:611">
P1: Teams without `CREATE_BENCHMARKS` can create a permitted type, then call `updateCompetitionFn` with `competitionType: "benchmark"` to bypass this new rollout gate. Apply the same entitlement check when an update selects benchmark.</violation>
<violation number="2" location="apps/wodsmith-start/src/server-fns/competition-fns.ts:611">
P0: Any unauthenticated caller can submit an entitled team's ID and create a benchmark owned by that team. Pair the entitlement check with session authentication and `MANAGE_COMPETITIONS` membership for `organizingTeamId` before creating it.</violation>
</file>
<file name="apps/wodsmith-start/src/components/organizer-competition-form.tsx">
<violation number="1" location="apps/wodsmith-start/src/components/organizer-competition-form.tsx:233">
P1: Editing an existing benchmark competition where the organizing team doesn't have benchmark creation privileges will silently reset the competition type to "in-person". The `competitionTypeOptions` filter correctly preserves the benchmark option in edit mode via `(isEditMode && initialCompetitionType === "benchmark")`, but the `useEffect` that resets the competition type when `canCreateBenchmarks` is false does not include this same carve-out, so the type gets overwritten on mount. Add the same edit-mode guard to the effect condition so an existing benchmark competition's type is not reset.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| }) | ||
|
|
||
| try { | ||
| await assertBenchmarkCreationAccess({ |
Contributor
There was a problem hiding this comment.
P0: Any unauthenticated caller can submit an entitled team's ID and create a benchmark owned by that team. Pair the entitlement check with session authentication and MANAGE_COMPETITIONS membership for organizingTeamId before creating it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/wodsmith-start/src/server-fns/competition-fns.ts, line 611:
<comment>Any unauthenticated caller can submit an entitled team's ID and create a benchmark owned by that team. Pair the entitlement check with session authentication and `MANAGE_COMPETITIONS` membership for `organizingTeamId` before creating it.</comment>
<file context>
@@ -607,6 +608,11 @@ export const createCompetitionFn = createServerFn({ method: "POST" })
})
try {
+ await assertBenchmarkCreationAccess({
+ teamId: data.organizingTeamId,
+ competitionType: data.competitionType,
</file context>
| }) | ||
|
|
||
| try { | ||
| await assertBenchmarkCreationAccess({ |
Contributor
There was a problem hiding this comment.
P1: Teams without CREATE_BENCHMARKS can create a permitted type, then call updateCompetitionFn with competitionType: "benchmark" to bypass this new rollout gate. Apply the same entitlement check when an update selects benchmark.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/wodsmith-start/src/server-fns/competition-fns.ts, line 611:
<comment>Teams without `CREATE_BENCHMARKS` can create a permitted type, then call `updateCompetitionFn` with `competitionType: "benchmark"` to bypass this new rollout gate. Apply the same entitlement check when an update selects benchmark.</comment>
<file context>
@@ -607,6 +608,11 @@ export const createCompetitionFn = createServerFn({ method: "POST" })
})
try {
+ await assertBenchmarkCreationAccess({
+ teamId: data.organizingTeamId,
+ competitionType: data.competitionType,
</file context>
| (isEditMode && initialCompetitionType === "benchmark"), | ||
| ) | ||
|
|
||
| useEffect(() => { |
Contributor
There was a problem hiding this comment.
P1: Editing an existing benchmark competition where the organizing team doesn't have benchmark creation privileges will silently reset the competition type to "in-person". The competitionTypeOptions filter correctly preserves the benchmark option in edit mode via (isEditMode && initialCompetitionType === "benchmark"), but the useEffect that resets the competition type when canCreateBenchmarks is false does not include this same carve-out, so the type gets overwritten on mount. Add the same edit-mode guard to the effect condition so an existing benchmark competition's type is not reset.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/wodsmith-start/src/components/organizer-competition-form.tsx, line 233:
<comment>Editing an existing benchmark competition where the organizing team doesn't have benchmark creation privileges will silently reset the competition type to "in-person". The `competitionTypeOptions` filter correctly preserves the benchmark option in edit mode via `(isEditMode && initialCompetitionType === "benchmark")`, but the `useEffect` that resets the competition type when `canCreateBenchmarks` is false does not include this same carve-out, so the type gets overwritten on mount. Add the same edit-mode guard to the effect condition so an existing benchmark competition's type is not reset.</comment>
<file context>
@@ -216,9 +216,28 @@ export function OrganizerCompetitionForm({
+ (isEditMode && initialCompetitionType === "benchmark"),
+ )
+
+ useEffect(() => {
+ if (
+ !canCreateBenchmarks &&
</file context>
Suggested change
| useEffect(() => { | |
| useEffect(() => { | |
| if ( | |
| !canCreateBenchmarks && | |
| !(isEditMode && initialCompetitionType === "benchmark") && | |
| form.getValues("competitionType") === "benchmark" | |
| ) { | |
| form.setValue("competitionType", "in-person") | |
| } | |
| }, [canCreateBenchmarks, form]) |
…ing lock, copy and seed fixes (#613) * feat(benchmark): apply meeting action items for benchmark refinements - Remove the 'Tier context is live' card and say 'half a point' in the scoring explainer on the organizer tiers page - Add a 'Link events to tier thresholds' section listing unlinked events; selecting one opens the add-test dialog locked to that event with name/scheme/score type/input unit prefilled from the event - Auto-select the 'time' input unit for time schemes in the test editor and bound the linked-event select to a scrollable ~10 items - Add cohosts/volunteerShifts/publicVolunteerSignup capabilities; benchmarks drop cohosts, volunteer shifts, and public volunteer signup (roster and waivers stay); sidebar items hidden and routes redirect defensively - Lock benchmark ranking algorithm to online (UI note + submit force + server-side guard) while keeping tiebreakers editable - Remove the dedicated leaderboard Affiliate column; affiliate stays as subtext under the athlete name - Label the Division and Athlete selects on the stats page - Rename 'Pre-published' to 'Auto-publish' in division results copy - Seed: vertical jump now scores in plain inches (points scheme) fixing a feet/inches mismatch; L-sit hold top tier raised to 3:00 per the CrossFit Journal standard - Update lat.md capability and organizer-dashboard docs Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R5MgTsR5FNrj7Tvu7T6Ebv * fix(benchmark): harden online-scoring lock and polish tier linking UX Review-round fixes from adversarial code review: - Resolve the effective leaderboard scoring config through a helper that forces the online algorithm for benchmark boards, so a fresh benchmark with no stored scoringConfig no longer silently ranks 'traditional' - Mirror the benchmark online force in the cohost scoring save path - Move the 'Link events to tier thresholds' card to the top of the tiers page per the meeting's 'list of events up here' - Treat emom as a time scheme for input-unit auto-selection - Drop unused timeCapSeconds plumbing from benchmark event options - Update lat.md docs for the read-time force and tier-linking UX Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R5MgTsR5FNrj7Tvu7T6Ebv * fix benchmark tiers for unmapped tests * fix benchmark review feedback --------- Co-authored-by: Claude <noreply@anthropic.com>
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
Prepares the HillerFit-derived benchmark leaderboard project for implementation with a build-ready documentation packet.
This explicitly keeps the shipped product generic: we are not building HillerFit-branded pages, routes, marketing surfaces, logos, theme treatments, or product navigation. The local
HillerFit_Training_Guide.pdfis only the source artifact for the first benchmark seed data; the UI remains WODsmith's generic benchmark board/stat-line experience.Artifacts
ai/research/hillerfit-benchmark-leaderboard-guide.mdai/research/hillerfit-benchmark-leaderboard/requirements.mdai/research/hillerfit-benchmark-leaderboard/technical-design.mdai/research/hillerfit-benchmark-leaderboard/tasks.mdai/research/hillerfit-benchmark-leaderboard/test-strategy.mdai/research/hillerfit-benchmark-leaderboard/traceability.mdai/research/hillerfit-benchmark-leaderboard/assumptions-and-decisions.mdai/research/hillerfit-benchmark-leaderboard/reviewer-alignment.mdVerification
lat search "benchmark leaderboard HillerFit branded pages training guide pdf capability registry M0a"lat expand "should be explicit in stating we are not building any hillerfit branded pages, we will just build out the benchmark against the training pdf"PATH="/Users/zacjones/.nvm/versions/node/v24.15.0/bin:$PATH" corepack pnpm lintpnpm lintandpnpm type-checkKnown existing LAT issue, unrelated to this packet:
lat checkstill fails on pre-existingcrewreferences inlat.md/crew.mdandapps/crew/test/routes/event-import-tabs.test.tsx.Stack Plan
This branch is the integration base for benchmark implementation PRs. Follow-up slices should target this branch and be merged back here, not into
main, until the benchmark implementation is complete.Summary by cubic
Adds a generic
benchmarkcompetition type with perpetual “Since …” leaderboards, a Stats page with inline submissions, and an organizer “Benchmark scoring” console. Benchmarks are selectable on create, discoverable at/benchmarks, and the nav link is gated by thebenchmark-comp-typeflag.benchmarkselectable in create; start date with optional end; home index hides perpetual boards; dedicated/benchmarks; nav link gated by PostHog flag.Written for commit 8f11595. Summary will update on new commits.
mainSummary by CodeRabbit