Merge bugFixes0.1.27 updates#10
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro (Legacy) Run ID: 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbit
✨ Finishing Touches📝 Generate docstrings
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 26
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/src/services/recording-capture.ts (1)
1335-1483:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winClear capture state when full recovery exhausts retries.
This path stops/finalizes the current capture before trying replacements. If every retry fails, it logs and returns but leaves
activeCaptureset, soisCapturing()staystruewhile no recorder is actually running and a newstartCapture()will still throw.Proposed fix
if (!canRetry) { recordCaptureRecoveryDiagnostic('capture_recovery_failed', { meetingId: capture.meetingId, sourceType: getSourceType(capture.sourceId), segmentIndex: capture.segmentIndex, reason, attempt, expectedAudio, error: serializeErrorForDiagnostics(err) }) captureRecordingRecoveryFailure(err, { meetingId: capture.meetingId, sourceType: getSourceType(capture.sourceId), segmentIndex: capture.segmentIndex, reason, attemptCount: attempt, expectedAudio, failureKind: 'failed' }) + if (activeCapture === capture) { + activeCapture = null + } console.error('Failed to recover capture after device change:', err) return }Also applies to: 1531-1533
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/renderer/src/services/recording-capture.ts` around lines 1335 - 1483, The failure path leaves activeCapture set when all recovery attempts exhaust, causing isCapturing() to stay true; modify the recovery-failure handling in the catch branch (the code that calls recordCaptureRecoveryDiagnostic and captureRecordingRecoveryFailure and then returns) to first finalize the current capture if not already finalized (await finalizeCapture(capture)), set activeCapture = null, then proceed with diagnostics/logging and return; make the same change in the other early-return site referenced (the similar block around the later return) so both paths clear activeCapture and finalize the capture before returning.
🤖 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 `@e2e/notes-ready-notification.spec.ts`:
- Around line 115-116: The geometry assertions using
metrics.dotOffsetFromTextCenter and metrics.actionsOffsetFromTextCenter are too
strict and can be flaky; relax the tolerance by updating the expectations in the
test (e2e/notes-ready-notification.spec.ts) to use a larger threshold (e.g.,
replace <= 1 with <= 2 or <= 3) so the assertions still enforce centering but
accommodate DPI/font/rendering variance.
In `@scripts/prepare-macos-mlx-runtime.js`:
- Around line 51-55: isRuntimeReady() currently only checks BUNDLE_FORMAT,
WHEEL_PLATFORM and PACKAGES in the marker and misses validating the runtime's
target and python version, causing stale bundles to be reused; update the marker
validation in isRuntimeReady() (the boolean return that inspects the marker
string) to also verify marker.includes(`target=${TARGET}`) and
marker.includes(`python=${PYTHON}`) (or the equivalent variables used in this
file), and apply the same additional validations to the other marker-checking
block later in the file (the second occurrence around the 156–160 region) so
both checks prevent reuse when target or python change.
In `@scripts/seed-dev-ask-ai-fixtures.mjs`:
- Around line 203-211: The seed routine currently skips writing most files when
a fixture ID exists, leaving stale metadata.json, transcript.json and removed
files; update the logic in the loop handling existing fixtures (where variables
like existing, fixture, meetingDir and writeFile are used) to either remove and
recreate the meetingDir or explicitly overwrite all fixture files
(metadata.json, transcript.json, segments.json, transcript.error, notes, etc.)
every run; ensure you delete or empty meetingDir before writing or
unconditionally write all expected files for fixture.id so the on-disk fixture
state always matches the script definitions.
In `@src/main/ipc/chat-ipc.ts`:
- Around line 145-148: withTimeout(...) currently returns the fallback (e.g. [])
on timeout so the calendar fetch appears successful and never triggers the catch
that sets calendarFailureUntil; change the call site around
withTimeout(calendarManager.fetchAllRecentEvents(...), []) and
withTimeout(calendarManager.fetchAllUpcomingEvents(), []) to detect the
timeout/fallback result and treat it as an error (either by throwing a new Error
or explicitly setting calendarFailureUntil) when the returned value equals the
fallback sentinel, so slow/timeout calendar fetches arm the backoff logic the
same way real exceptions do.
In `@src/main/ipc/llm-ipc.ts`:
- Around line 59-60: Wrap the optional callback invocation
onManualSegmentationRetry?.(meetingId) in a local try/catch so any thrown error
is caught and logged but does not prevent the subsequent call to
segmentationService.retry(meetingId); specifically, call
onManualSegmentationRetry(meetingId) inside try { ... } catch (err) { /* log
using the existing logger (e.g. processLogger.error or console.error) */ } then
always call segmentationService.retry(meetingId) outside the catch to ensure
retry runs best-effort.
In `@src/main/ipc/recording-ipc.ts`:
- Around line 615-630: The code currently enqueues transcription unconditionally
even when assembleRecordingAudioSegments throws; change the flow so
transcriptionService.enqueue(meetingId) and the subsequent logRecordingDebug
call only run when assembleRecordingAudioSegments succeeds: catch the error with
logAutodocFailure (and keep the console.error) but do NOT call
transcriptionService.enqueue or logRecordingDebug in the catch path (either move
those two calls inside the try after await assembleRecordingAudioSegments(...)
or return/rethrow inside the catch to prevent further execution); reference
assembleRecordingAudioSegments, transcriptionService.enqueue, logAutodocFailure,
and logRecordingDebug to locate and update the logic for
meetingId/postProcessStartedAt handling.
In `@src/main/services/__tests__/diarization.test.ts`:
- Around line 181-187: Replace the repeated "as any" casts by creating a single
typed internals object (e.g. `const internals = service as unknown as {
ensureModelReady: () => Promise<string>; resolveExistingModelPath: () =>
Promise<string>; isPythonEnvUsable: (pythonPath: string, modelPath: string) =>
Promise<boolean> }`) and then call vi.spyOn(internals, 'ensureModelReady'),
vi.spyOn(service, 'isReady'), vi.spyOn(internals, 'resolveExistingModelPath'),
and vi.spyOn(internals, 'isPythonEnvUsable') with the same mockResolvedValue
returns; this removes explicit any casts while keeping spies on the private
methods ensureModelReady, resolveExistingModelPath, and isPythonEnvUsable.
In `@src/main/services/__tests__/llm.test.ts`:
- Around line 146-166: The tests use repeated (provider as any) casts causing
lint failures; create a single typed test-harness interface (e.g.,
TestProviderHarness) that exposes private helpers parseResponse,
extractTimestampsMs, and parseTranscriptLines, then cast provider once via const
harness = provider as unknown as TestProviderHarness and replace all occurrences
of (provider as any).parseResponse / .extractTimestampsMs /
.parseTranscriptLines with harness.parseResponse / harness.extractTimestampsMs /
harness.parseTranscriptLines across the test file (including the other
occurrences noted) to eliminate no-explicit-any warnings.
In `@src/main/services/__tests__/whisper-onboarding-install.test.ts`:
- Around line 145-148: The tests currently early-return when process.arch !==
'arm64' (e.g., the "uses MLX Whisper by default on Apple Silicon..." test and
the other two at the mentioned ranges), which skips MLX paths on x64 CI; instead
of returning early, stub process.arch to 'arm64' for the test (use the existing
platform stubbing pattern used elsewhere or jest.spyOn/Object.defineProperty to
set process.arch) and restore the original value in afterEach/ finally so the
test runs deterministically on all runners; apply the same change to the other
two tests referenced (around lines 213-215 and 246-248) so all MLX-specific
assertions execute under a simulated arm64 environment.
- Around line 41-43: The test helper is forcing
AUTODOC_MAC_TRANSCRIPTION_BACKEND to 'whisper-cpp' when options?.macBackend is
undefined because the condition treats undefined as "not auto"; change the
condition to only set process.env.AUTODOC_MAC_TRANSCRIPTION_BACKEND when
options?.macBackend is explicitly provided and not equal to 'auto' (e.g., check
options?.macBackend !== undefined && options.macBackend !== 'auto' or use typeof
to guard), leaving the env var untouched when no override was requested; update
the block that references platform and options?.macBackend accordingly.
In `@src/main/services/auto-updater.ts`:
- Around line 75-77: The unconditional setTimeout(() => broadcast({ state:
'idle' }), 30_000) can clobber newer updater states; instead, replace this with
a guarded reset: either store the timer id and clearTimeout(...) whenever the
updater advances to a new state, or have the timeout handler check the current
updater state (e.g., an updaterState variable or a getUpdaterState() accessor)
and only call broadcast({ state: 'idle' }) if the state is still the
error/stalled state; update the logic around setTimeout and broadcast to ensure
the idle reset is canceled or skipped when a newer state exists.
In `@src/main/services/chat-retrieval.ts`:
- Around line 372-405: The direct-answer branches for intent 'count' and 'list'
currently use the full inventory variable; update these branches to run the same
question-filtering logic used in ranked retrieval (the code that produces the
filtered/matched set used later for ranking) and use that filtered set when
computing directAnswer, matched, and any counts, while keeping inventory as the
full library; specifically, call/reuse the existing filtering function or logic
(the same code that produces the matched/ranked results earlier) to produce a
filteredMatched variable and then compute directAnswer (`You have
${filteredMatched.length}...`) or pass filteredMatched to
formatRecordingListAnswer, and return matched: filteredMatched (leave matchMode
as 'direct-count'/'direct-list' and keep other timing fields intact).
In `@src/main/services/crypto.ts`:
- Around line 444-447: The eviction currently removes the cache index before
deleting the on-disk temp file—specifically the
mediaDecryptCache.delete(encPath) occurs prior to awaiting
fsp.unlink(entry.tempPath), which can orphan files if unlink fails; change the
sequence in the eviction logic inside the function that performs cache eviction
so that you first await fsp.unlink(entry.tempPath) (or attempt unlink and
confirm success), only then remove the entry from mediaDecryptCache and subtract
totalBytes (mediaDecryptCache.delete(encPath) and totalBytes -= entry.size); if
unlink fails, keep the cache entry (or reinsert it) and handle/log the error so
the tempPath remains tracked for future cleanup.
In `@src/main/services/media-http-server.ts`:
- Around line 279-283: stopRecordingMediaHttpServer currently calls
mediaServer?.close() and immediately clears decrypted temp files which can break
in-flight streams from
handleMediaRequest/getDecryptedTempPathForMedia/createReadStream; change
stopRecordingMediaHttpServer to wait for mediaServer to fully close before
calling clearMediaDecryptCache by awaiting the server close completion (wrap
mediaServer.close(...) in a Promise that resolves on the close callback or
error), then call clearMediaDecryptCache(), and handle any close errors without
deleting files prematurely.
In `@src/main/services/ollama-embedding.ts`:
- Around line 65-68: The embed() method currently returns whatever embeddings
the Ollama response contains without checking that the number of returned
embeddings matches the input texts array; update the logic in embed (referencing
data.embeddings, data.embedding and the texts parameter and this.model) to
validate counts: if data.embeddings is an array ensure data.embeddings.length
=== texts.length and throw a descriptive Error if not; if data.embedding is a
single array only accept it when texts.length === 1 (otherwise throw); keep
existing error path for no embeddings but make the message include this.model
and the expected vs actual counts for easier debugging.
- Around line 21-42: The current check permanently caches transient
unavailability by setting this.availability = false on any error/timeout; change
it to use a short TTL instead: add a timestamp property (e.g.,
this.availabilityCheckedAt or this.availabilityExpires) and, in the
availability-checking method, return cached this.availability only if it is true
OR if the cached value is false but the timestamp is still within the TTL
window; when performing the fetch to `${this.baseUrl}/api/tags` (using
AVAILABILITY_TIMEOUT_MS), update both this.availability and the timestamp/expiry
based on the result or catch, and ensure that after the TTL expires a new
network check is attempted instead of permanently returning false.
In `@src/main/services/recording-title.ts`:
- Around line 15-17: The code returns metadata.customTitle as-is which treats
whitespace-only strings as valid; change the check in the recording title logic
to trim the value first (use metadata?.customTitle?.trim()), and only return it
when the trimmed string is non-empty so whitespace-only titles fall through to
the existing fallback logic; update the condition around the
metadata.customTitle return in the function that handles title resolution to use
the trimmed value.
In `@src/main/services/segmentation.ts`:
- Around line 279-284: When macProcessingProfile is missing, proactively reset
the LLM provider tuning to avoid leaking previous job settings: after obtaining
macProcessingProfile via
getEffectiveMacProcessingProfile/getMacProcessingProfile, keep the existing
branch that applies profile settings using this.llmProvider.setModel and
this.llmProvider.setLowMemoryMode, and add an else branch that calls
this.llmProvider.setModel with a neutral value (e.g., null or default model) and
this.llmProvider.setLowMemoryMode(false) so prior low-memory or model overrides
are cleared when macProcessingProfile is undefined.
In `@src/main/services/whisper-manager.ts`:
- Around line 524-527: The MLX branch (isMlxWhisperSelected +
ensureMlxWhisperReady) currently returns immediately with no fallback; wrap the
call to ensureMlxWhisperReady in a try/catch, and if it throws or returns a
failure, log the error, clear/disable the MLX selection (so future checks of
isMlxWhisperSelected are false) and fall back to the non-MLX whisper path by
invoking the existing whisper.cpp/mac bootstrap (e.g., call
ensureWhisperCppReady or the equivalent non-MLX initialization used elsewhere);
make the same change for the other occurrence of this pattern around the 697-739
region so Apple Silicon setup failures degrade to the whisper.cpp flow instead
of leaving transcription unavailable.
In `@src/renderer/src/components/LowSpecMacProcessingBanner.tsx`:
- Around line 45-48: The dismiss click handler (dismiss function) can cause an
unhandled rejection from
window.electronAPI.invoke('prefs:set-low-spec-mac-processing-banner-dismissed',
true); wrap the await call in a try/catch inside dismiss, and on error log the
failure (or call a telemetry/error handler) and restore or keep the UI state
(e.g., call setVisible(true) or avoid closing the banner) so the UI doesn't
remain out of sync; ensure any caught error is swallowed or re-thrown only after
recovering UI state to prevent noisy unhandled rejections.
- Line 3: Add explicit type annotations: change the component signature
LowSpecMacProcessingBanner to declare a return type of JSX.Element | null, and
update the dismiss click handler to be declared async (): Promise<void>. Also
wrap the awaited IPC invoke call inside the dismiss handler (the
ipcRenderer.invoke / window.electron.ipcRenderer.invoke call) with try/catch and
handle or log errors so rejections don’t leak from the click handler.
In `@src/renderer/src/components/onboarding/MicPermissionStep.tsx`:
- Around line 164-165: The JSX text in the MicPermissionStep component contains
an unescaped apostrophe ("else's") which triggers react/no-unescaped-entities;
fix by replacing the raw apostrophe with an escaped entity or a JS string
expression (e.g., use "else's" or {"else's"}) in the JSX where that copy is
rendered so the lint rule is satisfied while preserving the original text.
In `@src/renderer/src/pages/MeetingDetail.tsx`:
- Around line 536-538: The event listener for 'recording:entry-updated'
currently only reloads 'recording:get-detail', so post-processed media can
remain stale; update the listener in MeetingDetail.tsx to also invoke
'recording:get-media' and call setMedia with the result (similarly ensure you
re-run 'transcription:get-transcript' and 'speakers:get' if needed), i.e., when
handling 'recording:entry-updated' call
window.electronAPI.invoke('recording:get-media', id).then(setMedia) (and mirror
the same change for the other similar block around the lines handling recording
updates at 566-573) so the audio/video player appears after post-processing.
In `@src/renderer/src/services/recording-capture.ts`:
- Around line 673-694: The watchdog retries are unbounded because audio-only
recoveries reuse the same CaptureHandles without advancing
capture.recoverySequence, so modify the recovery path in recording-capture.ts
(the block that checks recoveryValidation and calls queueCaptureRecovery) to
increment or cap capture.recoverySequence before queuing a retry (or set a
per-capture retry flag) so that each audio-only retry advances toward
MAX_RECOVERY_ATTEMPTS; specifically, update the branch that decides the retry
reason (pendingSources.includes('mic') ? 'watchdog:mic-route-changed' :
'watchdog:output-route-changed') to increment capture.recoverySequence (or mark
the capture as exhausted) and persist that change so subsequent watchdog
invocations see the increment and eventually hit the degraded terminal path.
In `@src/renderer/src/stores/chat.ts`:
- Around line 25-53: The runtime mutations currently allow messages to grow
unbounded even though persistence is capped; modify useChatStore's mutation
handlers (addMessage, updateMessage, appendToMessage and any other place that
calls set with messages) to enforce the 20-message cap at runtime by slicing the
resulting messages array to the last 20 items (e.g. compute newMessages = /*
result of push/map */ and then set({ messages: newMessages.slice(-20) })); keep
clearMessages as-is and leave partialize unchanged.
---
Outside diff comments:
In `@src/renderer/src/services/recording-capture.ts`:
- Around line 1335-1483: The failure path leaves activeCapture set when all
recovery attempts exhaust, causing isCapturing() to stay true; modify the
recovery-failure handling in the catch branch (the code that calls
recordCaptureRecoveryDiagnostic and captureRecordingRecoveryFailure and then
returns) to first finalize the current capture if not already finalized (await
finalizeCapture(capture)), set activeCapture = null, then proceed with
diagnostics/logging and return; make the same change in the other early-return
site referenced (the similar block around the later return) so both paths clear
activeCapture and finalize the capture before returning.
🪄 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: ASSERTIVE
Plan: Pro (Legacy)
Run ID: c4d36e98-914b-44e3-8403-02d25acdcb69
⛔ Files ignored due to path filters (9)
resources/macos-whisper-runtime/arm64/libggml-base.0.dylibis excluded by!**/*.dylibresources/macos-whisper-runtime/arm64/libggml-blas.sois excluded by!**/*.soresources/macos-whisper-runtime/arm64/libggml-cpu-apple_m1.sois excluded by!**/*.soresources/macos-whisper-runtime/arm64/libggml-cpu-apple_m2_m3.sois excluded by!**/*.soresources/macos-whisper-runtime/arm64/libggml-cpu-apple_m4.sois excluded by!**/*.soresources/macos-whisper-runtime/arm64/libggml-metal.sois excluded by!**/*.soresources/macos-whisper-runtime/arm64/libggml.0.dylibis excluded by!**/*.dylibresources/macos-whisper-runtime/arm64/libomp.dylibis excluded by!**/*.dylibresources/macos-whisper-runtime/arm64/libwhisper.1.dylibis excluded by!**/*.dylib
📒 Files selected for processing (83)
.github/workflows/build.yml.gitignoree2e/notes-ready-notification.spec.tse2e/onboarding-real-download.spec.tse2e/qa-repro.spec.tselectron-builder.ymlelectron.vite.config.tspackage.jsonresources/macos-whisper-runtime/arm64/whisper-cppresources/mlx-whisper-transcribe.pyscripts/prepare-macos-mlx-runtime.jsscripts/seed-dev-ask-ai-fixtures.mjssrc/main/index.tssrc/main/ipc/__tests__/calendar-ipc.test.tssrc/main/ipc/__tests__/chat-ipc.test.tssrc/main/ipc/__tests__/llm-ipc.test.tssrc/main/ipc/__tests__/prefs-ipc.test.tssrc/main/ipc/__tests__/recording-ipc.test.tssrc/main/ipc/__tests__/transcription-ipc.test.tssrc/main/ipc/calendar-ipc.tssrc/main/ipc/chat-ipc.tssrc/main/ipc/llm-ipc.tssrc/main/ipc/prefs-ipc.tssrc/main/ipc/recording-ipc.tssrc/main/ipc/transcription-ipc.tssrc/main/notification-window.tssrc/main/services/__tests__/application-install.test.tssrc/main/services/__tests__/auto-updater.test.tssrc/main/services/__tests__/chat-retrieval.test.tssrc/main/services/__tests__/diarization.test.tssrc/main/services/__tests__/llm.test.tssrc/main/services/__tests__/local-processing-coordinator.test.tssrc/main/services/__tests__/mac-processing-profile.test.tssrc/main/services/__tests__/notes-ready-notifier.test.tssrc/main/services/__tests__/ollama-onboarding-install.test.tssrc/main/services/__tests__/segmentation.test.tssrc/main/services/__tests__/test-runtime.test.tssrc/main/services/__tests__/transcription.test.tssrc/main/services/__tests__/whisper-manager.test.tssrc/main/services/__tests__/whisper-onboarding-install.test.tssrc/main/services/application-install.tssrc/main/services/auto-updater.tssrc/main/services/autodoc-log.tssrc/main/services/chat-retrieval.tssrc/main/services/crypto.tssrc/main/services/detection.tssrc/main/services/dev-runtime-paths.tssrc/main/services/diarization.tssrc/main/services/error-classification.tssrc/main/services/llm.tssrc/main/services/local-processing-coordinator.tssrc/main/services/mac-processing-profile.tssrc/main/services/media-http-server.tssrc/main/services/notes-ready-notifier.tssrc/main/services/ollama-embedding.tssrc/main/services/ollama-manager.tssrc/main/services/prefs-store.tssrc/main/services/recording-title.tssrc/main/services/segmentation.tssrc/main/services/test-runtime.tssrc/main/services/transcription.tssrc/main/services/whisper-manager.tssrc/preload/ipc.d.tssrc/renderer/src/App.test.tsxsrc/renderer/src/App.tsxsrc/renderer/src/components/LowSpecMacProcessingBanner.tsxsrc/renderer/src/components/Sidebar.test.tsxsrc/renderer/src/components/Sidebar.tsxsrc/renderer/src/components/onboarding/MicPermissionStep.tsxsrc/renderer/src/components/onboarding/TranscriptionStep.tsxsrc/renderer/src/components/onboarding/__tests__/MicPermissionStep.test.tsxsrc/renderer/src/components/onboarding/__tests__/OllamaStep.test.tsxsrc/renderer/src/components/onboarding/__tests__/TranscriptionStep.test.tsxsrc/renderer/src/pages/AskAI.test.tsxsrc/renderer/src/pages/AskAI.tsxsrc/renderer/src/pages/MeetingDetail.test.tsxsrc/renderer/src/pages/MeetingDetail.tsxsrc/renderer/src/services/__tests__/recording-capture.test.tssrc/renderer/src/services/recording-capture.tssrc/renderer/src/stores/chat.tssrc/renderer/src/test/fixtures.tssrc/shared/constants.tssrc/shared/types.ts
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)
src/main/services/llm.ts (1)
1559-1597: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueConsider extracting scoring magic numbers to named constants.
The scoring formula uses several magic numbers (2.8, 1.08, 0.8, 4, 2, 1.5, 1.25, 240_000, 0.45, 0.35) that control evidence matching behavior. Extracting these to named constants would improve readability and make future tuning easier.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/services/llm.ts` around lines 1559 - 1597, The scoreEvidenceWindow method contains many hard-coded "magic" numeric weights and thresholds (e.g., shared * 0.8, queryCoverage * 4, density * 2, quantityBonus * 1.5, distancePenalty cap 1.25 and divisor 240_000, windowLengthPenalty multiplier 0.45, etc.); extract these numbers into clearly named module-level or class-level constants (for example EVIDENCE_SHARED_WEIGHT, EVIDENCE_QUERY_COVERAGE_WEIGHT, EVIDENCE_DENSITY_WEIGHT, EVIDENCE_QUANTITY_BONUS_WEIGHT, EVIDENCE_DISTANCE_PENALTY_CAP, EVIDENCE_DISTANCE_DIVISOR, EVIDENCE_WINDOW_LENGTH_PENALTY_MULTIPLIER, etc.), replace the literals in scoreEvidenceWindow with those constants, and ensure names reflect their semantic role so tuning is centralized and readable while leaving the scoring logic unchanged.
♻️ Duplicate comments (1)
src/main/ipc/chat-ipc.ts (1)
176-178:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDo not short-circuit all
count/listintents to local recordings.Line 176 currently captures all
count/listquestions, so calendar inventory/schedule asks can still be answered from recordings only instead of calendar events.Suggested fix
- if (intent === 'count' || intent === 'list') { + if ( + (intent === 'count' || intent === 'list') && + !isMeetingInventoryQuestion(question) && + !isCalendarScheduleQuestion(normalizeRecordingSearchText(question)) + ) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/ipc/chat-ipc.ts` around lines 176 - 178, The current branch that treats any intent === 'count' || intent === 'list' as recordings-only is too broad; narrow the condition so you only short-circuit to recordings when the question is actually about local recordings. Update the block around intent (the variables intent, clearConversationScope(session), and recordingIndex.buildContext(question, [])) to first detect whether the list/count question targets recordings (e.g., via entity check or helper like isRecordingQuery(question/entities)) and only call recordingIndex.buildContext and clearConversationScope for those cases; otherwise fall through to the normal calendar/event handling flow so calendar inventory and scheduling queries still query calendar sources. Ensure you add/use a clear predicate function name (e.g., isRecordingQuery or isCalendarQuery) and keep existing calendar handling intact when predicate indicates calendar-related.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/services/__tests__/whisper-onboarding-install.test.ts`:
- Around line 32-37: The function setArch currently lacks an explicit return
type which violates the `@typescript-eslint/explicit-function-return-type` rule;
update the function signature for setArch(arch: NodeJS.Architecture) to declare
a return type of void (i.e., setArch(arch: NodeJS.Architecture): void) so the
function explicitly returns nothing while keeping the existing body that calls
Object.defineProperty on process.
In `@src/renderer/src/components/onboarding/MicPermissionStep.tsx`:
- Line 70: The async arrow function restoreStepState lacks an explicit return
type; update its signature to include a Promise return type (e.g.,
Promise<void>) so it satisfies `@typescript-eslint/explicit-function-return-type`.
Change the declaration of restoreStepState to declare the return type and ensure
any internal return values align with that type (adjust to Promise<void> unless
the function actually returns a value, in which case use the appropriate
Promise<T>).
---
Outside diff comments:
In `@src/main/services/llm.ts`:
- Around line 1559-1597: The scoreEvidenceWindow method contains many hard-coded
"magic" numeric weights and thresholds (e.g., shared * 0.8, queryCoverage * 4,
density * 2, quantityBonus * 1.5, distancePenalty cap 1.25 and divisor 240_000,
windowLengthPenalty multiplier 0.45, etc.); extract these numbers into clearly
named module-level or class-level constants (for example EVIDENCE_SHARED_WEIGHT,
EVIDENCE_QUERY_COVERAGE_WEIGHT, EVIDENCE_DENSITY_WEIGHT,
EVIDENCE_QUANTITY_BONUS_WEIGHT, EVIDENCE_DISTANCE_PENALTY_CAP,
EVIDENCE_DISTANCE_DIVISOR, EVIDENCE_WINDOW_LENGTH_PENALTY_MULTIPLIER, etc.),
replace the literals in scoreEvidenceWindow with those constants, and ensure
names reflect their semantic role so tuning is centralized and readable while
leaving the scoring logic unchanged.
---
Duplicate comments:
In `@src/main/ipc/chat-ipc.ts`:
- Around line 176-178: The current branch that treats any intent === 'count' ||
intent === 'list' as recordings-only is too broad; narrow the condition so you
only short-circuit to recordings when the question is actually about local
recordings. Update the block around intent (the variables intent,
clearConversationScope(session), and recordingIndex.buildContext(question, []))
to first detect whether the list/count question targets recordings (e.g., via
entity check or helper like isRecordingQuery(question/entities)) and only call
recordingIndex.buildContext and clearConversationScope for those cases;
otherwise fall through to the normal calendar/event handling flow so calendar
inventory and scheduling queries still query calendar sources. Ensure you
add/use a clear predicate function name (e.g., isRecordingQuery or
isCalendarQuery) and keep existing calendar handling intact when predicate
indicates calendar-related.
🪄 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: ASSERTIVE
Plan: Pro (Legacy)
Run ID: 8a1bf9d3-e796-401f-9b2d-a4f4b845d8d1
📒 Files selected for processing (20)
e2e/notes-ready-notification.spec.tsscripts/prepare-macos-mlx-runtime.jsscripts/seed-dev-ask-ai-fixtures.mjssrc/main/ipc/chat-ipc.tssrc/main/ipc/llm-ipc.tssrc/main/services/__tests__/chat-retrieval.test.tssrc/main/services/__tests__/whisper-onboarding-install.test.tssrc/main/services/auto-updater.tssrc/main/services/chat-retrieval.tssrc/main/services/crypto.tssrc/main/services/llm.tssrc/main/services/media-http-server.tssrc/main/services/ollama-embedding.tssrc/main/services/recording-title.tssrc/main/services/segmentation.tssrc/main/services/whisper-manager.tssrc/renderer/src/components/LowSpecMacProcessingBanner.tsxsrc/renderer/src/components/onboarding/MicPermissionStep.tsxsrc/renderer/src/pages/MeetingDetail.tsxsrc/renderer/src/stores/chat.ts
This was referenced Jun 3, 2026
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.
Summary
Validation