feat(meetings): long-form recorder + Windows installer/dep fixes (v1.6.0-rc1)#22
Conversation
- recording service with mic + system loopback capture (Linux/Windows) - recordings + recording_segments tables with startup recovery sweep - RPC: start/pause/resume/stop, file import, export (txt/md/json/srt), transcribe and summarize with cancellation - LLM summarization with OpenAI/Groq/OpenRouter/Ollama/custom presets; API key kept in OS keyring - LLM auto-rename: replace default timestamp titles with topic-derived titles after transcription (toggle in Meetings settings, on by default) - dashboard meetings list/recorder/detail pages, settings sections for meetings and AI summary - popup pill shows live meeting state and duration counter - redact API keys and bearer tokens in logs - disable chromium timer + wake-up throttling so long meetings survive the window losing focus - backend pushes recorder state to dashboard via Qt WebChannel at 4 Hz (replacing HTTP polling) so the dashboard timer keeps ticking even when Chromium freezes the renderer's fetch pipeline under Wayland/NVIDIA - wall-clock derived duration in the dashboard recorder context so the timer is never wrong even between push events - tray menu "Stop active recording" as throttling-immune escape hatch when the dashboard renderer's HTTP transport is dead - CancelToken for cooperative transcription cancellation
…cribe - popup: rewrite Hyprland rules with valid windowrulev2 syntax (`title:^(Recording)$` matcher, the previous `match:title Recording` was silently rejected by hyprctl, so the popup spawned in the middle of the screen on Wayland builds). Augment resize_popup() to dispatch movewindowpixel/resizewindowpixel since Qt's set_position() is a no-op on Wayland toplevels. Default AppRun to QT_QPA_PLATFORM=wayland;xcb so Hyprland gets the native plugin where the rules actually apply. - audio: register the voiceflow:// custom URL scheme before QApplication and install VoiceFlowAudioSchemeHandler on the default profile so the MeetingDetailPage <audio> element can stream WAVs with byte-range support for seek/scrub. Pure logic was already in audio_scheme.py — only the Qt glue was missing, which is why the play button silently no-op'd. - retranscribe: add transcript_model column (idempotent migration), parameterize MeetingsController.transcribe with model/device/language overrides via a per-recording dict, expose recordings_retranscribe and recordings_list_cached_models RPCs, and add the RetranscribeDialog modal on MeetingDetailPage with a chip showing which model produced the current transcript. - ci: wipe stale .venv/build/dist before each setup so reruns can't inherit a half-built tree; bump minimum Python to 3.10 (uv.lock regen).
- pnpm update: vite 6.4.1→6.4.2, plus transitive bumps that close rollup, lodash, flatted, minimatch, picomatch, postcss CVEs - uv lock --upgrade: orjson 3.11.8→3.11.9, pygments 2.19.2→2.20.0, pytest 9.0.2→9.0.3 (pillow + filelock already at safe versions in the lock — alerts were stale) tsc clean. pytest 289/289 (the 1 pre-existing theme-default failure is unchanged, not a regression).
The popup-dock fix shipped in 9f75281 added subprocess calls to hyprctl from inside the AppImage, but those calls inherited the AppImage's PyInstaller-injected LD_LIBRARY_PATH. The bundled libstdc++.so.6 is older than the system one, so hyprctl couldn't find GLIBCXX_3.4.32 / 3.4.34 / CXXABI_1.3.15 symbols required by libhyprutils, libhyprwire, and libre2 — every windowrulev2 + dispatch call failed silently in the build, leaving the popup centered again. Same pattern as a5b565c (v1.5.1): wrap the spawn with services.process_env.system_env() so LD_LIBRARY_PATH/LD_PRELOAD/ PYTHONPATH revert to their pre-bundle values, letting hyprctl link against the system libs.
Inno Setup leaves the previous install's _internal/ tree intact when
upgrading. When a Python wheel changes its compiled-extension filename
across versions (e.g. PyAV 15.x → 17.x renamed av/codec/codec.cp312-win_amd64.pyd
to av/codec/codec.abi3.pyd), both the old and new .pyd end up coexisting
and Python imports the stale version-tagged one first — ImportError on
missing symbols.
[InstallDelete] Type: filesandordirs Name: {app}\_internal wipes the
whole bundled python tree before [Files] lays down the new version. User
data in %USERPROFILE%\.VoiceFlow is untouched.
Adds a top-of-README experimental notice for the new Meetings flow (v1.6.0-rc1) and a dedicated showcase section between How It Works and Guided Setup. Covers what it does (mic + system capture, long-form recording, local transcription, BYO-LLM summaries, auto-rename, re-transcribe, seek-able playback, multi-format export) and the privacy posture (everything local except the single summary call to a user-configured endpoint). Includes two new screenshots: meetings library and meeting detail.
Shows the BYO-LLM provider picker (OpenAI / Groq / OpenRouter / Local Ollama / custom OpenAI-compatible endpoint) plus endpoint, model, API key (OS keychain), and connection-test fields. Sits between the feature list and the privacy posture paragraph so readers can see exactly how the summary step is configured.
… count
A Windows user reported every meeting recording produced a 44-byte WAV
(WAV header only, zero PCM frames) and the log showed:
[WARN] [meeting] preview start failed |
{"error": "Could not open audio device 8: Error opening InputStream:
Invalid number of channels [PaErrorCode -9998]"}
WASAPI loopback / Stereo Mix devices on Windows are stereo-only —
sd.InputStream(channels=1, ...) is rejected with paInvalidChannelCount.
SoundDeviceAudioSource was hard-coding channels=1 unconditionally, so
the meeting code's loopback branch never opened a stream and the
recorder's WAV writer flushed only its 44-byte header.
Fix: query sd.query_devices()[idx]["max_input_channels"] and try the
device-native channel count before falling back to mono. _make_pa_callback
already downmixes N-channel input to mono, so opening at channels=2 is
transparent to everything downstream.
Mic path is unchanged (mono first, stereo fallback as belt-and-braces).
Loopback path tries native first (typically 2), then mono.
Tested with a fake sounddevice module that mimics PortAudio's -9998
rejection; 5 new regression tests in test_audio_source_loopback_channels.py.
Full suite: 294 passed, 1 pre-existing failure unchanged.
The previous fix in 96b0b73 read sd.query_devices()[idx]['max_input_channels'] and clamped to 1 — but WASAPI loopback targets ARE output devices (speakers being captured). For those, max_input_channels=0 and the real stereo channel count lives under max_output_channels. So the prior fix ended up trying channels=1 just like before and PortAudio still rejected with `Invalid number of channels [PaErrorCode -9998]`. Live Windows user confirmed the same -9998 in the rebuilt artifact (sha 96b0b73, exe mtime 2026-05-12 20:29:08). This commit reads max_output_channels first for loopback, falls through input channels then mono if PortAudio still refuses. The downmix in _make_pa_callback already handles N→1 channel collapse, so opening channels=2 is transparent to the rest of the recorder. The regression tests in test_audio_source_loopback_channels.py were themselves wrong — they stubbed the loopback device with max_input_channels=2, which is not what real Windows enumeration looks like. Stubs corrected to mirror real WASAPI loopback enumeration (max_input_channels=0, max_output_channels=2), plus a mono-loopback case and a 3-step fallback test. 6/6 pass; full suite 295/295 (one unchanged pre-existing failure).
Replaces the longer marketing-flavored README with a denser, scannable version: dashboard hero → one-line tagline → download badges → "What it does" → Features → Meetings (experimental) → vs cloud → Install → Build from source → Stack → License. Same factual content; shorter, fewer headings, fewer screenshots inline (model-picker and onboarding shots dropped — they were duplicating what's on the website). Meetings section keeps the detail screenshot since it's the most visually distinctive part of the new feature.
Replaces the plain "VoiceFlow" H1 with the wordmark from public/. Uses <picture> with prefers-color-scheme so GitHub renders the correct variant on light or dark themes. Dashboard hero is moved below the tagline so the visual order reads logo → tagline → screenshot.
📝 WalkthroughWalkthroughAdds a complete Meetings feature (recording, transcription, summarization) with DB, audio, LLM, RPC, push events, React UI, and tests, plus CI/installer tweaks, version/dependency bumps, and README restructure. ChangesFull PR change set
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src-pyloid/server.py (1)
55-72:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSettings RPC still drops two Meetings toggles.
update_settingsonly whitelistsrecordingsAutoRenameTitle, sorecordingsAutoTranscribe/recordingsAutoSummarizeupdates from the Meetings settings UI are filtered out and won’t persist.Suggested patch (RPC boundary)
async def update_settings( *, @@ + recordingsAutoTranscribe: Optional[bool] = None, + recordingsAutoSummarize: Optional[bool] = None, recordingsAutoRenameTitle: Optional[bool] = None, ): @@ + if recordingsAutoTranscribe is not None: + kwargs["recordingsAutoTranscribe"] = recordingsAutoTranscribe + if recordingsAutoSummarize is not None: + kwargs["recordingsAutoSummarize"] = recordingsAutoSummarize if recordingsAutoRenameTitle is not None: kwargs["recordingsAutoRenameTitle"] = recordingsAutoRenameTitle(Also mirror these two keys in
AppController.get_settings()andAppController.update_settings()mapping.)Also applies to: 104-106
🤖 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-pyloid/server.py` around lines 55 - 72, The RPC-layer update_settings function is missing two Meetings toggles so updates to recordingsAutoTranscribe and recordingsAutoSummarize get dropped; add recordingsAutoTranscribe: Optional[bool] and recordingsAutoSummarize: Optional[bool] to the async def update_settings(...) parameter list and ensure these keys are forwarded into the settings merge logic there; also mirror those two keys in the AppController.get_settings() and AppController.update_settings() mappings so the controller exposes and persists them consistently with the RPC boundary.
🧹 Nitpick comments (9)
src-pyloid/services/recording/llm.py (1)
75-78: ⚡ Quick winAvoid swallowing all exceptions when reading error bodies.
The bare
except Exception: passhides diagnostic failures. Catch a narrower exception and log at debug level so troubleshooting keeps context.🤖 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-pyloid/services/recording/llm.py` around lines 75 - 78, Replace the bare "except Exception: pass" around resp.read() with a narrower catch (e.g., except (IOError, OSError) as e:) and log the failure at debug level instead of silencing it; call the module/logger instance (e.g., logger.debug or process_logger.debug) with a short message and include the exception (exc_info=True or include e) so diagnostic details are preserved while still treating the read failure as non-fatal.src-pyloid/services/recording/controller.py (1)
589-593: 💤 Low valueConsider expanding single-line if statements for PEP 8 compliance.
These lines compact the conditional assignments onto single lines, which PEP 8 discourages (E701). While functional, expanding them improves consistency with Python style conventions.
📐 Optional style improvement
- if "title" in fields: translated["title"] = fields["title"] - if "summary" in fields: translated["summary"] = fields["summary"] - if "notes" in fields: translated["notes"] = fields["notes"] - if "tags" in fields: translated["tags"] = fields["tags"] - if "language" in fields: translated["language"] = fields["language"] + if "title" in fields: + translated["title"] = fields["title"] + if "summary" in fields: + translated["summary"] = fields["summary"] + if "notes" in fields: + translated["notes"] = fields["notes"] + if "tags" in fields: + translated["tags"] = fields["tags"] + if "language" in fields: + translated["language"] = fields["language"]🤖 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-pyloid/services/recording/controller.py` around lines 589 - 593, The compact single-line conditionals like if "title" in fields: translated["title"] = fields["title"] violate PEP8 E701; replace each with a standard multi-line if block (e.g., if "title" in fields: newline + four-space indented translated["title"] = fields["title"]) for the keys "title", "summary", "notes", "tags", and "language" so the checks use expanded blocks operating on the translated and fields dicts.src-pyloid/services/database.py (1)
479-502: 💤 Low valueSQL injection warning is a false positive, but consider adding a defensive comment.
The static analysis tools flag the f-string usage in the SQL query, but the code is secure:
_RECORDING_UPDATABLE_FIELDSprovides a frozen whitelist (line 9)- Unknown fields are rejected before SQL construction (lines 480-483)
- All values are parameterized via the
paramsarrayConsider adding a comment above line 498 explaining that field names are validated against a whitelist to help future maintainers and static analysis tools understand the safety guarantee.
📝 Optional clarifying comment
sets.append("updated_at = ?") params.append(datetime.now().isoformat()) params.append(recording_id) + # Safe: field names in `sets` are validated against _RECORDING_UPDATABLE_FIELDS whitelist above conn = self._get_connection() try: conn.execute( f"UPDATE recordings SET {', '.join(sets)} WHERE id = ?", params )🤖 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-pyloid/services/database.py` around lines 479 - 502, The SQL f-string in update_recording is flagged but is safe because field names come from the frozen whitelist _RECORDING_UPDATABLE_FIELDS and values are parameterized via params; add a short defensive comment immediately above the conn.execute(...) (the f"UPDATE recordings SET {', '.join(sets)} WHERE id = ?") explaining that field names are validated against _RECORDING_UPDATABLE_FIELDS and that all values (including tags and updated_at) are passed as parameters to prevent SQL injection so static analyzers and future maintainers see the safety guarantee.src/lib/api.ts (1)
22-22: ⚡ Quick winUse the
@/alias for the types import.Switch
./typesto@/lib/typesto match the frontend import convention.As per coding guidelines, "Use
@/path alias forsrc/imports in frontend files (configured in tsconfig.json and vite.config.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 `@src/lib/api.ts` at line 22, The import in src/lib/api.ts currently uses a relative path ("./types"); update it to use the project path alias by replacing that import with "@/lib/types" so frontend code follows the tsconfig/vite alias convention and matches other imports (look for the import statement that ends with from "./types" in api.ts and change it to "@/lib/types").src/components/meetings/MeetingDetailPage.tsx (2)
98-114: 💤 Low valueTitle comparison doesn't trim both sides.
Line 100 compares
titleDraft.trim()withrecording.title(untrimmed). Ifrecording.titlehas leading/trailing whitespace, this will trigger an unnecessary save even when the user hasn't changed the content.♻️ Proposed fix
const handleSaveTitle = async () => { if (!recording) return; - if (titleDraft.trim() === recording.title) { + if (titleDraft.trim() === recording.title.trim()) { setEditingTitle(false); return; }🤖 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/components/meetings/MeetingDetailPage.tsx` around lines 98 - 114, The equality check in handleSaveTitle only trims titleDraft but not recording.title, causing unnecessary updates when recording.title contains whitespace; change the comparison to compare both trimmed values (e.g., compare titleDraft.trim() with recording.title.trim()) so no save is triggered for purely whitespace differences, and keep sending the trimmed title to api.recordingsUpdate and then setRecording with the updated trimmed value.
196-198: ⚡ Quick winAudio source path parsing may be fragile.
The code uses
audioRelpath.split("/").pop()to extract the filename. This assumes Unix-style paths and could break if the path uses backslashes or has unexpected formatting.♻️ Proposed fix
Consider using a more robust path extraction:
const audioSrc = recording.audioRelpath - ? `voiceflow://recording/${recording.audioRelpath.split("/").pop()}` + ? `voiceflow://recording/${recording.audioRelpath.split(/[\\/]/).pop()}` : null;Or better yet, if the backend always provides a consistent format, document that assumption in a comment.
🤖 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/components/meetings/MeetingDetailPage.tsx` around lines 196 - 198, The filename extraction using recording.audioRelpath.split("/").pop() is fragile; update the audioSrc construction (symbol: audioSrc, recording.audioRelpath) to first normalize separators (e.g. replace backslashes with forward slashes) or use a robust basename helper before building the voiceflow URL, e.g. normalize path = recording.audioRelpath.replace(/\\+/g, "/") then take the last segment, or import/implement a small basename(path) utility to handle both "/" and "\" and edge cases; keep behavior of producing null when audioRelpath is falsy.src/components/meetings/MeetingRecorderPage.tsx (1)
539-548: 💤 Low valueHardcoded locale in defaultTitle.
The function uses
"en-US"locale for formatting the date. Consider using the user's system locale (no argument totoLocaleString()) or making the locale configurable.♻️ Proposed fix
function defaultTitle(): string { const d = new Date(); - return d.toLocaleString("en-US", { + return d.toLocaleString(undefined, { weekday: "long", month: "short", day: "numeric", hour: "numeric", minute: "2-digit", }); }🤖 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/components/meetings/MeetingRecorderPage.tsx` around lines 539 - 548, The defaultTitle function hardcodes the "en-US" locale; update defaultTitle() to use the user's locale by calling toLocaleString() with no locale argument or accept a configurable locale parameter (e.g., defaultTitle(locale?: string)), then pass that locale into d.toLocaleString(locale ? locale : undefined, { weekday: ..., month: ..., day: ..., hour: ..., minute: ... }); keep the same formatting options and ensure callers (if adding a parameter) are updated to supply the configured/user locale.src/components/meetings/RetranscribeDialog.tsx (1)
51-84: ⚡ Quick winConsider stabilizing the useEffect dependencies.
The effect depends on
recording.transcriptModelandrecording.languagedirectly. If the parent component passes a newrecordingobject reference on each render (even with identical field values), this effect will re-run and re-fetch. Consider depending onrecording.idand extracting the fields inside the effect, or memoizing the recording object upstream.♻️ Proposed fix
useEffect(() => { if (!open) return; let cancelled = false; setLoading(true); Promise.all([ api.recordingsListCachedModels(), api.getSettings(), api.getOptions(), ]) .then(([models, settings, opts]) => { if (cancelled) return; setCachedModels(models); setOptions(opts); // Pre-select: prior transcript model → global default → first cached. const initialModel = recording.transcriptModel || settings.model || models.find((m) => m.cached)?.name || "tiny"; setModel(initialModel); setDevice(settings.device || "auto"); setLanguage(recording.language || settings.language || "auto"); }) .catch((err) => { console.error("retranscribe dialog fetch failed", err); toast.error("Could not load model list"); }) .finally(() => { if (!cancelled) setLoading(false); }); return () => { cancelled = true; }; - }, [open, recording.transcriptModel, recording.language]); + }, [open, recording.id, recording.transcriptModel, recording.language]);🤖 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/components/meetings/RetranscribeDialog.tsx` around lines 51 - 84, The effect in useEffect re-runs when the recording object reference changes because it depends on recording.transcriptModel and recording.language directly; change the dependency to a stable identifier (e.g., recording.id) and read recording.transcriptModel and recording.language inside the effect (or derive them into stable refs/variables before the hook) so the effect only re-fetches when the actual recording changes; update the dependency array for the effect that calls api.recordingsListCachedModels(), api.getSettings(), api.getOptions() to include open and recording.id (or other stable primitives) and leave out recording.transcriptModel and recording.language, and keep the existing state setters (setModel, setDevice, setLanguage) inside the effect body.src/components/meetings/LLMSettingsSection.tsx (1)
381-399: ⚡ Quick winConsider adding frontend validation before save.
The save button is enabled whenever
isDirtyis true, even ifendpointormodelfields are empty (which can happen with the "custom" preset). While the backend likely validates these fields, adding frontend validation would provide immediate feedback and improve the user experience.💡 Example validation
+ const canSave = + isDirty && + draft.endpoint.trim().length > 0 && + draft.model.trim().length > 0; + return ( <> {/* ... */} {isDirty && ( <div className="pt-4 border-t border-border flex items-center justify-end gap-2"> <Button /* ... */ > Discard </Button> - <Button size="sm" onClick={handleSave} disabled={saving}> + <Button size="sm" onClick={handleSave} disabled={saving || !canSave}> {saving ? "Saving…" : "Save changes"} </Button> </div> )}🤖 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/components/meetings/LLMSettingsSection.tsx` around lines 381 - 399, The save UI currently enables the Save button whenever isDirty is true even if required fields like endpoint or model (especially for the "custom" preset) are empty; add a frontend validation step that computes an isValid flag from the current draft (e.g., check draft.preset === "custom" -> draft.endpoint and draft.model are non-empty, and any other required fields) and use that to (1) disable the Save button (update the Button disabled prop to disabled={saving || !isValid}), (2) prevent handleSave from proceeding if isValid is false (early return or show validation errors), and (3) surface inline validation messages next to the endpoint/model inputs so users get immediate feedback; reference the draft state and handleSave/isDirty/saving symbols to implement these checks.
🤖 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-pyloid/server.py`:
- Line 631: Rename RPC parameters that shadow Python builtins: change any
parameter named id to recording_id and any parameter named format to
export_format in the recordings RPC functions (e.g., recordings_get and the
other recordings_* handlers referenced), update their function signatures and
all internal uses, and forward the renamed parameters to the underlying
controller calls (e.g., pass recording_id and export_format into
get_controller().meetings.* calls) so behavior is unchanged while avoiding Ruff
A002 builtin-argument-shadowing errors.
In `@src-pyloid/services/recording/llm.py`:
- Line 19: The logger domain used in the get_logger(...) call is not one of the
allowed domains; replace the current get_logger("llm") invocation with an
allowed domain (e.g., get_logger("model")) in the file so logging conforms to
the repository guideline; locate the get_logger call in this module (the log
variable assignment) and update the domain string to "model".
In `@src/components/meetings/AudioPlayer.tsx`:
- Around line 37-39: resolvedDurationMs is only initialized once from the
durationMs prop and can become stale if durationMs or src change; add a
useEffect that watches durationMs and src and calls
setResolvedDurationMs(durationMs ?? null) to resynchronize the state whenever
the prop or source swaps (also apply same sync to the other state updated at
lines 99-100), ensuring metadata load handlers still overwrite when they produce
a real duration.
- Line 18: The import in AudioPlayer.tsx uses a relative path for
formatDuration; update the import to use the project src alias (replace
"./utils" with "@/components/meetings/utils" or the correct "@/..." path) so the
symbol formatDuration is imported via the configured `@/` alias to match
tsconfig/vite conventions.
- Around line 136-145: The div with role="slider" in AudioPlayer.tsx exposes
slider semantics but lacks keyboard support; add an onKeyDown handler (e.g.,
handleSliderKeyDown) on that element to handle ArrowLeft/ArrowRight (seek small
step), PageUp/PageDown (seek larger step), and Home/End (seek to 0/total),
compute the new position relative to total and call the existing seek/scrub
logic (reuse or delegate to handleScrub or the component's seek method) and
update aria-valuenow/currentMs accordingly so keyboard users can change the
audio position.
In `@src/components/meetings/LLMSettingsSection.tsx`:
- Around line 102-118: The apiKeyDraft value is not trimmed before sending to
the backend which can cause auth failures; update usages of apiKeyDraft in
refreshModels, handleTest, and the dirty-check comparison to use
apiKeyDraft.trim() (or undefined if the trimmed string is empty) so you send a
trimmed key or undefined to api.llmListModels and other API calls and compare
trimmed values in the dirty check; locate the references to apiKeyDraft in the
functions refreshModels and handleTest and the dirty-check expression and
replace them with the trimmed/empty-to-undefined form.
---
Outside diff comments:
In `@src-pyloid/server.py`:
- Around line 55-72: The RPC-layer update_settings function is missing two
Meetings toggles so updates to recordingsAutoTranscribe and
recordingsAutoSummarize get dropped; add recordingsAutoTranscribe:
Optional[bool] and recordingsAutoSummarize: Optional[bool] to the async def
update_settings(...) parameter list and ensure these keys are forwarded into the
settings merge logic there; also mirror those two keys in the
AppController.get_settings() and AppController.update_settings() mappings so the
controller exposes and persists them consistently with the RPC boundary.
---
Nitpick comments:
In `@src-pyloid/services/database.py`:
- Around line 479-502: The SQL f-string in update_recording is flagged but is
safe because field names come from the frozen whitelist
_RECORDING_UPDATABLE_FIELDS and values are parameterized via params; add a short
defensive comment immediately above the conn.execute(...) (the f"UPDATE
recordings SET {', '.join(sets)} WHERE id = ?") explaining that field names are
validated against _RECORDING_UPDATABLE_FIELDS and that all values (including
tags and updated_at) are passed as parameters to prevent SQL injection so static
analyzers and future maintainers see the safety guarantee.
In `@src-pyloid/services/recording/controller.py`:
- Around line 589-593: The compact single-line conditionals like if "title" in
fields: translated["title"] = fields["title"] violate PEP8 E701; replace each
with a standard multi-line if block (e.g., if "title" in fields: newline +
four-space indented translated["title"] = fields["title"]) for the keys "title",
"summary", "notes", "tags", and "language" so the checks use expanded blocks
operating on the translated and fields dicts.
In `@src-pyloid/services/recording/llm.py`:
- Around line 75-78: Replace the bare "except Exception: pass" around
resp.read() with a narrower catch (e.g., except (IOError, OSError) as e:) and
log the failure at debug level instead of silencing it; call the module/logger
instance (e.g., logger.debug or process_logger.debug) with a short message and
include the exception (exc_info=True or include e) so diagnostic details are
preserved while still treating the read failure as non-fatal.
In `@src/components/meetings/LLMSettingsSection.tsx`:
- Around line 381-399: The save UI currently enables the Save button whenever
isDirty is true even if required fields like endpoint or model (especially for
the "custom" preset) are empty; add a frontend validation step that computes an
isValid flag from the current draft (e.g., check draft.preset === "custom" ->
draft.endpoint and draft.model are non-empty, and any other required fields) and
use that to (1) disable the Save button (update the Button disabled prop to
disabled={saving || !isValid}), (2) prevent handleSave from proceeding if
isValid is false (early return or show validation errors), and (3) surface
inline validation messages next to the endpoint/model inputs so users get
immediate feedback; reference the draft state and handleSave/isDirty/saving
symbols to implement these checks.
In `@src/components/meetings/MeetingDetailPage.tsx`:
- Around line 98-114: The equality check in handleSaveTitle only trims
titleDraft but not recording.title, causing unnecessary updates when
recording.title contains whitespace; change the comparison to compare both
trimmed values (e.g., compare titleDraft.trim() with recording.title.trim()) so
no save is triggered for purely whitespace differences, and keep sending the
trimmed title to api.recordingsUpdate and then setRecording with the updated
trimmed value.
- Around line 196-198: The filename extraction using
recording.audioRelpath.split("/").pop() is fragile; update the audioSrc
construction (symbol: audioSrc, recording.audioRelpath) to first normalize
separators (e.g. replace backslashes with forward slashes) or use a robust
basename helper before building the voiceflow URL, e.g. normalize path =
recording.audioRelpath.replace(/\\+/g, "/") then take the last segment, or
import/implement a small basename(path) utility to handle both "/" and "\" and
edge cases; keep behavior of producing null when audioRelpath is falsy.
In `@src/components/meetings/MeetingRecorderPage.tsx`:
- Around line 539-548: The defaultTitle function hardcodes the "en-US" locale;
update defaultTitle() to use the user's locale by calling toLocaleString() with
no locale argument or accept a configurable locale parameter (e.g.,
defaultTitle(locale?: string)), then pass that locale into
d.toLocaleString(locale ? locale : undefined, { weekday: ..., month: ..., day:
..., hour: ..., minute: ... }); keep the same formatting options and ensure
callers (if adding a parameter) are updated to supply the configured/user
locale.
In `@src/components/meetings/RetranscribeDialog.tsx`:
- Around line 51-84: The effect in useEffect re-runs when the recording object
reference changes because it depends on recording.transcriptModel and
recording.language directly; change the dependency to a stable identifier (e.g.,
recording.id) and read recording.transcriptModel and recording.language inside
the effect (or derive them into stable refs/variables before the hook) so the
effect only re-fetches when the actual recording changes; update the dependency
array for the effect that calls api.recordingsListCachedModels(),
api.getSettings(), api.getOptions() to include open and recording.id (or other
stable primitives) and leave out recording.transcriptModel and
recording.language, and keep the existing state setters (setModel, setDevice,
setLanguage) inside the effect body.
In `@src/lib/api.ts`:
- Line 22: The import in src/lib/api.ts currently uses a relative path
("./types"); update it to use the project path alias by replacing that import
with "@/lib/types" so frontend code follows the tsconfig/vite alias convention
and matches other imports (look for the import statement that ends with from
"./types" in api.ts and change it to "@/lib/types").
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: c0a767f9-5c29-405e-881f-173d3634d017
⛔ Files ignored due to path filters (7)
media/logo-dark.pngis excluded by!**/*.pngmedia/logo-light.pngis excluded by!**/*.pngmedia/meetings-ai-summary.pngis excluded by!**/*.pngmedia/meetings-detail.pngis excluded by!**/*.pngmedia/meetings-library.pngis excluded by!**/*.pngpnpm-lock.yamlis excluded by!**/pnpm-lock.yamluv.lockis excluded by!**/*.lock
📒 Files selected for processing (67)
.github/workflows/release.ymlREADME.mdinstaller/AppRuninstaller/voiceflow.isspackage.jsonpyproject.tomlsrc-pyloid/app_controller.pysrc-pyloid/main.pysrc-pyloid/server.pysrc-pyloid/services/database.pysrc-pyloid/services/logger.pysrc-pyloid/services/recording/__init__.pysrc-pyloid/services/recording/audio_scheme.pysrc-pyloid/services/recording/audio_scheme_handler.pysrc-pyloid/services/recording/audio_source.pysrc-pyloid/services/recording/clock.pysrc-pyloid/services/recording/controller.pysrc-pyloid/services/recording/export.pysrc-pyloid/services/recording/llm.pysrc-pyloid/services/recording/loopback_linux.pysrc-pyloid/services/recording/loopback_pulse.pysrc-pyloid/services/recording/loopback_windows.pysrc-pyloid/services/recording/recorder.pysrc-pyloid/services/recording/recovery.pysrc-pyloid/services/recording/secrets.pysrc-pyloid/services/recording/summary.pysrc-pyloid/services/recording/title.pysrc-pyloid/services/settings.pysrc-pyloid/services/transcription.pysrc-pyloid/tests/test_audio_scheme.pysrc-pyloid/tests/test_audio_source_loopback_channels.pysrc-pyloid/tests/test_audio_source_resample.pysrc-pyloid/tests/test_auto_title.pysrc-pyloid/tests/test_llm_provider.pysrc-pyloid/tests/test_loopback_discovery.pysrc-pyloid/tests/test_meeting_state_tick.pysrc-pyloid/tests/test_recording_export.pysrc-pyloid/tests/test_recording_recovery.pysrc-pyloid/tests/test_recording_service.pysrc-pyloid/tests/test_recordings_repo.pysrc-pyloid/tests/test_recordings_settings.pysrc-pyloid/tests/test_summary.pysrc-pyloid/tests/test_transcribe_file.pysrc/components/SettingsTab.tsxsrc/components/Sidebar.tsxsrc/components/meetings/AudioPlayer.tsxsrc/components/meetings/LLMSettingsSection.tsxsrc/components/meetings/LevelMeter.tsxsrc/components/meetings/MeetingDetailPage.tsxsrc/components/meetings/MeetingImportDialog.tsxsrc/components/meetings/MeetingRecorderContext.tsxsrc/components/meetings/MeetingRecorderPage.tsxsrc/components/meetings/MeetingsListPage.tsxsrc/components/meetings/MeetingsSettingsSection.tsxsrc/components/meetings/RetranscribeDialog.tsxsrc/components/meetings/StatusLine.tsxsrc/components/meetings/SummaryView.tsxsrc/components/meetings/TranscriptView.tsxsrc/components/meetings/utils.tssrc/components/ui/button.tsxsrc/components/ui/sheet.tsxsrc/index.csssrc/lib/api.tssrc/lib/constants.tssrc/lib/types.tssrc/pages/Dashboard.tsxsrc/pages/Popup.tsx
|
|
||
|
|
||
| @server.method() | ||
| async def recordings_get(*, id: int): |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
git ls-files src-pyloid/ | head -20Repository: infiniV/VoiceFlow
Length of output: 673
🏁 Script executed:
wc -l src-pyloid/server.pyRepository: infiniV/VoiceFlow
Length of output: 86
🏁 Script executed:
sed -n '625,665p' src-pyloid/server.pyRepository: infiniV/VoiceFlow
Length of output: 1207
🏁 Script executed:
sed -n '680,710p' src-pyloid/server.pyRepository: infiniV/VoiceFlow
Length of output: 887
🏁 Script executed:
find . -name "pyproject.toml" -o -name "ruff.toml" -o -name ".ruff.toml" | head -5Repository: infiniV/VoiceFlow
Length of output: 90
🏁 Script executed:
cat ruff.tomlRepository: infiniV/VoiceFlow
Length of output: 1176
🏁 Script executed:
cat pyproject.toml | grep -A 30 "\[tool.ruff"Repository: infiniV/VoiceFlow
Length of output: 43
🌐 Web query:
Ruff A002 rule Python linting
💡 Result:
Ruff rule A002 is builtin-argument-shadowing. What it checks: it flags function arguments whose names match Python builtins (for example, using list as an argument name). [1] Why it’s a problem: shadowing builtins makes code harder to read and can lead to non-obvious errors because readers may mistake the argument for the builtin (or vice versa). [1] Key exceptions / configuration behavior: - Functions decorated with @override or @overload are exempt by default. [1] - You can mark specific builtins as exceptions using lint.flake8-builtins.ignorelist. [1] How to disable (if needed): ignore the rule code A002 (or configure the relevant ignore list), or exclude it from your select set. (Ruff’s A rules are documented under its rules index, where A002 corresponds to builtin-argument-shadowing.) [2]
Citations:
- 1: https://docs.astral.sh/ruff/rules/builtin-argument-shadowing/
- 2: https://docs.astral.sh/ruff/rules/
Rename builtin-shadowing RPC parameters to avoid Ruff A002 lint violations.
The parameters id and format shadow Python builtins and trigger the A002 rule (builtin-argument-shadowing), which is enabled in this project. Rename to recording_id, export_format, and similar throughout the RPC methods and forward them appropriately to the underlying get_controller().meetings calls.
Applies to: 631, 639, 644, 654, 659, 666, 684, 702, 707
🧰 Tools
🪛 Ruff (0.15.12)
[error] 631-631: Function argument id is shadowing a Python builtin
(A002)
🤖 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-pyloid/server.py` at line 631, Rename RPC parameters that shadow Python
builtins: change any parameter named id to recording_id and any parameter named
format to export_format in the recordings RPC functions (e.g., recordings_get
and the other recordings_* handlers referenced), update their function
signatures and all internal uses, and forward the renamed parameters to the
underlying controller calls (e.g., pass recording_id and export_format into
get_controller().meetings.* calls) so behavior is unchanged while avoiding Ruff
A002 builtin-argument-shadowing errors.
| from services.logger import get_logger | ||
| from services.transcription import CancelToken | ||
|
|
||
| log = get_logger("llm") |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use an allowed domain in get_logger(...).
Line 19 uses "llm", but this repository limits logger domains for src-pyloid/**/*.py. Please switch to one of the allowed domains (likely "model" here).
As per coding guidelines src-pyloid/**/*.py: "Use domain-based logging with get_logger(domain) where domain is one of: model, audio, hotkey, settings, database, clipboard, window."
🤖 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-pyloid/services/recording/llm.py` at line 19, The logger domain used in
the get_logger(...) call is not one of the allowed domains; replace the current
get_logger("llm") invocation with an allowed domain (e.g., get_logger("model"))
in the file so logging conforms to the repository guideline; locate the
get_logger call in this module (the log variable assignment) and update the
domain string to "model".
| } from "react"; | ||
| import { Pause, Play } from "lucide-react"; | ||
| import { cn } from "@/lib/utils"; | ||
| import { formatDuration } from "./utils"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use the @/ alias instead of relative import.
Line 18 should use the configured src alias to match project import conventions.
As per coding guidelines src/**/*.{ts,tsx,js,jsx}: "Use @/ path alias for src/ imports in frontend files (configured in tsconfig.json and vite.config.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 `@src/components/meetings/AudioPlayer.tsx` at line 18, The import in
AudioPlayer.tsx uses a relative path for formatDuration; update the import to
use the project src alias (replace "./utils" with "@/components/meetings/utils"
or the correct "@/..." path) so the symbol formatDuration is imported via the
configured `@/` alias to match tsconfig/vite conventions.
| const [resolvedDurationMs, setResolvedDurationMs] = useState<number | null>( | ||
| durationMs ?? null, | ||
| ); |
There was a problem hiding this comment.
Keep resolved duration in sync with prop/source changes.
resolvedDurationMs is initialized from durationMs once; if parent updates durationMs (or src swaps), displayed total can stay stale until metadata reload.
Suggested fix
const [resolvedDurationMs, setResolvedDurationMs] = useState<number | null>(
durationMs ?? null,
);
+
+ useEffect(() => {
+ setResolvedDurationMs(durationMs ?? null);
+ setCurrentMs(0);
+ setPlaying(false);
+ }, [durationMs, src]);Also applies to: 99-100
🤖 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/components/meetings/AudioPlayer.tsx` around lines 37 - 39,
resolvedDurationMs is only initialized once from the durationMs prop and can
become stale if durationMs or src change; add a useEffect that watches
durationMs and src and calls setResolvedDurationMs(durationMs ?? null) to
resynchronize the state whenever the prop or source swaps (also apply same sync
to the other state updated at lines 99-100), ensuring metadata load handlers
still overwrite when they produce a real duration.
| <div | ||
| role="slider" | ||
| tabIndex={0} | ||
| aria-label="Audio position" | ||
| aria-valuemin={0} | ||
| aria-valuemax={total} | ||
| aria-valuenow={currentMs} | ||
| className="flex-1 h-6 flex items-center cursor-pointer group" | ||
| onClick={handleScrub} | ||
| > |
There was a problem hiding this comment.
Slider is not keyboard-operable despite role="slider".
Lines 136-145 expose slider semantics but miss Arrow/Home/End keyboard handling, which blocks keyboard-only seeking.
Suggested fix
<div
role="slider"
tabIndex={0}
aria-label="Audio position"
aria-valuemin={0}
aria-valuemax={total}
aria-valuenow={currentMs}
+ aria-valuetext={`${formatDuration(currentMs)} of ${formatDuration(total)}`}
className="flex-1 h-6 flex items-center cursor-pointer group"
onClick={handleScrub}
+ onKeyDown={(e) => {
+ const audio = audioRef.current;
+ if (!audio || !isFinite(audio.duration)) return;
+ const step = 5; // seconds
+ if (e.key === "ArrowRight") {
+ e.preventDefault();
+ audio.currentTime = Math.min(audio.duration, audio.currentTime + step);
+ } else if (e.key === "ArrowLeft") {
+ e.preventDefault();
+ audio.currentTime = Math.max(0, audio.currentTime - step);
+ } else if (e.key === "Home") {
+ e.preventDefault();
+ audio.currentTime = 0;
+ } else if (e.key === "End") {
+ e.preventDefault();
+ audio.currentTime = audio.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 `@src/components/meetings/AudioPlayer.tsx` around lines 136 - 145, The div with
role="slider" in AudioPlayer.tsx exposes slider semantics but lacks keyboard
support; add an onKeyDown handler (e.g., handleSliderKeyDown) on that element to
handle ArrowLeft/ArrowRight (seek small step), PageUp/PageDown (seek larger
step), and Home/End (seek to 0/total), compute the new position relative to
total and call the existing seek/scrub logic (reuse or delegate to handleScrub
or the component's seek method) and update aria-valuenow/currentMs accordingly
so keyboard users can change the audio position.
| const refreshModels = async () => { | ||
| if (!draft) return; | ||
| setLoadingModels(true); | ||
| try { | ||
| const list = await api.llmListModels( | ||
| draft.preset, | ||
| draft.endpoint, | ||
| apiKeyDraft || undefined, | ||
| ); | ||
| setModels(list); | ||
| } catch (err) { | ||
| console.error("list models failed", err); | ||
| toast.error("Could not list models — check endpoint / key"); | ||
| } finally { | ||
| setLoadingModels(false); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Trim whitespace from API key input before sending to backend.
Users commonly copy-paste API keys with accidental leading or trailing whitespace. The current code sends whitespace-only or untrimmed keys directly to the backend (line 109: apiKeyDraft || undefined), which will cause authentication failures that are difficult to debug. The same issue exists at lines 140 and 184.
🔧 Proposed fix
Apply .trim() when using apiKeyDraft:
const list = await api.llmListModels(
draft.preset,
draft.endpoint,
- apiKeyDraft || undefined,
+ apiKeyDraft.trim() || undefined,
);Apply the same fix at line 140 (in handleTest) and update line 184 in the dirty check:
- apiKeyDraft.length > 0;
+ apiKeyDraft.trim().length > 0;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const refreshModels = async () => { | |
| if (!draft) return; | |
| setLoadingModels(true); | |
| try { | |
| const list = await api.llmListModels( | |
| draft.preset, | |
| draft.endpoint, | |
| apiKeyDraft || undefined, | |
| ); | |
| setModels(list); | |
| } catch (err) { | |
| console.error("list models failed", err); | |
| toast.error("Could not list models — check endpoint / key"); | |
| } finally { | |
| setLoadingModels(false); | |
| } | |
| }; | |
| const refreshModels = async () => { | |
| if (!draft) return; | |
| setLoadingModels(true); | |
| try { | |
| const list = await api.llmListModels( | |
| draft.preset, | |
| draft.endpoint, | |
| apiKeyDraft?.trim() || undefined, | |
| ); | |
| setModels(list); | |
| } catch (err) { | |
| console.error("list models failed", err); | |
| toast.error("Could not list models — check endpoint / key"); | |
| } finally { | |
| setLoadingModels(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 `@src/components/meetings/LLMSettingsSection.tsx` around lines 102 - 118, The
apiKeyDraft value is not trimmed before sending to the backend which can cause
auth failures; update usages of apiKeyDraft in refreshModels, handleTest, and
the dirty-check comparison to use apiKeyDraft.trim() (or undefined if the
trimmed string is empty) so you send a trimmed key or undefined to
api.llmListModels and other API calls and compare trimmed values in the dirty
check; locate the references to apiKeyDraft in the functions refreshModels and
handleTest and the dirty-check expression and replace them with the
trimmed/empty-to-undefined form.
Summary
Replaces the current
main(which has a broken Windows release — see #18) with thefeat/meetingsline. Ships the Meetings feature as experimental and rolls up a stack of Windows/Linux fixes that have been validated locally.Tagged as
v1.6.0-rc1. Linux smoke-tested end-to-end. Windows is WIP — installer builds green and a clean install boots, but a separate WASAPI loopback edge case (see Known Issues) is still being chased on real hardware. That's not a regression frommain—main's Windows artifact doesn't run at all due to #18.What's in here
New: Meetings (experimental) — long-form recorder with system-audio capture, transcription, and AI summaries. Separate page on the dashboard; doesn't touch the existing hold-to-talk dictation flow.
Windows fixes
8b315af— installer wipes{app}\_internalon upgrade. Inno Setup was leaving the previous install's bundled Python tree behind, so stale.pydfiles from old wheels (PyAV in particular) shadowed the new build. This + the dep refresh in6f06170is the path that addresses Stuck in download for upgrade to 1.4 #18 — fresh installs get a consistent OpenSSL DLL set; upgrades from the broken v1.5.x payload no longer keep the mismatched_ssl.pyd/libssl-3-x64.dllaround.13d45be/96b0b73— WASAPI loopback opens use the device-native channel count (max_output_channels) instead of hard-coding mono, which PortAudio rejects with-9998.Linux fixes
7395059— scrubLD_LIBRARY_PATHbeforehyprctlsubprocess calls so they don't load the AppImage's bundledlibstdc++.9f75281— popup docks bottom-center on Wayland,voiceflow://audio playback fixed, retranscribe modal added.Other
6f06170— npm + pip dep bumps clearing dependabot alerts (also refreshes the bundled OpenSSL pair on Windows builds).Verification
pnpm tsc -b→ 0npx eslinton touched frontend → 0uv run pytest src-pyloid/tests/ --ignore=test_transcription.py→ 295 passed, 1 pre-existing failure (unchanged frommain), 5 skipped13d45be(run25761588376, 8m51s)Known issues (Windows, WIP — not regressions from main)
PaErrorCode -9998despite themax_output_channelsfallback — under active investigation with a live tester. Worst case it falls through to mic-only recording.recordingsAutoTranscribe/recordingsAutoSummarizesettings toggles are filtered at the RPC boundary (same scope asrecordingsAutoRenameTitle, which is wired). Will follow up.Why merge despite the Windows WIP
maincurrently ships a Windows build that hits_sslDLL load failure on launch (#18) — it's effectively unusable. This branch's Windows artifact at least installs and runs; the remaining loopback edge case is narrower than "the app won't start."Test plan
.pydfiles in_internal\, app launchesSummary by CodeRabbit
New Features
Improvements
Documentation