feat: estimate iRating changes in practice - #689
Conversation
📝 WalkthroughWalkthroughThe standings widget adds an optional ChangesStandings iRating estimation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant StandingsSettings
participant StandingsWidgetConfig
participant useDriverStandings
StandingsSettings->>StandingsWidgetConfig: Update estimateInPractice
StandingsWidgetConfig->>useDriverStandings: Provide standings configuration
useDriverStandings->>useDriverStandings: Evaluate iRating eligibility
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/frontend/components/Settings/sections/StandingsSettings.tsx`:
- Around line 155-171: Give the ToggleSwitch in the “Estimate During Practice”
setting an accessible name by associating it with the visible label text via
aria-labelledby, or by passing an accessible-label prop if supported. Update
ToggleSwitch as needed so its rendered switch button receives that name while
preserving the existing toggle behavior.
In `@src/frontend/components/Standings/hooks/useDriverStandings.tsx`:
- Around line 36-45: The official-race branch of shouldCalculateIRatingChange
must require sessionType === 'Race' so qualifying sessions return false; update
src/frontend/components/Standings/hooks/useDriverStandings.tsx lines 36-45
accordingly. Add an assertion in
src/frontend/components/Standings/hooks/useDriverStandings.spec.ts lines 7-37
verifying ('Race', true, 'Open Qualify', true) returns false.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d1d092e4-e5ad-4b41-acbd-6475dcc94428
📒 Files selected for processing (7)
src/frontend/components/Settings/sections/StandingsSettings.tsxsrc/frontend/components/Settings/types.tssrc/frontend/components/Standings/hooks/useDriverStandings.spec.tssrc/frontend/components/Standings/hooks/useDriverStandings.tsxsrc/types/defaultDashboard.spec.tssrc/types/defaultDashboard.tssrc/types/widgetConfigs.ts
| <div className="flex items-center justify-between gap-3 pl-8 mt-2 indent-8"> | ||
| <span className="text-sm text-slate-300"> | ||
| Estimate During Practice | ||
| </span> | ||
| <ToggleSwitch | ||
| enabled={ | ||
| settings.config.iratingChange.estimateInPractice ?? false | ||
| } | ||
| onToggle={(enabled) => | ||
| handleConfigChange({ | ||
| iratingChange: { | ||
| ...settings.config.iratingChange, | ||
| estimateInPractice: enabled, | ||
| }, | ||
| }) | ||
| } | ||
| /> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Give the switch an accessible name.
ToggleSwitch renders an unnamed role="switch" button. The sibling span does not label that button. Associate the text with the control through aria-labelledby, or extend ToggleSwitch with an accessible-label prop and pass Estimate During Practice.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/frontend/components/Settings/sections/StandingsSettings.tsx` around lines
155 - 171, Give the ToggleSwitch in the “Estimate During Practice” setting an
accessible name by associating it with the visible label text via
aria-labelledby, or by passing an accessible-label prop if supported. Update
ToggleSwitch as needed so its rendered switch button receives that name while
preserving the existing toggle behavior.
| export const shouldCalculateIRatingChange = ( | ||
| eventType: string | undefined, | ||
| isOfficial: boolean, | ||
| sessionType: string | undefined, | ||
| estimateInPractice: boolean | ||
| ) => | ||
| (eventType === 'Race' && isOfficial) || | ||
| (eventType === 'Practice' && | ||
| sessionType === 'Practice' && | ||
| estimateInPractice); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Exclude qualifying sessions during official race weekends.
Line 42 returns true for ('Race', true, 'Open Qualify', false). This enables hypothetical iRating changes during qualifying. Require sessionType === 'Race' for the official-race branch.
src/frontend/components/Standings/hooks/useDriverStandings.tsx#L36-L45: require a race session in the official-race eligibility branch.src/frontend/components/Standings/hooks/useDriverStandings.spec.ts#L7-L37: add an assertion that('Race', true, 'Open Qualify', true)returnsfalse.
📍 Affects 2 files
src/frontend/components/Standings/hooks/useDriverStandings.tsx#L36-L45(this comment)src/frontend/components/Standings/hooks/useDriverStandings.spec.ts#L7-L37
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/frontend/components/Standings/hooks/useDriverStandings.tsx` around lines
36 - 45, The official-race branch of shouldCalculateIRatingChange must require
sessionType === 'Race' so qualifying sessions return false; update
src/frontend/components/Standings/hooks/useDriverStandings.tsx lines 36-45
accordingly. Add an assertion in
src/frontend/components/Standings/hooks/useDriverStandings.spec.ts lines 7-37
verifying ('Race', true, 'Open Qualify', true) returns false.
![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: c5ede9f4de
ℹ️ 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".
| export interface StandingsWidgetSettings extends BaseWidgetSettings { | ||
| config: { | ||
| iratingChange: { enabled: boolean }; | ||
| iratingChange: { enabled: boolean; estimateInPractice?: boolean }; |
There was a problem hiding this comment.
This adds a persisted field to the Standings settings shape, but StandingsWidgetSettings still has no version field. Architecture rule R8.1 explicitly requires every changed settings shape to carry a version; without one, this schema cannot participate in the required versioned migration path when later changes become breaking.
AGENTS.md reference: AGENTS.md:L12-L14
Useful? React with 👍 / 👎.
| const iratingAugmentedGroupedByClass = shouldCalculateIRatingChange( | ||
| eventType, | ||
| isOfficial, | ||
| sessionType, | ||
| settings?.iratingChange?.estimateInPractice ?? false |
There was a problem hiding this comment.
When practice estimation is enabled, telemetry-driven standings recomputations now invoke augmentStandingsWithIRating, whose calculation allocates an N-by-N chance matrix, but this newly enabled high-frequency path has no perfMetrics.measure wrapper. Architecture rule R13.1 requires that instrumentation so regressions from large practice fields appear in the performance overlay.
AGENTS.md reference: AGENTS.md:L12-L14
Useful? React with 👍 / 👎.
1 participant
Description
Adds an opt-in standings setting to show hypothetical iRating changes during online practice sessions. The existing iRating calculation is reused and remains unchanged for official race weekends.
The new Estimate During Practice toggle is nested under the existing iRating Change display option and defaults to off, preserving current behavior for new and saved dashboards. Offline testing and qualifying sessions are excluded.
This is consistent with the Phase 2 settings direction in
docs/ARCHITECTURE_REVIEW.md: the new field is additive and is supplied through the existing default-config deep merge, so no breaking settings migration is required.Validation:
npm run test -- --no-coverage src/frontend/components/Standings/hooks/useDriverStandings.spec.ts src/types/defaultDashboard.spec.ts(46 tests passed)npm run lint -- --no-fixgit diff --checkScreenshots
Before
The iRating Change display option has no practice-session estimate control, and standalone online practice sessions do not show hypothetical changes.
After
The iRating Change display option includes an Estimate During Practice toggle. When enabled, online practice standings display hypothetical iRating changes using the current running order.
Type of Change
Checklist
npm testnpm run lintand fixed any issuesArchitecture Pre-PR Checklist
src/appSummary by CodeRabbit
New Features
Bug Fixes