feat(benchmark): meeting action items — gating, tier-linking UX, scoring lock, copy and seed fixes#613
Conversation
- 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
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
WalkthroughThis PR adds capability-based feature gating (cohosts, volunteer shifts, public volunteer signup) across sidebar UI and route loaders, forces benchmark competitions to always use the online scoring algorithm at save and read time, reworks benchmark test editor dialogs with event-linked prefill, removes the leaderboard affiliate column, updates benchmark seed thresholds, and renames "pre-publish" to "auto-publish" terminology. ChangesCapability-Gated Feature Access
Benchmark Online Scoring Lock
Benchmark Test Editor Dialogs & Event Linking
Leaderboard Affiliate Column Removal
Benchmark Seed Data Adjustments
Auto-Publish Wording & Stats UI
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![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: 45f6985f9e
ℹ️ 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".
| } | ||
|
|
||
| // Public volunteer signup is capability-gated (benchmark boards exclude it) | ||
| if (!competitionCan(competition.competitionType, "publicVolunteerSignup")) { |
There was a problem hiding this comment.
For benchmark boards this loader hides /volunteer, but the actual write paths (submitVolunteerSignupFn and especially the anonymous createAccountAndApplyAsVolunteerFn) still accept only competitionTeamId and create the volunteer invitation without resolving the competition type. A stale client or direct server-function call can therefore still create public volunteer applications for benchmark competitions, bypassing the new publicVolunteerSignup gate; please enforce the same capability check in the mutation/shared helper before creating the application.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (5)
apps/wodsmith-start/test/scripts/benchmark-seed-data.test.ts (1)
139-176: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a
//@lat:reference beside this test.
This benchmark seed-data test should carry the matching spec link inline to follow the test annotation rule.🤖 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/scripts/benchmark-seed-data.test.ts` around lines 139 - 176, Add the required inline // `@lat`: reference to this benchmark seed-data test so it satisfies the test annotation rule. Update the `it("scores vertical jump in plain inches and tops the L-sit at three minutes", ...)` block in `benchmark-seed-data.test.ts` and place the matching spec link beside the test declaration or its leading comment, keeping the rest of the assertions and helpers like `buildBenchmarkSeedRows` and `BENCHMARK_SEED_TESTS` unchanged.Source: Coding guidelines
apps/wodsmith-start/src/routes/compete/organizer/$competitionId/-components/scoring-settings-form.tsx (1)
30-43: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueLGTM!
The three-layer defense (init coercion, save re-force, server enforcement) is solid. One minor consistency note:
forceOnlineAlgorithmearly-returns whenalgorithm === "online"without clearingcustomTable, while the server-side enforcement inupdateCompetitionScoringConfigFnalways clears it. In practice this is harmless sincehandleAlgorithmChangealready clearscustomTablewhen switching to a non-custom algorithm, but you could align them if desired:Optional: align
forceOnlineAlgorithmwith server behaviorfunction forceOnlineAlgorithm(config: ScoringConfig): ScoringConfig { - if (config.algorithm === "online") return config - return { ...config, algorithm: "online", customTable: undefined } + if (config.algorithm === "online" && !config.customTable) return config + return { ...config, algorithm: "online", customTable: undefined } }Also applies to: 72-99, 124-124
🤖 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/scoring-settings-form.tsx around lines 30 - 43, `forceOnlineAlgorithm` is slightly inconsistent with `updateCompetitionScoringConfigFn` because it returns the config unchanged when `algorithm` is already "online", leaving any stale `customTable` intact. Update `forceOnlineAlgorithm` in `scoring-settings-form.tsx` to always normalize the config the same way the server does by clearing `customTable` whenever the online algorithm is enforced, and keep the save-path coercion and `handleAlgorithmChange` behavior aligned with that rule.apps/wodsmith-start/src/routes/compete/cohost/$competitionId/scoring.tsx (1)
79-82: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider centralizing the lock-algorithm capability check.
competitionCan(competition.competitionType, "benchmarkScoringTiers")is duplicated identically in this file,scoring/index.tsx, andsettings.tsx. Extracting a small helper (e.g.isScoringAlgorithmLocked(type)incapabilities.ts) would give a single place to update if the gating capability ever changes.♻️ Proposed helper
+// apps/wodsmith-start/src/lib/competitions/capabilities.ts +export function isScoringAlgorithmLocked(type: string): boolean { + return competitionCan(type, "benchmarkScoringTiers") +}- lockAlgorithmOnline={competitionCan( - competition.competitionType, - "benchmarkScoringTiers", - )} + lockAlgorithmOnline={isScoringAlgorithmLocked(competition.competitionType)}🤖 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/cohost/`$competitionId/scoring.tsx around lines 79 - 82, The lock-algorithm capability check is duplicated across scoring-related routes, so centralize it behind a shared helper instead of repeating competitionCan(competition.competitionType, "benchmarkScoringTiers"). Add a small function such as isScoringAlgorithmLocked(type) in capabilities.ts and use it from scoring.tsx, scoring/index.tsx, and settings.tsx so the gating logic lives in one place and is easy to update.apps/wodsmith-start/src/server/competition-leaderboard.ts (1)
263-279: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer a top-level type import over inline
import(...).
ScoringConfigis almost certainly already used/imported elsewhere in this file (e.g. viacalculateEventPoints,TiebreakerInput.scoringAlgorithm). Usingimport("@/types/scoring").ScoringConfiginline here is inconsistent with normal import style.✨ Proposed cleanup
+import type { ScoringConfig } from "`@/types/scoring`" ... export function resolveLeaderboardScoringConfig(params: { competitionType: string settings: CompetitionSettings | null | undefined -}): import("`@/types/scoring`").ScoringConfig { +}): ScoringConfig {🤖 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/competition-leaderboard.ts` around lines 263 - 279, The return type in resolveLeaderboardScoringConfig uses an inline import type, which is inconsistent with the rest of the file’s import style. Add a top-level ScoringConfig type import from "`@/types/scoring`" alongside the existing scoring-related imports, then update resolveLeaderboardScoringConfig to reference that imported type directly instead of import("`@/types/scoring`").ScoringConfig.apps/wodsmith-start/src/server-fns/cohost/cohost-competition-fns.ts (1)
408-423: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winForce-online save logic is correct; add unit coverage for this path.
The override (
algorithm: "online",customTable: undefined) correctly mirrorsresolveLeaderboardScoringConfig's read-time override and preservestiebreaker/statusHandling. However, onlycompetition-leaderboard-capability-gates.test.ts(read path) was added in this cohort — there's no test exercisingcohostUpdateScoringConfigFnforcing the algorithm on save for a benchmark competition, or leaving it untouched for non-benchmark types.Want me to draft a test for
cohostUpdateScoringConfigFncovering both branches?🤖 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/cohost/cohost-competition-fns.ts` around lines 408 - 423, Add unit coverage for the save-time override in cohostUpdateScoringConfigFn: verify that when competitionCan(..., "benchmarkScoringTiers") is true, the saved scoringConfig forces algorithm to "online" and clears customTable while preserving the other fields, and verify that non-benchmark competitions pass data.scoringConfig through unchanged. Use the existing resolveLeaderboardScoringConfig/read-path test behavior as the reference for the expected benchmark branch.
🤖 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.
Nitpick comments:
In `@apps/wodsmith-start/src/routes/compete/cohost/`$competitionId/scoring.tsx:
- Around line 79-82: The lock-algorithm capability check is duplicated across
scoring-related routes, so centralize it behind a shared helper instead of
repeating competitionCan(competition.competitionType, "benchmarkScoringTiers").
Add a small function such as isScoringAlgorithmLocked(type) in capabilities.ts
and use it from scoring.tsx, scoring/index.tsx, and settings.tsx so the gating
logic lives in one place and is easy to update.
In
`@apps/wodsmith-start/src/routes/compete/organizer/`$competitionId/-components/scoring-settings-form.tsx:
- Around line 30-43: `forceOnlineAlgorithm` is slightly inconsistent with
`updateCompetitionScoringConfigFn` because it returns the config unchanged when
`algorithm` is already "online", leaving any stale `customTable` intact. Update
`forceOnlineAlgorithm` in `scoring-settings-form.tsx` to always normalize the
config the same way the server does by clearing `customTable` whenever the
online algorithm is enforced, and keep the save-path coercion and
`handleAlgorithmChange` behavior aligned with that rule.
In `@apps/wodsmith-start/src/server-fns/cohost/cohost-competition-fns.ts`:
- Around line 408-423: Add unit coverage for the save-time override in
cohostUpdateScoringConfigFn: verify that when competitionCan(...,
"benchmarkScoringTiers") is true, the saved scoringConfig forces algorithm to
"online" and clears customTable while preserving the other fields, and verify
that non-benchmark competitions pass data.scoringConfig through unchanged. Use
the existing resolveLeaderboardScoringConfig/read-path test behavior as the
reference for the expected benchmark branch.
In `@apps/wodsmith-start/src/server/competition-leaderboard.ts`:
- Around line 263-279: The return type in resolveLeaderboardScoringConfig uses
an inline import type, which is inconsistent with the rest of the file’s import
style. Add a top-level ScoringConfig type import from "`@/types/scoring`"
alongside the existing scoring-related imports, then update
resolveLeaderboardScoringConfig to reference that imported type directly instead
of import("`@/types/scoring`").ScoringConfig.
In `@apps/wodsmith-start/test/scripts/benchmark-seed-data.test.ts`:
- Around line 139-176: Add the required inline // `@lat`: reference to this
benchmark seed-data test so it satisfies the test annotation rule. Update the
`it("scores vertical jump in plain inches and tops the L-sit at three minutes",
...)` block in `benchmark-seed-data.test.ts` and place the matching spec link
beside the test declaration or its leading comment, keeping the rest of the
assertions and helpers like `buildBenchmarkSeedRows` and `BENCHMARK_SEED_TESTS`
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 069c0960-5c71-4fab-b7e0-a1005ceec5a8
📒 Files selected for processing (36)
apps/wodsmith-start/scripts/seed/data/benchmark-training-guide.tsapps/wodsmith-start/src/components/benchmark-tiers/test-editor-dialogs.tsxapps/wodsmith-start/src/components/compete/scoring-config-form.tsxapps/wodsmith-start/src/components/competition-sidebar.tsxapps/wodsmith-start/src/components/events/event-details-form.tsxapps/wodsmith-start/src/components/online-competition-leaderboard-table.tsxapps/wodsmith-start/src/components/registration-sidebar.tsxapps/wodsmith-start/src/lib/competitions/capabilities.tsapps/wodsmith-start/src/routes/compete/$slug/stats.tsxapps/wodsmith-start/src/routes/compete/$slug/volunteer.tsxapps/wodsmith-start/src/routes/compete/cohost/$competitionId/scoring.tsxapps/wodsmith-start/src/routes/compete/organizer/$competitionId/-components/quick-actions-division-results.tsxapps/wodsmith-start/src/routes/compete/organizer/$competitionId/-components/scoring-settings-form.tsxapps/wodsmith-start/src/routes/compete/organizer/$competitionId/co-hosts.tsxapps/wodsmith-start/src/routes/compete/organizer/$competitionId/scoring/index.tsxapps/wodsmith-start/src/routes/compete/organizer/$competitionId/scoring/tiers.tsxapps/wodsmith-start/src/routes/compete/organizer/$competitionId/settings.tsxapps/wodsmith-start/src/routes/compete/organizer/$competitionId/volunteers/shifts.tsxapps/wodsmith-start/src/server-fns/cohost/cohost-competition-fns.tsapps/wodsmith-start/src/server-fns/competition-detail-fns.tsapps/wodsmith-start/src/server-fns/division-results-fns.tsapps/wodsmith-start/src/server/benchmark-scoring-tiers.tsapps/wodsmith-start/src/server/competition-leaderboard.tsapps/wodsmith-start/test/components/benchmark-test-editor-dialogs.test.tsxapps/wodsmith-start/test/components/competition-sidebar-capability-gates.test.tsapps/wodsmith-start/test/components/online-competition-leaderboard-table.test.tsxapps/wodsmith-start/test/components/registration-sidebar.test.tsxapps/wodsmith-start/test/components/scoring-config-form.test.tsxapps/wodsmith-start/test/lib/competitions/capabilities.test.tsapps/wodsmith-start/test/routes/compete/benchmark-stats-route.test.tsxapps/wodsmith-start/test/scripts/benchmark-seed-data.test.tsapps/wodsmith-start/test/server/benchmark-scoring-tiers.test.tsapps/wodsmith-start/test/server/competition-leaderboard-capability-gates.test.tslat.md/competition-type-capabilities.mdlat.md/domain.mdlat.md/organizer-dashboard.md
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
6 issues found across 36 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/components/benchmark-tiers/test-editor-dialogs.tsx">
<violation number="1" location="apps/wodsmith-start/src/components/benchmark-tiers/test-editor-dialogs.tsx:488">
P2: Time-based benchmark tests can still be saved with a non-time input unit: `handleSchemeChange` only sets `inputUnit` when the scheme changes, but the Input unit selector remains editable afterward. Consider enforcing `isTimeScheme(scheme) ? "time" : inputUnit` at save/update time or disabling/filtering the unit selector for time schemes.</violation>
</file>
<file name="apps/wodsmith-start/src/components/registration-sidebar.tsx">
<violation number="1" location="apps/wodsmith-start/src/components/registration-sidebar.tsx:395">
P2: Benchmark pages can still show volunteer dashboard/check-in UI for users with an existing volunteer team role because the new capability guard only wraps the non-volunteer signup card. Consider gating the `isVolunteer` dashboard/check-in block with the relevant volunteer/check-in capabilities too, so benchmark pages consistently hide volunteer surfaces.</violation>
</file>
<file name="apps/wodsmith-start/src/routes/compete/organizer/$competitionId/scoring/tiers.tsx">
<violation number="1" location="apps/wodsmith-start/src/routes/compete/organizer/$competitionId/scoring/tiers.tsx:350">
P2: The new event-link card only pre-fills scheme and score type, so adding tiers from a load/distance/calories event can still create the benchmark test with the default input unit (`reps`) and default category. Consider passing the event-derived category and input unit through `EventLinkOption` and applying them in `AddTestDialog` so the top-of-page workflow matches the event data it is linking.</violation>
</file>
<file name="apps/wodsmith-start/src/server/benchmark-scoring-tiers.ts">
<violation number="1" location="apps/wodsmith-start/src/server/benchmark-scoring-tiers.ts:539">
P2: Events whose workout has no explicit `scoreType` can prefill new benchmark tests with the dialog default `max`, even for time-based schemes that should use the scheme default. Consider normalizing nullable workout `scoreType` to the same default used by score submission before exposing it for event-derived prefill.</violation>
</file>
<file name="apps/wodsmith-start/src/components/compete/scoring-config-form.tsx">
<violation number="1" location="apps/wodsmith-start/src/components/compete/scoring-config-form.tsx:249">
P3: `ScoringConfigForm` now advertises `lockAlgorithmOnline` as hard-coding Online scoring, but inside this component it only hides the picker; a direct caller with a non-online `value.algorithm` would still show/edit the non-online preview and emit non-online config changes. Consider enforcing an effective online config in this component, or narrowing the prop name/contract to “hide algorithm picker.”</violation>
</file>
<file name="apps/wodsmith-start/src/routes/compete/$slug/volunteer.tsx">
<violation number="1" location="apps/wodsmith-start/src/routes/compete/$slug/volunteer.tsx:18">
P2: This loader-level redirect prevents navigation to the volunteer page for benchmark boards, but the underlying mutations (`submitVolunteerSignupFn`, `createAccountAndApplyAsVolunteerFn`) still accept a `competitionTeamId` and create volunteer applications without verifying the competition type supports public volunteer signup. A direct server-function call or stale client state could bypass this gate and create volunteer applications for benchmark competitions. Consider adding the same `competitionCan(competitionType, "publicVolunteerSignup")` check inside those mutations before creating the application.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| const handleSchemeChange = (value: string) => { | ||
| setScheme(value) | ||
| if (isTimeScheme(value)) { | ||
| setInputUnit("time") |
There was a problem hiding this comment.
P2: Time-based benchmark tests can still be saved with a non-time input unit: handleSchemeChange only sets inputUnit when the scheme changes, but the Input unit selector remains editable afterward. Consider enforcing isTimeScheme(scheme) ? "time" : inputUnit at save/update time or disabling/filtering the unit selector for time schemes.
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/benchmark-tiers/test-editor-dialogs.tsx, line 488:
<comment>Time-based benchmark tests can still be saved with a non-time input unit: `handleSchemeChange` only sets `inputUnit` when the scheme changes, but the Input unit selector remains editable afterward. Consider enforcing `isTimeScheme(scheme) ? "time" : inputUnit` at save/update time or disabling/filtering the unit selector for time schemes.</comment>
<file context>
@@ -462,10 +477,49 @@ export function AddTestDialog({
+ const handleSchemeChange = (value: string) => {
+ setScheme(value)
+ if (isTimeScheme(value)) {
+ setInputUnit("time")
+ } else {
+ setInputUnit((current) =>
</file context>
| {!isVolunteer && | ||
| competitionCan( | ||
| competition.competitionType, | ||
| "publicVolunteerSignup", |
There was a problem hiding this comment.
P2: Benchmark pages can still show volunteer dashboard/check-in UI for users with an existing volunteer team role because the new capability guard only wraps the non-volunteer signup card. Consider gating the isVolunteer dashboard/check-in block with the relevant volunteer/check-in capabilities too, so benchmark pages consistently hide volunteer surfaces.
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/registration-sidebar.tsx, line 395:
<comment>Benchmark pages can still show volunteer dashboard/check-in UI for users with an existing volunteer team role because the new capability guard only wraps the non-volunteer signup card. Consider gating the `isVolunteer` dashboard/check-in block with the relevant volunteer/check-in capabilities too, so benchmark pages consistently hide volunteer surfaces.</comment>
<file context>
@@ -309,96 +310,104 @@ export function RegistrationSidebar({
+ {!isVolunteer &&
+ competitionCan(
+ competition.competitionType,
+ "publicVolunteerSignup",
+ ) && (
+ <Card className="border-white/10 bg-white/5 backdrop-blur-md">
</file context>
| name: event.eventName, | ||
| linkedTestName: event.linkedTestName, | ||
| scheme: event.scheme, | ||
| scoreType: event.scoreType, |
There was a problem hiding this comment.
P2: The new event-link card only pre-fills scheme and score type, so adding tiers from a load/distance/calories event can still create the benchmark test with the default input unit (reps) and default category. Consider passing the event-derived category and input unit through EventLinkOption and applying them in AddTestDialog so the top-of-page workflow matches the event data it is linking.
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/compete/organizer/$competitionId/scoring/tiers.tsx, line 350:
<comment>The new event-link card only pre-fills scheme and score type, so adding tiers from a load/distance/calories event can still create the benchmark test with the default input unit (`reps`) and default category. Consider passing the event-derived category and input unit through `EventLinkOption` and applying them in `AddTestDialog` so the top-of-page workflow matches the event data it is linking.</comment>
<file context>
@@ -341,6 +342,16 @@ function BenchmarkScoringTiersEditor({
+ name: event.eventName,
+ linkedTestName: event.linkedTestName,
+ scheme: event.scheme,
+ scoreType: event.scoreType,
+ }))
+ const unlinkedEvents = summary.events.filter(
</file context>
| ? (testNameById.get(row.benchmarkTestId) ?? null) | ||
| : null, | ||
| scheme: row.scheme, | ||
| scoreType: row.scoreType, |
There was a problem hiding this comment.
P2: Events whose workout has no explicit scoreType can prefill new benchmark tests with the dialog default max, even for time-based schemes that should use the scheme default. Consider normalizing nullable workout scoreType to the same default used by score submission before exposing it for event-derived prefill.
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/benchmark-scoring-tiers.ts, line 539:
<comment>Events whose workout has no explicit `scoreType` can prefill new benchmark tests with the dialog default `max`, even for time-based schemes that should use the scheme default. Consider normalizing nullable workout `scoreType` to the same default used by score submission before exposing it for event-derived prefill.</comment>
<file context>
@@ -514,21 +512,32 @@ export async function loadBenchmarkEventTestOptions({
+ ? (testNameById.get(row.benchmarkTestId) ?? null)
+ : null,
+ scheme: row.scheme,
+ scoreType: row.scoreType,
+ }
}
</file context>
| } | ||
|
|
||
| // Public volunteer signup is capability-gated (benchmark boards exclude it) | ||
| if (!competitionCan(competition.competitionType, "publicVolunteerSignup")) { |
There was a problem hiding this comment.
P2: This loader-level redirect prevents navigation to the volunteer page for benchmark boards, but the underlying mutations (submitVolunteerSignupFn, createAccountAndApplyAsVolunteerFn) still accept a competitionTeamId and create volunteer applications without verifying the competition type supports public volunteer signup. A direct server-function call or stale client state could bypass this gate and create volunteer applications for benchmark competitions. Consider adding the same competitionCan(competitionType, "publicVolunteerSignup") check inside those mutations before creating the application.
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/compete/$slug/volunteer.tsx, line 18:
<comment>This loader-level redirect prevents navigation to the volunteer page for benchmark boards, but the underlying mutations (`submitVolunteerSignupFn`, `createAccountAndApplyAsVolunteerFn`) still accept a `competitionTeamId` and create volunteer applications without verifying the competition type supports public volunteer signup. A direct server-function call or stale client state could bypass this gate and create volunteer applications for benchmark competitions. Consider adding the same `competitionCan(competitionType, "publicVolunteerSignup")` check inside those mutations before creating the application.</comment>
<file context>
@@ -13,6 +14,11 @@ export const Route = createFileRoute("/compete/$slug/volunteer")({
}
+ // Public volunteer signup is capability-gated (benchmark boards exclude it)
+ if (!competitionCan(competition.competitionType, "publicVolunteerSignup")) {
+ throw redirect({ to: "/compete/$slug", params: { slug: params.slug } })
+ }
</file context>
| * Hard-codes the ranking algorithm to "online" (benchmark boards): hides | ||
| * the algorithm picker while keeping tiebreaker and DNF/DNS settings. | ||
| */ | ||
| lockAlgorithmOnline?: boolean |
There was a problem hiding this comment.
P3: ScoringConfigForm now advertises lockAlgorithmOnline as hard-coding Online scoring, but inside this component it only hides the picker; a direct caller with a non-online value.algorithm would still show/edit the non-online preview and emit non-online config changes. Consider enforcing an effective online config in this component, or narrowing the prop name/contract to “hide algorithm picker.”
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/scoring-config-form.tsx, line 249:
<comment>`ScoringConfigForm` now advertises `lockAlgorithmOnline` as hard-coding Online scoring, but inside this component it only hides the picker; a direct caller with a non-online `value.algorithm` would still show/edit the non-online preview and emit non-online config changes. Consider enforcing an effective online config in this component, or narrowing the prop name/contract to “hide algorithm picker.”</comment>
<file context>
@@ -242,6 +242,11 @@ interface ScoringConfigFormProps {
+ * Hard-codes the ranking algorithm to "online" (benchmark boards): hides
+ * the algorithm picker while keeping tiebreaker and DNF/DNS settings.
+ */
+ lockAlgorithmOnline?: boolean
}
</file context>
2 participants
Benchmark feature refinements from 7/7 meeting
Implements the action items from the Zac/Ian meeting reviewing the benchmark feature on this branch. Each item below lists what was decided, why it was addressed, the solution taken, and questions to consider while reviewing. Work was implemented by parallel subagents, then adversarially reviewed by two independent review agents; both HIGH-severity review findings were fixed in a follow-up round.
Action items addressed
1. Remove the "Tier context is live" card (organizer tiers page)
scoring/tiers.tsx(icon import cleaned up). The page now flows header → link-events → how-scoring-works → settings → categories → rating bands → thresholds.2. "Half a tier" → "half a point" + tier-count wording
maxTier(it rendered "10" because the training-guide battery has 10 tiers) — no change needed there. The wording is now "still earns half a point".3. "Link events to tier thresholds" section + derive test data from the event
scheme/scoreType). Category, score model, and thresholds remain for the organizer. All prefilled fields stay editable, and manual names are never clobbered. The per-category "Add event tiers" buttons remain as a secondary path (they get the same prefill for free). The event-edit page's inline dialog also passes the live scheme/scoreType now.benchmark_tests.timeCapMsexists but no editor flow writes it; follow-up?4. Time schemes auto-select the "time" input unit
time,time-with-cap, oremom(also time-entered on the athlete form) in the test editor auto-sets input unit "time"; leaving a time scheme reverts the unit (Add → default, Edit → the test's original) so a stale "time" unit can't corrupt threshold encoding. Choosing the hybrid score model (which forces time-with-cap) routes through the same logic.5. Bounded linked-event select
max-h-80) and scrolls. No search combobox — the meeting settled on bounded-with-scroll being fine.6. Benchmarks lose cohosts, volunteer shifts, and public volunteer signup
cohosts,volunteerShifts,publicVolunteerSignup) — true for in-person and online, false for benchmark, fail-closed for unknown types. Organizer sidebar hides Co-Hosts and Volunteer shifts for benchmarks (roster + registration questions stay); the co-hosts, volunteer-shifts, and public volunteer-signup routes redirect defensively; the "Sign up to volunteer" card is hidden on benchmark public pages. Truth-table, sidebar, and registration-sidebar tests extended; lat.md updated.inviteCohostFn, volunteer-shift fns,submitVolunteerSignupFn) are not capability-gated — the gate is UI + route loaders. Reachable only by scripted calls or a comp switched to benchmark after acquiring cohosts/shifts. Want server-side guards as a follow-up?7. Benchmark scoring hard-coded to online (tiebreakers kept)
onlineat init and on submit; the organizer and cohost save server fns force it server-side; and — per review finding —getCompetitionLeaderboardnow forces the effective algorithm to online for benchmark boards at read time, so a freshly created benchmark (which stores no scoringConfig and previously fell back to "traditional") ranks correctly without the organizer ever pressing Save. Tiebreaker and DNF/DNS settings remain fully editable.8. Leaderboard affiliate placement
9. Stats page athlete selector label
10. "Pre-published" → "Auto-publish"
resultsAutoPublishsettings key and identifiers are unchanged. lat.md updated.11. Seed data: vertical jump in inches
points+ inputUnitinches(the same pattern the seed already uses for "BikeErg Avg Watts"): thresholds and athlete entries are now plain integer inches in the same number space, verified end-to-end (parse → encode → tier comparison) by review.inputUnit(or adding an inches distance unit); worth a follow-up ticket. (b)inputUnitis stored as"inches"not"in"because both threshold encoders apply an inches→feet→mm conversion when the unit is exactly"in"— consequence: the Edit-test dialog's unit select shows empty for this test (harmless; unchanged fields aren't submitted).12. Seed data: L-sit hold top standard = 3:00
Miscellaneous action items identified but NOT acted on
maxTiermath, but a full organizer-flow E2E is a good follow-up.src/types/competitions.ts:65,src/server/competition-leaderboard.ts:249).Notes
/recall) was unreachable from this environment (worker fetch failed through the proxy), and the GitNexus MCP tools were not connected, so the mandatedgitnexus_impact/detect_changesruns could not be performed; blast radius was instead bounded by capability-gate tests, the full suite, and two independent review agents.pnpm type-check, Biome on all changed files, andlat checkall pass.🤖 Generated with Claude Code
https://claude.ai/code/session_01R5MgTsR5FNrj7Tvu7T6Ebv
Generated by Claude Code
mainSummary by cubic
Implements 7/7 benchmark meeting items: locks benchmark boards to online scoring, adds event-to-tier linking with smart prefills, gates benchmark-only capabilities, and makes copy/UI and seed fixes to improve clarity and data accuracy.
New Features
cohosts,volunteerShifts, andpublicVolunteerSignup; sidebar items hidden and routes redirect; volunteer roster and waivers remain.Refactors
Written for commit 45f6985. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes