Bot-review harvest (16 fixes), deterministic style/locale CI, reviewer configs - #1198
Merged
Conversation
… + reviewer configs Harvested and verified every CodeRabbit/Greptile finding from PRs #1175, #1189, #1192, #1195: 16 real ones fixed (fallback ASR preflight bypass, VRAM release on stream exit, typed 409 parity, uv env independence, path-privacy in errors, MCP clone_voice hardening, CaptureWidget WS guard, test hygiene), 4 refuted with evidence, rest documented as deliberate design or deferred. Deterministic CI replaces hand-enforcement: tests/test_changelog_style.py (quiet one-liner format) and tests/test_locale_parity.py (21-locale key/placeholder lockstep with a ratchet baseline) — the latter surfaced and fixes 151 already-broken locale strings. CodeRabbit/Greptile carry the house rules via .coderabbit.yaml + greptile.json; CLAUDE.md gains the harvest-before-merge and never-accept-as-is rules. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
|
Warning Ignoring CodeRabbit configuration file changes. For security, only the configuration from the base branch is applied for open source repositories. 📝 WalkthroughWalkthroughThis PR adds typed ASR missing-model handling, reliable streaming cleanup, audio validation, sanitized subprocess errors, frontend race protection, stricter model classification, locale parity checks, changelog linting, and expanded review and merge policies. ChangesRuntime and quality enforcement
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Capture
participant ASRPreflight
participant ASRStream
participant ModelBackend
Client->>Capture: start recording with model override
Capture->>ASRPreflight: validate requested model
ASRPreflight-->>Capture: typed missing-model error or proceed
Client->>ASRStream: start dubbing transcription
ASRStream->>ModelBackend: load ASR backend
ModelBackend-->>ASRStream: backend instance
ASRStream-->>Client: SSE result, error, and done events
ASRStream->>ModelBackend: unload backend on every terminal path
Possibly related PRs
🚥 Pre-merge checks | ✅ 6 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (6 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| _model = _model_task.result() | ||
| except Exception as e: | ||
| logger.exception("transcribe preflight: model load failed (job=%s)", job_id) | ||
| logger.exception("transcribe preflight: model load failed (job=%r)", job_id) |
| preflight_payload = e.payload | ||
| except Exception as e: | ||
| logger.exception("transcribe preflight: ASR load failed (job=%s)", job_id) | ||
| logger.exception("transcribe preflight: ASR load failed (job=%r)", job_id) |
| yield ev | ||
| except Exception as e: # noqa: BLE001 — last-resort stream finalizer | ||
| logger.exception("transcribe stream crashed (job=%s)", job_id) | ||
| logger.exception("transcribe stream crashed (job=%r)", job_id) |
Contributor
|
| Filename | Overview |
|---|---|
| backend/api/routers/dub_core.py | Adds _loaded_asr closure + gen() finally block for fire-and-forget executor unload on all exit paths; ASRModelMissingError caught in the ASR-load try/except to surface the typed download CTA. Implementation is correct; companion regression test has a small theoretical race (see comment). |
| backend/services/asr_backend.py | Introduces ASRModelMissingError, backend_id threading through _offline_asr_repo/asr_model_missing_error, and per-candidate preflight in load_active_asr_backend fallback loop. Correctly gates on tried being non-empty so callers handle the first-iteration check. |
| backend/api/routers/capture_ws.py | Passes raw ?model= query param as sherpa_model_id when spec is None so the preflight follows the Whisper execution path instead of the (possibly-installed) persisted sherpa preference. Covered by test_capture_ws_invalid_override_still_preflights_whisper. |
| backend/mcp_server.py | Adds _decode_ref_audio (data-URI stripping + validated base64) and _sniff_audio_ext (magic-byte container detection) helpers; clone_voice catches httpx errors with structured JSON responses. M4A extended-header caveat documented in-code per previous thread. |
| backend/services/sidecar_install.py | uv_subprocess_env now independently gates UV_CACHE_DIR and UV_PYTHON_INSTALL_DIR, returning None only when both are user-pinned. Prevents managed-Python downloads staying on the system drive when only UV_CACHE_DIR is pinned cross-drive. |
| backend/services/subprocess_backend.py | _os_exec_refusal builds the user-visible spawn error from errno/strerror only, avoiding the home-directory path leak that str(OSError) embeds via exc.filename. Log lines reduced to basename-only. |
| tests/test_locale_parity.py | New locale parity test with ratchet, placeholder-match, and corrupted-token checks. Ratchet now uses warnings.warn (not pytest.fail) for improvements, addressing the previous thread's CI-blocking concern. |
| tests/test_changelog_style.py | Deterministic changelog linter with date-epoch scoping, allowlist for owner-authored infra entries, and self-tests that verify each rule actually fires. |
| frontend/src/components/CaptureWidget.jsx | Adds wsHadFinalRef guard in the startRecording catch block so a late mic error/rejection doesn't clobber the already-terminal asr_model_missing state. |
| frontend/src/api/hooks.ts | useVisibleNotifications memoizes the filtered array with useMemo and strips raw data from the return to prevent consumers accidentally reading the unfiltered list. |
| frontend/src/i18n/locales/vi.json | Restores 31 V_0_-corrupted placeholder tokens to their correct {{name}} forms. The locale parity test now gates this class of corruption for all 21 locales. |
Reviews (2): Last reviewed commit: "config(review-bots): concise high-signal..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
backend/tests/conftest.py (1)
88-111: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the stale-module-alias lookup into a shared helper per file. Both
_clear_asr_installed_memoand its siblingasr_model_installedfixture independently re-implement the identicalimport types+vars(test_module)scan for aservices.asr_backendalias, now duplicated twice per file (four copies total across the two conftest files). One drift in this correctness-sensitive lookup (e.g., a third target module name) means editing four near-identical blocks instead of one.
backend/tests/conftest.py#L88-L111: factor the alias-scan (currently inlined in bothasr_model_installedL56-84 and_clear_asr_installed_memoL88-111) into a module-level helper, e.g._asr_backend_module_aliases(request) -> dict[int, types.ModuleType], and have both fixtures call it.tests/conftest.py#L192-L218: apply the same extraction againstasr_model_installed(L157-188) and_clear_asr_installed_memo(L192-218).♻️ Proposed helper (apply the same shape in both files)
+def _asr_backend_module_aliases(request): + """services.asr_backend plus any stale module-typed alias the test + module itself holds (see docstrings on the fixtures below).""" + import types + mod = sys.modules.get("services.asr_backend") + targets = {} if mod is None else {id(mod): mod} + test_module = getattr(request, "module", None) + if test_module is not None: + for val in vars(test_module).values(): + if (isinstance(val, types.ModuleType) + and getattr(val, "__name__", "") == "services.asr_backend"): + targets[id(val)] = val + return targets.values() + def _clear_asr_installed_memo(request): def _clear_all(): - import types - mod = sys.modules.get("services.asr_backend") - targets = {} if mod is None else {id(mod): mod} - test_module = getattr(request, "module", None) - if test_module is not None: - for val in vars(test_module).values(): - if (isinstance(val, types.ModuleType) - and getattr(val, "__name__", "") == "services.asr_backend"): - targets[id(val)] = val - for m in targets.values(): + for m in _asr_backend_module_aliases(request): getattr(m, "_INSTALLED_REPO_MEMO", set()).clear() _clear_all() yield _clear_all()🤖 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 `@backend/tests/conftest.py` around lines 88 - 111, Extract the duplicated stale-module alias scan into a shared module-level helper in backend/tests/conftest.py, such as _asr_backend_module_aliases(request), and update both asr_model_installed and _clear_asr_installed_memo to reuse it. Apply the same extraction and fixture updates in tests/conftest.py, including the affected _clear_asr_installed_memo site at tests/conftest.py lines 192-218; preserve the canonical module handling and deduplication by module identity.
🤖 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 `@backend/api/routers/dub_core.py`:
- Around line 1324-1334: Update the ASR cleanup around _b.unload() in the
finally block and the normal completion path to dispatch unload execution
through a thread-pool executor instead of calling it directly from the async
path. In the finally block, schedule the background cleanup so client-disconnect
CancelledError cannot abort the unload, while preserving the existing warning
logging for unload failures.
In `@tests/test_changelog_style.py`:
- Around line 85-86: Rename the generator variable `l` to `line` in both `any`
expressions defining `has_subsections` and `has_highlights`, updating each
`startswith` and `strip` reference while preserving the existing checks.
---
Nitpick comments:
In `@backend/tests/conftest.py`:
- Around line 88-111: Extract the duplicated stale-module alias scan into a
shared module-level helper in backend/tests/conftest.py, such as
_asr_backend_module_aliases(request), and update both asr_model_installed and
_clear_asr_installed_memo to reuse it. Apply the same extraction and fixture
updates in tests/conftest.py, including the affected _clear_asr_installed_memo
site at tests/conftest.py lines 192-218; preserve the canonical module handling
and deduplication by module identity.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 48647cb4-b345-467a-8deb-3b27a86b6890
📒 Files selected for processing (49)
.coderabbit.yamlCHANGELOG.mdCLAUDE.mdbackend/api/routers/capture_ws.pybackend/api/routers/dub_core.pybackend/api/routers/openai_compat.pybackend/mcp_server.pybackend/services/asr_backend.pybackend/services/sidecar_install.pybackend/services/subprocess_backend.pybackend/services/tts_backend.pybackend/tests/conftest.pydocs/generation-parameters.mddocs/install/macos.mddocs/install/troubleshooting.mdfrontend/src/api/hooks.tsfrontend/src/components/CaptureWidget.jsxfrontend/src/components/settings/models/sections.jsfrontend/src/i18n/locales/ar.jsonfrontend/src/i18n/locales/de.jsonfrontend/src/i18n/locales/es.jsonfrontend/src/i18n/locales/fr.jsonfrontend/src/i18n/locales/hi.jsonfrontend/src/i18n/locales/id.jsonfrontend/src/i18n/locales/it.jsonfrontend/src/i18n/locales/ja.jsonfrontend/src/i18n/locales/ko.jsonfrontend/src/i18n/locales/nl.jsonfrontend/src/i18n/locales/pl.jsonfrontend/src/i18n/locales/pt.jsonfrontend/src/i18n/locales/ru.jsonfrontend/src/i18n/locales/sv.jsonfrontend/src/i18n/locales/th.jsonfrontend/src/i18n/locales/tr.jsonfrontend/src/i18n/locales/uk.jsonfrontend/src/i18n/locales/vi.jsonfrontend/src/i18n/locales/zh-CN.jsonfrontend/src/i18n/locales/zh-TW.jsonfrontend/src/test/CaptureWidgetSetupRace.test.jsxfrontend/src/test/LogsFooterNotifications.test.jsxfrontend/src/test/modelStoreGrouping.test.jsxgreptile.jsontests/backend/services/test_binary_preflight.pytests/conftest.pytests/test_asr_model_missing.pytests/test_changelog_style.pytests/test_locale_parity.pytests/test_mcp_mount.pytests/test_uv_cross_drive.py
- fallback preflight pins the candidate backend id (Greptile) — also the CI empty-cache failure: deep-import fall-through tests get the asr_model_installed fixture - locale ratchet: improvement warns instead of failing CI (CodeRabbit) - stream-exit ASR unload runs on the GPU pool fire-and-forget instead of blocking the event loop (CodeRabbit) - E741 rename; M4A ftyp sniff scope documented (CodeRabbit) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment on lines
+1341
to
+1343
| lambda f: f.cancelled() | ||
| or (f.exception() and logger.warning( | ||
| "Failed to unload ASR backend: %s", f.exception())) |
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/api/routers/dub_core.py (1)
1279-1282: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winBlocking call inside async path (normal completion).
The
finallyblock was successfully updated to dispatch the unload to the GPU pool, but the normal completion path still calls_asr_backend.unload()directly. As noted in the previous review, this blocks the ASGI event loop (due togc.collect()and CUDA cache drops) and degrades responsiveness for all concurrent requests.Since this path does not run under
GeneratorExit, you can simplyawaitthe executor dispatch using the existingloopand_gpu_pool.🚀 Proposed fix to prevent event loop blocking
- try: - _asr_backend.unload() - except Exception as e: + try: + await loop.run_in_executor(_gpu_pool, _asr_backend.unload) + except Exception as e:🤖 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 `@backend/api/routers/dub_core.py` around lines 1279 - 1282, Update the normal-completion cleanup path around _asr_backend.unload() to dispatch unloading through the existing loop and _gpu_pool, awaiting the executor result instead of calling unload directly. Preserve the current exception handling and warning message while ensuring the async event loop is not blocked.
🧹 Nitpick comments (1)
backend/api/routers/dub_core.py (1)
1348-1351: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSuppress Ruff blind-exception warning.
You can add a
noqacomment to silence theBLE001warning for this best-effort fallback, maintaining consistency with the outer stream finalizer.✨ Proposed fix to silence the warning
- try: - _b.unload() - except Exception as e: + try: + _b.unload() + except Exception as e: # noqa: BLE001 - best-effort teardown🤖 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 `@backend/api/routers/dub_core.py` around lines 1348 - 1351, Add a Ruff `noqa: BLE001` suppression to the broad exception handler around `_b.unload()` in the ASR backend cleanup path, matching the existing outer stream finalizer convention while preserving the best-effort warning log.Source: Linters/SAST tools
🤖 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.
Outside diff comments:
In `@backend/api/routers/dub_core.py`:
- Around line 1279-1282: Update the normal-completion cleanup path around
_asr_backend.unload() to dispatch unloading through the existing loop and
_gpu_pool, awaiting the executor result instead of calling unload directly.
Preserve the current exception handling and warning message while ensuring the
async event loop is not blocked.
---
Nitpick comments:
In `@backend/api/routers/dub_core.py`:
- Around line 1348-1351: Add a Ruff `noqa: BLE001` suppression to the broad
exception handler around `_b.unload()` in the ASR backend cleanup path, matching
the existing outer stream finalizer convention while preserving the best-effort
warning log.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e04104f3-0296-48f6-858d-116b6e93a05e
📒 Files selected for processing (6)
backend/api/routers/dub_core.pybackend/mcp_server.pybackend/services/asr_backend.pybackend/tests/test_asr_deep_import_fallback.pytests/test_changelog_style.pytests/test_locale_parity.py
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/test_locale_parity.py
- backend/mcp_server.py
- tests/test_changelog_style.py
Owner directive: no fluff on PRs. Greptile: logic-only comments at max strictness, no diagrams/confidence sections, summary collapsed. CodeRabbit: three-sentence findings, no sequence diagrams or ASCII sketches, collapsed walkthrough, no per-push status comments, finishing touches off. Both: comment only when a finding changes what merges. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.coderabbit.yaml (1)
120-127: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSynchronize the changelog policy with the shipped format.
Both review configurations would flag the current
CHANGELOG.mdbecause its Unreleased content includes### CIand### Docsentries without(#NNN)references.
.coderabbit.yaml#L120-L127: update the rule or changeCHANGELOG.mdand its parity tests.greptile.json#L15-L19: apply the same rule change so the two reviewers enforce one contract.🤖 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 @.coderabbit.yaml around lines 120 - 127, Synchronize the CHANGELOG.md review policy with the shipped Unreleased format: update the rule at .coderabbit.yaml lines 120-127 and apply the identical contract at greptile.json lines 15-19, or consistently update CHANGELOG.md and its parity tests instead. Ensure both reviewers handle the existing CI and Docs sections without conflicting reference requirements.
🤖 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 `@greptile.json`:
- Around line 3-5: Update the commentTypes configuration in the Greptile
instructions to retain syntax alongside logic, or widen the list to cover all
categories required by the review policy; do not leave syntax excluded.
---
Outside diff comments:
In @.coderabbit.yaml:
- Around line 120-127: Synchronize the CHANGELOG.md review policy with the
shipped Unreleased format: update the rule at .coderabbit.yaml lines 120-127 and
apply the identical contract at greptile.json lines 15-19, or consistently
update CHANGELOG.md and its parity tests instead. Ensure both reviewers handle
the existing CI and Docs sections without conflicting reference requirements.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 95b2ffde-c3cb-4242-913b-e17aea1dca38
📒 Files selected for processing (2)
.coderabbit.yamlgreptile.json
Comment on lines
+3
to
+5
| "instructions": "Review as a panel of senior domain experts (ML inference, audio DSP, desktop systems). Comment ONLY on findings that would change what gets merged: a concrete bug, a violated house rule from CLAUDE.md, a real security/data risk. Per finding: at most three sentences — failure mode, line, fix. No praise, no restating the diff, no diagrams or mockups, no style commentary (CI linters gate style), no info-level observations. If nothing meets the bar, leave no comments. OmniVoice is a fully-local desktop app (no cloud, no API keys, no telemetry): flag any new outbound network call that is not GitHub Issues or a HuggingFace model download.", | ||
| "commentTypes": ["logic"], | ||
| "strictness": 3, |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== greptile.json =="
nl -ba greptile.json | sed -n '1,80p'
echo
echo "== repo files mentioning greptile/commentTypes =="
rg -n --hidden --glob '!**/.git/**' 'commentTypes|greptile' .
echo
echo "== candidate docs around Greptile config =="
fd -a -H -I 'CLAUDE.md|README.md|greptile.*|\.coderabbit.*' .Repository: debpalash/OmniVoice-Studio
Length of output: 224
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== greptile.json =="
nl -ba greptile.json | sed -n '1,120p'Repository: debpalash/OmniVoice-Studio
Length of output: 224
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== exact greptile.json lines =="
nl -ba greptile.json | sed -n '1,40p'
echo
echo "== any local mention of commentTypes semantics =="
rg -n --hidden --glob '!**/.git/**' 'commentTypes|syntax|logic|Greptile' . || trueRepository: debpalash/OmniVoice-Studio
Length of output: 236
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== greptile.json =="
python3 - <<'PY'
from pathlib import Path
for i, line in enumerate(Path("greptile.json").read_text().splitlines(), 1):
print(f"{i:>4} {line}")
PY
echo
echo "== local mentions of Greptile/commentTypes =="
rg -n --hidden --glob '!**/.git/**' 'commentTypes|Greptile|greptile' . || trueRepository: debpalash/OmniVoice-Studio
Length of output: 6755
🌐 Web query:
Greptile config commentTypes logic syntax official docs
💡 Result:
In Greptile, the commentTypes configuration allows you to filter the categories of feedback generated during code reviews [1][2]. Syntax and Configuration The setting is defined as an array of strings within your configuration file, either.greptile/config.json (recommended) or the legacy root greptile.json [3][1]. { "commentTypes": ["logic", "syntax"] } Valid Options By default, all four categories are enabled [1][2]. You can restrict the feedback to any subset of the following strings [4][1]: - logic: Flags business logic issues, algorithmic problems, and potential bugs [1][2]. - syntax: Flags language-specific best practices and proper usage patterns [1][2]. - style: Flags code formatting, naming conventions, and structural consistency [1][2]. - info: Provides informational comments about code context and patterns [4][1]. Key Details - Scope: The array you provide completely replaces the default [1]. If you specify only one type (e.g., ["logic"]), Greptile will ignore all other categories [1]. - Hierarchy: If using.greptile/config.json in a monorepo or nested directory structure, child configurations can override parent settings [3][1]. - Recommendation: Greptile officially recommends using.greptile/config.json for new and existing repositories [4][1].
Citations:
- 1: https://www.greptile.com/docs/code-review/controlling-nitpickiness
- 2: https://www.greptile.com/docs/code-review-bot/greptile-json
- 3: https://www.greptile.com/docs/code-review/greptile-config
- 4: https://www.greptile.com/docs/code-review/greptile-json-reference
Keep syntax in Greptile’s commentTypes list.
Line 4 replaces the default categories, so ["logic"] drops every non-logic finding. That can hide real merge blockers from Greptile; add syntax back at minimum, or widen the set to match the review policy.
🤖 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 `@greptile.json` around lines 3 - 5, Update the commentTypes configuration in
the Greptile instructions to retain syntax alongside logic, or widen the list to
cover all categories required by the review policy; do not leave syntax
excluded.
Source: MCP tools
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
2 participants
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.
Implements the "use the review bots properly" plan end-to-end.
_V_0__placeholders), all restored..coderabbit.yaml+greptile.jsonupdated with the changelog, bun.lock/version-lockstep, silent-download, and watermark rules; CLAUDE.md codifies harvest-before-merge and never-accept-as-is.The changelog linter validated this PR's own entries (and rejected my first draft — working as intended).
🤖 Generated with Claude Code
Summary
Fixed 16 CodeRabbit/Greptile findings across:
ASRModelMissingErrorand reran “no-download” missing-weight preflight per fallback candidate to prevent unintended multi‑GB auto-downloads; improvedws_transcribe“ASR model missing” preflight to use the client’s raw?model=override value; strengthenedopenai_compat/v1/audio/transcriptions409 payloads (OmniVoice-aware structureddetail+ humanmessage)._loaded_asrtracking so the SSE transcription stream reliably force-unloads the loaded ASR backend on early terminal errors and disconnects, with consistenterror+doneemission.clone_voicerobustness: added base64data:URI stripping, strict base64 decode with typed “invalid base64” errors, and magic-byte extension sniffing for uploaded reference audio (plus improved HTTP/transport/JSON failure selection).uv_subprocess_env()soUV_CACHE_DIR/UV_PYTHON_INSTALL_DIRoverrides behave correctly across drives; improved sidecar/env override logic; added KittenTTS graceful degradation ifchunk_textimport is unavailable; tightened ASR memo reset hermeticity in tests.Added deterministic CI quality gates:
CHANGELOG.md(date-scoped rules + highlights/one-line bullet/credits/ref checks).en.json, rejecting corrupted_V_…__placeholder tokens._V_0__-style corruption).Improved frontend correctness and user-facing error detail:
galleryvoice/profile error strings across locales to interpolate runtime{{message}}.UI changes
Behavior flow
flowchart TD A[ws_transcribe preflight] --> B{ASR model missing?} B -->|Yes| C[Derive sherpamodel_id from raw ?model= override (_requested_model)\nclose/error uses the exact client override] D[dub_transcribe_stream SSE gen()] --> E[Preflight/load ASR\ntrack loaded backend in _loaded_asr] E --> F{Terminal exit / typed error / crash / disconnect?} F -->|Yes| G[Emit structured SSE error/done as needed] G --> H[Force-unload tracked ASR backend to release VRAM] H --> I[Clear _loaded_asr backend to avoid double-unload] J[Fallback ASR candidate load] --> K{Missing weights detected?} K -->|Yes| L[Raise typed ASRModelMissingError\n(no-download install CTA; no auto-download)] K -->|No| M[Load fallback normally] N[OpenAI-compat transcription 409] --> O{ASR backend missing (TTS-only install)?} O -->|Yes| P[Return structured 409 detail dict + human-readable message]Tests & CI
asr_model_missingbehavior under invalid?model=overrides,_decode_ref_audio/_sniff_audio_ext(data-URI stripping, invalid base64 ->None, magic-bytes extension inference),uv_subprocess_envcross-drive behavior with pinnedUV_CACHE_DIR/UV_PYTHON_INSTALL_DIR,asr_model_missingduring mic setup,tests/test_changelog_style.pyandtests/test_locale_parity.pyto enforce deterministic changelog + 21-locale parity with ratchet baselines.Reviewer/merge-process configuration
.coderabbit.yaml,greptile.json, andCLAUDE.mdto: