feat: move remaining widgets to channels - #670
Conversation
|
Warning Review limit reached
Next review available in: 43 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThe change adds typed session-bar, standings, track-state, and lap-log snapshot channels. It introduces demand-driven lap-log processing, migrates frontend consumers from raw telemetry, updates widget runtime definitions and Storybook fixtures, and extends replay validation. ChangesTelemetry snapshot processing
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant SDKBridge
participant LapLogRuntime
participant LapLogProcessor
participant ChannelStore
participant LapTimeLog
SDKBridge->>LapLogRuntime: forward telemetry and session events
LapLogRuntime->>LapLogProcessor: process active frames
LapLogProcessor-->>LapLogRuntime: return versioned LapLogSnapshot
LapLogRuntime->>ChannelStore: publish lap-log.snapshot
ChannelStore-->>LapTimeLog: provide useLapLogSnapshot data
Possibly related PRs
🚥 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: 3
🧹 Nitpick comments (3)
src/frontend/context/CarSpeedStore/TopSpeedStoreUpdater.tsx (1)
13-20: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winDepend on the snapshot fields, not the snapshot object.
The effect depends on
snapshot. The channel publishes a new snapshot object on each tick, so the effect runs at the channel rate.useTopSpeedStore.setStatereceives a new object each time, so every store subscriber re-renders even when the three values are unchanged.
TrackTemperatureStoreUpdater.tsx(line 12) already uses scalar dependencies. Apply the same pattern here.♻️ Proposed fix to use scalar dependencies
const snapshot = useSessionBarSnapshot(); + const lastLapTopSpeed = snapshot?.lastLapTopSpeed; + const sessionBestTopSpeed = snapshot?.sessionBestTopSpeed; + const sessionNum = snapshot?.sessionNum; useEffect(() => { - if (!enabled || !snapshot) return; + if (!enabled || lastLapTopSpeed === undefined) return; useTopSpeedStore.setState({ - lastLapTopSpeed: snapshot.lastLapTopSpeed, - sessionBestTopSpeed: snapshot.sessionBestTopSpeed, - sessionNum: snapshot.sessionNum, + lastLapTopSpeed, + sessionBestTopSpeed, + sessionNum, }); - }, [enabled, snapshot]); + }, [enabled, lastLapTopSpeed, sessionBestTopSpeed, sessionNum]);🤖 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/context/CarSpeedStore/TopSpeedStoreUpdater.tsx` around lines 13 - 20, Update the useEffect in the TopSpeedStoreUpdater component to depend on snapshot.lastLapTopSpeed, snapshot.sessionBestTopSpeed, and snapshot.sessionNum individually rather than the snapshot object. Preserve the existing enabled and snapshot guard and setState fields, following the scalar-dependency pattern used by TrackTemperatureStoreUpdater.src/frontend/components/Standings/hooks/useSessionLapCount.tsx (1)
3-13: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winType
EMPTYagainstSessionTimingSnapshot.
EMPTYomits required snapshot fields, souseSessionLapCount()returns a drift-prone union type. Usesatisfies SessionTimingSnapshotso missing or renamed fields fail at compile time.🤖 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/useSessionLapCount.tsx` around lines 3 - 13, Type the EMPTY fallback against SessionTimingSnapshot by applying satisfies SessionTimingSnapshot to its definition. Ensure all required snapshot fields are present and correctly named while preserving useSessionLapCount’s existing fallback behavior.src/frontend/components/Standings/hooks/useTrackTemperature.tsx (1)
4-7: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the repeated temperature-unit union.
UseTrackTemperatureOptionsdeclares the same inline union twice. Define oneTemperatureUnitalias and use it for both properties.Proposed refactor
+type TemperatureUnit = 'Metric' | 'Imperial'; + interface UseTrackTemperatureOptions { - airTempUnit?: 'Metric' | 'Imperial'; - trackTempUnit?: 'Metric' | 'Imperial'; + airTempUnit?: TemperatureUnit; + trackTempUnit?: TemperatureUnit; }As per coding guidelines, use type aliases for union types.
🤖 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/useTrackTemperature.tsx` around lines 4 - 7, Define a shared TemperatureUnit type alias for the 'Metric' | 'Imperial' union, then update both airTempUnit and trackTempUnit in UseTrackTemperatureOptions to use that alias.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/processors/lapLogRuntime.ts`:
- Around line 21-40: Require a SessionLifecycle in LapLogRuntime’s constructor
and remove the optional lifecycle branch, ensuring the existing onLifecycle
handlers for sessionNumChange and disconnect are always registered. Update all
LapLogRuntime construction sites to provide the lifecycle rather than allowing
undefined, preserving snapshot cleanup on disconnect.
In `@src/frontend/components/SectorDelta/widgetRuntimeDefinition.ts`:
- Around line 7-12: Update the SectorDelta runtime definition’s channel
configuration so track-state.snapshot uses its declared max/default processor
rate instead of inheriting ratePreset: 'static' with an undefined rate. Add the
channel-specific rate override supported by the widget runtime, while leaving
the other channels and static preset unchanged.
In `@src/frontend/components/Standings/hooks/useTrackTemperature.tsx`:
- Around line 13-19: Keep missing temperatures nullable instead of converting
them to zero: in src/frontend/components/Standings/hooks/useTrackTemperature.tsx
lines 13-19, check trackTempVal before applying any fallback; at lines 29-31,
apply the same treatment to airTempVal; and in
src/frontend/components/Weather/hooks/useTrackTemperature.tsx lines 13-15,
preserve both nullable values until formatting so unavailable temperatures
remain represented correctly.
---
Nitpick comments:
In `@src/frontend/components/Standings/hooks/useSessionLapCount.tsx`:
- Around line 3-13: Type the EMPTY fallback against SessionTimingSnapshot by
applying satisfies SessionTimingSnapshot to its definition. Ensure all required
snapshot fields are present and correctly named while preserving
useSessionLapCount’s existing fallback behavior.
In `@src/frontend/components/Standings/hooks/useTrackTemperature.tsx`:
- Around line 4-7: Define a shared TemperatureUnit type alias for the 'Metric' |
'Imperial' union, then update both airTempUnit and trackTempUnit in
UseTrackTemperatureOptions to use that alias.
In `@src/frontend/context/CarSpeedStore/TopSpeedStoreUpdater.tsx`:
- Around line 13-20: Update the useEffect in the TopSpeedStoreUpdater component
to depend on snapshot.lastLapTopSpeed, snapshot.sessionBestTopSpeed, and
snapshot.sessionNum individually rather than the snapshot object. Preserve the
existing enabled and snapshot guard and setState fields, following the
scalar-dependency pattern used by TrackTemperatureStoreUpdater.
🪄 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: 671130d6-c2fe-4a33-a759-aaec4d37580c
📒 Files selected for processing (65)
.storybook/index.ts.storybook/sessionBarSnapshot.tsdocs/IMPLEMENTATION_PLAN.mdsrc/app/bridge/iracingSdk/iracingSdkBridge.tssrc/app/bridge/iracingSdk/mock-data/mockSdkBridge.tssrc/app/processors/LapLogProcessor.spec.tssrc/app/processors/LapLogProcessor.tssrc/app/processors/SessionBarProcessor.tssrc/app/processors/StandingsProcessor.tssrc/app/processors/lapLogRuntime.spec.tssrc/app/processors/lapLogRuntime.tssrc/frontend/components/CornerNameOverlay/widgetRuntimeDefinition.tssrc/frontend/components/Flag/widgetRuntimeDefinition.tssrc/frontend/components/GarageCover/widgetRuntimeDefinition.tssrc/frontend/components/HeartRate/widgetRuntimeDefinition.tssrc/frontend/components/InformationBar/widgetRuntimeDefinition.tssrc/frontend/components/LapTimeLog/hooks/useLapTimeLog.tssrc/frontend/components/LapTimeLog/widgetRuntimeDefinition.tssrc/frontend/components/Relative/widgetRuntimeDefinition.tssrc/frontend/components/SectorDelta/SectorDelta.stories.tsxsrc/frontend/components/SectorDelta/SectorDelta.tsxsrc/frontend/components/SectorDelta/components/SectorProgressIndicator.tsxsrc/frontend/components/SectorDelta/hooks/useCarouselWindow.tssrc/frontend/components/SectorDelta/widgetRuntimeDefinition.tssrc/frontend/components/Standings/Relative.stories.tsxsrc/frontend/components/Standings/Standings.stories.tsxsrc/frontend/components/Standings/components/TitleBar/TitleBar.tsxsrc/frontend/components/Standings/hooks/useBrakeBias.tsxsrc/frontend/components/Standings/hooks/useDriverIncidents.tsxsrc/frontend/components/Standings/hooks/useDriverPositions.tsxsrc/frontend/components/Standings/hooks/usePrecipitation.tsxsrc/frontend/components/Standings/hooks/useSessionBestLapTime.tsxsrc/frontend/components/Standings/hooks/useSessionCurrentTime.tsxsrc/frontend/components/Standings/hooks/useSessionLapCount.tsxsrc/frontend/components/Standings/hooks/useTrackTemperature.tsxsrc/frontend/components/Standings/hooks/useTrackWetness.tsxsrc/frontend/components/Standings/widgetRuntimeDefinition.tssrc/frontend/components/TelemetryInspector/widgetRuntimeDefinition.tssrc/frontend/components/TwitchChat/widgetRuntimeDefinition.tssrc/frontend/components/Weather/Weather.stories.tsxsrc/frontend/components/Weather/Weather.tsxsrc/frontend/components/Weather/hooks/useTrackTemperature.tsxsrc/frontend/components/Weather/widgetRuntimeDefinition.tssrc/frontend/components/Wind/Wind.stories.tsxsrc/frontend/components/Wind/Wind.tsxsrc/frontend/components/Wind/widgetRuntimeDefinition.tssrc/frontend/context/BattleGapStore/BattleGapStoreUpdater.tsxsrc/frontend/context/CarSpeedStore/TopSpeedStoreUpdater.tsxsrc/frontend/context/ChannelStore/index.tssrc/frontend/context/ChannelStore/useLapLogSnapshot.tssrc/frontend/context/PitLapStore/PitLapStoreUpdater.tsxsrc/frontend/context/PushToPassStore/PushToPassStoreUpdater.tsxsrc/frontend/context/TrackTemperatureStore/TrackTemperatureStoreUpdater.spec.tsxsrc/frontend/context/TrackTemperatureStore/TrackTemperatureStoreUpdater.tsxsrc/frontend/context/shared/useThrottledWeather.tsxsrc/frontend/context/shared/useTotalRaceLaps.tsxsrc/frontend/context/shared/useTotalRaceTime.tsxsrc/frontend/context/shared/useTotalRaceValue.tsxsrc/frontend/widgetRuntime.spec.tsxsrc/types/channels/channel.tstest-data/telemetry/ai-race-10min.golden.jsontools/telemetry-replay/lap-log-probe.tstools/telemetry-replay/run-curated-validation.tstools/telemetry-replay/session-bar-probe.tstools/telemetry-replay/standings-probe.ts
| constructor( | ||
| private readonly bus: ChannelBus, | ||
| lifecycle: SessionLifecycle | undefined, | ||
| private readonly metrics: PerformanceSections | ||
| ) { | ||
| this.disconnects = [ | ||
| bus.onSubscriberCountChanged((channel, count) => { | ||
| if (channel !== 'lap-log.snapshot') return; | ||
| if (count > 0) this.activate(); | ||
| else this.deactivate(); | ||
| }), | ||
| ]; | ||
| if (lifecycle) { | ||
| this.disconnects.push( | ||
| lifecycle.onSessionNumChange(() => | ||
| this.onLifecycle({ type: 'sessionNumChange' }) | ||
| ), | ||
| lifecycle.onDisconnect(() => this.onLifecycle({ type: 'disconnect' })) | ||
| ); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect direct bridge call sites and confirm that lifecycle is always supplied
# whenever channelBus is supplied.
rg -n -C 5 --glob '*.{ts,tsx}' '\bpublishIRacingSDKEvents\s*\(' src toolsRepository: tariknz/irdashies
Length of output: 7751
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== LapLogRuntime =="
fd -a 'lapLogRuntime\.ts$' src . | sed 's#^\./##' | while read -r f; do
echo "--- $f"
wc -l "$f"
cat -n "$f"
done
echo "== publishIRacingSDKEvents implementations =="
for f in src/app/bridge/iracingSdk/iracingSdkBridge.ts src/app/bridge/iracingSdk/mock-data/mockSdkBridge.ts; do
echo "--- $f"
wc -l "$f"
rg -n -C 12 -A 20 -B 10 'new LapLogRuntime|publishIRacingSDKEvents|stop\(\)|channelBus|lifecycle' "$f"
done
echo "== setup lifecycle creation =="
sed -n '1,120p' src/app/bridge/iracingSdk/setup.ts | cat -nRepository: tariknz/irdashies
Length of output: 24974
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== ChannelBus snapshot storage and clear semantics =="
fd -a 'channelBridge\.ts$' src | sed 's#^\./##' | while read -r f; do
echo "--- $f"
wc -l "$f"
rg -n -C 10 'latestSnapshots|publish\(|clearSnapshot|subscriberCount|subscribe' "$f"
done
echo "== Mock lifecycle API =="
fd -a 'sessionLifecycle\.ts$' src | sed 's#^\./##' | while read -r f; do
echo "--- $f"
wc -l "$f"
sed -n '1,180p' "$f" | cat -n
doneRepository: tariknz/irdashies
Length of output: 14940
Require lifecycle delivery for an active LapLogRuntime.
LapLogRuntime accepts an optional SessionLifecycle, but snapshot cleanup depends on onDisconnect. The iRacing bridge creates LapLogRuntime whenever channelBus is present, and demo mode can pass undefined; when all lap-log.snapshot subscribers unsubscribe after a session without a lifecycle disconnect, ChannelBus.latestSnapshots preserves the last value and sends it to later subscribers. Require SessionLifecycle here, or wire a public disconnect reset from bridge shutdown paths.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/processors/lapLogRuntime.ts` around lines 21 - 40, Require a
SessionLifecycle in LapLogRuntime’s constructor and remove the optional
lifecycle branch, ensuring the existing onLifecycle handlers for
sessionNumChange and disconnect are always registered. Update all LapLogRuntime
construction sites to provide the lifecycle rather than allowing undefined,
preserving snapshot cleanup on disconnect.
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/CornerNameOverlay/CornerNameOverlay.stories.tsx`:
- Around line 24-29: Update the default story decorators in CornerNameOverlay
stories to compose TelemetryDecorator with ChannelSnapshotDecorator, ensuring
the default story provides session data required by useLovelyTrackData while
preserving the existing track-state snapshot.
In `@tools/telemetry-replay/track-state-probe.ts`:
- Line 42: Update createTrackStateProbe() so its schemaVersion is 2 to reflect
the added SessionFlags field, then regenerate or update
test-data/telemetry/ai-race-10min.golden.json so the curated checkpoint hash
matches the new probe output.
🪄 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: 80181fc6-d617-422f-ad2b-dce3c172722f
📒 Files selected for processing (16)
.storybook/trackStateSnapshot.tssrc/app/processors/TrackStateProcessor.spec.tssrc/app/processors/TrackStateProcessor.tssrc/frontend/components/CornerNameOverlay/CornerNameOverlay.stories.tsxsrc/frontend/components/CornerNameOverlay/hooks/useCurrentSection.tssrc/frontend/components/CornerNameOverlay/widgetRuntimeDefinition.tssrc/frontend/components/Flag/Flag.tsxsrc/frontend/components/Flag/widgetRuntimeDefinition.tssrc/frontend/components/GarageCover/GarageCover.tsxsrc/frontend/components/GarageCover/widgetRuntimeDefinition.tssrc/frontend/components/SectorDelta/hooks/useLiveSectorDelta.tssrc/frontend/components/Standings/Relative.tsxsrc/frontend/widgetRuntime.spec.tsxsrc/types/channels/channel.tstest-data/telemetry/ai-race-10min.golden.jsontools/telemetry-replay/track-state-probe.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- src/frontend/components/CornerNameOverlay/widgetRuntimeDefinition.ts
- src/frontend/components/Flag/widgetRuntimeDefinition.ts
- src/frontend/components/GarageCover/widgetRuntimeDefinition.ts
- src/frontend/widgetRuntime.spec.tsx
- src/types/channels/channel.ts
| ChannelSnapshotDecorator({ | ||
| 'track-state.snapshot': { | ||
| ...trackStateStorySnapshot, | ||
| lapDistPct: 0.5, | ||
| }, | ||
| }), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 8 \
'TelemetryDecorator|ChannelSnapshotDecorator|useLovelyTrackData' \
.storybook src/frontend/components/CornerNameOverlayRepository: tariknz/irdashies
Length of output: 16066
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '.storybook files:'
git ls-files .storybook | sort
printf '\n.storybook/index.ts\n'
cat -n .storybook/index.ts
printf '\nStory metadata excerpt:\n'
sed -n '1,65p' src/frontend/components/CornerNameOverlay/CornerNameOverlay.stories.tsx | cat -n
printf '\nHooks excerpt:\n'
sed -n '1,120p' src/frontend/components/CornerNameOverlay/CornerNameOverlay.tsx | cat -n
sed -n '1,140p' src/frontend/components/CornerNameOverlay/hooks/useLovelyTrackData.ts | cat -n
sed -n '110,165p' src/frontend/components/CornerNameOverlay/hooks/useLovelyTrackData.ts | cat -nRepository: tariknz/irdashies
Length of output: 6888
Compose TelemetryDecorator into the default story metadata.
The default decorators only inject a track-state.snapshot channel value. useLovelyTrackData still depends on useSessionStore(s => s.session?.WeekendInfo?.TrackName), so the default story will not load track sections unless a provider supplies session. Add a default-story TelemetryDecorator(...) or an appropriate Storybook global provider so the default story matches the component stories guideline and data flow.
🤖 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/CornerNameOverlay/CornerNameOverlay.stories.tsx`
around lines 24 - 29, Update the default story decorators in CornerNameOverlay
stories to compose TelemetryDecorator with ChannelSnapshotDecorator, ensuring
the default story provides session data required by useLovelyTrackData while
preserving the existing track-state snapshot.
Source: Coding guidelines
1 participant
Description
Completes the penultimate Phase 4 migration described in
docs/ARCHITECTURE_REVIEW.mdanddocs/IMPLEMENTATION_PLAN.md.lap-log.snapshotfor Lap Time Log's distinct full-precision inputs, wired through live, curated-tape, mock, IPC, and browser-source channel paths.TelemetryProvider.Validation:
npm run lintnpm run test -- --no-coverage— 1,215 passed, 1 skippednpm run test:replay:curated— 36,000 frames, 70 session revisions, 14 probesnpm run build-storybookgit diff --checkArchitecture pre-PR checklist:
src/appScreenshots
Before
Normal renderers containing any remaining unmigrated widget mounted
TelemetryProviderand received the legacy telemetry payload at the SDK loop rate.After
Normal widgets consume only their declared typed snapshots. Telemetry Inspector retains the explicit development/debug raw-stream path. No intentional visual changes.
Type of Change
Checklist
npm testnpm run lintand fixed any issuesSummary by CodeRabbit