-
-
Notifications
You must be signed in to change notification settings - Fork 46
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Better roll #1157
Better roll #1157
Conversation
WalkthroughThe updates enhance error handling, streamline event timing management, and improve type safety across the application. Key changes include robust logging for unhandled errors, renaming Changes
Sequence Diagram(s)sequenceDiagram
participant A as Client
participant B as Server
participant C as EventTimer
A->>B: Request event information
B->>C: Check current events
C->>B: Return current and next events
B->>A: Send event details and logging info
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
c03aeb2
to
32d5245
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Outside diff range, codebase verification and nitpick comments (3)
apps/server/src/services/TimerService.ts (1)
12-12
: Class renaming incomplete!The class
TimerService
has not been fully renamed toEventTimer
. The following occurrences ofTimerService
need to be updated:
apps/server/src/services/TimerService.ts
apps/server/src/services/runtime-service/RuntimeService.ts
Please ensure all references to
TimerService
are updated toEventTimer
.Analysis chain
Class renaming approved!
The class
TimerService
has been renamed toEventTimer
. Ensure that all references toTimerService
have been updated accordingly.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all references to `TimerService` have been updated to `EventTimer`. # Test: Search for the old class name. Expect: No occurrences of `TimerService`. rg --type python -A 5 $'TimerService'Length of output: 67
Script:
#!/bin/bash # Description: Verify all references to `TimerService` have been updated to `EventTimer`. # Test: Search for the old class name. Expect: No occurrences of `TimerService`. rg --type ts -A 5 $'TimerService'Length of output: 4816
apps/server/src/services/__tests__/rollUtils.test.ts (2)
118-131
: Update the test case description.The test case is well-structured and correctly validates the expected behavior of the
getRollTimers
function. However, the description should be updated to reflect the correct timer value (20 instead of 10).- it('should start the third event if timer is at 10', () => { + it('should start the third event if timer is at 20', () => {
163-176
: Update the test case description.The test case is well-structured and correctly validates the expected behavior of the
getRollTimers
function. However, the description should be updated to reflect the correct event (eighth instead of seventh).- it('should start the seventh event if timer is at 75', () => { + it('should start the eighth event if timer is at 75', () => {
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (5)
apps/server/tsconfig.json
is excluded by none and included by noneapps/spec/roll.md
is excluded by none and included by nonepackages/types/src/definitions/core/OntimeEvent.type.ts
is excluded by none and included by nonepackages/types/src/index.ts
is excluded by none and included by nonepackages/utils/index.ts
is excluded by none and included by none
Files selected for processing (10)
- apps/server/src/app.ts (2 hunks)
- apps/server/src/services/TimerService.ts (2 hunks)
- apps/server/src/services/tests/rollUtils.test.ts (1 hunks)
- apps/server/src/services/tests/timerUtils.test.ts (2 hunks)
- apps/server/src/services/rollUtils.ts (1 hunks)
- apps/server/src/services/runtime-service/RuntimeService.ts (7 hunks)
- apps/server/src/services/timerUtils.ts (2 hunks)
- apps/server/src/stores/runtimeState.ts (4 hunks)
- packages/utils/src/rundown-utils/rundownUtils.test.ts (2 hunks)
- packages/utils/src/rundown-utils/rundownUtils.ts (2 hunks)
Additional context used
GitHub Check: unit-test
apps/server/src/stores/runtimeState.ts
[failure] 11-11:
'millisToString' is defined but never used. Allowed unused vars must match /^_/u
[failure] 546-546:
'loadRoll:' is defined but never used
Biome
apps/server/src/stores/runtimeState.ts
[error] 526-526: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 526-526: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
[error] 546-546: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 546-546: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
Additional comments not posted (58)
apps/server/src/services/TimerService.ts (9)
29-32
: Constructor changes approved!The constructor initializes the
_refreshInterval
and sets an interval to call theupdate
method.
Line range hint
35-37
:
FunctionsetOnUpdateCallback
approved!The function allows setting a callback for when the timer updates.
Line range hint
39-46
:
Functionstart
approved!The function starts the timer and sets an end callback.
Line range hint
48-53
:
Functionpause
approved!The function pauses the timer and clears the end callback.
Line range hint
55-60
:
Functionstop
approved!The function stops the timer and clears the end callback.
Line range hint
62-74
:
FunctionaddTime
approved!The function adds time to the running timer and renews the end callback.
Line range hint
76-80
:
Functionupdate
approved!The function updates the app at regular intervals and calls the update callback.
Line range hint
82-85
:
Functionroll
approved!The function loads roll information into the timer service.
Line range hint
87-89
:
Functionshutdown
approved!The function clears the interval and end callback.
apps/server/src/services/rollUtils.ts (5)
11-36
: Initial part ofgetRollTimers
approved!The function checks if the rundown wraps around midnight and handles cases before or after the rundown.
38-46
: Variable initialization ingetRollTimers
approved!The function initializes variables for the current and next events.
48-113
: Iteration and logic ingetRollTimers
approved!The function iterates through the playable events to determine the current and next events.
115-122
: Return statement ingetRollTimers
approved!The function returns the calculated current and next events.
124-157
: FunctionloadEventToStart
approved!The function handles the case where there is no current event and prepares the first event of the rundown.
apps/server/src/services/timerUtils.ts (9)
Line range hint
6-7
:
FunctionnormaliseEndTime
approved!The function normalizes the end time of an event that spans over midnight.
Line range hint
9-27
:
FunctiongetExpectedFinish
approved!The function calculates the expected finish time of a running timer.
Line range hint
29-50
:
FunctiongetCurrent
approved!The function calculates the running countdown of the timer.
Line range hint
52-71
:
FunctionskippedOutOfEvent
approved!The function checks whether the timer has skipped out of the event.
Line range hint
73-97
:
FunctiongetRuntimeOffset
approved!The function calculates the difference between the runtime and the schedule of an event.
Line range hint
99-109
:
FunctiongetTotalDuration
approved!The function calculates the total duration of a time span.
Line range hint
111-116
:
FunctiongetExpectedEnd
approved!The function calculates the expected end of the rundown.
Line range hint
118-123
:
FunctionisPlaybackActive
approved!The function checks whether the playback is considered to be active.
Line range hint
125-145
:
FunctiongetTimerPhase
approved!The function checks the current phase of the timer.
apps/server/src/app.ts (3)
22-22
: Import statement forconsoleError
looks good.The import statement correctly includes
consoleError
from./utils/console.js
.
288-290
: Unhandled promise rejection logging looks good.The conditional check and logging functionality for unhandled promise rejections are correctly implemented.
297-299
: Uncaught exception logging looks good.The conditional check and logging functionality for uncaught exceptions are correctly implemented.
packages/utils/src/rundown-utils/rundownUtils.test.ts (1)
302-311
: Test case forfilterPlayable
function looks good.The test case correctly verifies that the
filterPlayable
function filters out non-playable events.packages/utils/src/rundown-utils/rundownUtils.ts (1)
356-359
: Modification tofilterPlayable
function looks good.The function correctly filters the rundown to include only playable events and the return type is updated to
PlayableEvent[]
.apps/server/src/services/__tests__/rollUtils.test.ts (18)
58-71
: LGTM!The test case
should roll to the day after if timer is at 100
is well-structured and correctly validates the expected behavior of thegetRollTimers
function.
73-86
: LGTM!The test case
should be waiting to start if timer is at 0
is well-structured and correctly validates the expected behavior of thegetRollTimers
function.
88-101
: LGTM!The test case
should start the first event if timer is at 5
is well-structured and correctly validates the expected behavior of thegetRollTimers
function.
103-116
: LGTM!The test case
should start the second event if timer is at 15
is well-structured and correctly validates the expected behavior of thegetRollTimers
function.
133-146
: LGTM!The test case
should start the fifth event if timer is at 49
is well-structured and correctly validates the expected behavior of thegetRollTimers
function.
148-161
: LGTM!The test case
should start the seventh event if timer is at 63
is well-structured and correctly validates the expected behavior of thegetRollTimers
function.
213-225
: LGTM!The test case
should load the event in the time span
is well-structured and correctly validates the expected behavior of thegetRollTimers
function.
229-250
: LGTM!The test case
should prepare first event, if we are not yet in the rundown start
is well-structured and correctly validates the expected behavior of thegetRollTimers
function.
252-273
: LGTM!The test case
should prepare first event, if we are over the rundown end
is well-structured and correctly validates the expected behavior of thegetRollTimers
function.
275-295
: LGTM!The test case
should account for a rundown that goes through midnight
is well-structured and correctly validates the expected behavior of thegetRollTimers
function.
297-317
: LGTM!The test case
loads upcoming event while waiting to roll
is well-structured and correctly validates the expected behavior of thegetRollTimers
function.
342-355
: LGTM!The test case
if timer is at 0
for overlapping times is well-structured and correctly validates the expected behavior of thegetRollTimers
function.
357-370
: LGTM!The test case
if timer is at 10
for overlapping times is well-structured and correctly validates the expected behavior of thegetRollTimers
function.
372-385
: LGTM!The test case
if timer is at 15
for overlapping times is well-structured and correctly validates the expected behavior of thegetRollTimers
function.
387-400
: LGTM!The test case
if timer is at 20
for overlapping times is well-structured and correctly validates the expected behavior of thegetRollTimers
function.
402-415
: LGTM!The test case
if timer is at 25
for overlapping times is well-structured and correctly validates the expected behavior of thegetRollTimers
function.
420-441
: LGTM!The test case
should recognise a playing event where its schedule spans over midnight
is well-structured and correctly validates the expected behavior of thegetRollTimers
function.
443-469
: LGTM!The test case
if the start time is the day after end time, and both are later than now
is well-structured and correctly validates the expected behavior of thegetRollTimers
function.apps/server/src/services/__tests__/timerUtils.test.ts (8)
Line range hint
6-84
:
LGTM! The test cases forgetExpectedFinish()
are comprehensive.The test cases cover various scenarios, including edge cases, ensuring the function's robustness.
Line range hint
85-192
:
LGTM! The test cases forgetCurrent()
are comprehensive.The test cases cover various scenarios, including edge cases, ensuring the function's robustness.
Line range hint
193-220
:
LGTM! The test cases forgetExpectedFinish() and getCurrentTime() combined
are comprehensive.The test cases cover various scenarios, including edge cases, ensuring the combined behavior is robust.
Line range hint
221-305
:
LGTM! The test cases forskippedOutOfEvent()
are comprehensive.The test cases cover various scenarios, including edge cases, ensuring the function's robustness.
Line range hint
306-327
:
LGTM! The test cases fornormaliseEndTime()
are comprehensive.The test cases cover various scenarios, including edge cases, ensuring the function's robustness.
Line range hint
328-460
:
LGTM! The test cases forgetRuntimeOffset()
are comprehensive.The test cases cover various scenarios, including edge cases, ensuring the function's robustness.
Line range hint
461-494
:
LGTM! The test cases forgetTotalDuration()
are comprehensive.The test cases cover various scenarios, including edge cases, ensuring the function's robustness.
Line range hint
495-578
:
LGTM! The test cases forgetTimerPhase()
are comprehensive.The test cases cover various scenarios, including edge cases, ensuring the function's robustness.
apps/server/src/stores/runtimeState.ts (1)
156-159
: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
loadNow
match the updated documentation.Verification successful
Verified: All function calls to
loadNow
match the updated documentation.The
loadNow
function is used exclusively in theapps/server/src/stores/runtimeState.ts
file, and all instances are consistent with the updated parameter documentation.
apps/server/src/stores/runtimeState.ts
: All occurrences ofloadNow
match the updated documentation.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `loadNow` match the updated documentation. # Test: Search for the function usage. Expect: Only occurances of the updated documentation. rg --type python -A 5 $'loadNow'Length of output: 62
Script:
#!/bin/bash # Description: Verify all function calls to `loadNow` match the updated documentation. # Test: Search for the function usage. Expect: Only occurrences of the updated documentation. rg --type typescript -A 5 'loadNow'Length of output: 70
Script:
#!/bin/bash # Description: Verify all function calls to `loadNow` match the updated documentation. # Test: Search for the function usage. Expect: Only occurrences of the updated documentation. rg --type ts -A 5 'loadNow'Length of output: 2044
Script:
#!/bin/bash # Description: Verify all function calls to `loadNow` match the updated documentation. # Test: Search for any other occurrences of the function usage in the entire codebase. rg 'loadNow'Length of output: 334
apps/server/src/services/runtime-service/RuntimeService.ts (3)
Line range hint
43-56
:
LGTM!The changes to replace
TimerService
withEventTimer
are approved.
Line range hint
66-147
:
LGTM!The changes to streamline the conditions under which the event reloads occur and to handle edge cases related to the roll are approved.
519-527
: LGTM!The changes to enhance error handling in the
roll
method are approved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Outside diff range, codebase verification and nitpick comments (1)
apps/server/src/services/TimerService.ts (1)
12-12
: Class Renaming Incomplete: Update all references toTimerService
.The class
TimerService
has been renamed toEventTimer
, but there is still a reference toTimerService
in the error message string inapps/server/src/services/TimerService.ts
on line 12. Please update this reference toEventTimer
to ensure consistency across the codebase.
apps/server/src/services/TimerService.ts
: Line 12Analysis chain
Class Renaming: Ensure consistency across the codebase.
The class
TimerService
has been renamed toEventTimer
. Ensure that all references toTimerService
in the codebase are updated toEventTimer
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all references to `TimerService` have been updated to `EventTimer`. # Test: Search for the old class name. Expect: No occurrences of `TimerService`. rg --type ts 'TimerService'Length of output: 1212
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
packages/types/src/index.ts
is excluded by none and included by nonepackages/types/src/utils/guards.ts
is excluded by none and included by none
Files selected for processing (5)
- apps/server/src/services/TimerService.ts (4 hunks)
- apps/server/src/services/tests/rollUtils.test.ts (1 hunks)
- apps/server/src/services/rollUtils.ts (1 hunks)
- apps/server/src/services/runtime-service/RuntimeService.ts (10 hunks)
- apps/server/src/stores/runtimeState.ts (11 hunks)
Files skipped from review as they are similar to previous changes (3)
- apps/server/src/services/tests/rollUtils.test.ts
- apps/server/src/services/rollUtils.ts
- apps/server/src/services/runtime-service/RuntimeService.ts
Additional context used
Biome
apps/server/src/stores/runtimeState.ts
[error] 412-412: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 412-412: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
[error] 536-536: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 536-536: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
Additional comments not posted (5)
apps/server/src/services/TimerService.ts (1)
29-32
: Constructor Update: Ensure correct usage of the new class name.The constructor has been updated to use
EventTimer._refreshInterval
instead ofTimerService._refreshInterval
. This change is consistent with the class renaming.apps/server/src/stores/runtimeState.ts (4)
158-174
: Parameter Type Update: Ensure consistency withPlayableEvent
.The function
load
has been updated to handlePlayableEvent
instead ofOntimeEvent
. Ensure that the function logic is consistent with the new parameter type and that all references toOntimeEvent
are updated accordingly.
Line range hint
200-243
:
Parameter Type Update: Ensure consistency withPlayableEvent
andMaybeNumber
.The function
loadNow
has been updated to handlePlayableEvent
andMaybeNumber
foreventIndex
. Ensure that the function logic is consistent with the new parameter types and that all references to the old types are updated accordingly.
248-275
: Parameter Type Update: Ensure consistency withPlayableEvent
andMaybeNumber
.The function
loadNext
has been updated to handlePlayableEvent
andMaybeNumber
foreventIndex
. Ensure that the function logic is consistent with the new parameter types and that all references to the old types are updated accordingly.
509-574
: New Function: Ensure robust handling of playback state transitions.The function
roll
manages the playback state during transitions between events. Ensure that the function handles all edge cases and updates the runtime state correctly.Tools
Biome
[error] 536-536: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 536-536: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (1)
apps/server/src/services/TimerService.ts (1)
Line range hint
62-78
:
Remove Unused Label: DEVThe label
DEV
is not used by any break or continue statement and should be removed.- DEV: { - if (state.timer.expectedFinish === null) { - throw new Error('TimerService.addTime: expectedFinish is negative'); - } - }
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
packages/types/src/index.ts
is excluded by none and included by nonepackages/types/src/utils/guards.ts
is excluded by none and included by none
Files selected for processing (5)
- apps/server/src/services/TimerService.ts (4 hunks)
- apps/server/src/services/tests/rollUtils.test.ts (1 hunks)
- apps/server/src/services/rollUtils.ts (1 hunks)
- apps/server/src/services/runtime-service/RuntimeService.ts (12 hunks)
- apps/server/src/stores/runtimeState.ts (11 hunks)
Files skipped from review as they are similar to previous changes (2)
- apps/server/src/services/tests/rollUtils.test.ts
- apps/server/src/services/rollUtils.ts
Additional context used
Biome
apps/server/src/stores/runtimeState.ts
[error] 412-412: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 412-412: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
[error] 536-536: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 536-536: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
Additional comments not posted (27)
apps/server/src/services/TimerService.ts (9)
12-12
: Class Renaming: TimerService to EventTimerThe class
TimerService
has been renamed toEventTimer
. This change aligns with the new focus on event-driven timer mechanisms.
29-32
: Constructor Update: Ensure Consistent Usage of EventTimerThe constructor now sets the
_refreshInterval
usingEventTimer._refreshInterval
. Ensure that all references to the old class nameTimerService
have been updated toEventTimer
.
35-37
: LGTM: setOnUpdateCallback MethodThe
setOnUpdateCallback
method correctly assigns the callback function for timer updates.
Line range hint
39-46
:
LGTM: start MethodThe
start
method correctly interacts with theruntimeState
to start the timer and sets a timeout for the end callback.
Line range hint
48-53
:
LGTM: pause MethodThe
pause
method correctly interacts with theruntimeState
to pause the timer and clears the timeout for the end callback.
Line range hint
55-60
:
LGTM: stop MethodThe
stop
method correctly interacts with theruntimeState
to stop the timer and clears the timeout for the end callback.
Line range hint
80-85
:
LGTM: update MethodThe
update
method correctly interacts with theruntimeState
to update the timer and calls the update callback if set.
Line range hint
87-89
:
LGTM: roll MethodThe
roll
method correctly interacts with theruntimeState
to load roll information.
Line range hint
91-94
:
LGTM: shutdown MethodThe
shutdown
method correctly clears the interval and timeout to shut down the timer.apps/server/src/stores/runtimeState.ts (6)
158-182
: LGTM: load FunctionThe
load
function correctly filters playable events and updates theruntimeState
with the current and next events.
Line range hint
200-243
:
LGTM: loadNow FunctionThe
loadNow
function correctly updates theruntimeState
with the current event and its public counterpart.
248-273
: LGTM: loadNext FunctionThe
loadNext
function correctly updates theruntimeState
with the next event and its public counterpart.
281-283
: LGTM: resume FunctionThe
resume
function correctly loads the event and rundown from the restore point.
Line range hint
289-321
:
LGTM: reload FunctionThe
reload
function correctly reloads the current event and updates theruntimeState
.
509-574
: LGTM: roll FunctionThe
roll
function correctly manages the playback state and updates theruntimeState
with the next event.Tools
Biome
[error] 536-536: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 536-536: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
apps/server/src/services/runtime-service/RuntimeService.ts (12)
23-23
: Import Update: EventTimerThe import statement has been updated to import
EventTimer
instead ofTimerService
.
Line range hint
44-57
:
Constructor Update: Ensure Consistent Usage of EventTimerThe constructor now initializes
eventTimer
usingEventTimer
. Ensure that all references to the old class nameTimerService
have been updated toEventTimer
.
Line range hint
68-136
:
LGTM: checkTimerUpdate MethodThe
checkTimerUpdate
method correctly handles edge cases and updates theruntimeState
and integrations as needed.
Line range hint
148-155
:
LGTM: init MethodThe
init
method correctly sets the update callback and initializes the runtime service.
Line range hint
157-161
:
LGTM: shutdown MethodThe
shutdown
method correctly shuts down theeventTimer
.
Line range hint
221-258
:
LGTM: maybeUpdate MethodThe
maybeUpdate
method correctly handles changes to the runtime state and reloads events as needed.
Line range hint
269-278
:
LGTM: loadEvent MethodThe
loadEvent
method correctly loads events using theruntimeState
and updates the integrations.
Line range hint
280-292
:
LGTM: startById MethodThe
startById
method correctly starts an event by its ID using theruntimeState
.
Line range hint
294-304
:
LGTM: startByIndex MethodThe
startByIndex
method correctly starts an event by its index using theruntimeState
.
Line range hint
306-316
:
LGTM: startByCue MethodThe
startByCue
method correctly starts an event by its cue using theruntimeState
.
Line range hint
318-327
:
LGTM: loadById MethodThe
loadById
method correctly loads an event by its ID using theruntimeState
.
Line range hint
329-338
:
LGTM: loadByIndex MethodThe
loadByIndex
method correctly loads an event by its index using theruntimeState
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
packages/types/src/index.ts
is excluded by none and included by nonepackages/types/src/utils/guards.ts
is excluded by none and included by none
Files selected for processing (6)
- apps/server/src/services/TimerService.ts (4 hunks)
- apps/server/src/services/tests/rollUtils.test.ts (1 hunks)
- apps/server/src/services/rollUtils.ts (1 hunks)
- apps/server/src/services/runtime-service/RuntimeService.ts (12 hunks)
- apps/server/src/services/timerUtils.ts (3 hunks)
- apps/server/src/stores/runtimeState.ts (11 hunks)
Files skipped from review as they are similar to previous changes (2)
- apps/server/src/services/tests/rollUtils.test.ts
- apps/server/src/services/rollUtils.ts
Additional context used
Biome
apps/server/src/stores/runtimeState.ts
[error] 412-412: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 412-412: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
[error] 536-536: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 536-536: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
Additional comments not posted (18)
apps/server/src/services/TimerService.ts (2)
12-12
: Class Renaming Approved.The renaming of the class from
TimerService
toEventTimer
is consistent and clear.
29-32
: Verify the usage of the new class name in the codebase.Ensure that the new class name
EventTimer
is correctly referenced throughout the codebase.Verification successful
The new class name
EventTimer
is correctly referenced throughout the codebase.The following locations confirm the proper usage of
EventTimer
:
apps/server/src/services/TimerService.ts
apps/server/src/services/runtime-service/RuntimeService.ts
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all references to the new class name `EventTimer`. # Test: Search for the class usage. Expect: Only occurrences of the new class name. rg --type python -A 5 $'EventTimer'Length of output: 65
Script:
#!/bin/bash # Description: Verify all references to the new class name `EventTimer`. # Test: Search for the class usage. Expect: Only occurrences of the new class name. rg --type typescript -A 5 'EventTimer'Length of output: 73
Script:
#!/bin/bash # Description: Verify all references to the new class name `EventTimer`. # Test: Search for the class usage. Expect: Only occurrences of the new class name. rg -A 5 'EventTimer'Length of output: 3242
apps/server/src/services/timerUtils.ts (6)
Line range hint
6-6
:
Function Implementation Approved.The
normaliseEndTime
function is correctly implemented.
Line range hint
12-28
:
Function Implementation Approved.The
getExpectedFinish
function is correctly implemented.
Line range hint
33-48
:
Function Implementation Approved.The
getCurrent
function is correctly implemented.
Line range hint
96-109
:
Function Implementation Approved.The
skippedOutOfEvent
function is correctly implemented.
Line range hint
114-144
:
Function Implementation Approved.The
getRuntimeOffset
function is correctly implemented.
Line range hint
149-158
:
Function Implementation Approved.The
getTotalDuration
function is correctly implemented.apps/server/src/stores/runtimeState.ts (4)
25-25
: Import Statement Update Approved.The import statement update to include
loadRoll
is correctly implemented.
Line range hint
158-195
:
Function Update Approved.The function
load
update to handlePlayableEvent
andinitialData
is correctly implemented.
Line range hint
200-243
:
Function Update Approved.The function
loadNow
update to handlePlayableEvent
andeventIndex
is correctly implemented.
508-574
: Function Implementation Approved.The
roll
function is correctly implemented to govern the playback state during transitions between events.Tools
Biome
[error] 536-536: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 536-536: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
apps/server/src/services/runtime-service/RuntimeService.ts (6)
23-23
: Import statement forEventTimer
looks good.The import statement for
EventTimer
is correct and necessary.
56-57
: Constructor method update looks good.The constructor method correctly initializes the
eventTimer
property withEventTimer
.
Line range hint
88-97
: Modifications tocheckTimerUpdate
method look good.The changes to the control flow and the use of
eventTimer
are correct and necessary.
517-525
: Enhanced error handling inroll
method looks good.The try-catch block correctly enhances error handling and improves robustness.
549-549
: Modifications toresume
method look good.The changes to the use of
eventTimer
are correct and necessary.
570-570
: Initialization ofeventTimer
looks good.The
eventTimer
initialization correctly usesEventTimer
instead ofTimerService
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- apps/server/src/services/TimerService.ts (4 hunks)
- apps/server/src/services/runtime-service/RuntimeService.ts (12 hunks)
- apps/server/src/stores/runtimeState.ts (12 hunks)
Additional context used
Biome
apps/server/src/stores/runtimeState.ts
[error] 422-422: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 422-422: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
[error] 546-546: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 546-546: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
Additional comments not posted (12)
apps/server/src/services/TimerService.ts (3)
12-12
: Class name change is appropriate.The class name change from
TimerService
toEventTimer
is appropriate and ensures consistency.
29-32
: Static property reference updated correctly.The static property
_refreshInterval
is now accessed usingEventTimer
, ensuring consistency with the new class name.
107-107
: Integration withruntimeState
is appropriate.The
roll
function correctly integrates with theruntimeState
module by callingruntimeState.roll(rundown)
.apps/server/src/stores/runtimeState.ts (5)
4-6
: New imports are appropriate.The imports
MaybeNumber
andPlayableEvent
are necessary for the new functionalities introduced in the file.
25-25
: New import is appropriate.The import
loadRoll
fromrollUtils.js
is necessary for theroll
function implementation.
93-103
: State immutability is ensured.The
getState
function now creates a shallow copy of the state, ensuring immutability.
168-192
: Function parameter types updated correctly.The
load
function now acceptsPlayableEvent
andMaybeNumber
types, enhancing flexibility and accuracy.
518-584
:roll
function implementation is robust.The
roll
function effectively manages the playback state during transitions between events, updating theruntimeState
accordingly.Tools
Biome
[error] 546-546: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 546-546: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
apps/server/src/services/runtime-service/RuntimeService.ts (4)
23-23
: New import is appropriate.The import
EventTimer
fromTimerService.js
is necessary for the new timer management functionalities.
44-44
: Property type updated correctly.The
eventTimer
property type is updated toEventTimer
, reflecting the new timer management approach.
56-57
: Constructor parameter updated correctly.The constructor now accepts an
EventTimer
parameter, aligning with the new timer management approach.
508-533
: Error handling enhanced.The
roll
function now includes error handling with a try-catch block, enhancing robustness.
@@ -233,30 +226,31 @@ class RuntimeService { | |||
// 3. the edited event replaces next event | |||
let isNext = false; | |||
|
|||
// TODO: review logic |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reminder: Review TODO comment.
The maybeUpdate
function contains a TODO comment indicating that the logic needs to be reviewed.
Do you want me to help review the logic or open a GitHub issue to track this task?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Outside diff range, codebase verification and nitpick comments (1)
apps/server/src/services/runtime-service/RuntimeService.ts (1)
23-23
: Update the error message inEventTimer.ts
to reflect the new terminology.The presence of
TimerService
in the error message inEventTimer.ts
appears to be a leftover from the refactoring process. Please update it toEventTimer
.
- File:
apps/server/src/services/EventTimer.ts
- Line: 23
- Current:
throw new Error('TimerService.addTime: expectedFinish is negative');
- Suggested:
throw new Error('EventTimer.addTime: expectedFinish is negative');
Analysis chain
LGTM! But verify the usage of
EventTimer
in the codebase.The changes to replace
TimerService
withEventTimer
are approved.However, ensure that all instances of
TimerService
have been correctly replaced withEventTimer
.Also applies to: 44-44, 56-57
To ensure the replacement of
TimerService
withEventTimer
is complete, we need to investigate the context of the remaining instances inEventTimer.ts
,AuxTimerService.ts
, andintegration.controller.ts
.
To ensure the replacement of
TimerService
withEventTimer
is complete, we need to investigate ifTimerService
is defined or imported inEventTimer.ts
and whether it is used elsewhere in the file.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all instances of `TimerService` have been correctly replaced with `EventTimer`. # Test: Search for any remaining instances of `TimerService`. Expect: No occurrences. rg --type ts 'TimerService'Length of output: 1102
Script:
#!/bin/bash # Description: Investigate the context of `TimerService` in `EventTimer.ts`, `AuxTimerService.ts`, and `integration.controller.ts`. # Search for the context around `TimerService` in `EventTimer.ts` rg --type ts 'TimerService' -A 5 -B 5 apps/server/src/services/EventTimer.ts # Search for the context around `TimerService` in `AuxTimerService.ts` rg --type ts 'TimerService' -A 5 -B 5 apps/server/src/services/aux-timer-service/AuxTimerService.ts # Search for the context around `TimerService` in `integration.controller.ts` rg --type ts 'TimerService' -A 5 -B 5 apps/server/src/api-integration/integration.controller.tsLength of output: 3025
Script:
#!/bin/bash # Description: Investigate the definition or import of `TimerService` in `EventTimer.ts` and verify its usage. # Search for the definition or import of `TimerService` in `EventTimer.ts` rg --type ts 'TimerService' -A 0 -B 0 apps/server/src/services/EventTimer.ts # Search for the usage of `TimerService` in `EventTimer.ts` rg --type ts 'TimerService' apps/server/src/services/EventTimer.tsLength of output: 297
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (5)
apps/server/tsconfig.json
is excluded by none and included by nonepackages/types/src/definitions/core/OntimeEvent.type.ts
is excluded by none and included by nonepackages/types/src/index.ts
is excluded by none and included by nonepackages/types/src/utils/guards.ts
is excluded by none and included by nonepackages/utils/index.ts
is excluded by none and included by none
Files selected for processing (8)
- apps/server/src/services/EventTimer.ts (4 hunks)
- apps/server/src/services/tests/rollUtils.test.ts (1 hunks)
- apps/server/src/services/rollUtils.ts (1 hunks)
- apps/server/src/services/runtime-service/RuntimeService.ts (12 hunks)
- apps/server/src/services/timerUtils.ts (3 hunks)
- apps/server/src/stores/runtimeState.ts (12 hunks)
- packages/utils/src/rundown-utils/rundownUtils.test.ts (2 hunks)
- packages/utils/src/rundown-utils/rundownUtils.ts (2 hunks)
Files skipped from review as they are similar to previous changes (4)
- apps/server/src/services/tests/rollUtils.test.ts
- apps/server/src/services/rollUtils.ts
- packages/utils/src/rundown-utils/rundownUtils.test.ts
- packages/utils/src/rundown-utils/rundownUtils.ts
Additional context used
Biome
apps/server/src/stores/runtimeState.ts
[error] 422-422: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 422-422: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
[error] 546-546: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 546-546: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
Additional comments not posted (30)
apps/server/src/services/EventTimer.ts (6)
29-32
: Constructor looks good.The constructor correctly initializes the
_refreshInterval
and sets up the periodic update.
Line range hint
71-82
: MethodaddTime
looks good.The method correctly handles adding time to the timer and updating the
endCallback
.
Line range hint
86-91
: Methodupdate
looks good.The method correctly updates the timer state and calls the
onUpdateCallback
.
36-38
: MethodsetOnUpdateCallback
looks good.The method correctly sets the callback for timer updates.
Line range hint
113-115
: Methodshutdown
looks good.The method correctly clears the interval and timeout.
107-107
: LGTM! But verify the method usage in the codebase.The change to return the result of
runtimeState.roll(rundown)
is appropriate.However, ensure that all calls to the
roll
method handle the returned value correctly.apps/server/src/services/timerUtils.ts (8)
Line range hint
6-6
: FunctionnormaliseEndTime
looks good.The function correctly normalizes the end time for events spanning over midnight.
Line range hint
12-32
: FunctiongetExpectedFinish
looks good.The function correctly calculates the expected finish time of a running timer.
Line range hint
34-57
: FunctiongetCurrent
looks good.The function correctly calculates the running countdown for the timer.
Line range hint
96-111
: FunctionskippedOutOfEvent
looks good.The function correctly checks if the timer has skipped out of the event.
Line range hint
113-136
: FunctiongetRuntimeOffset
looks good.The function correctly calculates the difference between the runtime and the schedule of an event.
Line range hint
138-148
: FunctiongetTotalDuration
looks good.The function correctly calculates the total duration of a time span.
Line range hint
150-158
: FunctiongetExpectedEnd
looks good.The function correctly calculates the expected end of the rundown.
Line range hint
160-166
: FunctionisPlaybackActive
looks good.The function correctly checks if the playback is active.
apps/server/src/stores/runtimeState.ts (9)
Line range hint
168-204
: Functionload
looks good.The function correctly loads the event into the state with the updated parameter types.
Line range hint
210-253
: FunctionloadNow
looks good.The function correctly loads the current event and its public counterpart with the updated parameter types.
258-285
: FunctionloadNext
looks good.The function correctly loads the next event and its public counterpart with the updated parameter types.
291-293
: Functionresume
looks good.The function correctly resumes from the restore point with the provided parameters.
Line range hint
299-334
: Functionreload
looks good.The function correctly reloads the current event with the optional event parameter.
339-343
: FunctionreloadAll
looks good.The function correctly reloads all events without interrupting the timer.
Line range hint
345-383
: Functionstart
looks good.The function correctly starts the timer.
Line range hint
385-392
: Functionpause
looks good.The function correctly pauses the timer.
Line range hint
394-401
: Functionstop
looks good.The function correctly stops the timer.
apps/server/src/services/runtime-service/RuntimeService.ts (7)
229-229
: Reminder: Review TODO comment.The
maybeUpdate
function contains a TODO comment indicating that the logic needs to be reviewed.
264-264
: LGTM!The changes to the
loadEvent
function are approved.
508-541
: LGTM!The changes to the
roll
function, including error handling and logging, are approved.
88-94
: Verify consistency with theroll
method.Ensure that the changes to the
roll
method are consistent with its usage incheckTimerUpdate
.Verification successful
Verify the implementation and usage of the
roll
method inRuntimeService.ts
.Ensure that the
roll
method implementation inRuntimeService.ts
is consistent with its usage incheckTimerUpdate
.
The usage of the
roll
method incheckTimerUpdate
is consistent with its implementation.The
roll
method inRuntimeService.ts
is correctly utilized in thecheckTimerUpdate
method, with theskipCheck
parameter set totrue
, bypassing the initial validation and proceeding with the rolling logic.
- File:
apps/server/src/services/runtime-service/RuntimeService.ts
- Method:
roll
- Usage:
checkTimerUpdate
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the consistency of the `roll` method usage in `checkTimerUpdate`. # Test: Search for the `roll` method usage in the codebase. Expect: Consistent usage. rg --type ts 'roll'Length of output: 18702
Script:
#!/bin/bash # Extract the implementation of the `roll` method in `RuntimeService.ts` ast-grep --lang ts --pattern $'class RuntimeService { $$$ roll($_) { $$$ } $$$ }' apps/server/src/services/runtime-service/RuntimeService.tsLength of output: 49451
231-243
: Ensure correct handling of event reloading.Review the logic for reloading events to ensure it is correct and handles all edge cases.
Also applies to: 252-253
Verification successful
Event reloading logic is correctly implemented.
The
maybeUpdate
method inRuntimeService.ts
correctly handles event reloading by checking various conditions and reloading events as necessary. ThenotifyChanges
function inRundownService.ts
ensures thatmaybeUpdate
is called with the appropriateaffected
IDs, covering the required edge cases.
RuntimeService.ts
:
maybeUpdate(affectedIds?: string[]) { ... }
RundownService.ts
:
runtimeService.maybeUpdate(affected);
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the logic for reloading events in `maybeUpdate`. # Test: Search for the `maybeUpdate` method usage in the codebase. Expect: Correct handling of event reloading. rg --type ts 'maybeUpdate'Length of output: 228
Script:
#!/bin/bash # Description: Gather the definition of the `maybeUpdate` method in `RuntimeService.ts` and its usage in `RundownService.ts`. # Get the definition of `maybeUpdate` in `RuntimeService.ts` rg --type ts -A 20 'maybeUpdate' apps/server/src/services/runtime-service/RuntimeService.ts # Get the usage of `maybeUpdate` in `RundownService.ts` rg --type ts -A 10 'runtimeService.maybeUpdate' apps/server/src/services/rundown-service/RundownService.tsLength of output: 1350
Script:
#!/bin/bash # Description: Gather more context around the usage of `maybeUpdate` in `RundownService.ts`, specifically the definition and initialization of `affected`. # Get more lines around the usage of `maybeUpdate` in `RundownService.ts` rg --type ts -C 10 'runtimeService.maybeUpdate' apps/server/src/services/rundown-service/RundownService.tsLength of output: 738
563-563
: Verify consistency with theroll
method.Ensure that the changes to the
roll
method are consistent with its usage inresume
.Verification successful
Verified consistency with the
roll
method.The
resume
method inRuntimeService.ts
correctly calls theroll
method without parameters, which aligns with theroll
method's implementation.
resume
method callsroll
whenplayback
isPlayback.Roll
.roll
method has a default parameterskipCheck
set tofalse
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the consistency of the `roll` method usage in `resume`. # Test: Search for the `resume` method usage in the codebase. Expect: Consistent usage. rg --type ts 'resume'Length of output: 581
Script:
#!/bin/bash # Description: Verify the consistency of the `roll` method usage in `resume`. # Locate the `roll` method in the `RuntimeService.ts` file. ast-grep --lang typescript --pattern 'roll($_, $_) { $$$ }' --context 10Length of output: 517
Script:
#!/bin/bash # Description: Verify the consistency of the `roll` method usage in `resume`. # Locate the `roll` method in the `RuntimeService.ts` file. rg 'roll' apps/server/src/services/runtime-service/RuntimeService.ts -A 10Length of output: 2272
Script:
#!/bin/bash # Description: Verify the consistency of the `roll` method usage in `resume`. # Locate the `resume` method in the `RuntimeService.ts` file. rg 'resume' apps/server/src/services/runtime-service/RuntimeService.ts -A 10Length of output: 999
404-405
: Verify consistency withEventTimer
.Ensure that the changes to
EventTimer
are consistent with its usage instart
.Also applies to: 411-413
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Outside diff range, codebase verification and nitpick comments (1)
apps/server/src/services/runtime-service/RuntimeService.ts (1)
23-23
: Issues found with the update toEventTimer
.The following instances of
TimerService
were found in the codebase, indicating that not all references have been updated toEventTimer
:
apps/server/src/api-integration/integration.controller.ts
- Lines: 1, 3, 5, 7, 9, 11
apps/server/src/services/EventTimer.ts
- Line: 1
apps/server/src/services/aux-timer-service/AuxTimerService.ts
- Lines: 1, 3
Please update these instances to ensure consistent usage of
EventTimer
.Analysis chain
LGTM! Ensure consistent usage of
EventTimer
.The changes to use
EventTimer
instead ofTimerService
are approved.However, ensure that all instances of
TimerService
have been updated toEventTimer
throughout the codebase.Also applies to: 44-44, 56-57
Let's re-run the shell script without specifying the file type to search for
TimerService
across all files.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all instances of TimerService have been updated to EventTimer. # Test: Search for the usage of TimerService. Expect: No occurrences of TimerService. rg --type typescript 'TimerService'Length of output: 70
Script:
#!/bin/bash # Description: Verify all instances of TimerService have been updated to EventTimer. # Test: Search for the usage of TimerService. Expect: No occurrences of TimerService. rg 'TimerService'Length of output: 1092
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
packages/types/src/index.ts
is excluded by none and included by nonepackages/types/src/utils/guards.ts
is excluded by none and included by none
Files selected for processing (6)
- apps/server/src/services/EventTimer.ts (4 hunks)
- apps/server/src/services/tests/rollUtils.test.ts (1 hunks)
- apps/server/src/services/rollUtils.ts (1 hunks)
- apps/server/src/services/runtime-service/RuntimeService.ts (12 hunks)
- apps/server/src/services/timerUtils.ts (3 hunks)
- apps/server/src/stores/runtimeState.ts (12 hunks)
Files skipped from review as they are similar to previous changes (2)
- apps/server/src/services/tests/rollUtils.test.ts
- apps/server/src/services/rollUtils.ts
Additional context used
Biome
apps/server/src/stores/runtimeState.ts
[error] 422-422: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 422-422: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
[error] 513-513: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 513-513: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
[error] 554-554: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 554-554: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
Additional comments not posted (19)
apps/server/src/services/EventTimer.ts (4)
29-32
: Verify interval clearing.Ensure that the interval set in the constructor is cleared when the timer is stopped to prevent potential memory leaks.
107-107
: Verify return type usage.Ensure that the return type of the
roll
method aligns with the expected usage in the codebase.
Line range hint
62-75
:
Verify edge case handling for added time.Ensure that the
addTime
method correctly handles all edge cases, such as negative amounts and timer overflow.
Line range hint
77-80
:
Verify update callback handling.Ensure that the update callback in the
update
method is correctly handled and does not introduce any side effects.apps/server/src/services/timerUtils.ts (8)
Line range hint
11-30
:
Verify calculation accuracy.Ensure that the calculations in the
getExpectedFinish
function are accurate and handle all edge cases, such as events spanning over midnight.
Line range hint
32-52
:
Verify calculation accuracy.Ensure that the calculations in the
getCurrent
function are accurate and handle all edge cases, such as events spanning over midnight.
Line range hint
96-109
:
Verify logic accuracy.Ensure that the logic in the
skippedOutOfEvent
function is accurate and handles all edge cases, such as events spanning over midnight.
Line range hint
111-139
:
Verify calculation accuracy.Ensure that the calculations in the
getRuntimeOffset
function are accurate and handle all edge cases, such as events spanning over midnight.
Line range hint
141-153
:
Verify calculation accuracy.Ensure that the calculations in the
getTotalDuration
function are accurate and handle all edge cases, such as events spanning over midnight.
Line range hint
155-163
:
Verify calculation accuracy.Ensure that the calculations in the
getExpectedEnd
function are accurate and handle all edge cases, such as events spanning over midnight.
Line range hint
165-170
:
Verify logic accuracy.Ensure that the logic in the
isPlaybackActive
function is accurate and handles all edge cases.
Line range hint
172-194
:
Verify logic accuracy.Ensure that the logic in the
getTimerPhase
function is accurate and handles all edge cases.apps/server/src/stores/runtimeState.ts (4)
168-192
: Verify filtering logic accuracy.Ensure that the filtering logic in the
load
function is accurate and handles all edge cases.
Line range hint
210-253
:
Verify loading logic accuracy.Ensure that the loading logic in the
loadNow
function is accurate and handles all edge cases.
258-285
: Verify loading logic accuracy.Ensure that the loading logic in the
loadNext
function is accurate and handles all edge cases.
527-591
: Verify transition logic accuracy.Ensure that the transition logic in the
roll
function is accurate and handles all edge cases.Tools
Biome
[error] 554-554: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 554-554: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
apps/server/src/services/runtime-service/RuntimeService.ts (3)
508-541
: LGTM! Verify the error handling and logging.The changes to include a try-catch block for better error handling in the
roll
function are approved.However, ensure that the error handling and logging are correct and comprehensive.
Verification successful
Error handling and logging in the
roll
method are correct and comprehensive.The
roll
method inRuntimeService
includes a try-catch block that ensures any errors during execution are caught and logged appropriately usinglogger.error
.
apps/server/src/services/runtime-service/RuntimeService.ts
: Lines 508-541Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the error handling and logging in the `roll` method. # Test: Search for the `roll` method and its error handling. Expect: Correct and comprehensive error handling and logging. rg --type typescript -A 10 'roll'Length of output: 68
Script:
#!/bin/bash # Description: Verify the error handling and logging in the `roll` method. # Test: Search for the `roll` method and its error handling. Expect: Correct and comprehensive error handling and logging. rg --type ts -A 10 'roll'Length of output: 99857
88-94
: LGTM! Verify the conditions forroll
method call.The changes to the
roll
method call are approved.However, ensure that the conditions for calling the
roll
method are correct and comprehensive.Verification successful
The conditions for calling the
roll
method are correct and comprehensive.
- The first call to
this.roll(true)
is within a conditional block that ensures it is called only whenshouldCallRoll
orcheckNeedsEvent
is true.- The second call to
this.roll()
is within theresume
method and is called whenplayback
isPlayback.Roll
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the conditions for calling the `roll` method. # Test: Search for the `roll` method calls and their conditions. Expect: Correct and comprehensive conditions. rg --type typescript -A 5 'this.roll'Length of output: 72
Script:
#!/bin/bash # Description: Verify the conditions for calling the `roll` method. # Test: Search for the `roll` method calls and their conditions. Expect: Correct and comprehensive conditions. rg -A 5 'this.roll'Length of output: 1001
Script:
#!/bin/bash # Description: Verify the conditions for calling the `roll` method. # Test: Fetch more lines around `this.roll` calls to understand the conditions. rg -A 10 -B 10 'this.roll'Length of output: 3835
563-563
: LGTM! Verify the conditions forroll
method call.The changes to the
roll
method call in theresume
function are approved.However, ensure that the conditions for calling the
roll
method are correct and comprehensive.Verification successful
The conditions for calling the
roll
method in theresume
function are correct and comprehensive.
- The
resume
function checks theplayback
state from therestorePoint
and callsroll
if it isPlayback.Roll
.- It verifies if the
selectedEventId
corresponds to a validOntimeEvent
andPlayableEvent
before proceeding with the resume logic.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the conditions for calling the `roll` method in the `resume` function. # Test: Search for the `roll` method calls in the `resume` function and their conditions. Expect: Correct and comprehensive conditions. rg --type typescript -A 5 'resume'Length of output: 69
Script:
#!/bin/bash # Searching for the `resume` function in TypeScript files and displaying its content to verify the conditions for calling the `roll` method. ast-grep --lang typescript --pattern 'function resume($_) { $$$ }'Length of output: 68
Script:
#!/bin/bash # Searching for the `resume` method within the `RuntimeService` class and displaying its content to verify the conditions for calling the `roll` method. ast-grep --lang typescript --pattern 'class RuntimeService { $$$ resume($_) { $$$ } $$$ }'Length of output: 49390
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (5)
apps/server/tsconfig.json
is excluded by none and included by noneapps/spec/roll.md
is excluded by none and included by nonepackages/types/src/definitions/core/OntimeEvent.type.ts
is excluded by none and included by nonepackages/types/src/index.ts
is excluded by none and included by nonepackages/types/src/utils/guards.ts
is excluded by none and included by none
Files selected for processing (11)
- apps/server/src/services/EventTimer.ts (4 hunks)
- apps/server/src/services/tests/rollUtils.test.ts (1 hunks)
- apps/server/src/services/rollUtils.ts (1 hunks)
- apps/server/src/services/rundown-service/RundownService.ts (2 hunks)
- apps/server/src/services/rundown-service/rundownUtils.ts (3 hunks)
- apps/server/src/services/runtime-service/RuntimeService.ts (13 hunks)
- apps/server/src/services/timerUtils.ts (3 hunks)
- apps/server/src/stores/tests/runtimeState.test.ts (3 hunks)
- apps/server/src/stores/runtimeState.ts (13 hunks)
- packages/utils/src/rundown-utils/rundownUtils.test.ts (2 hunks)
- packages/utils/src/rundown-utils/rundownUtils.ts (17 hunks)
Files skipped from review as they are similar to previous changes (3)
- apps/server/src/services/EventTimer.ts
- apps/server/src/services/rollUtils.ts
- packages/utils/src/rundown-utils/rundownUtils.test.ts
Additional context used
GitHub Check: unit-test
apps/server/src/services/__tests__/rollUtils.test.ts
[failure] 67-67: src/services/tests/rollUtils.test.ts > loadRoll() > should roll to the day after if timer is at 100
AssertionError: expected { event: null, index: null } to strictly equal { event: { id: '1', …(3) }, …(2) }
- Expected
Received
Object {
- "event": Object {
"id": "1",
"isPublic": false,
"timeEnd": 10,
"timeStart": 5,
- },
- "index": 0,
- "isPending": true,
- "event": null,
- "index": null,
}❯ src/services/tests/rollUtils.test.ts:67:19
[failure] 79-79: src/services/tests/rollUtils.test.ts > loadRoll() > should be waiting to start if timer is at 0
AssertionError: expected { event: null, index: null } to strictly equal { event: { id: '1', …(3) }, …(2) }
- Expected
Received
Object {
- "event": Object {
"id": "1",
"isPublic": false,
"timeEnd": 10,
"timeStart": 5,
- },
- "index": 0,
- "isPending": true,
- "event": null,
- "index": null,
}❯ src/services/tests/rollUtils.test.ts:79:19
[failure] 90-90: src/services/tests/rollUtils.test.ts > loadRoll() > should start the first event if timer is at 5
AssertionError: expected { event: null, index: null } to strictly equal { event: { id: '1', …(3) }, index: +0 }
- Expected
Received
Object {
- "event": Object {
"id": "1",
"isPublic": false,
"timeEnd": 10,
"timeStart": 5,
- },
- "index": 0,
- "event": null,
- "index": null,
}❯ src/services/tests/rollUtils.test.ts:90:19
Biome
apps/server/src/stores/runtimeState.ts
[error] 431-431: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 431-431: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
[error] 488-488: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 488-488: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
[error] 525-525: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 525-525: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
[error] 566-566: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 566-566: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
Additional comments not posted (41)
apps/server/src/services/rundown-service/rundownUtils.ts (6)
1-2
: LGTM! Import statements updated correctly.The import statements have been updated to include
PlayableEvent
and utility functionsfilterPlayable
andfilterTimedEvents
, which are necessary for the new functionality.
24-24
: LGTM! Improved readability and modularity.The
getTimedEvents
function now uses thefilterTimedEvents
utility, which improves readability and modularity by abstracting the filtering logic.
30-31
: LGTM! Enhanced type safety and clarity.The
getPlayableEvents
function now uses thefilterPlayable
utility, which clarifies the intent of the function and enhances type safety.
54-59
: LGTM! Consistent with new filtering strategy.The
getNextEventWithCue
function now operates on theplayableEvents
array, aligning with the new filtering strategy and correctly filtering events based on the cue.
69-81
: LGTM! Consistent with new filtering strategy.The
findPrevious
function now operates on theplayableEvents
array, ensuring consistency with the new filtering strategy and correctly finding the previous event based on the current event ID.
88-101
: LGTM! Improved type specificity and consistency.The
findNext
function now operates on theplayableEvents
array and returnsPlayableEvent | null
, ensuring consistency with the new filtering strategy and improving type specificity.apps/server/src/stores/__tests__/runtimeState.test.ts (3)
1-1
: LGTM! Import statement updated correctly.The import statement has been updated to replace
OntimeEvent
withPlayableEvent
, aligning with the updated event type used in the tests.
14-15
: LGTM! Enhanced event management.The mock event object has been updated to include a new property,
skip
, set tofalse
, allowing for conditional handling based on theskip
property.
148-148
: LGTM! Ensured accurate state retrieval.The
getState()
function is now called after thestop()
function within the test case, ensuring that the state reflects the most current changes post-event manipulation.apps/server/src/services/timerUtils.ts (1)
96-100
: LGTM! Improved logic for handling skipped events.The
skippedOutOfEvent
function has been updated with new logic for handling skipped events, considering various conditions such as crossing midnight and time differences.apps/server/src/services/rundown-service/RundownService.ts (2)
23-23
: Import statement change looks good.The addition of
getTimedEvents
is necessary for the updated functionality in theupdateRuntimeOnChange
function.
218-219
: LGTM! But verify the functionality.The change to use
getTimedEvents()
instead ofgetPlayableEvents()
is approved.However, ensure that this change aligns with the intended functionality.
packages/utils/src/rundown-utils/rundownUtils.ts (7)
1-9
: Import statement change looks good.The import statement changes are necessary for the updated functionality in the file.
16-16
: Function signature change looks good.The change to accept
OntimeRundown
as a parameter improves type safety and clarity.
34-40
: Function logic and return type change looks good.The change to return
PlayableEvent | null
and include anisPlayableEvent
check ensures that only playable events are returned.
Line range hint
82-91
: Function logic and return type change looks good.The change to return
PlayableEvent | null
and include anisPlayableEvent
check ensures that only playable events are returned.
126-126
: Function signature change looks good.The change to accept
OntimeRundown
as a parameter improves type safety and clarity.
201-201
: Function signature change looks good.The change to accept
OntimeRundown
as a parameter improves type safety and clarity.
322-323
: Function return type change looks good.The change to return
PlayableEvent[]
improves semantic clarity, ensuring that the function's name and documentation accurately reflect its behavior.apps/server/src/services/__tests__/rollUtils.test.ts (7)
93-102
: Test case looks good:should start the second event if timer is at 15
.The test case is correctly testing the functionality of the
loadRoll
function.
104-113
: Test case looks good:should start the third event if timer is at 20
.The test case is correctly testing the functionality of the
loadRoll
function.
115-124
: Test case looks good:should start the fifth event if timer is at 49
.The test case is correctly testing the functionality of the
loadRoll
function.
126-135
: Test case looks good:should start the seventh event if timer is at 63
.The test case is correctly testing the functionality of the
loadRoll
function.
137-146
: Test case looks good:should start the eighth event if timer is at 75
.The test case is correctly testing the functionality of the
loadRoll
function.
150-191
: Test case looks good:should find an event that crosses midnight
.The test case is correctly testing the functionality of the
loadRoll
function.
193-227
: Test case looks good:should not skip to the second day
.The test case is correctly testing the functionality of the
loadRoll
function.apps/server/src/stores/runtimeState.ts (8)
539-608
: Ensure correct handling of playback state transitions inroll
function.Verify that the
roll
function correctly handles playback state transitions and updates the state.Verification successful
The
roll
function correctly handles playback state transitions and updates the state.The function effectively manages the playback state and ensures the state is updated as expected.
runtimeState.timer.playback
transitions are handled properly.runtimeState.eventNow
andruntimeState.eventNext
are updated correctly.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the handling of playback state transitions in the `roll` function. # Test: Search for the playback state transition logic. Expect: The function should correctly handle playback state transitions and update the state. rg --type js --pattern 'runtimeState.timer.playback|runtimeState.eventNow|runtimeState.eventNext' apps/server/src/stores/runtimeState.tsLength of output: 550
Script:
#!/bin/bash # Description: Verify the handling of playback state transitions in the `roll` function. # Test: Search for the playback state transition logic. Expect: The function should correctly handle playback state transitions and update the state. rg 'runtimeState.timer.playback|runtimeState.eventNow|runtimeState.eventNext' apps/server/src/stores/runtimeState.tsLength of output: 2522
Tools
Biome
[error] 566-566: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 566-566: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
44-44
: InitializeinitialTimer
properties correctly.Ensure that all properties in the
initialTimer
object are correctly initialized and used within the file.Verification successful
Initialize
initialTimer
properties correctly.All properties in the
initialTimer
object are correctly initialized and used within theapps/server/src/stores/runtimeState.ts
file. No further action is required.
addedTime
current
duration
elapsed
expectedFinish
finishedAt
phase
playback
secondaryTimer
startedAt
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the initialization and usage of `initialTimer` properties. # Test: Search for the usage of each `initialTimer` property. Expect: Each property should be used at least once. rg --type js --pattern 'addedTime|current|duration|elapsed|expectedFinish|finishedAt|phase|playback|secondaryTimer|startedAt' apps/server/src/stores/runtimeState.tsLength of output: 578
Script:
#!/bin/bash # Description: Verify the initialization and usage of `initialTimer` properties. # Test: Search for the usage of each `initialTimer` property. Expect: Each property should be used at least once. rg 'addedTime|current|duration|elapsed|expectedFinish|finishedAt|phase|playback|secondaryTimer|startedAt' apps/server/src/stores/runtimeState.tsLength of output: 5563
94-104
: Ensure shallow copy includes all necessary properties.Verify that the shallow copy created by the
getState
function includes all necessary properties of the state.Verification successful
Let's correct the command and verify the properties included in the shallow copy created by the
getState
function.
Verified: The shallow copy includes all necessary properties.
The
getState
function correctly creates a shallow copy of theruntimeState
object, including all necessary properties:eventNow
,eventNext
,publicEventNow
,publicEventNext
,runtime
,timer
, and_timer
.
eventNow
eventNext
publicEventNow
publicEventNext
runtime
timer
_timer
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the shallow copy created by the `getState` function includes all necessary properties. # Test: Search for the properties included in the shallow copy. Expect: All necessary properties should be included. rg --type js --pattern 'eventNow|eventNext|publicEventNow|publicEventNext|runtime|timer|_timer' apps/server/src/stores/runtimeState.tsLength of output: 548
Script:
#!/bin/bash # Description: Verify that the shallow copy created by the `getState` function includes all necessary properties. # Correct usage of rg to search for the properties in the getState function. rg 'eventNow|eventNext|publicEventNow|publicEventNext|runtime|timer|_timer' apps/server/src/stores/runtimeState.tsLength of output: 10901
3-5
: Ensure all imported modules are used.Verify that all imported modules are necessary and utilized within the file to avoid unused imports.
Also applies to: 8-8, 14-14, 27-27
Verification successful
All imported modules are used.
The verification confirms that all imported modules in
apps/server/src/stores/runtimeState.ts
are utilized within the file.
isPlayableEvent
MaybeNumber
MaybeString
PlayableEvent
calculateDuration
dayInMs
filterTimedEvents
getRelevantBlock
loadRoll
normaliseRollStart
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all imported modules are used in the file. # Test: Search for the usage of each imported module. Expect: Each module should be used at least once. rg --type js --pattern 'isPlayableEvent|MaybeNumber|MaybeString|PlayableEvent|calculateDuration|dayInMs|filterTimedEvents|getRelevantBlock|loadRoll|normaliseRollStart' apps/server/src/stores/runtimeState.tsLength of output: 620
Script:
#!/bin/bash # Description: Verify that all imported modules are used in the file. # Test: Search for the usage of each imported module. Expect: Each module should be used at least once. rg 'isPlayableEvent|MaybeNumber|MaybeString|PlayableEvent|calculateDuration|dayInMs|filterTimedEvents|getRelevantBlock|loadRoll|normaliseRollStart' apps/server/src/stores/runtimeState.tsLength of output: 2558
262-292
: Ensure correct handling of next event loading inloadNext
function.Verify that the
loadNext
function correctly handles the loading of the next event and updates the state.Verification successful
The
loadNext
function correctly handles the loading of the next event and updates the state.The function sets
runtimeState.eventNext
andruntimeState.publicEventNext
tonull
when necessary, assigns the next event to these properties, and includes conditions to check if both properties are set.
- Instances where
runtimeState.eventNext
andruntimeState.publicEventNext
are updated:
runtimeState.eventNext = null;
runtimeState.publicEventNext = null;
runtimeState.eventNext = event;
runtimeState.publicEventNext = event;
- Conditions checking if
runtimeState.eventNext
andruntimeState.publicEventNext
arenull
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the handling of next event loading in the `loadNext` function. # Test: Search for the next event loading logic. Expect: The function should correctly handle the loading of the next event and update the state. rg --type js --pattern 'runtimeState.eventNext|runtimeState.publicEventNext' apps/server/src/stores/runtimeState.tsLength of output: 529
Script:
#!/bin/bash # Description: Verify the handling of next event loading in the `loadNext` function. # Test: Search for the next event loading logic. Expect: The function should correctly handle the loading of the next event and update the state. rg 'runtimeState\.eventNext|runtimeState\.publicEventNext' apps/server/src/stores/runtimeState.tsLength of output: 630
Line range hint
211-252
: Ensure correct handling of event loading inloadNow
function.Verify that the
loadNow
function correctly handles the loading of events and updates the state.Verification successful
Ensure correct handling of event loading in
loadNow
function.The
loadNow
function correctly handles the loading of events and updates the state variablesruntimeState.eventNow
,runtimeState.publicEventNow
, andruntimeState.currentBlock
. The function resets the state wheneventIndex
is null, updates the state with the current event and block, and iterates backward to find a public event if the current event is not public.
runtimeState.eventNow
is updated with the current event.runtimeState.publicEventNow
is set to the current event if it is public, otherwise, it iterates backward to find a public event.runtimeState.currentBlock
is updated with the relevant block and retains thestartedAt
time if the block remains the same.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the handling of event loading in the `loadNow` function. # Test: Search for the event loading logic. Expect: The function should correctly handle the loading of events and update the state. rg --type js --pattern 'runtimeState.eventNow|runtimeState.publicEventNow|runtimeState.currentBlock' apps/server/src/stores/runtimeState.tsLength of output: 553
Script:
#!/bin/bash # Description: Verify the handling of event loading in the `loadNow` function. # Test: Search for the event loading logic. Expect: The function should correctly handle the loading of events and update the state. rg 'runtimeState.eventNow|runtimeState.publicEventNow|runtimeState.currentBlock' apps/server/src/stores/runtimeState.tsLength of output: 2570
Line range hint
308-340
: Ensure correct handling of hot reloading inreload
function.Verify that the
reload
function correctly handles the hot reloading of events and updates the state.Verification successful
Line range hint
308-340
: Ensure correct handling of hot reloading inreload
function.The
reload
function correctly handles hot reloading of events and updates the state as expected. ThereloadAll
function ensures that timed events are loaded before invokingreload
, which updates the state accordingly.
reload
function correctly updatesruntimeState
during hot reloading.reloadAll
function ensures proper invocation ofreload
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the handling of hot reloading in the `reload` function. # Test: Search for the hot reloading logic. Expect: The function should correctly handle the hot reloading of events and update the state. rg --type js --pattern 'runtimeState.reload' apps/server/src/stores/runtimeState.tsLength of output: 495
Script:
#!/bin/bash # Description: Verify the handling of hot reloading in the `reload` function. # Test: Search for the hot reloading logic. Expect: The function should correctly handle the hot reloading of events and update the state. rg 'runtimeState.reload' apps/server/src/stores/runtimeState.tsLength of output: 63
Script:
#!/bin/bash # Description: Verify the handling of hot reloading in the `reload` function. # Test: Search for the `reload` function definition and its references across the codebase. rg 'reload' apps/server/src/stores/runtimeState.tsLength of output: 456
Script:
#!/bin/bash # Description: Verify the implementation of `reloadAll` to understand the usage of `reload`. # Test: Search for the `reloadAll` function definition and its references across the codebase. rg 'reloadAll' apps/server/src/stores/runtimeState.tsLength of output: 105
Script:
#!/bin/bash # Description: Extract the full implementation of the `reloadAll` function to understand its usage of `reload`. # Extract the full implementation of the `reloadAll` function. rg -A 20 'export function reloadAll' apps/server/src/stores/runtimeState.tsLength of output: 706
169-193
: Verify filtering and loading logic inload
function.Ensure that the filtering and loading logic in the
load
function is correct and handles edge cases.apps/server/src/services/runtime-service/RuntimeService.ts (7)
Line range hint
44-57
: Ensure correct initialization ofeventTimer
property.Verify that the
RuntimeService
constructor correctly initializes theeventTimer
property withEventTimer
.Verification successful
Initialization of
eventTimer
property is correct.The
eventTimer
property is correctly initialized in theRuntimeService
constructor.
- Line 57:
this.eventTimer = eventTimer;
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the initialization of the `eventTimer` property in the `RuntimeService` constructor. # Test: Search for the initialization of the `eventTimer` property. Expect: The property should be correctly initialized with `EventTimer`. rg --type js --pattern 'this.eventTimer = eventTimer' apps/server/src/services/runtime-service/RuntimeService.tsLength of output: 526
Script:
#!/bin/bash # Description: Verify the initialization of the `eventTimer` property in the `RuntimeService` constructor. # Test: Search for the initialization of the `eventTimer` property. Expect: The property should be correctly initialized with `EventTimer`. rg 'this.eventTimer = eventTimer' apps/server/src/services/runtime-service/RuntimeService.tsLength of output: 128
4-4
: Ensure all imported modules are used.Verify that all imported modules are necessary and utilized within the file to avoid unused imports.
Also applies to: 23-23, 35-35
Verification successful
All imported modules are used.
The imported modules
isPlayableEvent
,EventTimer
, andcheckNeedsEvent
are all utilized within the fileapps/server/src/services/runtime-service/RuntimeService.ts
.
isPlayableEvent
is used on lines 4, 12, 14, and 15.EventTimer
is used on lines 5, 7, and 16.checkNeedsEvent
is used on line 8.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all imported modules are used in the file. # Test: Search for the usage of each imported module. Expect: Each module should be used at least once. rg --type js --pattern 'isPlayableEvent|EventTimer|checkNeedsEvent' apps/server/src/services/runtime-service/RuntimeService.tsLength of output: 540
Script:
#!/bin/bash # Description: Verify that all imported modules are used in the file. # Test: Search for the usage of each imported module. Expect: Each module should be used at least once. rg 'isPlayableEvent|EventTimer|checkNeedsEvent' apps/server/src/services/runtime-service/RuntimeService.tsLength of output: 590
562-562
: Ensure correct handling of playback state transitions inresume
method.Verify that the
resume
method correctly handles playback state transitions and event resumption.#!/bin/bash # Description: Verify the handling of playback state transitions in the `resume` method. # Test: Search for the playback <details> <summary>Verification successful</summary> --- <!-- [monologue] [task review_file_2/2 segment_8/10] Review `resume` method. [observations] The `resume` method in `RuntimeService.ts` is designed to handle playback state transitions and event resumption. It checks the playback state and selected event ID, verifies the event, and resumes the runtime state accordingly. [analysis] The method appears to handle playback state transitions and event resumption correctly. It includes checks for valid playback states and events, and it logs the resumption of playback. No issues were found in the implementation. [actions] Conclude the verification process by confirming that the `resume` method handles playback state transitions and event resumption correctly. --> **The `resume` method correctly handles playback state transitions and event resumption.** The method includes necessary checks for valid playback states and events, and it resumes the runtime state appropriately. - No issues found in the `resume` method implementation. </details> <details> <summary>Scripts executed</summary> The following scripts were executed for the analysis: --- Script: ```shell #!/bin/bash # Search for the implementation of the `resume` method in `RuntimeService.ts` to verify playback state transitions and event resumption handling. ast-grep --lang typescript --pattern 'class $_ { $$$ resume($_, $_) { $$$ } $$$ }' apps/server/src/services/runtime-service/RuntimeService.tsLength of output: 157
Script:
#!/bin/bash # Search for the `resume` method in `RuntimeService.ts` and include context lines to verify playback state transitions and event resumption handling. rg 'resume' -A 20 apps/server/src/services/runtime-service/RuntimeService.tsLength of output: 1883
88-94
: Ensure correct handling of timer updates incheckTimerUpdate
method.Verify that the
checkTimerUpdate
method correctly handles timer updates and playback state transitions.Verification successful
Ensure correct handling of timer updates in
checkTimerUpdate
method.The
checkTimerUpdate
method correctly handles timer updates and playback state transitions by checkingshouldCallRoll
,hasTimerFinished
, and invokingcheckNeedsEvent
.
checkTimerUpdate
method includes logic forshouldCallRoll
,hasTimerFinished
, andcheckNeedsEvent
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the handling of timer updates in the `checkTimerUpdate` method. # Test: Search for the timer update logic. Expect: The method should correctly handle timer updates and playback state transitions. rg --type js --pattern 'shouldCallRoll|hasTimerFinished|checkNeedsEvent' apps/server/src/services/runtime-service/RuntimeService.tsLength of output: 545
Script:
#!/bin/bash # Description: Verify the handling of timer updates in the `checkTimerUpdate` method. # Test: Search for the timer update logic. Expect: The method should correctly handle timer updates and playback state transitions. rg 'shouldCallRoll|hasTimerFinished|checkNeedsEvent' apps/server/src/services/runtime-service/RuntimeService.tsLength of output: 479
507-540
: Ensure correct handling of playback state transitions inroll
method.Verify that the
roll
method correctly handles playback state transitions and event loading.Verification successful
The
roll
method correctly handles playback state transitions and event loading.The method includes necessary checks, calls, and state management to ensure proper functionality.
- The
roll
method inRuntimeService
checks if rolling is allowed before proceeding.- It calls
this.eventTimer.roll
to perform the roll operation.- It logs and dispatches events based on the result of the roll.
- It checks for changes in playback state and logs them accordingly.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the handling of playback state transitions in the `roll` method. # Test: Search for the playback state transition logic. Expect: The method should correctly handle playback state transitions and event loading. rg --type js --pattern 'this.eventTimer.roll' apps/server/src/services/runtime-service/RuntimeService.tsLength of output: 516
Script:
#!/bin/bash # Description: Verify the handling of playback state transitions in the `roll` method. # Test: Search for the playback state transition logic. Expect: The method should correctly handle playback state transitions and event loading. rg 'this.eventTimer.roll' apps/server/src/services/runtime-service/RuntimeService.tsLength of output: 136
Script:
#!/bin/bash # Description: Verify the handling of playback state transitions in the `roll` method. # Test 1: Search for the implementation of `roll` method in the eventTimer class. ast-grep --lang typescript --pattern 'class $_ { $$$ roll($_) { $$$ } $$$ }' # Test 2: Search for the state management logic around `this.eventTimer.roll` in the `roll` method. rg -A 10 -B 10 'this.eventTimer.roll' apps/server/src/services/runtime-service/RuntimeService.tsLength of output: 57005
403-412
: Ensure correct handling of playback state transitions instart
method.Verify that the
start
method correctly handles playback state transitions and updates the state.Verification successful
The
start
method correctly handles playback state transitions and updates the state.The method retrieves the previous state, validates if playback can start, attempts to start the event timer, logs the new state, and dispatches an event if the timer started successfully.
start
method implementation atapps/server/src/services/runtime-service/RuntimeService.ts
lines 403-412Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the handling of playback state transitions in the `start` method. # Test: Search for the playback state transition logic. Expect: The method should correctly handle playback state transitions and update the state. rg --type js --pattern 'this.eventTimer.start' apps/server/src/services/runtime-service/RuntimeService.tsLength of output: 517
Script:
#!/bin/bash # Description: Verify the handling of playback state transitions in the `start` method. # Test: Search for the playback state transition logic. Expect: The method should correctly handle playback state transitions and update the state. rg 'this.eventTimer.start' apps/server/src/services/runtime-service/RuntimeService.tsLength of output: 85
Script:
#!/bin/bash # Description: Search for the implementation of the `start` method in the `RuntimeService.ts` file. # Test: Locate the `start` method implementation to understand how playback state transitions are handled. rg 'start(' apps/server/src/services/runtime-service/RuntimeService.ts -A 10Length of output: 141
Script:
#!/bin/bash # Description: Search for the implementation of the `start` method in the `RuntimeService.ts` file. # Test: Locate the `start` method implementation to understand how playback state transitions are handled. rg 'start\(' apps/server/src/services/runtime-service/RuntimeService.ts -A 10Length of output: 1922
Line range hint
573-573
: Ensure correct handling of user time addition inaddTime
method.Verify that the
addTime
method correctly handles the addition of user time and updates the state.Verification successful
The
addTime
method correctly handles the addition of user time and updates the state.The method implementation in
apps/server/src/stores/runtimeState.ts
ensures that the timer state is correctly updated and handles various edge cases.
apps/server/src/stores/runtimeState.ts
(lines 424-462)Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the handling of user time addition in the `addTime` method. # Test: Search for the user time addition logic. Expect: The method should correctly handle the addition of user time and update the state. rg --type js --pattern 'this.eventTimer.addTime' apps/server/src/services/runtime-service/RuntimeService.tsLength of output: 519
Script:
#!/bin/bash # Description: Verify the handling of user time addition in the `addTime` method. # Test: Search for the user time addition logic. Expect: The method should correctly handle the addition of user time and update the state. rg 'this.eventTimer.addTime' apps/server/src/services/runtime-service/RuntimeService.tsLength of output: 128
Script:
#!/bin/bash # Description: Verify the implementation of the `addTime` method in the `eventTimer` class or object. # Test: Search for the implementation of the `addTime` method. Expect: The method should correctly handle the addition of user time and update the state. rg 'addTime' --type jsLength of output: 22
Script:
#!/bin/bash # Description: Verify the implementation of the `addTime` method in the codebase. # Test: Use ast-grep to search for the implementation of the `addTime` method. Expect: The method should correctly handle the addition of user time and update the state. ast-grep --lang typescript --pattern 'addTime($_)'Length of output: 129
Script:
#!/bin/bash # Description: Verify the implementation of the `addTime` method in the codebase. # Test: Use ast-grep to search for the definition of the `addTime` method. Expect: The method should correctly handle the addition of user time and update the state. ast-grep --lang typescript --pattern 'function addTime($_) { $$$ }'Length of output: 3089
47372c0
to
c5d47f3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (4)
apps/server/tsconfig.json
is excluded by none and included by nonepackages/types/src/definitions/core/OntimeEvent.type.ts
is excluded by none and included by nonepackages/types/src/index.ts
is excluded by none and included by nonepackages/types/src/utils/guards.ts
is excluded by none and included by none
Files selected for processing (11)
- apps/server/src/services/EventTimer.ts (4 hunks)
- apps/server/src/services/tests/rollUtils.test.ts (1 hunks)
- apps/server/src/services/rollUtils.ts (1 hunks)
- apps/server/src/services/rundown-service/RundownService.ts (2 hunks)
- apps/server/src/services/rundown-service/rundownUtils.ts (3 hunks)
- apps/server/src/services/runtime-service/RuntimeService.ts (14 hunks)
- apps/server/src/services/timerUtils.ts (3 hunks)
- apps/server/src/stores/tests/runtimeState.test.ts (3 hunks)
- apps/server/src/stores/runtimeState.ts (13 hunks)
- packages/utils/src/rundown-utils/rundownUtils.test.ts (2 hunks)
- packages/utils/src/rundown-utils/rundownUtils.ts (17 hunks)
Files skipped from review as they are similar to previous changes (6)
- apps/server/src/services/EventTimer.ts
- apps/server/src/services/tests/rollUtils.test.ts
- apps/server/src/services/rundown-service/RundownService.ts
- apps/server/src/stores/tests/runtimeState.test.ts
- packages/utils/src/rundown-utils/rundownUtils.test.ts
- packages/utils/src/rundown-utils/rundownUtils.ts
Additional context used
Biome
apps/server/src/stores/runtimeState.ts
[error] 431-431: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 431-431: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
[error] 488-488: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 488-488: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
[error] 525-525: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 525-525: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
[error] 566-566: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 566-566: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
Additional comments not posted (33)
apps/server/src/services/rundown-service/rundownUtils.ts (9)
1-2
: LGTM! Imports are updated correctly.The import statements have been updated to include
PlayableEvent
and utility functionsfilterPlayable
andfilterTimedEvents
.
24-24
: Refactoring improves readability and performance.The function
getTimedEvents
now uses thefilterTimedEvents
utility, enhancing readability and performance.
30-31
: Refactoring clarifies intent and enhances type safety.The function
getPlayableEvents
now uses thefilterPlayable
utility and returns an array ofPlayableEvent
types.
54-59
: Ensures consistency with the new filtering strategy.The function
getNextEventWithCue
now uses theplayableEvents
array, ensuring consistency.
69-81
: Ensures consistency with the new filtering strategy.The function
findPrevious
now uses theplayableEvents
array, ensuring consistency.
88-101
: Ensures consistency with the new filtering strategy and enhances type safety.The function
findNext
now uses theplayableEvents
array and returns aPlayableEvent
ornull
.
Line range hint
104-113
: No changes detected.The function
getPaginated
has not been modified.
6-9
: No changes detected.The function
getNormalisedRundown
has not been modified.
12-15
: No changes detected.The function
getRundown
has not been modified.apps/server/src/services/rollUtils.ts (3)
1-2
: LGTM! Imports are updated correctly.The import statements bring in various utilities and types necessary for the roll functionality.
Also applies to: 4-5, 7-7
94-102
: LGTM! Function checks necessary conditions correctly.The function
checkNeedsEvent
correctly checks if an event load needs to be triggered.
107-109
: LGTM! Function normalizes event start time correctly.The function
normaliseRollStart
correctly checks whether the event start is the day after.apps/server/src/services/timerUtils.ts (10)
1-3
: LGTM! Imports are updated correctly.The import statements bring in various utilities and types necessary for the timer functionality.
Line range hint
6-6
: LGTM! Function normalizes end time correctly.The function
normaliseEndTime
correctly handles events that span over midnight.
Line range hint
9-31
: LGTM! Function calculates expected finish time correctly.The function
getExpectedFinish
correctly calculates the expected finish time of a running timer.
Line range hint
34-57
: LGTM! Function calculates current time correctly.The function
getCurrent
correctly calculates the running countdown.
96-100
: LGTM! Function checks necessary conditions correctly.The function
skippedOutOfEvent
correctly checks whether the timer has skipped out of the event.
Line range hint
103-131
: LGTM! Function calculates runtime offset correctly.The function
getRuntimeOffset
correctly calculates the difference between the runtime and the schedule of an event.
Line range hint
134-143
: LGTM! Function calculates total duration correctly.The function
getTotalDuration
correctly calculates the total duration of a time span.
Line range hint
146-150
: LGTM! Function calculates expected end correctly.The function
getExpectedEnd
correctly calculates the expected end of the rundown.
Line range hint
153-155
: LGTM! Function checks necessary conditions correctly.The function
isPlaybackActive
correctly checks whether the playback is considered to be active.
Line range hint
158-174
: LGTM! Function checks timer phase correctly.The function
getTimerPhase
correctly checks the current phase of the running timer.apps/server/src/stores/runtimeState.ts (6)
262-292
: Ensure correct handling ofeventIndex
inloadNext
function.The
loadNext
function now accepts an array ofOntimeEvent
and an optionaleventIndex
. Ensure that theeventIndex
is correctly handled and defaults toruntimeState.runtime.selectedEventIndex
if not provided.Verification successful
The handling of
eventIndex
in theloadNext
function is correct.The
eventIndex
parameter correctly defaults toruntimeState.runtime.selectedEventIndex
if not provided, ensuring proper functionality.
loadNext
function definition and usage ofeventIndex
:
apps/server/src/stores/runtimeState.ts
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the handling of `eventIndex` in the `loadNext` function. # Test: Search for the usage of `eventIndex` in the `loadNext` function. Expect: Correct handling and defaulting to `runtimeState.runtime.selectedEventIndex`. rg --type-add 'ts' --type ts 'function loadNext' -A 10Length of output: 114
Script:
#!/bin/bash # Description: Verify the handling of `eventIndex` in the `loadNext` function. # Test: Search for the definition and usage of `eventIndex` in the `loadNext` function. Expect: Correct handling and defaulting to `runtimeState.runtime.selectedEventIndex`. rg 'function loadNext' -t ts -A 30Length of output: 2153
Line range hint
211-252
:
Ensure correct handling ofeventIndex
inloadNow
function.The
loadNow
function now accepts an array ofOntimeEvent
and an optionaleventIndex
. Ensure that theeventIndex
is correctly handled and defaults toruntimeState.runtime.selectedEventIndex
if not provided.Verification successful
Ensure correct handling of
eventIndex
inloadNow
function.The
loadNow
function correctly defaults theeventIndex
toruntimeState.runtime.selectedEventIndex
if not provided and handles it appropriately within the function.
eventIndex
is defaulted toruntimeState.runtime.selectedEventIndex
.- The function resets the state if
eventIndex
isnull
.- Proper handling of the event and updating of the state is present.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the handling of `eventIndex` in the `loadNow` function. # Test: Search for the usage of `eventIndex` in the `loadNow` function. Expect: Correct handling and defaulting to `runtimeState.runtime.selectedEventIndex`. rg --type-add 'ts' --type ts 'function loadNow' -A 10Length of output: 113
Script:
#!/bin/bash # Description: Verify the handling of `eventIndex` in the `loadNow` function. # Test: Search for the usage of `eventIndex` in the `loadNow` function. Expect: Correct handling and defaulting to `runtimeState.runtime.selectedEventIndex`. rg 'function loadNow' -A 50Length of output: 3570
169-181
: Ensure correct type usage forload
function parameters.The
load
function now accepts aPlayableEvent
instead of anOntimeEvent
. Ensure that thePlayableEvent
type is correctly used throughout the codebase.Verification successful
Ensure correct type usage for
load
function parameters.The
PlayableEvent
type is correctly used throughout the codebase, including in theload
function and other related functions and files.
- Files and lines where
PlayableEvent
is used:
packages/types/src/index.ts
packages/utils/src/rundown-utils/rundownUtils.ts
packages/types/src/utils/guards.ts
packages/types/src/definitions/core/OntimeEvent.type.ts
apps/server/src/stores/runtimeState.ts
apps/server/src/stores/__tests__/runtimeState.test.ts
apps/server/src/services/rollUtils.ts
apps/server/src/services/runtime-service/RuntimeService.ts
apps/server/src/services/rundown-service/rundownUtils.ts
apps/server/src/services/rundown-service/RundownService.ts
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `PlayableEvent` type in the codebase. # Test: Search for the usage of `PlayableEvent` type. Expect: Correct usage throughout the codebase. rg --type-add 'ts' --type ts 'PlayableEvent'Length of output: 102
Script:
#!/bin/bash # Description: Verify the usage of `PlayableEvent` type in the codebase. # Test: Search for the usage of `PlayableEvent` type. Expect: Correct usage throughout the codebase. rg 'PlayableEvent'Length of output: 5105
Line range hint
308-340
:
Ensure correct handling of optionalPlayableEvent
parameter inreload
function.The
reload
function now accepts an optionalPlayableEvent
parameter. Ensure that the function correctly updates the state based on the provided event.Verification successful
Let's correct the
rg
command to search for thereload
function in TypeScript files and capture enough lines to verify the handling of thePlayableEvent
parameter.
Verified: Correct handling of optional
PlayableEvent
parameter inreload
function.The
reload
function correctly updates the state based on the providedPlayableEvent
parameter or resets the timer when no event is provided.
- The function updates
runtimeState.eventNow
and associated timer properties when an event is passed.- When no event is passed, it resets the timer properties to their initial states.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the handling of optional `PlayableEvent` parameter in the `reload` function. # Test: Search for the usage of `PlayableEvent` parameter in the `reload` function. Expect: Correct handling and state updates. rg --type-add 'ts' --type ts 'function reload' -A 10Length of output: 112
Script:
#!/bin/bash # Description: Verify the handling of optional `PlayableEvent` parameter in the `reload` function. # Corrected Test: Search for the usage of `PlayableEvent` parameter in the `reload` function. Expect: Correct handling and state updates. rg 'function reload' -A 30 --glob '*.ts'Length of output: 4546
183-191
: Verify the correctness ofloadNow
andloadNext
function calls.The
loadNow
andloadNext
functions are called with the updated parameters. Ensure that these functions are correctly handling the new parameters.Verification successful
The
loadNow
andloadNext
functions correctly handle the updated parameters.
- The
loadNow
function sets various runtime state properties based on the current event, ensuring that the state is updated correctly.- The
loadNext
function iterates through the events to set the next event and the next public event, handling the parameters as expected.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of `loadNow` and `loadNext` function calls. # Test: Search for the implementation of `loadNow` and `loadNext` functions. Expect: Correct handling of new parameters. rg --type-add 'ts' --type ts 'function loadNow' -A 10 rg --type-add 'ts' --type ts 'function loadNext' -A 10Length of output: 228
Script:
#!/bin/bash # Description: Verify the correctness of `loadNow` and `loadNext` function calls. # Test: Search for the implementation of `loadNow` and `loadNext` functions. Expect: Correct handling of new parameters. rg 'function loadNow' -A 10 rg 'function loadNext' -A 10Length of output: 1689
Script:
#!/bin/bash # Description: Extract more lines of `loadNow` and `loadNext` function implementations for verification. # Test: Extract 30 lines of the `loadNow` function implementation. rg 'function loadNow' -A 30 # Test: Extract 30 lines of the `loadNext` function implementation. rg 'function loadNext' -A 30Length of output: 4533
539-608
: Ensure correct handling of roll state and event loading inroll
function.The
roll
function now returns an object witheventId
anddidStart
properties. Ensure that the roll state and event loading logic are correctly implemented.Verification successful
Ensure correct handling of roll state and event loading in
roll
function.The
roll
function now returns an object witheventId
anddidStart
properties. The roll state and event loading logic are correctly implemented and update the runtime state and timer state as expected.
- The function updates the playback state to
Playback.Roll
when an event is running.- It finds the next event if no event is running and handles pending events appropriately.
- The function also updates the timer properties and runtime state correctly.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the handling of roll state and event loading in the `roll` function. # Test: Search for the usage of roll state and event loading logic in the `roll` function. Expect: Correct implementation and state updates. rg --type-add 'ts' --type ts 'function roll' -A 10Length of output: 110
Script:
#!/bin/bash # Description: Verify the handling of roll state and event loading in the `roll` function. # Test: Search for the full implementation of the `roll` function to ensure correct handling of roll state and event loading. ast-grep --lang typescript --pattern $'function roll($_) { $$$ }'Length of output: 68
Script:
#!/bin/bash # Description: Search for the term `roll` in TypeScript files to gather more context about its definition and usage. # Test: Search for the term `roll` in TypeScript files. rg 'roll' --type ts -A 20Length of output: 153243
Tools
Biome
[error] 566-566: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 566-566: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
apps/server/src/services/runtime-service/RuntimeService.ts (5)
507-540
: Ensure correct handling of roll state and error handling inroll
method.The
roll
method has changes in its parameters, error handling, and internal logic. Ensure that the method correctly handles the roll state and dispatches the appropriate integration service events.Verification successful
The
roll
method correctly handles the roll state and error handling.The method implementation includes appropriate checks, error handling, and dispatches integration service events as expected.
- The
skipCheck
parameter is correctly used to bypass the roll check.- The
try-catch
block ensures errors are logged.- Integration service events are dispatched using
process.nextTick
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the handling of roll state and error handling in the `roll` method. # Test: Search for the usage of roll state and error handling logic in the `roll` method. Expect: Correct implementation and dispatches. rg --type-add 'ts' --type ts 'function roll' -A 10Length of output: 110
Script:
#!/bin/bash # Description: Verify the handling of roll state and error handling in the `roll` method. # Test: Search for the usage of roll state and error handling logic in the `roll` method. Expect: Correct implementation and dispatches. rg 'roll\(skipCheck: boolean = false\)' -A 30 apps/server/src/services/runtime-service/RuntimeService.tsLength of output: 991
56-56
: Ensure correct handling of timer updates and roll state incheckTimerUpdate
method.The
checkTimerUpdate
method has changes in its internal logic for handling timer updates and roll state. Ensure that the method correctly handles these updates and dispatches the appropriate integration service events.Also applies to: 88-94
Verification successful
Ensure correct handling of timer updates and roll state in
checkTimerUpdate
method.The
checkTimerUpdate
method correctly handles timer updates and roll state, including dispatching the appropriate integration service events.
- The method dispatches integration events when the timer phase changes.
- It handles edge cases related to roll, including dispatching events and calling the roll method.
- It processes actions related to the timer finishing, including dispatching events and handling end actions.
- It updates the timer appropriately.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the handling of timer updates and roll state in the `checkTimerUpdate` method. # Test: Search for the usage of timer updates and roll state logic in the `checkTimerUpdate` method. Expect: Correct implementation and dispatches. rg --type-add 'ts' --type ts 'function checkTimerUpdate' -A 10Length of output: 122
Script:
#!/bin/bash # Description: Verify the handling of timer updates and roll state in the `checkTimerUpdate` method. # Test: Search for the usage of timer updates and roll state logic in the `checkTimerUpdate` method. Expect: Correct implementation and dispatches. rg 'checkTimerUpdate' -A 50Length of output: 9742
23-23
: Ensure correct usage ofEventTimer
type.The
eventTimer
property type has been changed fromTimerService
toEventTimer
. Ensure that theEventTimer
type is correctly used throughout the class.Also applies to: 44-57
Verification successful
Ensure correct usage of
EventTimer
type.The
eventTimer
property type has been changed fromTimerService
toEventTimer
. TheEventTimer
type is correctly imported and used in theRuntimeService
class. Ensure that the methods and properties ofEventTimer
are used correctly inRuntimeService
.
apps/server/src/services/runtime-service/RuntimeService.ts
import { EventTimer } from '../EventTimer.js';
private eventTimer: EventTimer;
constructor(eventTimer: EventTimer)
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `EventTimer` type in the codebase. # Test: Search for the usage of `EventTimer` type. Expect: Correct usage throughout the codebase. rg --type-add 'ts' --type ts 'EventTimer'Length of output: 99
Script:
#!/bin/bash # Description: Verify the usage of `EventTimer` type in the codebase. # Test: Search for the usage of `EventTimer` type. Expect: Correct usage throughout the codebase. rg --type-add 'ts:*.ts' --type ts 'EventTimer'Length of output: 674
562-562
: Ensure correct handling of playback state resumption inresume
method.The
resume
method has changes in its parameters and internal logic. Ensure that the method correctly handles resuming the playback state from a restore point.Also applies to: 538-540
Verification successful
The
resume
method correctly handles playback state resumption by delegating to theload
function.The
load
function inapps/server/src/stores/runtimeState.ts
ensures that the playback state is correctly resumed by clearing the current state, filtering the rundown, and loading the relevant events into memory. It also updates the runtime state and sets the playback state toArmed
.
apps/server/src/stores/runtimeState.ts: resume
methodapps/server/src/stores/runtimeState.ts: load
functionScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the handling of playback state resumption in the `resume` method. # Test: Search for the usage of playback state resumption logic in the `resume` method. Expect: Correct implementation and state updates. rg --type-add 'ts' --type ts 'function resume' -A 10Length of output: 112
Script:
#!/bin/bash # Description: Verify the handling of playback state resumption in the `resume` method. # Test: Search for the usage of playback state resumption logic in the `resume` method. Expect: Correct implementation and state updates. rg 'function resume' -A 20Length of output: 1680
Script:
#!/bin/bash # Description: Verify the handling of playback state resumption in the `load` function. # Test: Search for the implementation of the `load` function and include sufficient context lines for review. rg 'function load' -A 20Length of output: 14554
403-412
: Ensure correct handling of event timer start and playback state instart
method.The
start
method has changes in its internal logic for starting the event timer and handling playback state. Ensure that the method correctly handles these updates and dispatches the appropriate integration service events.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- apps/server/src/services/runtime-service/RuntimeService.ts (14 hunks)
- apps/server/src/stores/runtimeState.ts (13 hunks)
Additional context used
Biome
apps/server/src/stores/runtimeState.ts
[error] 431-431: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 431-431: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
[error] 488-488: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 488-488: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
[error] 525-525: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 525-525: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
[error] 589-589: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 589-589: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
Additional comments not posted (11)
apps/server/src/stores/runtimeState.ts (6)
262-292
: Verify the correctness of event index handling.Ensure that the logic to handle the event index is correct and that edge cases are covered.
169-181
: Verify the correctness of event filtering and error handling.Ensure that the logic to filter and find events is correct and that the error handling covers all edge cases.
Verification successful
Verify the correctness of event filtering and error handling.
The
load
function correctly filters events and handles errors. It ensures that the event list is not empty, the event is found, and the event is playable before proceeding. The function also updates the runtime state and handles initial data correctly.
- File:
apps/server/src/stores/runtimeState.ts
- Lines: 168-206
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of event filtering and error handling in the `load` function. # Test: Search for the function usage. Expect: Correct filtering and error handling. rg --type python -A 10 $'function load('Length of output: 120
Script:
#!/bin/bash # Description: Verify the correctness of event filtering and error handling in the `load` function. # Test: Search for the function definition and its usage in TypeScript files. rg --type typescript -A 10 'function load('Length of output: 124
Script:
#!/bin/bash # Description: Verify the correctness of event filtering and error handling in the `load` function. # Test: Search for the function definition and its usage in TypeScript files. rg --type typescript -A 10 'function load('Length of output: 124
Script:
#!/bin/bash # Description: Verify the correctness of event filtering and error handling in the `load` function. # Test: Search for the function definition and its usage in TypeScript files using ast-grep. ast-grep --lang ts --pattern 'function load($_, $_, $_): boolean { $$$ }'Length of output: 3014
Line range hint
211-252
:
Verify the correctness of event index handling.Ensure that the logic to handle the event index is correct and that edge cases are covered.
Verification successful
Verify the correctness of event index handling.
The
loadNow
function appears to handle the event index correctly by checking if theeventIndex
isnull
and setting the appropriate state. The function is called with different parameters, and the logic seems to cover various edge cases, such as when no event index is provided or when iterating backwards to find a public event.
- The function correctly resets the state when
eventIndex
isnull
.- It sets the
eventNow
andcurrentBlock
based on the providedeventIndex
.- It maintains the
startedAt
time if the block remains the same.- It handles the public event logic by iterating backwards if no public event is found.
The function's implementation and usage indicate that it correctly handles the event index in different scenarios.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of event index handling in the `loadNow` function. # Test: Search for the function usage. Expect: Correct handling of event index. rg --type python -A 10 $'function loadNow('Length of output: 129
Script:
#!/bin/bash # Description: Verify the correctness of event index handling in the `loadNow` function. # Test: Search for the function usage. Expect: Correct handling of event index. rg --type js -A 10 'function loadNow('Length of output: 125
Script:
#!/bin/bash # Description: Verify the correctness of event index handling in the `loadNow` function. # Test: Search for the function usage. Expect: Correct handling of event index. rg --type js -A 10 'function loadNow'Length of output: 39
Script:
#!/bin/bash # Description: Verify the correctness of event index handling in the `loadNow` function. # Test 1: Search for the function definition. rg --type js --type ts 'function loadNow' # Test 2: Search for the function usage. rg --type js --type ts 'loadNow('Length of output: 306
Script:
#!/bin/bash # Description: Verify the correctness of event index handling in the `loadNow` function. # Test: Search for the function usage. rg --type js --type ts 'loadNow\('Length of output: 404
569-575
: Verify the correctness of restore point handling.Ensure that the logic to handle the restore point is correct and that edge cases are covered.
539-567
: Verify the error handling.Ensure that the error handling for the roll operation covers all edge cases.
539-567
: Verify the correctness of playback control and error handling.Ensure that the logic to control playback during event transitions is correct and that the error handling covers all edge cases.
apps/server/src/services/runtime-service/RuntimeService.ts (5)
56-59
: Verify the initialization logic.Ensure that the initialization logic in the constructor is correct and that the transition from
TimerService
toEventTimer
is seamless.Verification successful
Initialization logic is correct.
The constructor for
RuntimeService
has been correctly updated to useEventTimer
, and the instantiation ofRuntimeService
witheventTimer
is properly handled.
RuntimeService
constructor definition and usage are consistent and correct.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the initialization logic in the constructor. # Test: Search for the constructor usage. Expect: Correct initialization logic. rg --type python -A 10 $'constructor(eventTimer: EventTimer)'Length of output: 93
Script:
#!/bin/bash # Description: Verify the initialization logic in the constructor. # Search for the constructor definition in the RuntimeService class. ast-grep --lang typescript --pattern $'class RuntimeService { constructor(eventTimer: EventTimer) { $$$ } }' -A 10 # Search for the usage of the RuntimeService constructor to ensure proper initialization. rg 'new RuntimeService' -A 5Length of output: 734
Line range hint
569-575
:
Verify the correctness of restore point handling.Ensure that the logic to handle the restore point is correct and that edge cases are covered.
56-59
: Verify the correctness of update result handling and integration notifications.Ensure that the logic to handle the update result and notify integrations is correct and that edge cases are covered.
Verification successful
Let's correct the
rg
command to search for thecheckTimerUpdate
function in TypeScript files and ensure proper syntax.
Let's adjust the
rg
command to correctly search for thecheckTimerUpdate
function definition in TypeScript files.
Let's adjust the
rg
command to search for thecheckTimerUpdate
function without using parentheses in the pattern.
Verify the correctness of update result handling and integration notifications.
The
checkTimerUpdate
function inRuntimeService.ts
correctly handles the update result and notifies integrations based on the timer phase changes. The function checks if the timer phase has changed and dispatches the appropriate integration events.
apps/server/src/services/runtime-service/RuntimeService.ts: checkTimerUpdate({ shouldCallRoll, hasTimerFinished }: runtimeState.UpdateResult) {
The function implementation appears to be correct and covers the necessary logic for handling updates and notifying integrations.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of update result handling and integration notifications in the `checkTimerUpdate` function. # Test: Search for the function usage. Expect: Correct handling of update result and integration notifications. rg --type python -A 10 $'function checkTimerUpdate('Length of output: 156
Script:
#!/bin/bash # Description: Verify the correctness of update result handling and integration notifications in the `checkTimerUpdate` function. # Test: Search for the function definition and context. Expect: Correct handling of update result and integration notifications. rg --type ts -A 10 'function checkTimerUpdate('Length of output: 152
Script:
#!/bin/bash # Description: Verify the correctness of update result handling and integration notifications in the `checkTimerUpdate` function. # Test: Search for the function definition and context. Expect: Correct handling of update result and integration notifications. rg --type ts -A 10 'checkTimerUpdate('Length of output: 125
Script:
#!/bin/bash # Description: Verify the correctness of update result handling and integration notifications in the `checkTimerUpdate` function. # Test: Search for the function definition and context. Expect: Correct handling of update result and integration notifications. rg --type ts -A 10 'checkTimerUpdate'Length of output: 2205
507-514
: Verify the correctness of roll operation handling.Ensure that the logic to handle the roll operation for a loaded event is correct and that edge cases are covered.
520-553
: Verify the correctness of playback control and error handling.Ensure that the logic to control playback during event transitions is correct and that the error handling covers all edge cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
Outside diff range, codebase verification and nitpick comments (1)
apps/server/src/stores/runtimeState.ts (1)
Line range hint
310-356
:
Remove unusedDEV
label.The
DEV
label is not used by any break or continue statement and should be removed.- DEV: { - if (runtimeState.eventNow === null) { - throw new Error('runtimeState.roll: invalid state received'); - } - }
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (5)
apps/server/tsconfig.json
is excluded by none and included by noneapps/spec/roll.md
is excluded by none and included by nonepackages/types/src/definitions/core/OntimeEvent.type.ts
is excluded by none and included by nonepackages/types/src/index.ts
is excluded by none and included by nonepackages/types/src/utils/guards.ts
is excluded by none and included by none
Files selected for processing (11)
- apps/server/src/services/EventTimer.ts (4 hunks)
- apps/server/src/services/tests/rollUtils.test.ts (1 hunks)
- apps/server/src/services/rollUtils.ts (1 hunks)
- apps/server/src/services/rundown-service/RundownService.ts (2 hunks)
- apps/server/src/services/rundown-service/rundownUtils.ts (3 hunks)
- apps/server/src/services/runtime-service/RuntimeService.ts (12 hunks)
- apps/server/src/services/timerUtils.ts (3 hunks)
- apps/server/src/stores/tests/runtimeState.test.ts (3 hunks)
- apps/server/src/stores/runtimeState.ts (15 hunks)
- packages/utils/src/rundown-utils/rundownUtils.test.ts (2 hunks)
- packages/utils/src/rundown-utils/rundownUtils.ts (17 hunks)
Files skipped from review due to trivial changes (1)
- packages/utils/src/rundown-utils/rundownUtils.test.ts
Files skipped from review as they are similar to previous changes (7)
- apps/server/src/services/EventTimer.ts
- apps/server/src/services/tests/rollUtils.test.ts
- apps/server/src/services/rundown-service/RundownService.ts
- apps/server/src/services/rundown-service/rundownUtils.ts
- apps/server/src/services/runtime-service/RuntimeService.ts
- apps/server/src/stores/tests/runtimeState.test.ts
- packages/utils/src/rundown-utils/rundownUtils.ts
Additional context used
GitHub Check: unit-test
apps/server/src/services/rollUtils.ts
[failure] 4-4:
'RuntimeState' is defined but never used. Allowed unused vars must match /^_/u
[failure] 5-5:
'timerConfig' is defined but never used. Allowed unused vars must match /^_/u
[failure] 7-7:
'skippedOutOfEvent' is defined but never used. Allowed unused vars must match /^_/u
Biome
apps/server/src/stores/runtimeState.ts
[error] 449-449: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 449-449: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
[error] 506-506: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 506-506: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
[error] 543-543: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 543-543: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
[error] 605-605: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 605-605: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
Additional comments not posted (24)
apps/server/src/services/rollUtils.ts (1)
94-96
: LGTM!The function is simple and correct.
apps/server/src/services/timerUtils.ts (8)
Line range hint
6-8
:
LGTM!The function is simple and correct.
Line range hint
9-31
:
LGTM!The function is well-structured and handles various scenarios correctly.
Line range hint
32-52
:
LGTM!The function is well-structured and handles various scenarios correctly.
Line range hint
53-74
:
LGTM!The function is well-structured and handles various scenarios correctly.
Line range hint
75-92
:
LGTM!The function is well-structured and handles various scenarios correctly.
Line range hint
93-104
:
LGTM!The function is simple and correct.
Line range hint
105-112
:
LGTM!The function is well-structured and handles various scenarios correctly.
Line range hint
113-118
:
LGTM!The function is simple and correct.
apps/server/src/stores/runtimeState.ts (15)
96-106
: LGTM!The function is simple and correct.
Line range hint
107-127
:
LGTM!The function is well-structured and handles various scenarios correctly.
Line range hint
128-136
:
LGTM!The function is simple and correct.
Line range hint
137-150
:
LGTM!The function is well-structured and handles various scenarios correctly.
Line range hint
171-207
:
LGTM!The function is well-structured and handles various scenarios correctly.
Line range hint
213-259
:
LGTM!The function is well-structured and handles various scenarios correctly.
264-299
: LGTM!The function is well-structured and handles various scenarios correctly.
302-306
: LGTM!The function is well-structured and handles various scenarios correctly.
Line range hint
310-356
:
LGTM!The function is well-structured and handles various scenarios correctly.
366-370
: LGTM!The function is well-structured and handles various scenarios correctly.
Line range hint
371-413
:
LGTM!The function is well-structured and handles various scenarios correctly.
Line range hint
414-421
:
LGTM!The function is well-structured and handles various scenarios correctly.
Line range hint
422-429
:
LGTM!The function is well-structured and handles various scenarios correctly.
441-453
: LGTM!The function is well-structured and handles various scenarios correctly.
Tools
Biome
[error] 449-449: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 449-449: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
Line range hint
506-553
:
LGTM!The function is well-structured
Tools
Biome
[error] 543-543: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 543-543: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
[error] 605-605: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 605-605: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Caution
Inline review comments failed to post
Actionable comments posted: 6
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- apps/server/src/services/rollUtils.ts (1 hunks)
- apps/server/src/services/runtime-service/RuntimeService.ts (12 hunks)
- apps/server/src/stores/runtimeState.ts (15 hunks)
Additional context used
GitHub Check: unit-test
apps/server/src/services/rollUtils.ts
[failure] 4-4:
'RuntimeState' is defined but never used. Allowed unused vars must match /^_/u
[failure] 5-5:
'timerConfig' is defined but never used. Allowed unused vars must match /^_/u
[failure] 7-7:
'skippedOutOfEvent' is defined but never used. Allowed unused vars must match /^_/u
Biome
apps/server/src/stores/runtimeState.ts
[error] 449-449: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 449-449: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
[error] 506-506: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 506-506: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
[error] 543-543: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 543-543: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
[error] 605-605: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 605-605: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
Additional comments not posted (13)
apps/server/src/services/rollUtils.ts (1)
94-96
: LGTM!The
normaliseRollStart
function is straightforward and does not have any apparent issues.apps/server/src/services/runtime-service/RuntimeService.ts (4)
86-103
: Improve error handling incheckTimerUpdate
.The
checkTimerUpdate
method has robust error handling and logging for different timer phases, but ensure that the logic for handling edge cases is thoroughly tested.Ensure that edge cases related to
skippedOutOfEvent
andhasTimerFinished
are covered in your tests.
515-522
: Enhance error handling inrollLoaded
method.The
rollLoaded
method now includes a try-catch block for error handling, which improves robustness. Ensure that errors are properly logged and handled.
528-561
: Improve error handling inroll
method.The
roll
method now includes a try-catch block for error handling, which improves robustness. Ensure that errors are properly logged and handled.
56-57
: Update constructor parameter.The constructor parameter
timerService
should be updated toeventTimer
to match the changes.- constructor(timerService: TimerService) { - this.eventTimer = timerService; + constructor(eventTimer: EventTimer) { + this.eventTimer = eventTimer;Likely invalid or redundant comment.
apps/server/src/stores/runtimeState.ts (8)
Line range hint
171-209
:
LGTM! But verify the logic.The
load
function has been updated to filter playable events and ensure that only valid events are processed during state transitions. Ensure that the logic is thoroughly tested.Ensure that all edge cases related to event loading are covered in your tests.
Line range hint
213-260
:
LGTM! But verify the logic.The
loadNow
function has been updated to accept arrays ofPlayableEvent
and an optional event index, enhancing its flexibility. Ensure that the logic is thoroughly tested.Ensure that all edge cases related to event loading are covered in your tests.
264-294
: LGTM! But verify the logic.The
loadNext
function has been updated to accept arrays ofPlayableEvent
and an optional event index, enhancing its flexibility. Ensure that the logic is thoroughly tested.Ensure that all edge cases related to event loading are covered in your tests.
Line range hint
310-356
:
LGTM! But verify the logic.The
reload
function has been updated to handle hot reloading of events without interrupting the timer. Ensure that the logic is thoroughly tested.Ensure that all edge cases related to event reloading are covered in your tests.
555-645
: LGTM! But verify the logic.The
roll
function has undergone substantial restructuring to improve playback transitions and ensure timer accuracy. Ensure that the logic is thoroughly tested.Ensure that all edge cases related to event rolling are covered in your tests.
Tools
Biome
[error] 605-605: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 605-605: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
Line range hint
441-478
:
LGTM! But verify the logic.The
addTime
function appears to be correctly handling the addition of time to the timer. Ensure that the logic is thoroughly tested.Ensure that all edge cases related to adding time are covered in your tests.
Tools
Biome
[error] 449-449: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 449-449: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
Line range hint
484-548
:
LGTM! But verify the logic.The
update
function appears to be correctly handling the update of the timer state. Ensure that the logic is thoroughly tested.Ensure that all edge cases related to updating the timer state are covered in your tests.
Tools
Biome
[error] 506-506: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 506-506: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
541-551
: LGTM! But verify the logic.The
updateIfWaitingToRoll
function appears to be correctly handling the update of the timer state when waiting to roll. Ensure that the logic is thoroughly tested.Ensure that all edge cases related to updating the timer state when waiting to roll are covered in your tests.
Tools
Biome
[error] 543-543: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 543-543: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
Comments failed to post (6)
apps/server/src/services/rollUtils.ts
4-7: Remove unused imports.
The imports
RuntimeState
,timerConfig
, andskippedOutOfEvent
are not used and should be removed.- import { RuntimeState } from '../stores/runtimeState.js'; - import { timerConfig } from '../config/config.js'; - import { skippedOutOfEvent } from './timerUtils.js';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.
Tools
GitHub Check: unit-test
[failure] 4-4:
'RuntimeState' is defined but never used. Allowed unused vars must match /^_/u
[failure] 5-5:
'timerConfig' is defined but never used. Allowed unused vars must match /^_/u
[failure] 7-7:
'skippedOutOfEvent' is defined but never used. Allowed unused vars must match /^_/u
12-89: Optimize the logic for checking if an event is in the future.
The logic for checking if an event is in the future can be optimized by combining the conditions.
- const isFromDayBefore = normalEnd > dayInMs && timeNow < event.timeEnd; - const hasStarted = isFromDayBefore || timeNow >= event.timeStart; - if (hasStarted) { + if ((normalEnd > dayInMs && timeNow < event.timeEnd) || timeNow >= event.timeStart) { return { event, index: i }; }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.export function loadRoll( timedEvents: OntimeEvent[], timeNow: number, ): { event: PlayableEvent | null; index: MaybeNumber; isPending?: boolean; } { const { firstEvent } = getFirstEvent(timedEvents); const { lastEvent } = getLastEvent(timedEvents); if (!firstEvent || !lastEvent) { return { event: null, index: null }; } // check that the rundown wraps around midnight const wrapsAroundMidnight = firstEvent.timeStart > lastEvent.timeEnd; if (!wrapsAroundMidnight) { // check whether we are before or after the rundown const lastNormalEnd = normaliseEndTime(lastEvent.timeStart, lastEvent.timeEnd); const isAfterRundown = timeNow > lastNormalEnd; const isBeforeRundown = timeNow < firstEvent.timeStart && !isAfterRundown; if (isAfterRundown || isBeforeRundown) { return { event: firstEvent, index: 0, isPending: true }; } } // we know we are in the middle of the rundown and we need to find the current event // account for number of times we went over midnight let daySpan = 0; for (let i = 0; i < timedEvents.length; i++) { const event = timedEvents[i]; if (!isPlayableEvent(event)) { continue; } // we check if event crosses midnight if (event.timeStart > event.timeEnd) { daySpan++; } const correctedDays = dayInMs * daySpan; const correctedStart = event.timeStart + correctedDays; const correctedEnd = event.timeEnd + correctedDays; /** * there are 3 possible states for an event * 1. event is already finished * 2. event is running * 3. event is in the future */ // 1. event is already finished // when does the event end (handle midnight) const normalEnd = normaliseEndTime(correctedStart, correctedEnd); if (normalEnd <= timeNow) { continue; } // 2. event is running and is the first event in our time slot if ((normalEnd > dayInMs && timeNow < event.timeEnd) || timeNow >= event.timeStart) { return { event, index: i }; } // 3. event will run in the future // we set the isPending flag to indicate that the event is currently playing return { event, index: i, isPending: true }; } // in case we were unable to find anything, we load the first event return { event: firstEvent, index: 0, isPending: true }; }
apps/server/src/services/runtime-service/RuntimeService.ts
238-252: Review and address TODO comment in
maybeUpdate
method.The
maybeUpdate
method contains a TODO comment indicating that the logic needs to be reviewed.Do you want me to help review the logic or open a GitHub issue to track this task?
apps/server/src/stores/runtimeState.ts
543-546: Remove unused
DEV
label.The
DEV
label is not used by any break or continue statement and should be removed.- // eslint-disable-next-line no-unused-labels -- dev code path - DEV: { - if (runtimeState.eventNow === null || runtimeState._timer.secondaryTarget === null) { - throw new Error('runtimeState.updateIfWaitingToRoll: invalid state received'); - } - }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.if (runtimeState.eventNow === null || runtimeState._timer.secondaryTarget === null) { throw new Error('runtimeState.updateIfWaitingToRoll: invalid state received'); }
Tools
Biome
[error] 543-543: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 543-543: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
506-509: Remove unused
DEV
label.The
DEV
label is not used by any break or continue statement and should be removed.- // eslint-disable-next-line no-unused-labels -- dev code path - DEV: { - if (!runtimeState.timer.duration) { - throw new Error('runtimeState.update: invalid state received'); - } - }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.if (!runtimeState.timer.duration) { throw new Error('runtimeState.update: invalid state received'); }
Tools
Biome
[error] 506-506: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 506-506: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
449-453: Remove unused
DEV
label.The
DEV
label is not used by any break or continue statement and should be removed.- // eslint-disable-next-line no-unused-labels -- dev code path - DEV: { - if (runtimeState.timer.expectedFinish === null) { - throw new Error('runtimeState.addTime: invalid state received'); - } - }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.if (runtimeState.timer.expectedFinish === null) { throw new Error('runtimeState.addTime: invalid state received'); }
Tools
Biome
[error] 449-449: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 449-449: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Outside diff range, codebase verification and nitpick comments (3)
packages/utils/src/date-utils/checkIsNow.ts (1)
1-1
: Consider renamingdayInMs
toMILLIS_PER_DAY
for consistency.The constant
dayInMs
can be renamed toMILLIS_PER_DAY
to maintain consistency withMILLIS_PER_HOUR
in the test file.- import { dayInMs } from './conversionUtils.js'; + import { MILLIS_PER_DAY } from './conversionUtils.js';packages/utils/src/date-utils/checkIsNow.test.ts (1)
1-2
: Consider renamingMILLIS_PER_HOUR
toMILLIS_PER_HOUR
for consistency.Ensure that the naming of constants is consistent across the codebase.
- import { MILLIS_PER_HOUR } from './conversionUtils'; + import { MILLIS_PER_HOUR } from './conversionUtils.js';apps/server/src/services/runtime-service/RuntimeService.ts (1)
4-4
: Ensure consistent import ordering.Consider grouping and ordering imports logically (e.g., external libraries, internal modules).
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
packages/utils/index.ts
is excluded by none and included by none
Files selected for processing (5)
- apps/server/src/services/rollUtils.ts (1 hunks)
- apps/server/src/services/runtime-service/RuntimeService.ts (12 hunks)
- apps/server/src/stores/runtimeState.ts (15 hunks)
- packages/utils/src/date-utils/checkIsNow.test.ts (1 hunks)
- packages/utils/src/date-utils/checkIsNow.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- apps/server/src/services/rollUtils.ts
Additional context used
Biome
apps/server/src/stores/runtimeState.ts
[error] 449-449: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 449-449: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
[error] 506-506: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 506-506: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
[error] 543-543: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 543-543: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
[error] 617-617: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 617-617: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
Additional comments not posted (17)
packages/utils/src/date-utils/checkIsNow.ts (1)
6-8
: Ensure correct handling of events rolling over midnight.The function correctly normalizes the end time for events that roll over midnight.
packages/utils/src/date-utils/checkIsNow.test.ts (4)
5-10
: Comprehensive test forcheckIsNow
function.The test correctly checks if the current time is between the start and end times.
12-17
: Comprehensive test forcheckIsNow
function.The test correctly checks if the current time is before the start time.
19-24
: Comprehensive test forcheckIsNow
function.The test correctly checks if the current time is after the end time.
26-28
: Comprehensive test forcheckIsNow
function with events rolling over midnight.The test correctly checks if the function handles events that roll over midnight.
apps/server/src/services/runtime-service/RuntimeService.ts (7)
44-44
: Update type declaration foreventTimer
.The type declaration for
eventTimer
has been updated fromTimerService
toEventTimer
.
56-57
: Update constructor parameter type.The constructor parameter type has been updated from
TimerService
toEventTimer
.
86-103
: Refactor roll logic for better readability.The roll logic has been refactored to handle different scenarios more clearly.
238-238
: Reminder: Review TODO comment.The
maybeUpdate
function contains a TODO comment indicating that the logic needs to be reviewed.
412-421
: Enhance logging for start method.The start method now includes logging for the new state after starting the event.
515-522
: Improve error handling inrollLoaded
method.The
rollLoaded
method now includes a try-catch block for better error handling.
528-561
: Improve error handling inroll
method.The
roll
method now includes a try-catch block for better error handling.apps/server/src/stores/runtimeState.ts (5)
171-183
: Verify the correct use ofPlayableEvent
.Ensure that the
PlayableEvent
type is correctly used throughout the function and that all references toevent
are valid.Verification successful
Verified the correct use of
PlayableEvent
.The
PlayableEvent
type is correctly used throughout theload
function and other related functions. All references toevent
are valid.
apps/server/src/stores/runtimeState.ts
: Verified correct usage ofPlayableEvent
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct usage of `PlayableEvent` type in the `load` function. # Test: Search for the `PlayableEvent` type usage. Expect: Correct usage throughout the function. rg --type-add 'ts' --type ts 'PlayableEvent' -A 10Length of output: 108
Script:
#!/bin/bash # Description: Verify the correct usage of `PlayableEvent` type in the `load` function. # Test: Search for the `PlayableEvent` type usage in TypeScript files. Expect: Correct usage throughout the function. rg 'PlayableEvent' -t ts -A 10Length of output: 32166
Line range hint
310-340
:
Verify the correct handling of hot reloading and edge cases.Ensure that the function correctly updates the current event and handles edge cases with roll.
Verification successful
Verified the correct handling of hot reloading and edge cases in the
reload
function.The function updates the current event and synchronizes the timer, handling different playback states and edge cases appropriately.
- The
reload
function correctly updatesruntimeState.eventNow
.- It synchronizes the timer with the new event details.
- Edge cases with roll are handled by adjusting the secondary target and timer.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct handling of hot reloading and edge cases in the `reload` function. # Test: Search for the hot reloading and edge case handling. Expect: Correct handling throughout the function. rg --type-add 'ts' --type ts 'reload' -A 10Length of output: 101
Script:
#!/bin/bash # Description: Verify the correct handling of hot reloading and edge cases in the `reload` function. # Test: Search for the `reload` function definition and include enough context lines for verification. rg 'export function reload' -A 50Length of output: 7476
555-657
: Verify the correct handling of playback transitions and timer accuracy.Ensure that the function correctly updates the playback state and timer properties.
Verification successful
Verified the correct handling of playback transitions and timer accuracy.
The
roll
function correctly updates the playback state and timer properties as expected.
- The function transitions between different playback states (
Playback.Play
,Playback.Roll
,Playback.Armed
).- It updates the timer properties (
expectedFinish
,duration
,current
,elapsed
,startedAt
,secondaryTimer
) accurately.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct handling of playback transitions and timer accuracy in the `roll` function. # Test: Search for the playback transitions and timer accuracy handling. Expect: Correct handling throughout the function. rg --type-add 'ts' --type ts 'roll' -A 10Length of output: 99
Script:
#!/bin/bash # Description: Verify the correct handling of playback transitions and timer accuracy in the `roll` function. # Test: Search for the playback transitions and timer accuracy handling. Expect: Correct handling throughout the function. rg 'playback' -A 10 apps/server/src/stores/runtimeState.ts rg 'timer' -A 10 apps/server/src/stores/runtimeState.tsLength of output: 20505
Tools
Biome
[error] 617-617: Unexpected label.
Only loops should be labeled.
The use of labels for other statements is suspicious and unfamiliar.(lint/suspicious/noConfusingLabels)
[error] 617-617: Unused label.
The label is not used by any break statement and continue statement.
Safe fix: Remove the unused label.(lint/correctness/noUnusedLabels)
264-294
: Verify the correct handling of event index.Ensure that the event index handling is correct and that the function correctly loads the next event and its public counterpart.
Verification successful
Verification successful: The event index is correctly handled within the
loadNext
function.The function properly checks for
null
values and iterates over the events starting from the correct index to find the next playable and public events.
apps/server/src/stores/runtimeState.ts
: TheloadNext
function correctly handles the event index.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct handling of event index in the `loadNext` function. # Test: Search for the event index usage. Expect: Correct handling throughout the function. rg --type-add 'ts' --type ts 'eventIndex' -A 10Length of output: 105
Script:
#!/bin/bash # Description: Verify the correct handling of event index in the `loadNext` function. # Test: Search for the event index usage in TypeScript files. Expect: Correct handling throughout the function. rg --type-add 'ts:*.ts' --type ts 'eventIndex' -A 10Length of output: 43441
Line range hint
213-254
:
Verify the correct handling of event index.Ensure that the event index handling is correct and that the function correctly loads the current event and its public counterpart.
Verification successful
The handling of
eventIndex
in theloadNow
function is correct. The function properly checks for null values, ensures valid indices, and updates the state accordingly. The backward iteration to find public events is also correctly implemented.
apps/server/src/stores/runtimeState.ts
: Verified the correct handling ofeventIndex
in theloadNow
function.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct handling of event index in the `loadNow` function. # Test: Search for the event index usage. Expect: Correct handling throughout the function. rg --type-add 'ts' --type ts 'eventIndex' -A 10Length of output: 105
Script:
#!/bin/bash # Description: Verify the correct handling of event index in the `loadNow` function. # Test: Search for the event index usage in TypeScript files. Expect: Correct handling throughout the function. rg 'eventIndex' -t ts -A 10Length of output: 43416
This PR attempts to restructure and provide documentation to roll mode
See the specification in the code
TODO
Implement roll()
Implement getRollTimers()
Implement onUpdate()
Revise edge case for hot reload
Revise edge cases for rundown over midnight
Revise edge case for multiple days
Revise edge cases around integrations
Revise edge cases around start and end time / planned start and planned end / actual start and expected end
how can roll account for delays?
how can roll account for added time