test: 100% branch coverage for renderer modules - #24
Merged
Conversation
Signed-off-by: Robbin "Roboroads" Schepers <robbin@deschepers.nl>
Signed-off-by: Robbin "Roboroads" Schepers <robbin@deschepers.nl>
Signed-off-by: Robbin "Roboroads" Schepers <robbin@deschepers.nl>
Signed-off-by: Robbin "Roboroads" Schepers <robbin@deschepers.nl>
Signed-off-by: Robbin "Roboroads" Schepers <robbin@deschepers.nl>
Signed-off-by: Robbin "Roboroads" Schepers <robbin@deschepers.nl>
Signed-off-by: Robbin "Roboroads" Schepers <robbin@deschepers.nl>
Signed-off-by: Robbin "Roboroads" Schepers <robbin@deschepers.nl>
Signed-off-by: Robbin Roboroads Schepers <robbin@deschepers.nl>
Signed-off-by: Robbin "Roboroads" Schepers <robbin@deschepers.nl>
Signed-off-by: Robbin Roboroads Schepers <robbin@deschepers.nl>
Signed-off-by: Robbin "Roboroads" Schepers <robbin@deschepers.nl>
Signed-off-by: Robbin Roboroads Schepers <robbin@deschepers.nl>
… gaps Signed-off-by: Robbin "Roboroads" Schepers <robbin@deschepers.nl>
Signed-off-by: Robbin "Roboroads" Schepers <robbin@deschepers.nl>
…aveformDisplay handleSeekBarClick Signed-off-by: Robbin Roboroads Schepers <robbin@deschepers.nl>
Signed-off-by: Robbin "Roboroads" Schepers <robbin@deschepers.nl>
Signed-off-by: Robbin Roboroads Schepers <robbin@deschepers.nl>
Signed-off-by: Robbin Roboroads Schepers <robbin@deschepers.nl>
Signed-off-by: Robbin "Roboroads" Schepers <robbin@deschepers.nl>
Signed-off-by: Robbin "Roboroads" Schepers <robbin@deschepers.nl>
Signed-off-by: Robbin "Roboroads" Schepers <robbin@deschepers.nl>
Signed-off-by: Robbin "Roboroads" Schepers <robbin@deschepers.nl>
Signed-off-by: Robbin Roboroads Schepers <robbin@deschepers.nl>
Signed-off-by: Robbin "Roboroads" Schepers <robbin@deschepers.nl>
Signed-off-by: Robbin "Roboroads" Schepers <robbin@deschepers.nl>
Signed-off-by: Robbin Roboroads Schepers <robbin@deschepers.nl>
Signed-off-by: Robbin "Roboroads" Schepers <robbin@deschepers.nl>
Signed-off-by: Robbin "Roboroads" Schepers <robbin@deschepers.nl>
|
✅ All compliance checks are now passing — thanks for updating your PR! |
Signed-off-by: Robbin "Roboroads" Schepers <robbin@deschepers.nl>
Signed-off-by: Robbin Roboroads Schepers <robbin@deschepers.nl>
Signed-off-by: Robbin "Roboroads" Schepers <robbin@deschepers.nl>
Signed-off-by: Robbin Roboroads Schepers <robbin@deschepers.nl>
Signed-off-by: Robbin "Roboroads" Schepers <robbin@deschepers.nl>
Signed-off-by: Robbin Roboroads Schepers <robbin@deschepers.nl>
…utput Signed-off-by: Robbin "Roboroads" Schepers <robbin@deschepers.nl>
Use v8 ignore start/stop around keyed {#each} blocks whose compiler-generated
UPDATE path is unreachable in jsdom; fix LocalOutput ignore count off-by-one.
Signed-off-by: Robbin "Roboroads" Schepers <robbin@deschepers.nl>
Roboroads
force-pushed
the
feat/test-coverage
branch
from
June 4, 2026 10:11
77b9cd7 to
7ce68bb
Compare
The electron package requires a downloaded binary; loading it in Node/jsdom crashes without it. Mocking at the module level prevents the binary lookup. Signed-off-by: Robbin "Roboroads" Schepers <robbin@deschepers.nl>
Roboroads
marked this pull request as ready for review
June 4, 2026 11:32
Roboroads
enabled auto-merge (squash)
June 4, 2026 11:44
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds CI test execution, Vitest coverage configuration, renderer and main-process tests, and coverage-ignore annotations across audio, UI, windowing, IPC, and library code. ChangesUnified test and coverage expansion
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes |
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/shared/package.json (1)
23-31:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPin
@vitest/coverage-v8to the samevitestversion
packages/shared/package.jsonhas@vitest/coverage-v8at^4.1.8whilevitestis^2.0.0(also mismatched inpackages/audio-worklet/package.json). Vitest requires these packages to be kept at the exact same version; otherwise coverage collection can fail at runtime."`@vitest/coverage-v8`": "^4.1.8", "vitest": "^2.0.0"Update
@vitest/coverage-v8to matchvitest(aspackages/streamline/package.jsonalready does with^2.0.0) and refresh the lockfile.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/shared/package.json` around lines 23 - 31, The dependency "`@vitest/coverage-v8`" in packages/shared/package.json is pinned to "^4.1.8" while "vitest" is "^2.0.0"; update the "`@vitest/coverage-v8`" entry to the same version spec as "vitest" (e.g., "^2.0.0") so both packages match (also check packages/audio-worklet/package.json for the same mismatch), then run your package manager to regenerate the lockfile (install/update) so the lockfile reflects the change.packages/streamline/renderer/src/audio/mixer-bridge.ts (1)
64-71:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPrevent duplicate destination connections when toggling soft clip.
Line 70 reconnects
tapNodetoaudioCtx.destinationevery call, but this path is never disconnected here. RepeatedsetSoftClipEnabled(...)calls can accumulate connections and double-route audio. Add a targeted disconnect (or connect once during init) before reconnecting.Proposed fix
export function setSoftClipEnabled(enabled: boolean): void { if (!masterBus || !softClipper || !tapNode) return; @@ if (enabled) { masterBus.connect(softClipper); softClipper.connect(tapNode); } else { masterBus.connect(tapNode); } + try { + tapNode.disconnect(getAudioContext().destination); + } catch { + /* not connected */ + } tapNode.connect(getAudioContext().destination); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/streamline/renderer/src/audio/mixer-bridge.ts` around lines 64 - 71, The tapNode is being reconnected to the audio destination every time soft clipping is toggled which can create duplicate connections; modify the toggle logic around masterBus, softClipper and tapNode so you first remove any existing connection from tapNode to getAudioContext().destination (e.g. call a targeted disconnect on tapNode for that destination) before re-routing (masterBus.connect(softClipper) / softClipper.connect(tapNode) or masterBus.connect(tapNode)), or alternatively move the tapNode.connect(getAudioContext().destination) to the one-time initialization path so it is only connected once; locate and update the code that performs these connects involving masterBus, softClipper, tapNode and getAudioContext() to ensure a single destination connection.
🧹 Nitpick comments (2)
packages/streamline/renderer/src/hotkeys/reset-to-default.ts (1)
39-40: 🏗️ Heavy liftAvoid blanket coverage ignore on the full reset flow.
Line 39 suppresses coverage for a large stateful path (
resetToDefault) that is mock-testable. Prefer testing this flow and limiting ignore pragmas to genuinely unreachable branches.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/streamline/renderer/src/hotkeys/reset-to-default.ts` around lines 39 - 40, The coverage suppression currently applied over the entire resetToDefault function hides a mock-testable stateful flow; remove or narrow the /* v8 ignore next 32 */ pragma so only truly unreachable branches are ignored, and add unit tests that mock the IPC channel and the involved stores to exercise resetToDefault; specifically, extract any complex sub-steps inside resetToDefault (e.g., IPC call handlers, store reset helpers) into smaller functions (helpers referenced by resetToDefault) so you can unit-test those pieces, then write tests that simulate successful and error IPC responses and verify store resets and side effects rather than suppressing coverage for the whole function.packages/streamline/renderer/src/windows/WindowManager.component.spec.ts (1)
35-64: ⚡ Quick winMove
window.streamlinesetup/cleanup to hooks to avoid leakage on failed assertions.Right now cleanup is inline in each test; if a test throws before
delete, subsequent tests can inherit stale global state.♻️ Suggested refactor
describe('WindowManager', () => { + beforeEach(() => { + allMapMock.clear(); + (window as unknown as Record<string, unknown>).streamline = { + api: { + layout: { + list: vi.fn().mockResolvedValue([]), + load: vi.fn().mockResolvedValue(null) + } + } + }; + }); + + afterEach(() => { + delete (window as unknown as Record<string, unknown>).streamline; + }); + it('renders without crashing when instanceStore is empty', () => { - allMapMock.clear(); - (window as unknown as Record<string, unknown>).streamline = { - api: { - layout: { - list: vi.fn().mockResolvedValue([]), - load: vi.fn().mockResolvedValue(null) - } - } - }; const { container } = render(WindowManager); expect(container.querySelector('div.absolute')).toBeTruthy(); - delete (window as unknown as Record<string, unknown>).streamline; }); it('renders wrapper container', () => { - allMapMock.clear(); - (window as unknown as Record<string, unknown>).streamline = { - api: { - layout: { - list: vi.fn().mockResolvedValue([]), - load: vi.fn().mockResolvedValue(null) - } - } - }; const { container } = render(WindowManager); const root = container.firstElementChild; expect(root?.className).toContain('absolute'); - delete (window as unknown as Record<string, unknown>).streamline; }); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/streamline/renderer/src/windows/WindowManager.component.spec.ts` around lines 35 - 64, The tests set and delete the global (window as unknown as Record<string, unknown>).streamline inline which can leak state if a test throws; move the setup into a beforeEach that calls allMapMock.clear() and assigns window.streamline.api.layout.list and .load to vi.fn().mockResolvedValue(...) and move the deletion into an afterEach that deletes window.streamline so each spec (e.g., the two tests referencing WindowManager) runs with a fresh global and always cleans up; keep test bodies focused on rendering assertions and reuse the same mock functions created in the hooks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/test.yml:
- Around line 12-14: The workflow uses mutable action tags and leaves checkout
credentials persisted; update the three actions referenced (actions/checkout,
pnpm/action-setup, actions/setup-node) to be pinned to their respective full
commit SHAs instead of tags and set persist-credentials: false on the
actions/checkout step; locate the checkout step named actions/checkout@v4 and
add persist-credentials: false, and replace the tag strings for
pnpm/action-setup@v4 and actions/setup-node@v4 with their exact commit SHA pins.
In `@packages/audio-worklet/package.json`:
- Line 31: The package.json has a major-version mismatch between vitest and
`@vitest/coverage-v8`: the dependency "`@vitest/coverage-v8`" is ^4.1.8 while
"vitest" is ^2.0.0; update them to be compatible by either upgrading "vitest" to
a v4.x release (e.g., ^4.1.8) to match "`@vitest/coverage-v8`" or downgrade/pin
"`@vitest/coverage-v8`" to a v2-compatible release so their major versions align;
modify the package.json dependency entries for "vitest" and/or
"`@vitest/coverage-v8`" accordingly and run install to verify CI coverage works.
In `@packages/audio-worklet/vitest.config.mts`:
- Line 9: Update the coverage thresholds object so it enforces non-zero gates
instead of disabling them; replace the current thresholds: { lines: 0,
functions: 0, branches: 0, statements: 0 } with stricter values (at minimum set
branches: 100, and ideally set lines, functions, and statements to 100) in the
vitest configuration where the thresholds object is defined so the coverage gate
will fail on regressions.
In
`@packages/streamline/renderer/src/components/forms/NumberField.component.spec.ts`:
- Around line 22-27: The test "clamps to min on input" in
NumberField.component.spec.ts currently asserts the raw typed value ('2')
instead of the expected clamped minimum; update the assertion to expect the min
value (e.g., '10') after firing the input event on the rendered NumberField
(render(NumberField, { label: 'L', value: 50, min: 10 })) using the same input
element variable so the test verifies that NumberField enforces the min clamp.
In `@packages/streamline/renderer/src/hotkeys/binder.spec.ts`:
- Around line 36-40: The test should assert teardown removes the exact function
that was registered, not any Function; spy on window.addEventListener when
calling initHotkeyBinder(), capture the handler argument from the
addEventListener call (from the call args for 'keydown'), then call the returned
teardown and assert window.removeEventListener was called with 'keydown' and
that same captured handler (use the exact reference from the add spy) instead of
expect.any(Function); reference initHotkeyBinder and the teardown variable in
the assertion.
In `@packages/streamline/renderer/src/hotkeys/reset-to-default.ts`:
- Around line 74-75: The call to resetToDefault() can reject and currently is
invoked without a rejection handler; update the invocation of resetToDefault
(the reset-to-default IPC flow) to handle promise failures—either await it
inside a try/catch or append a .catch(err => /* handle/log error */) (e.g. void
resetToDefault().catch(err => processLogger.error("resetToDefault failed",
err))); ensure the catch logs or otherwise handles the error to prevent
unhandled promise rejections.
In `@packages/streamline/renderer/src/modules/context.spec.ts`:
- Around line 30-52: The tests create spies on console methods inside each it
block (vi.spyOn(console, 'info'|'warn'|'error')) but call spy.mockRestore()
inside the test body which can be skipped on failures; move restoration to a
shared afterEach to guarantee cleanup. Update the spec to keep references to the
spies (or re-spy per test) and add an afterEach that calls mockRestore() for any
active spies, or alternatively use vi.restoreAllMocks() in afterEach; apply this
change for the tests that use createModuleContext and ctx.log (the tests
referencing console.info/warn/error and createModuleContext).
In `@packages/streamline/renderer/src/modules/deck/Deck.component.spec.ts`:
- Around line 303-314: The test in Deck.component.spec.ts is race-prone because
it queries the close button globally and asserts dialog removal synchronously;
instead, scope the queries to the opened dialog and wait for its removal: after
opening the modal via render(Deck) and clicking the gearButton, locate the
dialog element (e.g., via getByRole('dialog') or
document.querySelector('[role="dialog"]')), use within(dialog) to find the close
button rather than a global query (closeButton), click that scoped close button,
and then use waitFor or waitForElementToBeRemoved on the dialog element to
assert the modal is removed.
In `@packages/streamline/renderer/src/modules/deck/Deck.svelte`:
- Around line 384-389: The test-only global window.__deck_setArtwork assigned in
Deck.svelte (the block that sets (window as unknown as { __deck_setArtwork?:
(url: string | null) => void }).__deck_setArtwork = (url) => { artworkDataUrl =
url; }) must be cleared during component teardown; add an onDestroy handler
(imported from 'svelte' if needed) that, when import.meta.env.MODE === 'test',
removes or sets window.__deck_setArtwork = undefined (or delete
window.__deck_setArtwork) to avoid leaking the closure across tests and ensure
artworkDataUrl is not referenced after destroy.
In `@packages/streamline/src/encoders/encoder-process.spec.ts`:
- Around line 157-161: The test emits stderr data but never asserts the logging
side-effect; update the test to spy/mock the logger and assert a debug call when
stderr data is emitted: create a spy for the logger's debug method (or a mock
logger object) and pass it into EncoderProcess (the same constructor used in the
test), call encoderProcess.start(), emit on (fakeProcess.stderr as EventEmitter)
the Buffer.from('ffmpeg output'), then assert the spy was called with a message
containing 'ffmpeg output' (or otherwise matches the expected log string) to
verify EncoderProcess handles stderr events and logs via its logger.
In `@packages/streamline/src/encoders/ffmpeg-path.spec.ts`:
- Around line 24-29: The test currently only compares two identical return
values which would pass even without caching; modify the spec for getFFmpegPath
so it proves caching by making the mocked ffmpeg-static change (or its factory
record calls) after the first invocation and asserting the second
getFFmpegPath() still returns the original value and that the ffmpeg-static
mock/factory was not invoked again. Concretely, adjust the vi.doMock for
'ffmpeg-static' to expose a mutable return or a call counter, call
getFFmpegPath() once to capture the initial path, change the mock’s return (or
assert the counter) and then call getFFmpegPath() again expecting the initial
cached path and no additional mock invocation. Ensure you reference
getFFmpegPath and the 'ffmpeg-static' mock in the updated assertions.
In `@packages/streamline/src/encoders/ffmpeg-path.ts`:
- Around line 9-13: The code currently calls ffmpegStatic!.replace(...) which
will throw if ffmpegStatic is null; update the logic around ffmpegStatic and
_path to first check if ffmpegStatic is non-null (e.g., if (!ffmpegStatic) throw
new Error('ffmpeg-static binary not found') or return a sensible fallback), then
perform the replace for the app.asar path only when ffmpegStatic is a string;
ensure all uses of ffmpegStatic and _path (the variables in this block) handle
the null case rather than force-unwrapping with !.
In `@packages/streamline/src/encoders/manager.spec.ts`:
- Around line 119-132: The test currently only asserts mockEncoderStart was
invoked; update it to also validate the password callback passed by startEncoder
returns null for configs without passwordRef: after calling
startEncoder(fileConfig, fakeWindow) and asserting mockEncoderStart was called,
inspect the arguments of mockEncoderStart (via its mock.calls), locate the
password callback argument supplied to the encoder constructor invoked by
startEncoder, call that callback and assert it returns null. Ensure you
reference startEncoder and mockEncoderStart when locating and testing the
callback.
In `@packages/streamline/src/ipc/handlers/encoder.spec.ts`:
- Around line 48-52: The tests mutate shared encoder window state via
setEncoderWindow(fakeWindow) but the beforeEach only clears mocks; update
beforeEach in encoder.spec.ts to also reset the encoder window state (either
call a provided reset function or call setEncoderWindow(undefined) / null)
before each test so registerEncoderHandlers() runs against a clean state;
specifically add a call to reset the encoder window state alongside
vi.clearAllMocks() and before registerEncoderHandlers() to avoid order-dependent
tests involving setEncoderWindow and registerEncoderHandlers.
In `@packages/streamline/src/ipc/handlers/hotkeys.spec.ts`:
- Around line 41-57: Tests call the IPC handlers without awaiting their
promises, which can let async failures go unnoticed; make the two tests async
and await the handler invocations returned by getHandler('hotkey:save') and
getHandler('hotkey:delete') respectively (i.e., change the test callbacks to
async and prefix getHandler(...)(...) with await) so mockDb assertions run after
the handler completes.
In `@packages/streamline/src/ipc/handlers/settings.spec.ts`:
- Around line 32-60: Tests share mocked state across cases causing flaky
expectations; add test isolation by clearing/resetting mocks before each test.
In the spec add a beforeEach/afterEach that calls vi.resetAllMocks() or
vi.clearAllMocks() and explicitly clears any mocked ipcMain.handle and mockDb
methods (e.g., vi.mocked(ipcMain.handle).mockClear(), mockDb.get.mockClear(),
mockDb.values.mockClear(), mockDb.run.mockClear()) so registerSettingsHandlers,
getHandler, and mockDb call counts are fresh for every it block.
In `@packages/streamline/src/library/watcher.spec.ts`:
- Line 22: The test variable chokidarInstance should be made optional and
explicitly reset in a beforeEach to avoid order-dependent failures: change its
declaration (chokidarInstance) to an optional type (e.g., add | undefined) and
add a beforeEach that sets chokidarInstance = undefined (and any mock reset you
need) so each test starts with a clean watcher state; update any places that
assume non-null to handle the optional (or assert existence in the specific
tests that require it).
---
Outside diff comments:
In `@packages/shared/package.json`:
- Around line 23-31: The dependency "`@vitest/coverage-v8`" in
packages/shared/package.json is pinned to "^4.1.8" while "vitest" is "^2.0.0";
update the "`@vitest/coverage-v8`" entry to the same version spec as "vitest"
(e.g., "^2.0.0") so both packages match (also check
packages/audio-worklet/package.json for the same mismatch), then run your
package manager to regenerate the lockfile (install/update) so the lockfile
reflects the change.
In `@packages/streamline/renderer/src/audio/mixer-bridge.ts`:
- Around line 64-71: The tapNode is being reconnected to the audio destination
every time soft clipping is toggled which can create duplicate connections;
modify the toggle logic around masterBus, softClipper and tapNode so you first
remove any existing connection from tapNode to getAudioContext().destination
(e.g. call a targeted disconnect on tapNode for that destination) before
re-routing (masterBus.connect(softClipper) / softClipper.connect(tapNode) or
masterBus.connect(tapNode)), or alternatively move the
tapNode.connect(getAudioContext().destination) to the one-time initialization
path so it is only connected once; locate and update the code that performs
these connects involving masterBus, softClipper, tapNode and getAudioContext()
to ensure a single destination connection.
---
Nitpick comments:
In `@packages/streamline/renderer/src/hotkeys/reset-to-default.ts`:
- Around line 39-40: The coverage suppression currently applied over the entire
resetToDefault function hides a mock-testable stateful flow; remove or narrow
the /* v8 ignore next 32 */ pragma so only truly unreachable branches are
ignored, and add unit tests that mock the IPC channel and the involved stores to
exercise resetToDefault; specifically, extract any complex sub-steps inside
resetToDefault (e.g., IPC call handlers, store reset helpers) into smaller
functions (helpers referenced by resetToDefault) so you can unit-test those
pieces, then write tests that simulate successful and error IPC responses and
verify store resets and side effects rather than suppressing coverage for the
whole function.
In `@packages/streamline/renderer/src/windows/WindowManager.component.spec.ts`:
- Around line 35-64: The tests set and delete the global (window as unknown as
Record<string, unknown>).streamline inline which can leak state if a test
throws; move the setup into a beforeEach that calls allMapMock.clear() and
assigns window.streamline.api.layout.list and .load to
vi.fn().mockResolvedValue(...) and move the deletion into an afterEach that
deletes window.streamline so each spec (e.g., the two tests referencing
WindowManager) runs with a fresh global and always cleans up; keep test bodies
focused on rendering assertions and reuse the same mock functions created in the
hooks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1effd62e-e16b-4eaa-b25b-26c4adb87e0e
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (97)
.github/workflows/test.yml.idea/vcs.xmlpackage.jsonpackages/audio-worklet/package.jsonpackages/audio-worklet/vitest.config.mtspackages/shared/package.jsonpackages/shared/vitest.config.mtspackages/streamline/package.jsonpackages/streamline/renderer/src/audio/context.spec.tspackages/streamline/renderer/src/audio/context.tspackages/streamline/renderer/src/audio/mixer-bridge.spec.tspackages/streamline/renderer/src/audio/mixer-bridge.tspackages/streamline/renderer/src/audio/port.spec.tspackages/streamline/renderer/src/components/DbMeter.component.spec.tspackages/streamline/renderer/src/components/IconButton.component.spec.tspackages/streamline/renderer/src/components/Modal.component.spec.tspackages/streamline/renderer/src/components/Modal.sveltepackages/streamline/renderer/src/components/OnAirIndicator.component.spec.tspackages/streamline/renderer/src/components/StatusBar.component.spec.tspackages/streamline/renderer/src/components/StatusBar.sveltepackages/streamline/renderer/src/components/Toast.component.spec.tspackages/streamline/renderer/src/components/ToastContainer.component.spec.tspackages/streamline/renderer/src/components/forms/BitrateField.component.spec.tspackages/streamline/renderer/src/components/forms/ComboField.component.spec.tspackages/streamline/renderer/src/components/forms/ComboField.sveltepackages/streamline/renderer/src/components/forms/FilenameField.component.spec.tspackages/streamline/renderer/src/components/forms/FolderField.component.spec.tspackages/streamline/renderer/src/components/forms/FormField.component.spec.tspackages/streamline/renderer/src/components/forms/NumberField.component.spec.tspackages/streamline/renderer/src/components/forms/NumberField.sveltepackages/streamline/renderer/src/components/forms/SampleRateField.component.spec.tspackages/streamline/renderer/src/components/forms/SelectField.component.spec.tspackages/streamline/renderer/src/components/forms/TextField.component.spec.tspackages/streamline/renderer/src/drag-drop/song-drag.spec.tspackages/streamline/renderer/src/first-run/setup.spec.tspackages/streamline/renderer/src/hotkeys/binder.spec.tspackages/streamline/renderer/src/hotkeys/reset-to-default.spec.tspackages/streamline/renderer/src/hotkeys/reset-to-default.tspackages/streamline/renderer/src/hotkeys/store.svelte.spec.tspackages/streamline/renderer/src/layout/persistence.svelte.spec.tspackages/streamline/renderer/src/layout/persistence.svelte.tspackages/streamline/renderer/src/layout/store.svelte.spec.tspackages/streamline/renderer/src/modules/context.spec.tspackages/streamline/renderer/src/modules/crossfader/Crossfader.component.spec.tspackages/streamline/renderer/src/modules/crossfader/Crossfader.sveltepackages/streamline/renderer/src/modules/deck/Deck.component.spec.tspackages/streamline/renderer/src/modules/deck/Deck.sveltepackages/streamline/renderer/src/modules/deck/DeckSettingsModal.component.spec.tspackages/streamline/renderer/src/modules/deck/WaveformDisplay.component.spec.tspackages/streamline/renderer/src/modules/deck/WaveformDisplay.sveltepackages/streamline/renderer/src/modules/deck/deck-audio.tspackages/streamline/renderer/src/modules/deck/waveform-worker.spec.tspackages/streamline/renderer/src/modules/deck/waveform-worker.tspackages/streamline/renderer/src/modules/encoders/EncoderModal.component.spec.tspackages/streamline/renderer/src/modules/encoders/EncoderModal.sveltepackages/streamline/renderer/src/modules/encoders/Encoders.component.spec.tspackages/streamline/renderer/src/modules/encoders/Encoders.sveltepackages/streamline/renderer/src/modules/instance-store.svelte.spec.tspackages/streamline/renderer/src/modules/local-output/LocalOutput.component.spec.tspackages/streamline/renderer/src/modules/local-output/LocalOutput.sveltepackages/streamline/renderer/src/modules/microphone/Microphone.component.spec.tspackages/streamline/renderer/src/modules/microphone/Microphone.sveltepackages/streamline/renderer/src/modules/queue/Queue.component.spec.tspackages/streamline/renderer/src/modules/queue/Queue.sveltepackages/streamline/renderer/src/modules/queue/QueueSettingsModal.component.spec.tspackages/streamline/renderer/src/modules/queue/rotation.tspackages/streamline/renderer/src/modules/register-builtins.spec.tspackages/streamline/renderer/src/windows/WindowManager.component.spec.tspackages/streamline/renderer/src/windows/WindowManager.sveltepackages/streamline/renderer/src/windows/WindowWrapper.component.spec.tspackages/streamline/renderer/src/windows/WindowWrapper.sveltepackages/streamline/renderer/src/windows/use-interact.spec.tspackages/streamline/renderer/src/windows/use-interact.tspackages/streamline/src/audio/pcm-receiver.spec.tspackages/streamline/src/audio/pcm-receiver.tspackages/streamline/src/audio/port.spec.tspackages/streamline/src/db/migrate.spec.tspackages/streamline/src/encoders/encoder-process.spec.tspackages/streamline/src/encoders/encoder-process.tspackages/streamline/src/encoders/ffmpeg-path.spec.tspackages/streamline/src/encoders/ffmpeg-path.tspackages/streamline/src/encoders/manager.spec.tspackages/streamline/src/encoders/manager.tspackages/streamline/src/ipc/handlers/encoder.spec.tspackages/streamline/src/ipc/handlers/hotkeys.spec.tspackages/streamline/src/ipc/handlers/layout.spec.tspackages/streamline/src/ipc/handlers/library.spec.tspackages/streamline/src/ipc/handlers/secret.spec.tspackages/streamline/src/ipc/handlers/settings.spec.tspackages/streamline/src/ipc/handlers/system.spec.tspackages/streamline/src/ipc/handlers/system.tspackages/streamline/src/library/scan-worker.tspackages/streamline/src/library/scanner.spec.tspackages/streamline/src/library/watcher.spec.tspackages/streamline/src/update-check.spec.tspackages/streamline/src/update-check.tspackages/streamline/vitest.config.mts
💤 Files with no reviewable changes (1)
- .idea/vcs.xml
…icate audio connection Signed-off-by: Robbin "Roboroads" Schepers <robbin@deschepers.nl>
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/streamline/renderer/src/audio/mixer-bridge.ts (1)
50-69:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPrevent duplicate
softClipper → tapNodeconnections when toggling soft clip repeatedly.Repeated
setSoftClipEnabled(true)calls can create parallel duplicate edges becausesoftClipper.disconnect(tapNode)is never done before reconnecting, which can increase output level unexpectedly.Suggested fix
export function setSoftClipEnabled(enabled: boolean): void { if (!masterBus || !softClipper || !tapNode) return; // Use targeted disconnects to avoid severing LocalOutput gainNode connections try { masterBus.disconnect(softClipper); } catch { /* not connected */ } try { masterBus.disconnect(tapNode); } catch { /* not connected */ } + try { + softClipper.disconnect(tapNode); + } catch { + /* not connected */ + } if (enabled) { masterBus.connect(softClipper); softClipper.connect(tapNode); } else { masterBus.connect(tapNode); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/streamline/renderer/src/audio/mixer-bridge.ts` around lines 50 - 69, setSoftClipEnabled currently reconnects softClipper → tapNode on repeated enables without ever disconnecting that edge, causing duplicate parallel connections; update setSoftClipEnabled to explicitly disconnect softClipper from tapNode (wrapped in a try/catch like the other disconnects) before calling softClipper.connect(tapNode) when enabled, and ensure you likewise disconnect softClipper from tapNode when disabling so only one path (masterBus→tapNode or masterBus→softClipper→tapNode) exists; reference the setSoftClipEnabled function and the masterBus, softClipper, and tapNode nodes when making this change.packages/streamline/renderer/src/modules/deck/WaveformDisplay.svelte (1)
54-65:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGuard zero-width click geometry before computing seek ratio.
When
rect.widthis0, both click handlers can emit invalid seek values. Add a width guard before dividing.Suggested fix
function handleCanvasClick(e: MouseEvent) { if (!canvas || duration === 0) return; const rect = canvas.getBoundingClientRect(); + if (rect.width <= 0) return; onSeek(((e.clientX - rect.left) / rect.width) * duration); } function handleSeekBarClick(e: MouseEvent) { if (duration === 0) return; const rect = (e.currentTarget as HTMLElement).getBoundingClientRect(); + if (rect.width <= 0) return; onSeek(((e.clientX - rect.left) / rect.width) * duration); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/streamline/renderer/src/modules/deck/WaveformDisplay.svelte` around lines 54 - 65, Both click handlers compute a seek ratio by dividing by rect.width without guarding for zero; update handleCanvasClick and handleSeekBarClick to early-return when rect.width is 0 (or <= 0) before performing the division so you never pass NaN/Infinity into onSeek. Specifically, after obtaining rect via getBoundingClientRect() in handleCanvasClick and handleSeekBarClick, check if rect.width <= 0 and return early (preserving existing duration checks) to avoid invalid seek values.
♻️ Duplicate comments (2)
packages/streamline/renderer/src/modules/deck/Deck.component.spec.ts (1)
303-315:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winWait for dialog removal after clicking Close.
Line 314 asserts synchronously; this can still race modal teardown and intermittently fail.
Suggested fix
await fireEvent.click(closeButton); - expect(document.querySelector('[role="dialog"]')).toBeNull(); + await waitFor(() => { + expect(document.querySelector('[role="dialog"]')).toBeNull(); + }); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/streamline/renderer/src/modules/deck/Deck.component.spec.ts` around lines 303 - 315, The test currently clicks the Close button and then asserts synchronously that the dialog is gone, which can race; after locating closeButton (from dialog.querySelector) and firing the click, change the final assertion to wait for the dialog to be removed (e.g., use waitFor or waitForElementToBeRemoved) targeting the same selector '[role="dialog"]' so the test awaits modal teardown instead of asserting immediately.packages/streamline/src/encoders/ffmpeg-path.spec.ts (1)
24-29:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStrengthen the cache test so it actually proves memoization.
Line 28 compares two identical calls and still passes if caching is removed. Mutate a mocked input between calls and assert the second result stays pinned to the first value.
Suggested fix
it('caches the result on repeated calls', async () => { - vi.doMock('electron', () => ({ app: { isPackaged: false } })); - vi.doMock('ffmpeg-static', () => ({ default: '/path/ffmpeg' })); + const appMock = { isPackaged: true }; + vi.doMock('electron', () => ({ app: appMock })); + vi.doMock('ffmpeg-static', () => ({ + default: '/app/app.asar/node_modules/ffmpeg-static/bin/ffmpeg' + })); const { getFFmpegPath } = await import('./ffmpeg-path'); - expect(getFFmpegPath()).toBe(getFFmpegPath()); + const first = getFFmpegPath(); + appMock.isPackaged = false; + expect(getFFmpegPath()).toBe(first); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/streamline/src/encoders/ffmpeg-path.spec.ts` around lines 24 - 29, The test currently calls getFFmpegPath() twice with the same mocked input so it passes even without memoization; change the test to mock 'ffmpeg-static' to one value, import getFFmpegPath and call it once to capture the first result, then re-mock 'ffmpeg-static' to a different value and call getFFmpegPath() again and assert the second call equals the first. Reference getFFmpegPath and the 'ffmpeg-static' mock so the test proves the function caches/memoizes its result.
🧹 Nitpick comments (3)
packages/streamline/renderer/src/windows/WindowManager.component.spec.ts (1)
35-48: ⚡ Quick winMove
window.streamlinesetup/teardown into test hooks for isolation.
delete window.streamlineis inside each test body, so a failing assertion can leak global state into subsequent tests. UsebeforeEach/afterEachto make cleanup unconditional.Suggested refactor
describe('WindowManager', () => { + beforeEach(() => { + (window as unknown as Record<string, unknown>).streamline = { + api: { + layout: { + list: vi.fn().mockResolvedValue([]), + load: vi.fn().mockResolvedValue(null) + } + } + }; + }); + + afterEach(() => { + delete (window as unknown as Record<string, unknown>).streamline; + }); + it('renders without crashing when instanceStore is empty', () => { allMapMock.clear(); - (window as unknown as Record<string, unknown>).streamline = { - api: { - layout: { - list: vi.fn().mockResolvedValue([]), - load: vi.fn().mockResolvedValue(null) - } - } - }; const { container } = render(WindowManager); expect(container.querySelector('div.absolute')).toBeTruthy(); - delete (window as unknown as Record<string, unknown>).streamline; }); it('renders wrapper container', () => { allMapMock.clear(); - (window as unknown as Record<string, unknown>).streamline = { - api: { - layout: { - list: vi.fn().mockResolvedValue([]), - load: vi.fn().mockResolvedValue(null) - } - } - }; const { container } = render(WindowManager); const root = container.firstElementChild; expect(root?.className).toContain('absolute'); - delete (window as unknown as Record<string, unknown>).streamline; }); });Also applies to: 50-64
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/streamline/renderer/src/windows/WindowManager.component.spec.ts` around lines 35 - 48, The tests in WindowManager.component.spec.ts set window.streamline inside individual it blocks and delete it at the end, which can leak global state on failed assertions; move the setup into a beforeEach that assigns (window as unknown as Record<string, unknown>).streamline = { api: { layout: { list: vi.fn().mockResolvedValue([]), load: vi.fn().mockResolvedValue(null) } } } and move the cleanup into an afterEach that unconditionally deletes (window as unknown as Record<string, unknown>).streamline; update both the test at 'renders without crashing when instanceStore is empty' and the similar test around lines 50-64 to remove inline setup/teardown so they rely on the shared hooks and keep assertions unchanged.packages/streamline/renderer/src/windows/WindowWrapper.component.spec.ts (1)
45-161: ⚡ Quick winReset shared mocks between tests to keep assertions independent.
updateMock,bringToFrontMock, andupdateInstanceMockare shared across cases. Add abeforeEach(() => vi.clearAllMocks())in this suite to prevent state bleed as tests grow.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/streamline/renderer/src/windows/WindowWrapper.component.spec.ts` around lines 45 - 161, Tests share mocks (updateMock, bringToFrontMock, updateInstanceMock) causing state bleed; add a beforeEach in the WindowWrapper suite that calls vi.clearAllMocks() to reset these mocks between tests so each it block starts with clean mock state—insert a beforeEach(() => vi.clearAllMocks()) inside the describe('WindowWrapper', ...) block near the top of the file.package.json (1)
22-22: ⚡ Quick winAlign Vitest coverage provider major versions across the workspace.
Root now brings in
@vitest/coverage-v8@^4.1.8while the app package is on Vitest 2 + coverage-v8 2. Keeping these aligned (or removing the root copy if unused) reduces dependency drift and avoids accidental toolchain mismatch later.#!/bin/bash # Verify Vitest + coverage provider versions across workspace package manifests. fd -a "package.json" | xargs -I{} sh -c ' printf "\n== %s ==\n" "{}" jq -r '"'"' .devDependencies // {} | { vitest: .vitest, "`@vitest/coverage-v8`": .["`@vitest/coverage-v8`"] } | to_entries[] | select(.value != null) | "\(.key)=\(.value)" '"'"' "{}" '🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@package.json` at line 22, The root package.json currently lists "`@vitest/coverage-v8`": "^4.1.8" which mismatches the app package's Vitest + coverage provider major version; either update the root dependency to match the app's vitest major (e.g., align "`@vitest/coverage-v8`" to the same major as the app) or remove the root entry if it's unused. Locate the root package.json dependency "`@vitest/coverage-v8`" and either change its version string to the app's matching major version alongside "vitest" or delete the entry, then run yarn/npm install and re-run the workspace version-check script to confirm consistency.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/shared/vitest.config.mts`:
- Line 9: The coverage gate is effectively disabled because the thresholds
object in vitest.config.mts (the thresholds: { lines: 0, functions: 0, branches:
0, statements: 0 } entry) uses zeros; update that thresholds object to non-zero
enforced values (for example set branches: 100 and sensible minima for lines,
functions, and statements like 80–90 depending on policy) so CI fails on
coverage regressions—locate the thresholds object in the exported config and
replace the zero values with the agreed enforcement numbers.
In `@packages/streamline/renderer/src/windows/use-interact.ts`:
- Around line 17-22: Remove the misleading "v8 ignore next — fallback
unreachable" comments from the width/height getters in use-interact.ts so the
default fallbacks (opts.minWidth ?? 200 and opts.minHeight ?? 150) are treated
as covered logic; update tests to assert the fallback clamp behavior when
InteractOptions omits minWidth/minHeight. Locate the getters named width and
height in the useInteract hook (they reference opts.minWidth and opts.minHeight)
and simply delete the ignore annotations and add/adjust unit tests to validate
the 200x150 default clamp.
In `@packages/streamline/src/ipc/handlers/secret.spec.ts`:
- Around line 57-61: The test for SECRET_SET currently toggles encryption
availability with isAvailableMock.mockReturnValue(false) and asserts
encryptStringMock was called but never asserts the warning branch; update the
test (the it block using getHandler('secret:set')) to also assert that the
mocked warning logger was invoked (e.g., expect(warnMock).toHaveBeenCalled() or
expect(warnMock).toHaveBeenCalledWith(expect.stringContaining('encryption')/the
exact message used in the handler)) so the test fails if the warning emission
regresses; keep the existing encryptStringMock assertion and add the warning
assertion after invoking getHandler('secret:set')(null, 'ref', 'val').
---
Outside diff comments:
In `@packages/streamline/renderer/src/audio/mixer-bridge.ts`:
- Around line 50-69: setSoftClipEnabled currently reconnects softClipper →
tapNode on repeated enables without ever disconnecting that edge, causing
duplicate parallel connections; update setSoftClipEnabled to explicitly
disconnect softClipper from tapNode (wrapped in a try/catch like the other
disconnects) before calling softClipper.connect(tapNode) when enabled, and
ensure you likewise disconnect softClipper from tapNode when disabling so only
one path (masterBus→tapNode or masterBus→softClipper→tapNode) exists; reference
the setSoftClipEnabled function and the masterBus, softClipper, and tapNode
nodes when making this change.
In `@packages/streamline/renderer/src/modules/deck/WaveformDisplay.svelte`:
- Around line 54-65: Both click handlers compute a seek ratio by dividing by
rect.width without guarding for zero; update handleCanvasClick and
handleSeekBarClick to early-return when rect.width is 0 (or <= 0) before
performing the division so you never pass NaN/Infinity into onSeek.
Specifically, after obtaining rect via getBoundingClientRect() in
handleCanvasClick and handleSeekBarClick, check if rect.width <= 0 and return
early (preserving existing duration checks) to avoid invalid seek values.
---
Duplicate comments:
In `@packages/streamline/renderer/src/modules/deck/Deck.component.spec.ts`:
- Around line 303-315: The test currently clicks the Close button and then
asserts synchronously that the dialog is gone, which can race; after locating
closeButton (from dialog.querySelector) and firing the click, change the final
assertion to wait for the dialog to be removed (e.g., use waitFor or
waitForElementToBeRemoved) targeting the same selector '[role="dialog"]' so the
test awaits modal teardown instead of asserting immediately.
In `@packages/streamline/src/encoders/ffmpeg-path.spec.ts`:
- Around line 24-29: The test currently calls getFFmpegPath() twice with the
same mocked input so it passes even without memoization; change the test to mock
'ffmpeg-static' to one value, import getFFmpegPath and call it once to capture
the first result, then re-mock 'ffmpeg-static' to a different value and call
getFFmpegPath() again and assert the second call equals the first. Reference
getFFmpegPath and the 'ffmpeg-static' mock so the test proves the function
caches/memoizes its result.
---
Nitpick comments:
In `@package.json`:
- Line 22: The root package.json currently lists "`@vitest/coverage-v8`": "^4.1.8"
which mismatches the app package's Vitest + coverage provider major version;
either update the root dependency to match the app's vitest major (e.g., align
"`@vitest/coverage-v8`" to the same major as the app) or remove the root entry if
it's unused. Locate the root package.json dependency "`@vitest/coverage-v8`" and
either change its version string to the app's matching major version alongside
"vitest" or delete the entry, then run yarn/npm install and re-run the workspace
version-check script to confirm consistency.
In `@packages/streamline/renderer/src/windows/WindowManager.component.spec.ts`:
- Around line 35-48: The tests in WindowManager.component.spec.ts set
window.streamline inside individual it blocks and delete it at the end, which
can leak global state on failed assertions; move the setup into a beforeEach
that assigns (window as unknown as Record<string, unknown>).streamline = { api:
{ layout: { list: vi.fn().mockResolvedValue([]), load:
vi.fn().mockResolvedValue(null) } } } and move the cleanup into an afterEach
that unconditionally deletes (window as unknown as Record<string,
unknown>).streamline; update both the test at 'renders without crashing when
instanceStore is empty' and the similar test around lines 50-64 to remove inline
setup/teardown so they rely on the shared hooks and keep assertions unchanged.
In `@packages/streamline/renderer/src/windows/WindowWrapper.component.spec.ts`:
- Around line 45-161: Tests share mocks (updateMock, bringToFrontMock,
updateInstanceMock) causing state bleed; add a beforeEach in the WindowWrapper
suite that calls vi.clearAllMocks() to reset these mocks between tests so each
it block starts with clean mock state—insert a beforeEach(() =>
vi.clearAllMocks()) inside the describe('WindowWrapper', ...) block near the top
of the file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a39eae30-d6ad-4ae3-8dbe-71b649aee595
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (99)
.github/workflows/test.yml.idea/vcs.xmlpackage.jsonpackages/audio-worklet/package.jsonpackages/audio-worklet/src/tap-processor.tspackages/audio-worklet/vitest.config.mtspackages/shared/package.jsonpackages/shared/vitest.config.mtspackages/streamline/package.jsonpackages/streamline/renderer/src/audio/context.spec.tspackages/streamline/renderer/src/audio/context.tspackages/streamline/renderer/src/audio/mixer-bridge.spec.tspackages/streamline/renderer/src/audio/mixer-bridge.tspackages/streamline/renderer/src/audio/port.spec.tspackages/streamline/renderer/src/components/DbMeter.component.spec.tspackages/streamline/renderer/src/components/IconButton.component.spec.tspackages/streamline/renderer/src/components/Modal.component.spec.tspackages/streamline/renderer/src/components/Modal.sveltepackages/streamline/renderer/src/components/OnAirIndicator.component.spec.tspackages/streamline/renderer/src/components/StatusBar.component.spec.tspackages/streamline/renderer/src/components/StatusBar.sveltepackages/streamline/renderer/src/components/Toast.component.spec.tspackages/streamline/renderer/src/components/ToastContainer.component.spec.tspackages/streamline/renderer/src/components/forms/BitrateField.component.spec.tspackages/streamline/renderer/src/components/forms/ComboField.component.spec.tspackages/streamline/renderer/src/components/forms/ComboField.sveltepackages/streamline/renderer/src/components/forms/FilenameField.component.spec.tspackages/streamline/renderer/src/components/forms/FolderField.component.spec.tspackages/streamline/renderer/src/components/forms/FormField.component.spec.tspackages/streamline/renderer/src/components/forms/NumberField.component.spec.tspackages/streamline/renderer/src/components/forms/NumberField.sveltepackages/streamline/renderer/src/components/forms/SampleRateField.component.spec.tspackages/streamline/renderer/src/components/forms/SelectField.component.spec.tspackages/streamline/renderer/src/components/forms/TextField.component.spec.tspackages/streamline/renderer/src/drag-drop/song-drag.spec.tspackages/streamline/renderer/src/first-run/setup.spec.tspackages/streamline/renderer/src/hotkeys/binder.spec.tspackages/streamline/renderer/src/hotkeys/reset-to-default.spec.tspackages/streamline/renderer/src/hotkeys/reset-to-default.tspackages/streamline/renderer/src/hotkeys/store.svelte.spec.tspackages/streamline/renderer/src/layout/persistence.svelte.spec.tspackages/streamline/renderer/src/layout/persistence.svelte.tspackages/streamline/renderer/src/layout/store.svelte.spec.tspackages/streamline/renderer/src/modules/context.spec.tspackages/streamline/renderer/src/modules/crossfader/Crossfader.component.spec.tspackages/streamline/renderer/src/modules/crossfader/Crossfader.sveltepackages/streamline/renderer/src/modules/deck/Deck.component.spec.tspackages/streamline/renderer/src/modules/deck/Deck.sveltepackages/streamline/renderer/src/modules/deck/DeckSettingsModal.component.spec.tspackages/streamline/renderer/src/modules/deck/WaveformDisplay.component.spec.tspackages/streamline/renderer/src/modules/deck/WaveformDisplay.sveltepackages/streamline/renderer/src/modules/deck/deck-audio.tspackages/streamline/renderer/src/modules/deck/waveform-worker.spec.tspackages/streamline/renderer/src/modules/deck/waveform-worker.tspackages/streamline/renderer/src/modules/encoders/EncoderModal.component.spec.tspackages/streamline/renderer/src/modules/encoders/EncoderModal.sveltepackages/streamline/renderer/src/modules/encoders/Encoders.component.spec.tspackages/streamline/renderer/src/modules/encoders/Encoders.sveltepackages/streamline/renderer/src/modules/instance-store.svelte.spec.tspackages/streamline/renderer/src/modules/local-output/LocalOutput.component.spec.tspackages/streamline/renderer/src/modules/local-output/LocalOutput.sveltepackages/streamline/renderer/src/modules/microphone/Microphone.component.spec.tspackages/streamline/renderer/src/modules/microphone/Microphone.sveltepackages/streamline/renderer/src/modules/queue/Queue.component.spec.tspackages/streamline/renderer/src/modules/queue/Queue.sveltepackages/streamline/renderer/src/modules/queue/QueueSettingsModal.component.spec.tspackages/streamline/renderer/src/modules/queue/rotation.tspackages/streamline/renderer/src/modules/register-builtins.spec.tspackages/streamline/renderer/src/windows/WindowManager.component.spec.tspackages/streamline/renderer/src/windows/WindowManager.sveltepackages/streamline/renderer/src/windows/WindowWrapper.component.spec.tspackages/streamline/renderer/src/windows/WindowWrapper.sveltepackages/streamline/renderer/src/windows/use-interact.spec.tspackages/streamline/renderer/src/windows/use-interact.tspackages/streamline/src/audio/pcm-receiver.spec.tspackages/streamline/src/audio/pcm-receiver.tspackages/streamline/src/audio/port.spec.tspackages/streamline/src/db/migrate.spec.tspackages/streamline/src/encoders/encoder-process.spec.tspackages/streamline/src/encoders/encoder-process.tspackages/streamline/src/encoders/ffmpeg-path.spec.tspackages/streamline/src/encoders/ffmpeg-path.tspackages/streamline/src/encoders/manager.spec.tspackages/streamline/src/encoders/manager.tspackages/streamline/src/ipc/handlers/encoder.spec.tspackages/streamline/src/ipc/handlers/encoder.tspackages/streamline/src/ipc/handlers/hotkeys.spec.tspackages/streamline/src/ipc/handlers/layout.spec.tspackages/streamline/src/ipc/handlers/library.spec.tspackages/streamline/src/ipc/handlers/secret.spec.tspackages/streamline/src/ipc/handlers/settings.spec.tspackages/streamline/src/ipc/handlers/system.spec.tspackages/streamline/src/ipc/handlers/system.tspackages/streamline/src/library/scan-worker.tspackages/streamline/src/library/scanner.spec.tspackages/streamline/src/library/watcher.spec.tspackages/streamline/src/update-check.spec.tspackages/streamline/src/update-check.tspackages/streamline/vitest.config.mts
💤 Files with no reviewable changes (1)
- .idea/vcs.xml
… correctness, defensive guards Signed-off-by: Robbin "Roboroads" Schepers <robbin@deschepers.nl>
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/streamline/renderer/src/modules/encoders/EncoderModal.svelte (1)
72-100:⚠️ Potential issue | 🟠 Major | ⚡ Quick winIncomplete v8 ignore directive — else block not covered.
The coverage ignore at line 72 specifies
next 11, which covers only lines 73–83 (theif (type === 'file')branch). Theelsebranch (lines 85–100) also callsonSave()with network encoder parameters and is equally untestable in jsdom, yet it remains uncovered by the directive.🔧 Proposed fix
Extend the ignore range to cover the entire if/else block:
const sampleRateOut = snapSampleRate(sampleRate); - /* v8 ignore next 11 — save() requires Electron IPC (safeStorage); not testable in jsdom */ + /* v8 ignore next 28 — save() requires Electron IPC (safeStorage); not testable in jsdom */ if (type === 'file') { const pathTemplate = composePathTemplate(folder, filename); onSave({This will correctly exclude lines 73–100 (both the
ifandelsebranches) from coverage.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/streamline/renderer/src/modules/encoders/EncoderModal.svelte` around lines 72 - 100, The v8 coverage ignore currently only covers the `if (type === 'file')` branch; extend the ignore directive so it spans the entire if/else block that calls `onSave` (including the `composePathTemplate` usage and the `else` branch that passes `host`, `port`, `mount`, `username`, and `passwordRef`) so both branches are excluded from coverage; update the `/* v8 ignore next N */` count to cover all lines from the `if (type === 'file')` start through the end of the `else` block (the lines that invoke `onSave` in both branches).
♻️ Duplicate comments (2)
packages/streamline/src/ipc/handlers/hotkeys.spec.ts (1)
41-57:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAwait async handler invocations in save/delete tests.
Line 48 and Line 54 should be awaited so async handler failures cannot be skipped.
Suggested fix
- it('HOTKEY_SAVE upserts binding', () => { + it('HOTKEY_SAVE upserts binding', async () => { const binding: HotkeyBinding = { id: 'h1', instanceId: 'i1', action: 'play', accelerator: 'F1' }; - getHandler('hotkey:save')(null, binding); + await getHandler('hotkey:save')(null, binding); expect(mockDb.insert).toHaveBeenCalledOnce(); expect(mockDb.run).toHaveBeenCalledOnce(); }); - it('HOTKEY_DELETE deletes by id', () => { - getHandler('hotkey:delete')(null, 'h1'); + it('HOTKEY_DELETE deletes by id', async () => { + await getHandler('hotkey:delete')(null, 'h1'); expect(mockDb.delete).toHaveBeenCalledOnce(); expect(mockDb.run).toHaveBeenCalledOnce(); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/streamline/src/ipc/handlers/hotkeys.spec.ts` around lines 41 - 57, The tests "HOTKEY_SAVE upserts binding" and "HOTKEY_DELETE deletes by id" call async IPC handlers via getHandler('hotkey:save') and getHandler('hotkey:delete') but do not await them; update both tests to be async (add async to the it callback) and await the invocation of the handler (await getHandler(...)(null, ...)) so any thrown/rejected errors are surfaced and the subsequent expectations (mockDb.insert/delete and mockDb.run) run after the handler completes.packages/streamline/renderer/src/hotkeys/reset-to-default.ts (1)
74-75:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDo not silently swallow reset failures.
Line 75 suppresses all
resetToDefault()errors with an empty catch, so reset failures become invisible to users and debugging.Suggested failure handling
const onKeyDown = createTripleTapHandler(() => { /* v8 ignore next — resetToDefault IPC flow not exercised in unit tests */ - void resetToDefault().catch(() => {}); + void resetToDefault().catch((error) => { + console.error('resetToDefault failed', error); + eventBus.emit('toast:show', { + message: 'Failed to reset layout and hotkeys', + type: 'error' + }); + }); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/streamline/renderer/src/hotkeys/reset-to-default.ts` around lines 74 - 75, The current call to resetToDefault() swallows all errors with an empty catch; change this so failures are surfaced: replace the empty catch at the reset-to-default call with a catch that logs the error (e.g., console.error or the renderer logging utility) and surfaces user-visible feedback (e.g., dispatch an IPC/error-notification or call an existing showErrorNotification function) and/or rethrow so upstream can handle it; update the catch block around resetToDefault() in reset-to-default.ts to log the error and trigger a UI notification or IPC event named for reset failures.
🧹 Nitpick comments (4)
packages/streamline/src/ipc/handlers/system.spec.ts (1)
92-96: ⚡ Quick winTest coverage gap:
selectFoldernon-canceled path is untested.The test suite covers the
canceledpath forsystem:selectFolder(lines 92–96) but does not exercise the success path where a folder is selected. This mirrors the pattern used forsystem:selectFile(lines 81–90), which tests both canceled and non-canceled outcomes.While the implementation in
system.tsline 33 marks this branch with/* v8 ignore next */, adding a test for the non-canceled path would increase confidence that the handler behaves correctly when users actually select a folder.🧪 Suggested test case
it('SYSTEM_SELECT_FOLDER returns null when canceled', async () => { dialogMock.showOpenDialog.mockResolvedValueOnce({ canceled: true, filePaths: [] }); const result = await (getHandler('system:selectFolder')(null) as Promise<unknown>); expect(result).toBeNull(); }); + + it('SYSTEM_SELECT_FOLDER returns folder path when not canceled', async () => { + dialogMock.showOpenDialog.mockResolvedValueOnce({ canceled: false, filePaths: ['/chosen/folder'] }); + const result = await (getHandler('system:selectFolder')(null) as Promise<unknown>); + expect(result).toBe('/chosen/folder'); + }); it('SYSTEM_GET_DEFAULT_RECORDINGS_FOLDER returns path with StreamlineRecordings', () => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/streamline/src/ipc/handlers/system.spec.ts` around lines 92 - 96, Add a unit test for the non-canceled path of the "system:selectFolder" handler: mock dialogMock.showOpenDialog to resolve to { canceled: false, filePaths: ['/some/path'] }, call getHandler('system:selectFolder')(null) and assert the result equals the selected folder path (e.g., filePaths[0]); mirror the pattern used in the "system:selectFile" tests so the success branch in the selectFolder handler (referenced by getHandler and the "system:selectFolder" handler in system.spec.ts) is covered.packages/streamline/renderer/src/audio/mixer-bridge.spec.ts (1)
10-38: ⚡ Quick winRemove or document the orphaned tanh curve tests.
The
buildTanhCurvehelper and its test suite are no longer connected to the production code. The actual tanh curve is built inline inmixer-bridge.ts(lines 28–34) and excluded from coverage (/* v8 ignore next 27 */at line 21). These tests validate a local copy of the algorithm that doesn't match the production code path, adding no coverage and potentially causing confusion.🧹 Recommended cleanup
Either:
- Remove the dead tests (preferred if the production code will remain coverage-ignored):
-function buildTanhCurve(size = 256): Float32Array { - const curve = new Float32Array(size); - for (let i = 0; i < size; i++) { - const x = (i * 2) / (size - 1) - 1; - curve[i] = Math.tanh(x * 2); - } - return curve; -} - -describe('soft-clip tanh curve', () => { - it('is monotonically increasing', () => { - const curve = buildTanhCurve(); - for (let i = 1; i < curve.length; i++) { - expect(curve[i]).toBeGreaterThanOrEqual(curve[i - 1]); - } - }); - - it('clamps output to [-1, 1]', () => { - const curve = buildTanhCurve(); - expect(curve[0]).toBeGreaterThanOrEqual(-1); - expect(curve[curve.length - 1]).toBeLessThanOrEqual(1); - }); - - it('maps center to 0', () => { - const curve = buildTanhCurve(); - const mid = Math.floor(curve.length / 2); - expect(Math.abs(curve[mid])).toBeLessThan(0.01); - }); -}); - describe('mixer-bridge module', () => {
- Or extract and export
buildTanhCurvefrommixer-bridge.tsso it can be unit tested independently (if you want coverage of the curve logic).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/streamline/renderer/src/audio/mixer-bridge.spec.ts` around lines 10 - 38, The test helper buildTanhCurve and its suite are orphaned because the real tanh curve is generated inline in mixer-bridge.ts (inside the production function) and excluded from coverage; either delete the test file (remove buildTanhCurve and the "soft-clip tanh curve" describe block) if you intend to keep the production curve coverage-ignored, or refactor the production code to extract and export a reusable function (e.g., export buildTanhCurve or export getTanhCurve) from mixer-bridge.ts and update the tests to import and assert that exported function so the test covers the actual implementation.packages/streamline/renderer/src/modules/queue/QueueSettingsModal.component.spec.ts (1)
77-80: 💤 Low valueConsider adding a length check before array access.
The test accesses
checkboxes[0]without verifying the array is non-empty. In this test's context (one deck is rendered), this is safe, but a defensive check would make the test more robust.🛡️ Optional defensive check
const checkboxes = container.querySelectorAll( 'input[type="checkbox"]' ) as NodeListOf<HTMLInputElement>; +expect(checkboxes.length).toBeGreaterThan(0); expect(checkboxes[0].checked).toBe(true);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/streamline/renderer/src/modules/queue/QueueSettingsModal.component.spec.ts` around lines 77 - 80, Add a defensive length check before accessing checkboxes[0] in the QueueSettingsModal.component.spec.ts test: verify the NodeList returned by container.querySelectorAll('input[type="checkbox"]') has length > 0 (or use expect(checkboxes.length).toBeGreaterThan(0)) and then assert on checkboxes[0].checked; update the test around the checkboxes variable to guard the array access and keep the existing expectation of true for the first checkbox.packages/streamline/renderer/src/modules/local-output/LocalOutput.svelte (1)
88-90: ⚡ Quick winRemove unnecessary
passthroughhelper.The
passthrough<T>(value: T): Tfunction is an identity function used only once (line 103). Svelte can bindselectedDeviceIddirectly without wrapping it.♻️ Simplify by removing passthrough
- function passthrough<T>(value: T): T { - return value; - } - /* v8 ignore next 2 — instanceId prop is always a non-empty string; ?? '' branch unreachable */And update line 103:
<select id={deviceSelectId} class="rounded border border-primary-700 bg-primary-800 px-2 py-1 text-sm text-primary-100" - value={passthrough(selectedDeviceId)} + value={selectedDeviceId} onchange={(e) => changeDevice((e.target as HTMLSelectElement).value)} >Also applies to: 103-103
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/streamline/renderer/src/modules/local-output/LocalOutput.svelte` around lines 88 - 90, Remove the unnecessary identity helper passthrough<T>(value: T): T and its single usage; replace the call that wraps selectedDeviceId with a direct binding to selectedDeviceId (remove passthrough(...) wherever used, e.g. the instance referencing selectedDeviceId) and delete the passthrough function declaration (function passthrough) from the module to simplify the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/audio-worklet/vitest.config.mts`:
- Around line 9-11: The config currently sets 100% coverage thresholds while
only including src/tap-processor.ts, but that file is fully wrapped in V8 ignore
comments (the v8 ignore start/stop at the top and bottom of tap-processor.ts),
so coverage will be missing or inconsistent; fix by either removing
'src/tap-processor.ts' from the vitest config include array (or lowering
thresholds in the thresholds config) if the file can't be run in Node, or by
editing tap-processor.ts to narrow the /* v8 ignore start/stop */ blocks to only
the AudioWorklet registration bits so meaningful code remains testable and
coverage can be collected against the current thresholds.
In `@packages/streamline/renderer/src/hotkeys/reset-to-default.spec.ts`:
- Around line 151-157: The test currently asserts teardown removes a keydown
listener with expect.any(Function) which can yield false positives; change it to
capture the actual callback passed to window.addEventListener (by stubbing
window.addEventListener and reading its first call's second argument), then call
initResetToDefaultHotkey(), call teardown(), and assert
window.removeEventListener was called with 'keydown' and that exact captured
callback; reference the initResetToDefaultHotkey function and the
window.addEventListener/removeEventListener stubs to locate the code to update.
In
`@packages/streamline/renderer/src/modules/deck/WaveformDisplay.component.spec.ts`:
- Around line 101-113: The test "does not seek when seekbar width is zero" for
WaveformDisplay should explicitly mock the seek bar's bounding rectangle to
width: 0 before firing the click; locate the seekBar element (queried by
container.querySelector('[role="slider"]') in the test), stub its
getBoundingClientRect to return an object with width: 0 (and any required x/left
values), then fire the click and assert onSeek was not called; ensure the mock
is set up on the seekBar instance used by the test so the width check in the
component uses the mocked zero width.
In `@packages/streamline/src/update-check.ts`:
- Around line 28-30: Remove the "/* v8 ignore next 2 */" annotation that skips
the major/minor version checks in update-check.ts and add unit tests in
update-check.spec.ts (the "resolveUpdate — stable" suite) that exercise those
branches: add cases using resolveUpdate (or the same exported comparator) for
major divergence (e.g., "v1.0.0" vs "v0.0.1") and minor divergence (e.g.,
"v0.1.0" vs "v0.0.1") and assert the expected true/false outcomes so the rMaj
!== cMaj and rMin !== cMin paths are covered.
- Around line 48-49: Add a unit test in the "resolveUpdate — nightly" suite
(update-check.spec.ts) that supplies a ReleaseInfo where name: null and asserts
resolveUpdate behavior for that branch (so the null-name fallback is exercised),
then remove the /* v8 ignore next — null branch for release.name not exercised
in unit tests */ annotation on the line that computes remoteSha (the ternary
using release.name ? parseNightlySha(release.name) : null) so the branch is
covered by tests.
---
Outside diff comments:
In `@packages/streamline/renderer/src/modules/encoders/EncoderModal.svelte`:
- Around line 72-100: The v8 coverage ignore currently only covers the `if (type
=== 'file')` branch; extend the ignore directive so it spans the entire if/else
block that calls `onSave` (including the `composePathTemplate` usage and the
`else` branch that passes `host`, `port`, `mount`, `username`, and
`passwordRef`) so both branches are excluded from coverage; update the `/* v8
ignore next N */` count to cover all lines from the `if (type === 'file')` start
through the end of the `else` block (the lines that invoke `onSave` in both
branches).
---
Duplicate comments:
In `@packages/streamline/renderer/src/hotkeys/reset-to-default.ts`:
- Around line 74-75: The current call to resetToDefault() swallows all errors
with an empty catch; change this so failures are surfaced: replace the empty
catch at the reset-to-default call with a catch that logs the error (e.g.,
console.error or the renderer logging utility) and surfaces user-visible
feedback (e.g., dispatch an IPC/error-notification or call an existing
showErrorNotification function) and/or rethrow so upstream can handle it; update
the catch block around resetToDefault() in reset-to-default.ts to log the error
and trigger a UI notification or IPC event named for reset failures.
In `@packages/streamline/src/ipc/handlers/hotkeys.spec.ts`:
- Around line 41-57: The tests "HOTKEY_SAVE upserts binding" and "HOTKEY_DELETE
deletes by id" call async IPC handlers via getHandler('hotkey:save') and
getHandler('hotkey:delete') but do not await them; update both tests to be async
(add async to the it callback) and await the invocation of the handler (await
getHandler(...)(null, ...)) so any thrown/rejected errors are surfaced and the
subsequent expectations (mockDb.insert/delete and mockDb.run) run after the
handler completes.
---
Nitpick comments:
In `@packages/streamline/renderer/src/audio/mixer-bridge.spec.ts`:
- Around line 10-38: The test helper buildTanhCurve and its suite are orphaned
because the real tanh curve is generated inline in mixer-bridge.ts (inside the
production function) and excluded from coverage; either delete the test file
(remove buildTanhCurve and the "soft-clip tanh curve" describe block) if you
intend to keep the production curve coverage-ignored, or refactor the production
code to extract and export a reusable function (e.g., export buildTanhCurve or
export getTanhCurve) from mixer-bridge.ts and update the tests to import and
assert that exported function so the test covers the actual implementation.
In `@packages/streamline/renderer/src/modules/local-output/LocalOutput.svelte`:
- Around line 88-90: Remove the unnecessary identity helper
passthrough<T>(value: T): T and its single usage; replace the call that wraps
selectedDeviceId with a direct binding to selectedDeviceId (remove
passthrough(...) wherever used, e.g. the instance referencing selectedDeviceId)
and delete the passthrough function declaration (function passthrough) from the
module to simplify the code.
In
`@packages/streamline/renderer/src/modules/queue/QueueSettingsModal.component.spec.ts`:
- Around line 77-80: Add a defensive length check before accessing checkboxes[0]
in the QueueSettingsModal.component.spec.ts test: verify the NodeList returned
by container.querySelectorAll('input[type="checkbox"]') has length > 0 (or use
expect(checkboxes.length).toBeGreaterThan(0)) and then assert on
checkboxes[0].checked; update the test around the checkboxes variable to guard
the array access and keep the existing expectation of true for the first
checkbox.
In `@packages/streamline/src/ipc/handlers/system.spec.ts`:
- Around line 92-96: Add a unit test for the non-canceled path of the
"system:selectFolder" handler: mock dialogMock.showOpenDialog to resolve to {
canceled: false, filePaths: ['/some/path'] }, call
getHandler('system:selectFolder')(null) and assert the result equals the
selected folder path (e.g., filePaths[0]); mirror the pattern used in the
"system:selectFile" tests so the success branch in the selectFolder handler
(referenced by getHandler and the "system:selectFolder" handler in
system.spec.ts) is covered.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b4280f91-445a-4c04-9963-7ae6f4be5de6
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (98)
.github/workflows/test.yml.idea/vcs.xmlpackage.jsonpackages/audio-worklet/package.jsonpackages/audio-worklet/src/tap-processor.tspackages/audio-worklet/vitest.config.mtspackages/shared/package.jsonpackages/shared/vitest.config.mtspackages/streamline/package.jsonpackages/streamline/renderer/src/audio/context.spec.tspackages/streamline/renderer/src/audio/context.tspackages/streamline/renderer/src/audio/mixer-bridge.spec.tspackages/streamline/renderer/src/audio/mixer-bridge.tspackages/streamline/renderer/src/audio/port.spec.tspackages/streamline/renderer/src/components/DbMeter.component.spec.tspackages/streamline/renderer/src/components/IconButton.component.spec.tspackages/streamline/renderer/src/components/Modal.component.spec.tspackages/streamline/renderer/src/components/Modal.sveltepackages/streamline/renderer/src/components/OnAirIndicator.component.spec.tspackages/streamline/renderer/src/components/StatusBar.component.spec.tspackages/streamline/renderer/src/components/StatusBar.sveltepackages/streamline/renderer/src/components/Toast.component.spec.tspackages/streamline/renderer/src/components/ToastContainer.component.spec.tspackages/streamline/renderer/src/components/forms/BitrateField.component.spec.tspackages/streamline/renderer/src/components/forms/ComboField.component.spec.tspackages/streamline/renderer/src/components/forms/ComboField.sveltepackages/streamline/renderer/src/components/forms/FilenameField.component.spec.tspackages/streamline/renderer/src/components/forms/FolderField.component.spec.tspackages/streamline/renderer/src/components/forms/FormField.component.spec.tspackages/streamline/renderer/src/components/forms/NumberField.component.spec.tspackages/streamline/renderer/src/components/forms/NumberField.sveltepackages/streamline/renderer/src/components/forms/SampleRateField.component.spec.tspackages/streamline/renderer/src/components/forms/SelectField.component.spec.tspackages/streamline/renderer/src/components/forms/TextField.component.spec.tspackages/streamline/renderer/src/drag-drop/song-drag.spec.tspackages/streamline/renderer/src/first-run/setup.spec.tspackages/streamline/renderer/src/hotkeys/binder.spec.tspackages/streamline/renderer/src/hotkeys/reset-to-default.spec.tspackages/streamline/renderer/src/hotkeys/reset-to-default.tspackages/streamline/renderer/src/hotkeys/store.svelte.spec.tspackages/streamline/renderer/src/layout/persistence.svelte.spec.tspackages/streamline/renderer/src/layout/persistence.svelte.tspackages/streamline/renderer/src/layout/store.svelte.spec.tspackages/streamline/renderer/src/modules/context.spec.tspackages/streamline/renderer/src/modules/crossfader/Crossfader.component.spec.tspackages/streamline/renderer/src/modules/crossfader/Crossfader.sveltepackages/streamline/renderer/src/modules/deck/Deck.component.spec.tspackages/streamline/renderer/src/modules/deck/Deck.sveltepackages/streamline/renderer/src/modules/deck/DeckSettingsModal.component.spec.tspackages/streamline/renderer/src/modules/deck/WaveformDisplay.component.spec.tspackages/streamline/renderer/src/modules/deck/WaveformDisplay.sveltepackages/streamline/renderer/src/modules/deck/deck-audio.tspackages/streamline/renderer/src/modules/deck/waveform-worker.spec.tspackages/streamline/renderer/src/modules/deck/waveform-worker.tspackages/streamline/renderer/src/modules/encoders/EncoderModal.component.spec.tspackages/streamline/renderer/src/modules/encoders/EncoderModal.sveltepackages/streamline/renderer/src/modules/encoders/Encoders.component.spec.tspackages/streamline/renderer/src/modules/encoders/Encoders.sveltepackages/streamline/renderer/src/modules/instance-store.svelte.spec.tspackages/streamline/renderer/src/modules/local-output/LocalOutput.component.spec.tspackages/streamline/renderer/src/modules/local-output/LocalOutput.sveltepackages/streamline/renderer/src/modules/microphone/Microphone.component.spec.tspackages/streamline/renderer/src/modules/microphone/Microphone.sveltepackages/streamline/renderer/src/modules/queue/Queue.component.spec.tspackages/streamline/renderer/src/modules/queue/Queue.sveltepackages/streamline/renderer/src/modules/queue/QueueSettingsModal.component.spec.tspackages/streamline/renderer/src/modules/queue/rotation.tspackages/streamline/renderer/src/modules/register-builtins.spec.tspackages/streamline/renderer/src/windows/WindowManager.component.spec.tspackages/streamline/renderer/src/windows/WindowManager.sveltepackages/streamline/renderer/src/windows/WindowWrapper.component.spec.tspackages/streamline/renderer/src/windows/WindowWrapper.sveltepackages/streamline/renderer/src/windows/use-interact.spec.tspackages/streamline/src/audio/pcm-receiver.spec.tspackages/streamline/src/audio/pcm-receiver.tspackages/streamline/src/audio/port.spec.tspackages/streamline/src/db/migrate.spec.tspackages/streamline/src/encoders/encoder-process.spec.tspackages/streamline/src/encoders/encoder-process.tspackages/streamline/src/encoders/ffmpeg-path.spec.tspackages/streamline/src/encoders/ffmpeg-path.tspackages/streamline/src/encoders/manager.spec.tspackages/streamline/src/encoders/manager.tspackages/streamline/src/ipc/handlers/encoder.spec.tspackages/streamline/src/ipc/handlers/encoder.tspackages/streamline/src/ipc/handlers/hotkeys.spec.tspackages/streamline/src/ipc/handlers/layout.spec.tspackages/streamline/src/ipc/handlers/library.spec.tspackages/streamline/src/ipc/handlers/secret.spec.tspackages/streamline/src/ipc/handlers/settings.spec.tspackages/streamline/src/ipc/handlers/system.spec.tspackages/streamline/src/ipc/handlers/system.tspackages/streamline/src/library/scan-worker.tspackages/streamline/src/library/scanner.spec.tspackages/streamline/src/library/watcher.spec.tspackages/streamline/src/update-check.spec.tspackages/streamline/src/update-check.tspackages/streamline/vitest.config.mts
💤 Files with no reviewable changes (1)
- .idea/vcs.xml
Signed-off-by: Robbin "Roboroads" Schepers <robbin@deschepers.nl>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/audio-worklet/vitest.config.mts (1)
7-10:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing coverage thresholds defeats the 100% coverage goal.
The configuration enables V8 coverage but doesn't set any thresholds. Without explicit thresholds, the coverage gate won't enforce the PR's stated goal of "100% branch coverage." Regressions will pass silently.
📊 Proposed fix to add coverage thresholds
coverage: { provider: 'v8', + thresholds: { lines: 100, functions: 100, branches: 100, statements: 100 }, exclude: ['src/index.ts'] }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/audio-worklet/vitest.config.mts` around lines 7 - 10, The coverage block currently sets provider and exclude but lacks enforcement thresholds; add a thresholds object to the coverage config (under the existing coverage object) that enforces 100% for branches (and optionally lines, functions, statements) globally and/or per-file. Update the coverage config to include a thresholds key with values such as global: { branches: 100, lines: 100, functions: 100, statements: 100 } so the V8 coverage provider actually fails on regressions while keeping the existing exclude array intact.
🧹 Nitpick comments (6)
packages/streamline/src/audio/pcm-receiver.spec.ts (1)
64-66: ⚡ Quick winWeak assertion — test does not verify dropout count behavior.
The test only checks the return type, not the actual count value or behavior. Consider extending this test to verify that
getDropoutCount()returns 0 initially, or merge this assertion into the existing dropout increment test (lines 51-62) which already callsgetDropoutCount().♻️ Suggested consolidation
it('increments dropoutCount when target has no consumer', () => { const before = getDropoutCount(); const msg: PcmMessage = { buffer: new ArrayBuffer(100), frames: 10, sampleRate: 48000, channels: 2, encoderTargets: ['no-such-enc'] }; handlePcmMessage(msg); expect(getDropoutCount()).toBe(before + 1); + expect(typeof getDropoutCount()).toBe('number'); }); - - it('getDropoutCount returns current dropout count', () => { - expect(typeof getDropoutCount()).toBe('number'); - });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/streamline/src/audio/pcm-receiver.spec.ts` around lines 64 - 66, The test for getDropoutCount currently only asserts the return type; update it to assert actual behavior by verifying getDropoutCount() returns 0 initially (or move this assertion into the existing dropout increment test that exercises the increment behavior on lines 51-62) so the suite confirms both the initial state and that getDropoutCount() increases after invoking the dropout increment logic; locate and modify the test that references getDropoutCount() (or merge into the existing increment test) to include an explicit numeric expectation rather than just typeof.packages/streamline/renderer/src/modules/encoders/EncoderModal.svelte (1)
72-100: ⚡ Quick winOverly broad v8 ignore suppresses coverage for testable code.
The ignore comment on Line 72 covers the entire save payload construction (lines 73–100), but neither the file-type branch (lines 73–84) nor the network-type branch (lines 86–100) make IPC calls—they only build config objects and invoke
onSave. The accompanying test suite (EncoderModal.component.spec.tslines 123–147, 149–164, 181–199) successfully exercises these paths by mockingwindow.streamline.apiand clicking the Save button, confirming this code is testable in jsdom.The password-related IPC calls (lines 58–61 and 66–69) already have targeted ignore comments. Consider removing the ignore directive on Line 72 to restore coverage for the payload-construction logic, which does not depend on Electron-specific APIs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/streamline/renderer/src/modules/encoders/EncoderModal.svelte` around lines 72 - 100, Remove the broad v8 ignore that currently spans the payload construction and onSave invocation; this code (composePathTemplate and both branches that call onSave) is testable in jsdom. Narrow or delete the ignore so only the actual IPC-dependent safeStorage calls remain ignored (keep the targeted ignores around the password-related IPC calls), leaving the composePathTemplate + onSave branches uncovered so tests can measure coverage.packages/streamline/renderer/src/modules/microphone/Microphone.svelte (1)
205-207: Clarify or update the v8 ignore comment.Similar to LocalOutput.svelte, the comment states "instanceId prop is always a non-empty string; ?? '' branch unreachable," but the visible code shows simple string concatenation without a
??operator. If this refers to Svelte compiler-generated code, consider clarifying the comment to prevent confusion.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/streamline/renderer/src/modules/microphone/Microphone.svelte` around lines 205 - 207, Update the confusing v8 ignore comment above the micDeviceSelectId and micVolSliderId declarations: either remove the "?? '' branch unreachable" phrase or reword it to explicitly state that instanceId is guaranteed non-empty by the Svelte component (or that the comment refers to Svelte-compiled code which inserts a fallback), matching the style used in LocalOutput.svelte; reference the symbols micDeviceSelectId, micVolSliderId and the instanceId prop so reviewers can locate and confirm the clarification.packages/streamline/renderer/src/modules/encoders/Encoders.component.spec.ts (1)
157-172: ⚡ Quick winStrengthen the partial-order test assertion.
The test verifies that configs missing from the stored order JSON are placed at the end, but only checks the first item's position. To fully validate the behavior, assert that all missing configs (enc-a and enc-c) appear after the ordered config (enc-b).
✅ Recommended assertion improvements
const names = getAllByText(/Encoder [ABC]/); expect(names[0].textContent).toBe('Encoder B'); + // Verify missing configs appear after ordered ones + expect(names[1].textContent).toMatch(/Encoder [AC]/); + expect(names[2].textContent).toMatch(/Encoder [AC]/);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/streamline/renderer/src/modules/encoders/Encoders.component.spec.ts` around lines 157 - 172, The test only asserts the first item is 'Encoder B' but should verify the full ordering; after rendering (using makeStreamline and importing Encoders.svelte) capture the elements via getAllByText (as assigned to names) and assert their sequence includes 'Encoder B' first and that 'Encoder A' and 'Encoder C' follow it (e.g. compare names.map(n => n.textContent) against the expected ordered array or assert indexOf('Encoder B') < indexOf('Encoder A') and < indexOf('Encoder C')). Ensure you reference the existing variables configA/configB/configC, makeStreamline, Encoders.svelte import, and the getAllByText/names usage so the new assertions are added immediately after the existing expect(names[0].textContent).toBe('Encoder B') check.packages/streamline/renderer/src/modules/local-output/LocalOutput.svelte (1)
92-94: Clarify or update the v8 ignore comment.The comment states "instanceId prop is always a non-empty string; ?? '' branch unreachable," but the code shows simple string concatenation without a visible
??operator. If this refers to Svelte compiler-generated null-guard code, consider clarifying the comment to mention "compiler-generated" to avoid confusion.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/streamline/renderer/src/modules/local-output/LocalOutput.svelte` around lines 92 - 94, Update the V8 ignore comment above the deviceSelectId and volSliderId declarations to explicitly reference the Svelte/compiler-generated null-guard rather than implying a visible "??" in this code; e.g. state that instanceId is always a non-empty string and any compiler-generated nullish fallback (e.g. "?? ''") is unreachable. Target the comment near the deviceSelectId and volSliderId lines and mention instanceId and the Svelte compiler to avoid confusion.packages/streamline/renderer/src/modules/encoders/Encoders.svelte (1)
145-146: Clarify the necessity of v8 ignore foractiveLiveText.The derived value
activeLiveTextis a simple string interpolation ofactiveCount(which is always a number from.length). The v8 ignore comment suggests this guards against a null branch, butactiveLiveTextis actively used in the template at line 237. If the derived value is reached in tests, this annotation may be unnecessary.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/streamline/renderer/src/modules/encoders/Encoders.svelte` around lines 145 - 146, Remove or replace the ambiguous "v8 ignore next 2" comment before the activeLiveText declaration: either delete the v8-ignore lines if they are unnecessary, or replace them with a concise comment that states "activeCount is always a number (derived from .length) and activeLiveText is always a string used in the template" so the intent is clear; ensure you touch the const activeLiveText = $derived(`${activeCount} live`) declaration (and adjust tests if needed) so the code comment accurately reflects why no null-branch exists.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/streamline/renderer/src/modules/crossfader/Crossfader.svelte`:
- Around line 98-102: Remove the `{!true /* v8 ignore ... */}` interpolations
inside the <select> (they render the literal "false" between options) and
instead place the v8-ignore comment where it won't emit template text — e.g.,
move the `/* v8 ignore next 2 — deckLabel/deckId ... */` comment into the
<script> block immediately above the deckLabel/deckId function definitions (or
use a non-rendering mechanism such as a JS comment in the script) so the
coverage directive still applies but no runtime text node is produced in the
template.
In
`@packages/streamline/renderer/src/modules/deck/DeckSettingsModal.component.spec.ts`:
- Around line 18-27: The tests in DeckSettingsModal.component.spec.ts use
container.querySelector('input[type="checkbox"]') with a type assertion, which
can be null and gives poor failure messages; update both tests (the one calling
render(DeckSettingsModal, { sendMetadata: false, ... }) and the similar
sendMetadata: true test) to first assert the element exists (e.g.,
expect(element).not.toBeNull()) or throw a clear error if null before accessing
.checked, locating the element via
container.querySelector('input[type="checkbox"]') so the failure message
explicitly states the checkbox was not found instead of causing a runtime null
access.
---
Duplicate comments:
In `@packages/audio-worklet/vitest.config.mts`:
- Around line 7-10: The coverage block currently sets provider and exclude but
lacks enforcement thresholds; add a thresholds object to the coverage config
(under the existing coverage object) that enforces 100% for branches (and
optionally lines, functions, statements) globally and/or per-file. Update the
coverage config to include a thresholds key with values such as global: {
branches: 100, lines: 100, functions: 100, statements: 100 } so the V8 coverage
provider actually fails on regressions while keeping the existing exclude array
intact.
---
Nitpick comments:
In `@packages/streamline/renderer/src/modules/encoders/EncoderModal.svelte`:
- Around line 72-100: Remove the broad v8 ignore that currently spans the
payload construction and onSave invocation; this code (composePathTemplate and
both branches that call onSave) is testable in jsdom. Narrow or delete the
ignore so only the actual IPC-dependent safeStorage calls remain ignored (keep
the targeted ignores around the password-related IPC calls), leaving the
composePathTemplate + onSave branches uncovered so tests can measure coverage.
In
`@packages/streamline/renderer/src/modules/encoders/Encoders.component.spec.ts`:
- Around line 157-172: The test only asserts the first item is 'Encoder B' but
should verify the full ordering; after rendering (using makeStreamline and
importing Encoders.svelte) capture the elements via getAllByText (as assigned to
names) and assert their sequence includes 'Encoder B' first and that 'Encoder A'
and 'Encoder C' follow it (e.g. compare names.map(n => n.textContent) against
the expected ordered array or assert indexOf('Encoder B') < indexOf('Encoder A')
and < indexOf('Encoder C')). Ensure you reference the existing variables
configA/configB/configC, makeStreamline, Encoders.svelte import, and the
getAllByText/names usage so the new assertions are added immediately after the
existing expect(names[0].textContent).toBe('Encoder B') check.
In `@packages/streamline/renderer/src/modules/encoders/Encoders.svelte`:
- Around line 145-146: Remove or replace the ambiguous "v8 ignore next 2"
comment before the activeLiveText declaration: either delete the v8-ignore lines
if they are unnecessary, or replace them with a concise comment that states
"activeCount is always a number (derived from .length) and activeLiveText is
always a string used in the template" so the intent is clear; ensure you touch
the const activeLiveText = $derived(`${activeCount} live`) declaration (and
adjust tests if needed) so the code comment accurately reflects why no
null-branch exists.
In `@packages/streamline/renderer/src/modules/local-output/LocalOutput.svelte`:
- Around line 92-94: Update the V8 ignore comment above the deviceSelectId and
volSliderId declarations to explicitly reference the Svelte/compiler-generated
null-guard rather than implying a visible "??" in this code; e.g. state that
instanceId is always a non-empty string and any compiler-generated nullish
fallback (e.g. "?? ''") is unreachable. Target the comment near the
deviceSelectId and volSliderId lines and mention instanceId and the Svelte
compiler to avoid confusion.
In `@packages/streamline/renderer/src/modules/microphone/Microphone.svelte`:
- Around line 205-207: Update the confusing v8 ignore comment above the
micDeviceSelectId and micVolSliderId declarations: either remove the "?? ''
branch unreachable" phrase or reword it to explicitly state that instanceId is
guaranteed non-empty by the Svelte component (or that the comment refers to
Svelte-compiled code which inserts a fallback), matching the style used in
LocalOutput.svelte; reference the symbols micDeviceSelectId, micVolSliderId and
the instanceId prop so reviewers can locate and confirm the clarification.
In `@packages/streamline/src/audio/pcm-receiver.spec.ts`:
- Around line 64-66: The test for getDropoutCount currently only asserts the
return type; update it to assert actual behavior by verifying getDropoutCount()
returns 0 initially (or move this assertion into the existing dropout increment
test that exercises the increment behavior on lines 51-62) so the suite confirms
both the initial state and that getDropoutCount() increases after invoking the
dropout increment logic; locate and modify the test that references
getDropoutCount() (or merge into the existing increment test) to include an
explicit numeric expectation rather than just typeof.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5dc548f8-0ae0-4175-bb97-b0c63f34afdd
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (98)
.github/workflows/test.yml.idea/vcs.xmlpackage.jsonpackages/audio-worklet/package.jsonpackages/audio-worklet/src/tap-processor.tspackages/audio-worklet/vitest.config.mtspackages/shared/package.jsonpackages/shared/vitest.config.mtspackages/streamline/package.jsonpackages/streamline/renderer/src/audio/context.spec.tspackages/streamline/renderer/src/audio/context.tspackages/streamline/renderer/src/audio/mixer-bridge.spec.tspackages/streamline/renderer/src/audio/mixer-bridge.tspackages/streamline/renderer/src/audio/port.spec.tspackages/streamline/renderer/src/components/DbMeter.component.spec.tspackages/streamline/renderer/src/components/IconButton.component.spec.tspackages/streamline/renderer/src/components/Modal.component.spec.tspackages/streamline/renderer/src/components/Modal.sveltepackages/streamline/renderer/src/components/OnAirIndicator.component.spec.tspackages/streamline/renderer/src/components/StatusBar.component.spec.tspackages/streamline/renderer/src/components/StatusBar.sveltepackages/streamline/renderer/src/components/Toast.component.spec.tspackages/streamline/renderer/src/components/ToastContainer.component.spec.tspackages/streamline/renderer/src/components/forms/BitrateField.component.spec.tspackages/streamline/renderer/src/components/forms/ComboField.component.spec.tspackages/streamline/renderer/src/components/forms/ComboField.sveltepackages/streamline/renderer/src/components/forms/FilenameField.component.spec.tspackages/streamline/renderer/src/components/forms/FolderField.component.spec.tspackages/streamline/renderer/src/components/forms/FormField.component.spec.tspackages/streamline/renderer/src/components/forms/NumberField.component.spec.tspackages/streamline/renderer/src/components/forms/NumberField.sveltepackages/streamline/renderer/src/components/forms/SampleRateField.component.spec.tspackages/streamline/renderer/src/components/forms/SelectField.component.spec.tspackages/streamline/renderer/src/components/forms/TextField.component.spec.tspackages/streamline/renderer/src/drag-drop/song-drag.spec.tspackages/streamline/renderer/src/first-run/setup.spec.tspackages/streamline/renderer/src/hotkeys/binder.spec.tspackages/streamline/renderer/src/hotkeys/reset-to-default.spec.tspackages/streamline/renderer/src/hotkeys/reset-to-default.tspackages/streamline/renderer/src/hotkeys/store.svelte.spec.tspackages/streamline/renderer/src/layout/persistence.svelte.spec.tspackages/streamline/renderer/src/layout/persistence.svelte.tspackages/streamline/renderer/src/layout/store.svelte.spec.tspackages/streamline/renderer/src/modules/context.spec.tspackages/streamline/renderer/src/modules/crossfader/Crossfader.component.spec.tspackages/streamline/renderer/src/modules/crossfader/Crossfader.sveltepackages/streamline/renderer/src/modules/deck/Deck.component.spec.tspackages/streamline/renderer/src/modules/deck/Deck.sveltepackages/streamline/renderer/src/modules/deck/DeckSettingsModal.component.spec.tspackages/streamline/renderer/src/modules/deck/WaveformDisplay.component.spec.tspackages/streamline/renderer/src/modules/deck/WaveformDisplay.sveltepackages/streamline/renderer/src/modules/deck/deck-audio.tspackages/streamline/renderer/src/modules/deck/waveform-worker.spec.tspackages/streamline/renderer/src/modules/deck/waveform-worker.tspackages/streamline/renderer/src/modules/encoders/EncoderModal.component.spec.tspackages/streamline/renderer/src/modules/encoders/EncoderModal.sveltepackages/streamline/renderer/src/modules/encoders/Encoders.component.spec.tspackages/streamline/renderer/src/modules/encoders/Encoders.sveltepackages/streamline/renderer/src/modules/instance-store.svelte.spec.tspackages/streamline/renderer/src/modules/local-output/LocalOutput.component.spec.tspackages/streamline/renderer/src/modules/local-output/LocalOutput.sveltepackages/streamline/renderer/src/modules/microphone/Microphone.component.spec.tspackages/streamline/renderer/src/modules/microphone/Microphone.sveltepackages/streamline/renderer/src/modules/queue/Queue.component.spec.tspackages/streamline/renderer/src/modules/queue/Queue.sveltepackages/streamline/renderer/src/modules/queue/QueueSettingsModal.component.spec.tspackages/streamline/renderer/src/modules/queue/rotation.tspackages/streamline/renderer/src/modules/register-builtins.spec.tspackages/streamline/renderer/src/windows/WindowManager.component.spec.tspackages/streamline/renderer/src/windows/WindowManager.sveltepackages/streamline/renderer/src/windows/WindowWrapper.component.spec.tspackages/streamline/renderer/src/windows/WindowWrapper.sveltepackages/streamline/renderer/src/windows/use-interact.spec.tspackages/streamline/src/audio/pcm-receiver.spec.tspackages/streamline/src/audio/pcm-receiver.tspackages/streamline/src/audio/port.spec.tspackages/streamline/src/db/migrate.spec.tspackages/streamline/src/encoders/encoder-process.spec.tspackages/streamline/src/encoders/encoder-process.tspackages/streamline/src/encoders/ffmpeg-path.spec.tspackages/streamline/src/encoders/ffmpeg-path.tspackages/streamline/src/encoders/manager.spec.tspackages/streamline/src/encoders/manager.tspackages/streamline/src/ipc/handlers/encoder.spec.tspackages/streamline/src/ipc/handlers/encoder.tspackages/streamline/src/ipc/handlers/hotkeys.spec.tspackages/streamline/src/ipc/handlers/layout.spec.tspackages/streamline/src/ipc/handlers/library.spec.tspackages/streamline/src/ipc/handlers/secret.spec.tspackages/streamline/src/ipc/handlers/settings.spec.tspackages/streamline/src/ipc/handlers/system.spec.tspackages/streamline/src/ipc/handlers/system.tspackages/streamline/src/library/scan-worker.tspackages/streamline/src/library/scanner.spec.tspackages/streamline/src/library/watcher.spec.tspackages/streamline/src/update-check.spec.tspackages/streamline/src/update-check.tspackages/streamline/vitest.config.mts
💤 Files with no reviewable changes (1)
- .idea/vcs.xml
Signed-off-by: Robbin "Roboroads" Schepers <robbin@deschepers.nl>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/streamline/renderer/src/modules/crossfader/Crossfader.svelte (1)
98-102:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove
{!true /* v8 ignore ... */}interpolations that render literal "false" text nodes.The
{!true /* v8 ignore start */}and{!true /* v8 ignore stop */}interpolations (lines 98, 102, 114, 118) will render the string "false" as text nodes inside the<select>elements. In Svelte, mustache-style interpolation{expression}outputs text;{!true}evaluates tofalseand renders as a text node.The coverage directive for
deckLabel/deckIdis already correctly placed in the script block (line 28). Remove the{!true ...}interpolations from the template to avoid rendering unwanted text nodes.Proposed fix
<option value="">-- None --</option> - <!-- prettier-ignore --> - {!true /* v8 ignore start */} {`#each` decks as d (d.id)}<option value={deckId(d.id)} label={deckLabel(d.title, d.id)} ></option>{/each} - <!-- prettier-ignore --> - {!true /* v8 ignore stop */} </select>Apply the same fix to both select elements (lines 98-102 and 114-118).
Also applies to: 114-118
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/streamline/renderer/src/modules/crossfader/Crossfader.svelte` around lines 98 - 102, Remove the stray Svelte interpolations `{!true /* v8 ignore start */}` and `{!true /* v8 ignore stop */}` that are rendering the literal "false" text nodes inside the <select> blocks; locate the select blocks iterating `decks` (the each block using `deckId(d.id)` and `deckLabel(d.title, d.id)`) and simply delete those `{!true ...}` expressions (they are redundant because the coverage directives for `deckLabel`/`deckId` live in the script), applying the same removal to both places where they appear.
🧹 Nitpick comments (3)
packages/streamline/vitest.config.mts (1)
19-19: ⚡ Quick winExclude
**/*.test.tsfrom coverage to match existing test/spec pattern parity
vitest.config.mtscoverage excludes only**/*.spec.tswhile coverage still includes allsrc/**/*.tsandrenderer/src/**/*.ts. There are currently no*.test.tsfiles underpackages/streamline/srcorpackages/streamline/renderer/src, so the 100% gate won’t break today—but excluding**/*.test.tsprevents future additions from being counted as source.Proposed config tweak
coverage: { provider: 'v8', thresholds: { lines: 100, functions: 100, branches: 100, statements: 100 }, include: ['src/**/*.ts', 'renderer/src/**/*.ts', 'renderer/src/**/*.svelte'], exclude: [ '**/*.spec.ts', + '**/*.test.ts', 'src/main.ts',🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/streamline/vitest.config.mts` at line 19, In vitest.config.mts update the coverage.exclude list used by Vitest (the array that currently contains '**/*.spec.ts') to also exclude '**/*.test.ts' so test files with either .spec.ts or .test.ts aren't treated as source; locate the coverage.exclude array in the config and add the '**/*.test.ts' pattern alongside '**/*.spec.ts' to maintain parity with the existing test naming convention..github/workflows/test.yml (1)
10-10: ⚡ Quick winAdd a job timeout to prevent stuck CI runs.
If tests hang, this job can run indefinitely and delay feedback. Add
timeout-minutesat the job level.Suggested patch
jobs: test: runs-on: ubuntu-latest + timeout-minutes: 20 steps:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/test.yml at line 10, The GitHub Actions job currently lacks a timeout and can hang indefinitely; update the workflow job that contains "runs-on: ubuntu-latest" to include a job-level timeout by adding a `timeout-minutes` key (for example `timeout-minutes: 30`) under the same job mapping so the runner will cancel stuck runs after the configured time; ensure the new key is placed at the same indentation level as `runs-on` within the job definition.packages/streamline/renderer/src/modules/deck/Deck.component.spec.ts (1)
74-94: 💤 Low valueConsider adding cleanup for test globals.
While
beforeEachsets upwindow.streamlinebefore each test, adding anafterEachhook to deletewindow.streamlineand__deckAudiowould improve test hygiene and prevent potential cross-suite pollution.🧹 Optional cleanup pattern
beforeEach(() => { stateEvents = []; layoutUpdateInstance.mockReset(); instanceUpdate.mockReset(); eventBus.on('deck:d1:state', (payload) => { stateEvents.push(payload as DeckStatePayload); }); (window as unknown as { streamline: unknown }).streamline = { api: { library: { readAudioFile: vi.fn().mockResolvedValue(new ArrayBuffer(0)), getCoverArt: vi.fn().mockResolvedValue(null), getSongByPath: vi.fn().mockResolvedValue(null), getFileMetadata: vi.fn().mockResolvedValue(null), loadWaveform: vi.fn().mockResolvedValue(null), saveWaveform: vi.fn() } }, getPathForFile: vi.fn() }; }); + + afterEach(() => { + delete (window as unknown as Record<string, unknown>).streamline; + delete (globalThis as unknown as Record<string, unknown>).__deckAudio; + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/streamline/renderer/src/modules/deck/Deck.component.spec.ts` around lines 74 - 94, Add an afterEach cleanup that removes the test globals set in beforeEach: delete (window as unknown as { streamline?: unknown }).streamline and delete (window as unknown as { __deckAudio?: unknown }).__deckAudio (or the actual global name used) to avoid cross-test pollution; place this afterEach alongside the existing beforeEach in Deck.component.spec.ts so that the mocked window.streamline and any __deckAudio created during tests are removed after each spec.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/streamline/src/audio/port.spec.ts`:
- Around line 26-41: Reset the didFinishLoad callback in the test setup so it
can't hold a listener from a prior test: in the beforeEach that sets up
mockWebContents (where you call vi.clearAllMocks(), vi.resetModules(),
channelRef.current = makeChannel(), and create mockWebContents/postMessage),
explicitly reassign didFinishLoad to undefined (or a no-op) before registering
the new mock on handler; this ensures any subsequent test calls to
didFinishLoad!() only invoke the callback installed in that test via
mockWebContents.on.
---
Duplicate comments:
In `@packages/streamline/renderer/src/modules/crossfader/Crossfader.svelte`:
- Around line 98-102: Remove the stray Svelte interpolations `{!true /* v8
ignore start */}` and `{!true /* v8 ignore stop */}` that are rendering the
literal "false" text nodes inside the <select> blocks; locate the select blocks
iterating `decks` (the each block using `deckId(d.id)` and `deckLabel(d.title,
d.id)`) and simply delete those `{!true ...}` expressions (they are redundant
because the coverage directives for `deckLabel`/`deckId` live in the script),
applying the same removal to both places where they appear.
---
Nitpick comments:
In @.github/workflows/test.yml:
- Line 10: The GitHub Actions job currently lacks a timeout and can hang
indefinitely; update the workflow job that contains "runs-on: ubuntu-latest" to
include a job-level timeout by adding a `timeout-minutes` key (for example
`timeout-minutes: 30`) under the same job mapping so the runner will cancel
stuck runs after the configured time; ensure the new key is placed at the same
indentation level as `runs-on` within the job definition.
In `@packages/streamline/renderer/src/modules/deck/Deck.component.spec.ts`:
- Around line 74-94: Add an afterEach cleanup that removes the test globals set
in beforeEach: delete (window as unknown as { streamline?: unknown }).streamline
and delete (window as unknown as { __deckAudio?: unknown }).__deckAudio (or the
actual global name used) to avoid cross-test pollution; place this afterEach
alongside the existing beforeEach in Deck.component.spec.ts so that the mocked
window.streamline and any __deckAudio created during tests are removed after
each spec.
In `@packages/streamline/vitest.config.mts`:
- Line 19: In vitest.config.mts update the coverage.exclude list used by Vitest
(the array that currently contains '**/*.spec.ts') to also exclude
'**/*.test.ts' so test files with either .spec.ts or .test.ts aren't treated as
source; locate the coverage.exclude array in the config and add the
'**/*.test.ts' pattern alongside '**/*.spec.ts' to maintain parity with the
existing test naming convention.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 91905201-cdb7-473a-878f-a76e32ba04a9
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (98)
.github/workflows/test.yml.idea/vcs.xmlpackage.jsonpackages/audio-worklet/package.jsonpackages/audio-worklet/src/tap-processor.tspackages/audio-worklet/vitest.config.mtspackages/shared/package.jsonpackages/shared/vitest.config.mtspackages/streamline/package.jsonpackages/streamline/renderer/src/audio/context.spec.tspackages/streamline/renderer/src/audio/context.tspackages/streamline/renderer/src/audio/mixer-bridge.spec.tspackages/streamline/renderer/src/audio/mixer-bridge.tspackages/streamline/renderer/src/audio/port.spec.tspackages/streamline/renderer/src/components/DbMeter.component.spec.tspackages/streamline/renderer/src/components/IconButton.component.spec.tspackages/streamline/renderer/src/components/Modal.component.spec.tspackages/streamline/renderer/src/components/Modal.sveltepackages/streamline/renderer/src/components/OnAirIndicator.component.spec.tspackages/streamline/renderer/src/components/StatusBar.component.spec.tspackages/streamline/renderer/src/components/StatusBar.sveltepackages/streamline/renderer/src/components/Toast.component.spec.tspackages/streamline/renderer/src/components/ToastContainer.component.spec.tspackages/streamline/renderer/src/components/forms/BitrateField.component.spec.tspackages/streamline/renderer/src/components/forms/ComboField.component.spec.tspackages/streamline/renderer/src/components/forms/ComboField.sveltepackages/streamline/renderer/src/components/forms/FilenameField.component.spec.tspackages/streamline/renderer/src/components/forms/FolderField.component.spec.tspackages/streamline/renderer/src/components/forms/FormField.component.spec.tspackages/streamline/renderer/src/components/forms/NumberField.component.spec.tspackages/streamline/renderer/src/components/forms/NumberField.sveltepackages/streamline/renderer/src/components/forms/SampleRateField.component.spec.tspackages/streamline/renderer/src/components/forms/SelectField.component.spec.tspackages/streamline/renderer/src/components/forms/TextField.component.spec.tspackages/streamline/renderer/src/drag-drop/song-drag.spec.tspackages/streamline/renderer/src/first-run/setup.spec.tspackages/streamline/renderer/src/hotkeys/binder.spec.tspackages/streamline/renderer/src/hotkeys/reset-to-default.spec.tspackages/streamline/renderer/src/hotkeys/reset-to-default.tspackages/streamline/renderer/src/hotkeys/store.svelte.spec.tspackages/streamline/renderer/src/layout/persistence.svelte.spec.tspackages/streamline/renderer/src/layout/persistence.svelte.tspackages/streamline/renderer/src/layout/store.svelte.spec.tspackages/streamline/renderer/src/modules/context.spec.tspackages/streamline/renderer/src/modules/crossfader/Crossfader.component.spec.tspackages/streamline/renderer/src/modules/crossfader/Crossfader.sveltepackages/streamline/renderer/src/modules/deck/Deck.component.spec.tspackages/streamline/renderer/src/modules/deck/Deck.sveltepackages/streamline/renderer/src/modules/deck/DeckSettingsModal.component.spec.tspackages/streamline/renderer/src/modules/deck/WaveformDisplay.component.spec.tspackages/streamline/renderer/src/modules/deck/WaveformDisplay.sveltepackages/streamline/renderer/src/modules/deck/deck-audio.tspackages/streamline/renderer/src/modules/deck/waveform-worker.spec.tspackages/streamline/renderer/src/modules/deck/waveform-worker.tspackages/streamline/renderer/src/modules/encoders/EncoderModal.component.spec.tspackages/streamline/renderer/src/modules/encoders/EncoderModal.sveltepackages/streamline/renderer/src/modules/encoders/Encoders.component.spec.tspackages/streamline/renderer/src/modules/encoders/Encoders.sveltepackages/streamline/renderer/src/modules/instance-store.svelte.spec.tspackages/streamline/renderer/src/modules/local-output/LocalOutput.component.spec.tspackages/streamline/renderer/src/modules/local-output/LocalOutput.sveltepackages/streamline/renderer/src/modules/microphone/Microphone.component.spec.tspackages/streamline/renderer/src/modules/microphone/Microphone.sveltepackages/streamline/renderer/src/modules/queue/Queue.component.spec.tspackages/streamline/renderer/src/modules/queue/Queue.sveltepackages/streamline/renderer/src/modules/queue/QueueSettingsModal.component.spec.tspackages/streamline/renderer/src/modules/queue/rotation.tspackages/streamline/renderer/src/modules/register-builtins.spec.tspackages/streamline/renderer/src/windows/WindowManager.component.spec.tspackages/streamline/renderer/src/windows/WindowManager.sveltepackages/streamline/renderer/src/windows/WindowWrapper.component.spec.tspackages/streamline/renderer/src/windows/WindowWrapper.sveltepackages/streamline/renderer/src/windows/use-interact.spec.tspackages/streamline/src/audio/pcm-receiver.spec.tspackages/streamline/src/audio/pcm-receiver.tspackages/streamline/src/audio/port.spec.tspackages/streamline/src/db/migrate.spec.tspackages/streamline/src/encoders/encoder-process.spec.tspackages/streamline/src/encoders/encoder-process.tspackages/streamline/src/encoders/ffmpeg-path.spec.tspackages/streamline/src/encoders/ffmpeg-path.tspackages/streamline/src/encoders/manager.spec.tspackages/streamline/src/encoders/manager.tspackages/streamline/src/ipc/handlers/encoder.spec.tspackages/streamline/src/ipc/handlers/encoder.tspackages/streamline/src/ipc/handlers/hotkeys.spec.tspackages/streamline/src/ipc/handlers/layout.spec.tspackages/streamline/src/ipc/handlers/library.spec.tspackages/streamline/src/ipc/handlers/secret.spec.tspackages/streamline/src/ipc/handlers/settings.spec.tspackages/streamline/src/ipc/handlers/system.spec.tspackages/streamline/src/ipc/handlers/system.tspackages/streamline/src/library/scan-worker.tspackages/streamline/src/library/scanner.spec.tspackages/streamline/src/library/watcher.spec.tspackages/streamline/src/update-check.spec.tspackages/streamline/src/update-check.tspackages/streamline/vitest.config.mts
💤 Files with no reviewable changes (1)
- .idea/vcs.xml
Signed-off-by: Robbin "Roboroads" Schepers <robbin@deschepers.nl>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/streamline/renderer/src/layout/persistence.svelte.ts (1)
7-18:⚠️ Potential issue | 🟠 MajorFix off-by-one in
v8 ignore next 10for the$effectblock
/* v8 ignore next 10 */on line 7 ignores lines 8–17 ofpackages/streamline/renderer/src/layout/persistence.svelte.ts, but the$effectcallback closes on line 18 (});), so that line may still count toward coverage. Change tov8 ignore next 11or wrap the block withv8 ignore start/v8 ignore stop.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/streamline/renderer/src/layout/persistence.svelte.ts` around lines 7 - 18, The v8 coverage pragma currently uses "v8 ignore next 10" but the $effect block spans 11 lines (closing on the line after the ignored range), so update the pragma to "v8 ignore next 11" or replace it with a paired "v8 ignore start" / "v8 ignore stop" around the entire $effect block; locate the block by the $effect(...) callback that references layoutStore.active, $state.snapshot(layout), DEBOUNCE_MS, and the window.streamline.api.layout.save(...) call and adjust the v8 ignore directives to cover the full callback including its closing "});".
♻️ Duplicate comments (1)
packages/streamline/renderer/src/modules/crossfader/Crossfader.svelte (1)
98-102:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove
{!true /* v8 ignore ... */}from<select>markup.Line 98 and Line 114 use boolean interpolation as a coverage marker inside
<select>. This is a risky pattern in Svelte templates and can introduce unintended text nodes instead of remaining non-rendering metadata.Suggested minimal patch
- <!-- prettier-ignore --> - {!true /* v8 ignore start */} - {`#each` decks as d (d.id)}<option value={deckId(d.id)} label={deckLabel(d.title, d.id)} - ></option>{/each} - <!-- prettier-ignore --> - {!true /* v8 ignore stop */} + {`#each` decks as d (d.id)} + <option value={deckId(d.id)} label={deckLabel(d.title, d.id)}></option> + {/each} ... - <!-- prettier-ignore --> - {!true /* v8 ignore start */} - {`#each` decks as d (d.id)}<option value={deckId(d.id)} label={deckLabel(d.title, d.id)} - ></option>{/each} - <!-- prettier-ignore --> - {!true /* v8 ignore stop */} + {`#each` decks as d (d.id)} + <option value={deckId(d.id)} label={deckLabel(d.title, d.id)}></option> + {/each}In Svelte template markup, what is rendered by `{false}` or `{!true}` inside normal element content (not attributes)?Also applies to: 114-118
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/streamline/renderer/src/modules/crossfader/Crossfader.svelte` around lines 98 - 102, Remove the boolean interpolations `{!true /* v8 ignore start */}` and `{!true /* v8 ignore stop */}` inside the <select> markup in Crossfader.svelte (they can create real text nodes); instead delete those expressions and, if you need non-rendering markers, replace them with HTML comments like `<!-- v8 ignore start -->` / `<!-- v8 ignore stop -->` around the `{`#each` decks as d (d.id)}` block that renders `<option value={deckId(d.id)} label={deckLabel(d.title, d.id)}></option>`, leaving the decks/ deckId / deckLabel logic unchanged.
🧹 Nitpick comments (6)
packages/streamline/renderer/src/modules/queue/Queue.component.spec.ts (1)
423-438: ⚡ Quick winStrengthen the dedup test so it actually proves deduplication.
expect(loadSongCalls).toEqual([])can still pass when duplicates are not deduplicated, so this test may miss regressions in the dedup path. Add an assertion for the single-deck outcome (e.g., the “Autoplay needs a second linked deck” warning) to make the behavior unambiguous.♻️ Suggested assertion tightening
it('deduplicates linked deck IDs when filtering active decks', () => { settingsHolder.current = JSON.stringify({ autoplay: true, linkedDeckIds: ['d1', 'd1'] }); layoutInstancesHolder.current = [{ id: 'd1', moduleId: 'deck' }]; const loadSongCalls: unknown[] = []; + const toasts: Array<{ message: string; type: string }> = []; eventBus.on('deck:d1:load-song', () => loadSongCalls.push('d1')); + eventBus.on('toast:show', (payload) => + toasts.push(payload as { message: string; type: string }) + ); render(Queue, { instanceId: 'q-dedup' }); queueAddSong()(makeSong()); eventBus.emit('deck:d1:state', { state: 'unloaded' as DeckState }); eventBus.emit('deck:d1:ended', undefined); expect(loadSongCalls).toEqual([]); + expect(toasts).toEqual([{ message: 'Autoplay needs a second linked deck', type: 'warning' }]); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/streamline/renderer/src/modules/queue/Queue.component.spec.ts` around lines 423 - 438, The test currently only checks loadSongCalls and can pass even if deduplication is broken; update the "deduplicates linked deck IDs when filtering active decks" test that renders Queue and calls queueAddSong() to also assert the single-deck outcome so dedup is actually verified: after emitting deck:d1:state and deck:d1:ended, assert that the component/emitter emitted the "Autoplay needs a second linked deck" warning (or the equivalent message/event produced by Queue when only one linked deck remains) and/or that the internal single-deck flag/state is set; reference the same test setup (settingsHolder.current, layoutInstancesHolder.current, loadSongCalls, eventBus, Queue, queueAddSong) and add an expectation for the warning/event in addition to expect(loadSongCalls).toEqual([]).packages/streamline/src/audio/port.spec.ts (1)
57-64: ⚡ Quick winAdd assertion before invoking
didFinishLoad!()to catch registration failures.The past review recommended adding
expect(didFinishLoad).toBeTypeOf('function')before callingdidFinishLoad!()to ensure the listener was actually registered. This assertion is still missing. Without it, ifcreateAudioPortsilently fails to register the listener, the test will crash with a confusing error instead of a clear assertion failure.🛡️ Recommended assertion
it('starts port1 and transfers port2 on did-finish-load', async () => { const { createAudioPort } = await import('./port'); createAudioPort(mockWindow as unknown as import('electron').BrowserWindow); const channel = channelRef.current as ReturnType<typeof makeChannel>; + expect(didFinishLoad).toBeTypeOf('function'); didFinishLoad!(); expect(channel.port1.start).toHaveBeenCalledOnce(); expect(mockWebContents.postMessage).toHaveBeenCalledWith('audio:port', null, expect.any(Array)); });Apply the same pattern at lines 66-75, 77-88, and 90-99.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/streamline/src/audio/port.spec.ts` around lines 57 - 64, Add an explicit assertion that the didFinishLoad listener was registered before invoking it: in the test case that calls createAudioPort(mockWindow...), assert expect(typeof didFinishLoad).toBe('function') immediately before calling didFinishLoad!(); do the same in the other related test blocks (the ones covering the other ranges) so that createAudioPort and the listener registration (didFinishLoad) are validated before invocation.packages/streamline/src/audio/pcm-receiver.ts (1)
7-8: Tighten thePcmMessageguard:buffernull seems impossible; focus onframes===0.
PcmMessageis only constructed withbuffer: new ArrayBuffer(...)inpackages/streamline/src/audio/pcm-receiver.spec.tsand frompackages/streamline/renderer/src/audio/port.tsviasendPcm(buffer: ArrayBuffer, frames: number, ...), which always passes a non-nullArrayBufferinto{ buffer, frames, ... }. This makes the!msg.bufferportion ofpackages/streamline/src/audio/pcm-receiver.ts’s guard likely redundant.frames === 0is the only remaining meaningful edge case (existing tests use positiveframes), so either add a unit test forframes=0or remove the guard if upstream guarantees non-empty chunks.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/streamline/src/audio/pcm-receiver.ts` around lines 7 - 8, The guard in pcm-receiver.ts currently checks "!msg.buffer || msg.frames === 0" but PcmMessage is always constructed with a non-null ArrayBuffer (see PcmMessage usage and sendPcm), so tighten the check to only guard the meaningful edge case: replace the guard with a single check for msg.frames === 0 (e.g., if (msg.frames === 0) return;) or alternatively assert/throw if buffer is unexpectedly null using a non-null assertion for PcmMessage; update or add a unit test in pcm-receiver.spec.ts for frames === 0 if you remove the buffer null branch.packages/streamline/src/ipc/handlers/system.spec.ts (1)
70-74: ⚡ Quick winTighten log-folder assertion to the exact expected path.
stringContaining('logs')can pass even when the handler uses the wrong base directory.Proposed assertion improvement
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { join } from 'node:path'; @@ it('SYSTEM_OPEN_LOG_FOLDER calls shell.openPath with logs dir', async () => { appMock.getPath.mockReturnValue('/fake/userData'); await getHandler('system:openLogFolder')(null); - expect(shellMock.openPath).toHaveBeenCalledWith(expect.stringContaining('logs')); + expect(shellMock.openPath).toHaveBeenCalledWith(join('/fake/userData', 'logs')); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/streamline/src/ipc/handlers/system.spec.ts` around lines 70 - 74, The test currently uses a loose assertion expect.stringContaining('logs'); update the assertion in the "SYSTEM_OPEN_LOG_FOLDER calls shell.openPath with logs dir" test to assert the exact expected path produced from appMock.getPath plus the "logs" folder (use the same path-joining logic as the handler, e.g., path.join(appMock.getPath(), 'logs')) and assert shellMock.openPath was called with that exact normalized path (reference: getHandler('system:openLogFolder'), appMock.getPath, shellMock.openPath).packages/streamline/src/ipc/handlers/layout.spec.ts (1)
112-119: ⚡ Quick winAssert imported instance-to-layout linkage explicitly.
This test can still pass if imported instances keep the old
layoutId, which would break parent-child linkage when persisted.Proposed test hardening
it('LAYOUT_IMPORT assigns new UUIDs and returns layout', () => { const original = makeLayout(); const json = JSON.stringify(original); const result = getHandler('layout:import')(null, json) as Layout; expect(result.id).not.toBe(original.id); expect(result.instances[0].id).not.toBe(original.instances[0].id); + expect(result.instances[0].layoutId).toBe(result.id); expect(result.isActive).toBe(false); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/streamline/src/ipc/handlers/layout.spec.ts` around lines 112 - 119, The test currently checks that imported layout and instance IDs change but doesn't assert that each imported instance's layoutId was updated to the new layout id; update the LAYOUT_IMPORT spec (the getHandler('layout:import') result handling) to assert that each instance's layoutId equals result.id (and does not equal original.id), e.g., iterate result.instances and check instance.layoutId === result.id (or at least assert result.instances[0].layoutId === result.id) to ensure parent-child linkage is preserved after import.packages/streamline/renderer/src/layout/store.svelte.spec.ts (1)
32-32: ⚡ Quick winType cast bypasses type safety.
The cast
null as unknown as Layoutforces null into a Layout-typed parameter, which is a type lie. IflayoutStore.set()is designed to accept null (as evidenced by the test on lines 51–53 verifying null handling), the type signature should beLayout | nullinstead of requiring a cast.♻️ Update the store type signature
Update
layoutStore.set()signature to acceptLayout | null:-set(layout: Layout): void { +set(layout: Layout | null): void {Then remove the cast in the test:
-layoutStore.set(null as unknown as Layout); +layoutStore.set(null);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/streamline/renderer/src/layout/store.svelte.spec.ts` at line 32, The test is using a type-unsafe cast (`null as unknown as Layout`) because the store API requires a non-null Layout; change the store's type signature so its setter accepts `Layout | null` (update the exported/store type where `layoutStore.set` is declared) and then remove the cast in the spec so the test can call `layoutStore.set(null)` directly; ensure any related types or interfaces referencing Layout are adjusted to accommodate null where appropriate.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/streamline/renderer/src/modules/deck/deck-audio.ts`:
- Line 38: The v8 ignore count at the top of deck-audio.ts is too short and
currently skips lines 39–139 but needs to include getPosition() through line 140
and the blank line 141; update the comment on line 38 from "/* v8 ignore next
101 */" to "/* v8 ignore next 103 */" to cover the extra two lines, then re-run
tests/lint to ensure no overlap with getDuration() (which starts at
getDuration()) and the subsequent "/* v8 ignore next 3 */" at the next block.
In
`@packages/streamline/renderer/src/modules/local-output/LocalOutput.component.spec.ts`:
- Around line 31-41: Tests mutate navigator.mediaDevices without restoring it,
causing cross-suite leakage; capture the original descriptor (via
Object.getOwnPropertyDescriptor or store navigator.mediaDevices) before you
redefine it in the spec (the block that sets Object.defineProperty on navigator,
and any similar mocks at lines referenced), and restore the original descriptor
in an afterEach/afterAll cleanup so the global navigator is returned to its
prior state; update the LocalOutput.component.spec.ts to save the original
value/descriptor, replace the mediaDevices for the test, and restore it in
afterEach to avoid flakiness across suites.
---
Outside diff comments:
In `@packages/streamline/renderer/src/layout/persistence.svelte.ts`:
- Around line 7-18: The v8 coverage pragma currently uses "v8 ignore next 10"
but the $effect block spans 11 lines (closing on the line after the ignored
range), so update the pragma to "v8 ignore next 11" or replace it with a paired
"v8 ignore start" / "v8 ignore stop" around the entire $effect block; locate the
block by the $effect(...) callback that references layoutStore.active,
$state.snapshot(layout), DEBOUNCE_MS, and the
window.streamline.api.layout.save(...) call and adjust the v8 ignore directives
to cover the full callback including its closing "});".
---
Duplicate comments:
In `@packages/streamline/renderer/src/modules/crossfader/Crossfader.svelte`:
- Around line 98-102: Remove the boolean interpolations `{!true /* v8 ignore
start */}` and `{!true /* v8 ignore stop */}` inside the <select> markup in
Crossfader.svelte (they can create real text nodes); instead delete those
expressions and, if you need non-rendering markers, replace them with HTML
comments like `<!-- v8 ignore start -->` / `<!-- v8 ignore stop -->` around the
`{`#each` decks as d (d.id)}` block that renders `<option value={deckId(d.id)}
label={deckLabel(d.title, d.id)}></option>`, leaving the decks/ deckId /
deckLabel logic unchanged.
---
Nitpick comments:
In `@packages/streamline/renderer/src/layout/store.svelte.spec.ts`:
- Line 32: The test is using a type-unsafe cast (`null as unknown as Layout`)
because the store API requires a non-null Layout; change the store's type
signature so its setter accepts `Layout | null` (update the exported/store type
where `layoutStore.set` is declared) and then remove the cast in the spec so the
test can call `layoutStore.set(null)` directly; ensure any related types or
interfaces referencing Layout are adjusted to accommodate null where
appropriate.
In `@packages/streamline/renderer/src/modules/queue/Queue.component.spec.ts`:
- Around line 423-438: The test currently only checks loadSongCalls and can pass
even if deduplication is broken; update the "deduplicates linked deck IDs when
filtering active decks" test that renders Queue and calls queueAddSong() to also
assert the single-deck outcome so dedup is actually verified: after emitting
deck:d1:state and deck:d1:ended, assert that the component/emitter emitted the
"Autoplay needs a second linked deck" warning (or the equivalent message/event
produced by Queue when only one linked deck remains) and/or that the internal
single-deck flag/state is set; reference the same test setup
(settingsHolder.current, layoutInstancesHolder.current, loadSongCalls, eventBus,
Queue, queueAddSong) and add an expectation for the warning/event in addition to
expect(loadSongCalls).toEqual([]).
In `@packages/streamline/src/audio/pcm-receiver.ts`:
- Around line 7-8: The guard in pcm-receiver.ts currently checks "!msg.buffer ||
msg.frames === 0" but PcmMessage is always constructed with a non-null
ArrayBuffer (see PcmMessage usage and sendPcm), so tighten the check to only
guard the meaningful edge case: replace the guard with a single check for
msg.frames === 0 (e.g., if (msg.frames === 0) return;) or alternatively
assert/throw if buffer is unexpectedly null using a non-null assertion for
PcmMessage; update or add a unit test in pcm-receiver.spec.ts for frames === 0
if you remove the buffer null branch.
In `@packages/streamline/src/audio/port.spec.ts`:
- Around line 57-64: Add an explicit assertion that the didFinishLoad listener
was registered before invoking it: in the test case that calls
createAudioPort(mockWindow...), assert expect(typeof
didFinishLoad).toBe('function') immediately before calling didFinishLoad!(); do
the same in the other related test blocks (the ones covering the other ranges)
so that createAudioPort and the listener registration (didFinishLoad) are
validated before invocation.
In `@packages/streamline/src/ipc/handlers/layout.spec.ts`:
- Around line 112-119: The test currently checks that imported layout and
instance IDs change but doesn't assert that each imported instance's layoutId
was updated to the new layout id; update the LAYOUT_IMPORT spec (the
getHandler('layout:import') result handling) to assert that each instance's
layoutId equals result.id (and does not equal original.id), e.g., iterate
result.instances and check instance.layoutId === result.id (or at least assert
result.instances[0].layoutId === result.id) to ensure parent-child linkage is
preserved after import.
In `@packages/streamline/src/ipc/handlers/system.spec.ts`:
- Around line 70-74: The test currently uses a loose assertion
expect.stringContaining('logs'); update the assertion in the
"SYSTEM_OPEN_LOG_FOLDER calls shell.openPath with logs dir" test to assert the
exact expected path produced from appMock.getPath plus the "logs" folder (use
the same path-joining logic as the handler, e.g., path.join(appMock.getPath(),
'logs')) and assert shellMock.openPath was called with that exact normalized
path (reference: getHandler('system:openLogFolder'), appMock.getPath,
shellMock.openPath).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 186b2815-0ebe-4e17-8981-93005a2cddfe
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (98)
.github/workflows/test.yml.idea/vcs.xmlpackage.jsonpackages/audio-worklet/package.jsonpackages/audio-worklet/src/tap-processor.tspackages/audio-worklet/vitest.config.mtspackages/shared/package.jsonpackages/shared/vitest.config.mtspackages/streamline/package.jsonpackages/streamline/renderer/src/audio/context.spec.tspackages/streamline/renderer/src/audio/context.tspackages/streamline/renderer/src/audio/mixer-bridge.spec.tspackages/streamline/renderer/src/audio/mixer-bridge.tspackages/streamline/renderer/src/audio/port.spec.tspackages/streamline/renderer/src/components/DbMeter.component.spec.tspackages/streamline/renderer/src/components/IconButton.component.spec.tspackages/streamline/renderer/src/components/Modal.component.spec.tspackages/streamline/renderer/src/components/Modal.sveltepackages/streamline/renderer/src/components/OnAirIndicator.component.spec.tspackages/streamline/renderer/src/components/StatusBar.component.spec.tspackages/streamline/renderer/src/components/StatusBar.sveltepackages/streamline/renderer/src/components/Toast.component.spec.tspackages/streamline/renderer/src/components/ToastContainer.component.spec.tspackages/streamline/renderer/src/components/forms/BitrateField.component.spec.tspackages/streamline/renderer/src/components/forms/ComboField.component.spec.tspackages/streamline/renderer/src/components/forms/ComboField.sveltepackages/streamline/renderer/src/components/forms/FilenameField.component.spec.tspackages/streamline/renderer/src/components/forms/FolderField.component.spec.tspackages/streamline/renderer/src/components/forms/FormField.component.spec.tspackages/streamline/renderer/src/components/forms/NumberField.component.spec.tspackages/streamline/renderer/src/components/forms/NumberField.sveltepackages/streamline/renderer/src/components/forms/SampleRateField.component.spec.tspackages/streamline/renderer/src/components/forms/SelectField.component.spec.tspackages/streamline/renderer/src/components/forms/TextField.component.spec.tspackages/streamline/renderer/src/drag-drop/song-drag.spec.tspackages/streamline/renderer/src/first-run/setup.spec.tspackages/streamline/renderer/src/hotkeys/binder.spec.tspackages/streamline/renderer/src/hotkeys/reset-to-default.spec.tspackages/streamline/renderer/src/hotkeys/reset-to-default.tspackages/streamline/renderer/src/hotkeys/store.svelte.spec.tspackages/streamline/renderer/src/layout/persistence.svelte.spec.tspackages/streamline/renderer/src/layout/persistence.svelte.tspackages/streamline/renderer/src/layout/store.svelte.spec.tspackages/streamline/renderer/src/modules/context.spec.tspackages/streamline/renderer/src/modules/crossfader/Crossfader.component.spec.tspackages/streamline/renderer/src/modules/crossfader/Crossfader.sveltepackages/streamline/renderer/src/modules/deck/Deck.component.spec.tspackages/streamline/renderer/src/modules/deck/Deck.sveltepackages/streamline/renderer/src/modules/deck/DeckSettingsModal.component.spec.tspackages/streamline/renderer/src/modules/deck/WaveformDisplay.component.spec.tspackages/streamline/renderer/src/modules/deck/WaveformDisplay.sveltepackages/streamline/renderer/src/modules/deck/deck-audio.tspackages/streamline/renderer/src/modules/deck/waveform-worker.spec.tspackages/streamline/renderer/src/modules/deck/waveform-worker.tspackages/streamline/renderer/src/modules/encoders/EncoderModal.component.spec.tspackages/streamline/renderer/src/modules/encoders/EncoderModal.sveltepackages/streamline/renderer/src/modules/encoders/Encoders.component.spec.tspackages/streamline/renderer/src/modules/encoders/Encoders.sveltepackages/streamline/renderer/src/modules/instance-store.svelte.spec.tspackages/streamline/renderer/src/modules/local-output/LocalOutput.component.spec.tspackages/streamline/renderer/src/modules/local-output/LocalOutput.sveltepackages/streamline/renderer/src/modules/microphone/Microphone.component.spec.tspackages/streamline/renderer/src/modules/microphone/Microphone.sveltepackages/streamline/renderer/src/modules/queue/Queue.component.spec.tspackages/streamline/renderer/src/modules/queue/Queue.sveltepackages/streamline/renderer/src/modules/queue/QueueSettingsModal.component.spec.tspackages/streamline/renderer/src/modules/queue/rotation.tspackages/streamline/renderer/src/modules/register-builtins.spec.tspackages/streamline/renderer/src/windows/WindowManager.component.spec.tspackages/streamline/renderer/src/windows/WindowManager.sveltepackages/streamline/renderer/src/windows/WindowWrapper.component.spec.tspackages/streamline/renderer/src/windows/WindowWrapper.sveltepackages/streamline/renderer/src/windows/use-interact.spec.tspackages/streamline/src/audio/pcm-receiver.spec.tspackages/streamline/src/audio/pcm-receiver.tspackages/streamline/src/audio/port.spec.tspackages/streamline/src/db/migrate.spec.tspackages/streamline/src/encoders/encoder-process.spec.tspackages/streamline/src/encoders/encoder-process.tspackages/streamline/src/encoders/ffmpeg-path.spec.tspackages/streamline/src/encoders/ffmpeg-path.tspackages/streamline/src/encoders/manager.spec.tspackages/streamline/src/encoders/manager.tspackages/streamline/src/ipc/handlers/encoder.spec.tspackages/streamline/src/ipc/handlers/encoder.tspackages/streamline/src/ipc/handlers/hotkeys.spec.tspackages/streamline/src/ipc/handlers/layout.spec.tspackages/streamline/src/ipc/handlers/library.spec.tspackages/streamline/src/ipc/handlers/secret.spec.tspackages/streamline/src/ipc/handlers/settings.spec.tspackages/streamline/src/ipc/handlers/system.spec.tspackages/streamline/src/ipc/handlers/system.tspackages/streamline/src/library/scan-worker.tspackages/streamline/src/library/scanner.spec.tspackages/streamline/src/library/watcher.spec.tspackages/streamline/src/update-check.spec.tspackages/streamline/src/update-check.tspackages/streamline/vitest.config.mts
💤 Files with no reviewable changes (1)
- .idea/vcs.xml
…te ignore hacks Signed-off-by: Robbin "Roboroads" Schepers <robbin@deschepers.nl>
Signed-off-by: Robbin "Roboroads" Schepers <robbin@deschepers.nl>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add test coverage across all renderer modules (Encoders, Deck, Queue, Microphone, LocalOutput, StatusBar, forms) using v8 ignore annotations for Electron IPC/jsdom-incompatible paths and by extracting Svelte compiler-generated dirty-check branches to
$derivedhelpers. Covers Svelte 5?? ''null-guard branches, class array&&expressions, and drag-and-drop event handlers.Summary by CodeRabbit
Release Notes
Tests
Documentation
Chores