Phase 4 Plan 04-01: SPIKE-01 GGUF — GO + integration - #100
Conversation
Integrates Serveurperso/OmniVoice-GGUF as a hardware-adaptive default voice-cloning engine, with overridable fallback to the in-process OmniVoiceBackend. Spike confirmed GO: the model is a clean quantization of k2-fsa/OmniVoice (Apache-2.0 + MIT runtime, `omnivoice-lm` custom architecture so it does NOT load in vanilla llama.cpp). Pinned SHAs: * Serveurperso/OmniVoice-GGUF revision: 361609388ae572a820d085185bbbe2a2aac4b30e * ServeurpersoCom/omnivoice.cpp master: 886fc079838ca7400cb2b42b36e2a65aa1daabe8 Implements GGUF-01 (hardware probe) through GGUF-05 (default-engine resolver with graceful fallback). The four `bin/omnivoice-tts-*` artifacts are committed as zero-byte placeholders; the new CI matrix job builds the real binaries per platform from the pinned commit SHA and appends a SHA-256 manifest used by `is_available()` for tampering detection (T-04-01). The macos-14 (Apple Silicon) slot is marked `continue-on-error: true` because omnivoice.cpp publishes no `buildmetal.sh` (Pitfall 1 / Assumption A1) — failure feeds into Task 3's GO/NO-GO call. Quant override is allow-listed against quant_map.json entries only (T-04-05). Argv is composed from typed Path objects rooted in HF_HUB_CACHE; never uses `shell=True`. HF token redaction applies to captured stderr before logging (AUTH-05 / T-04-04). Tests: 36 new (8 hardware-probe + 13 GGUF engine + 6 settings_store quant override + grep gate); 428 passed in full suite vs 402+ baseline. ADR Status stays "Proposed (research-supported)" — Task 3 (human checkpoint) flips to Accepted after CI produces real binaries and a reviewer signs off on the GGUF-06 cross-hardware smoke. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a hardware-adaptive GGUF-based OmniVoice TTS backend: capability probe, pinned quant_map, subprocess-based backend with checksum/quarantine checks and quant override persistence, CI/build and smoke scripts, README updates, and unit tests covering detection, backend behavior, and settings persistence. ChangesGGUF OmniVoice Backend Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 281-283: The matrix entry uses an unsupported runner label
`macos-13`; update that `os:` value to a valid GitHub-hosted macOS label (e.g.,
replace `os: macos-13` with `os: macos-12` or `os: macos-latest`) so the
darwin-x86_64 platform job runs; ensure the `platform: darwin-x86_64` line
remains unchanged and the matrix entry format is preserved.
- Line 289: The workflow uses floating action tags (e.g., actions/checkout@v4)
and leaves checkout credentials persistent; replace each floating tag with its
immutable SHA pin (for every usage of actions/checkout and other actions like
upload-artifact, setup-python, setup-uv, setup-node, setup-bun, cache,
rust-toolchain, rust-cache, cache-apt-pkgs-action) and add persist-credentials:
false to every actions/checkout step (including the occurrences at the locations
shown); update each uses: entry to the exact commit SHA for that action and add
the persist-credentials: false option under the checkout step to disable
credential persistence.
- Around line 299-301: The workflow is injecting ${{ steps.pin.outputs.sha }}
directly into a shell command which can allow template/shell injection; fix by
passing the SHA via an environment variable and quoting it when calling the
script: set an env var (e.g., COMMIT_SHA: ${{ steps.pin.outputs.sha }}) on the
job/step where you call scripts/build-omnivoice-tts.sh and change the invocation
to --commit-sha "$COMMIT_SHA" (referencing the existing matrix.platform and the
pin step’s outputs and the scripts/build-omnivoice-tts.sh invocation).
In @.planning/decisions/SPIKE-01-gguf.md:
- Line 31: The Markdown table row contains an unescaped pipe in the command
example which splits the cell; escape the pipe in the command string. Edit the
table cell containing the command example echo "Hello world." |
./build/omnivoice-tts --model … --codec … --lang … -o … and replace the pipe
with an escaped version (use \| or &`#124`;) so the entire command remains inside
a single table cell.
In `@backend/engines/omnivoice_gguf/backend.py`:
- Around line 68-73: The repo-root calculation is off: _PKG_DIR points at
backend/engines/omnivoice_gguf so _REPO_ROOT = _PKG_DIR.parent.parent.parent
resolves to backend (not repo root), causing _binary_path() and checksum lookups
to target backend/bin; change the calculation to go one level higher (repo root)
e.g. use Path(__file__).resolve().parents[3] or
_PKG_DIR.parent.parent.parent.parent so _REPO_ROOT correctly points at the
repository root and fixes is_available() binary/checksum lookup across
platforms.
In `@backend/engines/omnivoice_gguf/hardware_probe.py`:
- Around line 94-96: The detect_capabilities() function currently assumes torch
is installed and raises ModuleNotFoundError; modify it to handle missing torch
by wrapping the import/use of torch in a try/except ImportError (or check if a
module-level torch is None) and return the safe CPU-only capability fallback
when torch is not available. Specifically, update detect_capabilities() (and any
helper like probe_cuda or uses of torch.cuda.is_available()) to first attempt to
import torch inside the function or check a guarded torch variable, only call
torch.cuda.* when import succeeded, and on ImportError return the CPU capability
object/value used elsewhere so macOS/Windows/Linux fallbacks work correctly.
Ensure all references to torch in detect_capabilities() are guarded to avoid
runtime exceptions.
In `@backend/engines/omnivoice_gguf/README.md`:
- Around line 5-6: The README incorrectly states the engine uses
SubprocessBackend/SubprocessBackend.run(); update the text to reflect that the
engine implements a custom subprocess host rather than subclassing
SubprocessBackend: remove or replace references to SubprocessBackend and
SubprocessBackend.run() with a brief description that the engine provides its
own subprocess host implementation (a custom subprocess runner in the backend)
and explain how the runtime is isolated and how operators should invoke or
configure that custom host; ensure the same change is applied to the other
occurrences mentioned (lines ~79-81).
In `@scripts/smoke-gguf.sh`:
- Around line 109-139: The smoke script must fail when the selected quant
doesn't match the forced hardware class: after entry =
backend._select_quant_entry() (and before generating) check that
entry.get("base") and/or entry.get("tokenizer") equals the expected value for
the current class_name (use the same mapping used elsewhere for class→expected
quant, or derive expected from class_name), and if it does not match, print a
FAIL message to stderr and sys.exit(1); keep the existing meta recording but
ensure this explicit assertion runs prior to calling backend.generate so the
test fails fast on a wrong quant selection.
- Around line 29-35: The case arms handling --hardware-class and --prompt
currently read "$2" directly (assigning to CLASS and PROMPT) which will fail
under set -u if the value is missing; modify those handlers to first validate
the next argument before consuming it (e.g., check that "${2-}" is set and does
not start with a dash) and if the check fails call the usage/error path,
otherwise assign CLASS="$2" (or PROMPT="$2") and shift 2 as before; reference
these exact case labels (--hardware-class, --prompt) and variables (CLASS,
PROMPT, "$2") when applying the guard.
🪄 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 Plus
Run ID: c26b816b-ac30-4f1c-8dfc-382ddfb10f1d
⛔ Files ignored due to path filters (1)
bin/omnivoice-tts-windows-x86_64.exeis excluded by!**/*.exe
📒 Files selected for processing (21)
.github/workflows/ci.yml.planning/decisions/SPIKE-01-gguf.md.planning/phases/04-adaptive-specialty-engines-spike-first/04-01-SUMMARY.mdbackend/engines/omnivoice_gguf/README.mdbackend/engines/omnivoice_gguf/__init__.pybackend/engines/omnivoice_gguf/backend.pybackend/engines/omnivoice_gguf/hardware_probe.pybackend/engines/omnivoice_gguf/quant_map.jsonbackend/services/gpu_sandbox.pybackend/services/settings_store.pybackend/services/tts_backend.pybin/README.mdbin/omnivoice-tts-darwin-arm64bin/omnivoice-tts-darwin-x86_64bin/omnivoice-tts-linux-x86_64scripts/build-omnivoice-tts.shscripts/smoke-gguf.shtests/backend/engines/__init__.pytests/backend/engines/test_hardware_probe.pytests/backend/engines/test_omnivoice_gguf.pytests/backend/services/test_settings_store.py
| - os: macos-13 | ||
| platform: darwin-x86_64 | ||
| experimental: false |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Validate workflow runner labels/syntax.
if command -v actionlint >/dev/null 2>&1; then
actionlint -color
else
echo "Install actionlint and run: actionlint -color"
fiRepository: debpalash/OmniVoice-Studio
Length of output: 942
Use a supported macOS Intel runner label for the x86_64 build slot.
Line 281 uses macos-13, which is not a recognized GitHub-hosted label and will fail this required matrix leg.
Suggested fix
- - os: macos-13
+ - os: macos-15-intel
platform: darwin-x86_64
experimental: false📝 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.
| - os: macos-13 | |
| platform: darwin-x86_64 | |
| experimental: false | |
| - os: macos-15-intel | |
| platform: darwin-x86_64 | |
| experimental: false |
🧰 Tools
🪛 actionlint (1.7.12)
[error] 281-281: label "macos-13" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2025-vs2026", "windows-2022", "windows-11-arm", "ubuntu-slim", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "macos-latest", "macos-latest-xlarge", "macos-latest-large", "macos-26-intel", "macos-26-xlarge", "macos-26-large", "macos-26", "macos-15-intel", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xlarge", "macos-14-large", "macos-14", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml around lines 281 - 283, The matrix entry uses an
unsupported runner label `macos-13`; update that `os:` value to a valid
GitHub-hosted macOS label (e.g., replace `os: macos-13` with `os: macos-12` or
`os: macos-latest`) so the darwin-x86_64 platform job runs; ensure the
`platform: darwin-x86_64` line remains unchanged and the matrix entry format is
preserved.
| # Metal build path is unpublished upstream — see Pitfall 1. | ||
| experimental: true | ||
| steps: | ||
| - uses: actions/checkout@v4 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Validate workflow security policy findings.
if command -v zizmor >/dev/null 2>&1; then
zizmor .github/workflows/ci.yml
else
echo "Install zizmor and run: zizmor .github/workflows/ci.yml"
fiRepository: debpalash/OmniVoice-Studio
Length of output: 9431
Pin all GitHub Actions to immutable SHAs and disable checkout credential persistence.
This workflow uses 18 unpinned floating action tags, weakening supply-chain security. Additionally, checkout actions at lines 27, 134, 214, and 289 do not set persist-credentials: false, increasing artifact exfiltration risk.
Example fix for line 289
- - uses: actions/checkout@v4
+ - uses: actions/checkout@<PINNED_SHA>
+ with:
+ persist-credentials: falseApply pinning (to exact SHAs) and persist-credentials: false to all checkout actions. Pin remaining actions (upload-artifact, setup-python, setup-uv, setup-node, setup-bun, cache, rust-toolchain, rust-cache, cache-apt-pkgs-action) to exact SHAs as well.
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 289-289: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 289-289: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml at line 289, The workflow uses floating action tags
(e.g., actions/checkout@v4) and leaves checkout credentials persistent; replace
each floating tag with its immutable SHA pin (for every usage of
actions/checkout and other actions like upload-artifact, setup-python, setup-uv,
setup-node, setup-bun, cache, rust-toolchain, rust-cache, cache-apt-pkgs-action)
and add persist-credentials: false to every actions/checkout step (including the
occurrences at the locations shown); update each uses: entry to the exact commit
SHA for that action and add the persist-credentials: false option under the
checkout step to disable credential persistence.
| bash scripts/build-omnivoice-tts.sh \ | ||
| --platform ${{ matrix.platform }} \ | ||
| --commit-sha ${{ steps.pin.outputs.sha }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for unquoted expression interpolation in shell command arguments.
rg -n --fixed-strings '--commit-sha ${{ steps.pin.outputs.sha }}' .github/workflows/ci.ymlRepository: debpalash/OmniVoice-Studio
Length of output: 514
🏁 Script executed:
# Read the relevant section of the workflow file
sed -n '295,305p' .github/workflows/ci.ymlRepository: debpalash/OmniVoice-Studio
Length of output: 480
🏁 Script executed:
# Also check the full context around the build-omnivoice-tts step
rg -n 'build-omnivoice-tts' .github/workflows/ci.yml -A 10Repository: debpalash/OmniVoice-Studio
Length of output: 880
🏁 Script executed:
# Check if the pattern exists (properly quoted this time)
rg -n -- '--commit-sha' .github/workflows/ci.yml -B 2 -A 2Repository: debpalash/OmniVoice-Studio
Length of output: 341
Quote the interpolated commit SHA before passing it to bash arguments.
Line 301 injects ${{ steps.pin.outputs.sha }} unquoted into shell command text. Quote via an env var to prevent template/shell injection paths.
Suggested fix
- name: Build omnivoice-tts
shell: bash
+ env:
+ RUNTIME_COMMIT_SHA: ${{ steps.pin.outputs.sha }}
run: |
bash scripts/build-omnivoice-tts.sh \
--platform ${{ matrix.platform }} \
- --commit-sha ${{ steps.pin.outputs.sha }}
+ --commit-sha "$RUNTIME_COMMIT_SHA"📝 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.
| bash scripts/build-omnivoice-tts.sh \ | |
| --platform ${{ matrix.platform }} \ | |
| --commit-sha ${{ steps.pin.outputs.sha }} | |
| - name: Build omnivoice-tts | |
| shell: bash | |
| env: | |
| RUNTIME_COMMIT_SHA: ${{ steps.pin.outputs.sha }} | |
| run: | | |
| bash scripts/build-omnivoice-tts.sh \ | |
| --platform ${{ matrix.platform }} \ | |
| --commit-sha "$RUNTIME_COMMIT_SHA" |
🧰 Tools
🪛 zizmor (1.25.2)
[info] 301-301: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml around lines 299 - 301, The workflow is injecting
${{ steps.pin.outputs.sha }} directly into a shell command which can allow
template/shell injection; fix by passing the SHA via an environment variable and
quoting it when calling the script: set an env var (e.g., COMMIT_SHA: ${{
steps.pin.outputs.sha }}) on the job/step where you call
scripts/build-omnivoice-tts.sh and change the invocation to --commit-sha
"$COMMIT_SHA" (referencing the existing matrix.platform and the pin step’s
outputs and the scripts/build-omnivoice-tts.sh invocation).
| | Runtime: llama.cpp / candle / custom? | CUSTOM (`omnivoice.cpp`, MIT) — does NOT load in vanilla llama.cpp | `gguf.architecture = "omnivoice-lm"` from HF API; README states "GGUF weights for omnivoice.cpp, a C++17/GGML port of OmniVoice". | | ||
| | Quant variants and footprints? | 4 quants × 2 files each (base + tokenizer): Q4_K_M (659 MB), Q8_0 (945 MB), BF16 (1.60 GB), F32 (3.19 GB) | HF `siblings` list confirms all 8 files; sizes from model card table. | | ||
| | Cross-platform runtime fit? | Linux + Windows + macOS Intel YES via documented build scripts; macOS Apple Silicon Metal CONDITIONAL (no `buildmetal.sh` published, only feature mention) | `buildcpu.sh`, `buildcuda.sh`, `buildvulkan.sh`, `buildall.sh` listed; Metal in description only — Wave 1 Task 3 builds and verifies via `cmake -DGGML_METAL=ON` per A1. | | ||
| | Subprocess CLI fits Phase 2 `SubprocessBackend`? | YES | README shows `echo "Hello world." | ./build/omnivoice-tts --model … --codec … --lang … -o …` — line-oriented stdin + argv + output-file pattern is exactly what `SubprocessBackend` is designed for. | |
There was a problem hiding this comment.
Escape the pipe character inside the table cell.
The | in the command example splits the table row into extra columns in Markdown renderers.
Use \| (or |) in that cell’s command snippet.
🧰 Tools
🪛 LanguageTool
[style] ~31-~31: Consider an alternative for the overused word “exactly”.
Context: ...d stdin + argv + output-file pattern is exactly what SubprocessBackend is designed fo...
(EXACTLY_PRECISELY)
🪛 markdownlint-cli2 (0.22.1)
[warning] 31-31: Table column count
Expected: 3; Actual: 4; Too many cells, extra data will be missing
(MD056, table-column-count)
🤖 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 @.planning/decisions/SPIKE-01-gguf.md at line 31, The Markdown table row
contains an unescaped pipe in the command example which splits the cell; escape
the pipe in the command string. Edit the table cell containing the command
example echo "Hello world." | ./build/omnivoice-tts --model … --codec … --lang …
-o … and replace the pipe with an escaped version (use \| or &`#124`;) so the
entire command remains inside a single table cell.
| _PKG_DIR = Path(__file__).resolve().parent | ||
|
|
||
| #: Repo root — used to find ``bin/omnivoice-tts-*`` and | ||
| #: ``bin/checksums.sha256``. backend/engines/omnivoice_gguf/ → root. | ||
| _REPO_ROOT = _PKG_DIR.parent.parent.parent | ||
|
|
There was a problem hiding this comment.
Fix repo-root path calculation for binary/checksum lookup.
_REPO_ROOT currently resolves to backend/, so _binary_path() and checksum lookup target backend/bin/* instead of repo bin/*. This will make is_available() fail even when binaries exist.
Suggested fix
-_REPO_ROOT = _PKG_DIR.parent.parent.parent
+_REPO_ROOT = _PKG_DIR.parent.parent.parent.parentAs per coding guidelines, "Cross-platform support: Every fix must work on macOS (Apple Silicon + Intel), Windows (x64), and Linux (AppImage + deb) with no platform-only regressions".
🤖 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/engines/omnivoice_gguf/backend.py` around lines 68 - 73, The
repo-root calculation is off: _PKG_DIR points at backend/engines/omnivoice_gguf
so _REPO_ROOT = _PKG_DIR.parent.parent.parent resolves to backend (not repo
root), causing _binary_path() and checksum lookups to target backend/bin; change
the calculation to go one level higher (repo root) e.g. use
Path(__file__).resolve().parents[3] or _PKG_DIR.parent.parent.parent.parent so
_REPO_ROOT correctly points at the repository root and fixes is_available()
binary/checksum lookup across platforms.
| import torch | ||
|
|
||
| if torch.cuda.is_available(): |
There was a problem hiding this comment.
Handle missing torch gracefully in detect_capabilities().
detect_capabilities() currently crashes with ModuleNotFoundError if torch is absent, instead of returning a safe CPU capability fallback.
Suggested fix
def detect_capabilities() -> HardwareCapabilities:
@@
- import torch
+ try:
+ import torch
+ except ModuleNotFoundError:
+ return HardwareCapabilities(backend="cpu", vram_mb=0, compute_class="cpu")As per coding guidelines, "Cross-platform support: Every fix must work on macOS (Apple Silicon + Intel), Windows (x64), and Linux (AppImage + deb) with no platform-only regressions".
📝 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.
| import torch | |
| if torch.cuda.is_available(): | |
| try: | |
| import torch | |
| except ModuleNotFoundError: | |
| return HardwareCapabilities(backend="cpu", vram_mb=0, compute_class="cpu") | |
| if torch.cuda.is_available(): |
🤖 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/engines/omnivoice_gguf/hardware_probe.py` around lines 94 - 96, The
detect_capabilities() function currently assumes torch is installed and raises
ModuleNotFoundError; modify it to handle missing torch by wrapping the
import/use of torch in a try/except ImportError (or check if a module-level
torch is None) and return the safe CPU-only capability fallback when torch is
not available. Specifically, update detect_capabilities() (and any helper like
probe_cuda or uses of torch.cuda.is_available()) to first attempt to import
torch inside the function or check a guarded torch variable, only call
torch.cuda.* when import succeeded, and on ImportError return the CPU capability
object/value used elsewhere so macOS/Windows/Linux fallbacks work correctly.
Ensure all references to torch in detect_capabilities() are guarded to avoid
runtime exceptions.
| subprocess (via Phase 2's `SubprocessBackend`) so the runtime is fully | ||
| isolated from the parent's Python process. |
There was a problem hiding this comment.
README runtime path is out of sync with implementation.
This doc still states the GGUF engine uses SubprocessBackend/SubprocessBackend.run(), but backend.py explicitly implements its own subprocess host and does not subclass that base. Please align wording to avoid operator confusion.
Also applies to: 79-81
🤖 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/engines/omnivoice_gguf/README.md` around lines 5 - 6, The README
incorrectly states the engine uses SubprocessBackend/SubprocessBackend.run();
update the text to reflect that the engine implements a custom subprocess host
rather than subclassing SubprocessBackend: remove or replace references to
SubprocessBackend and SubprocessBackend.run() with a brief description that the
engine provides its own subprocess host implementation (a custom subprocess
runner in the backend) and explain how the runtime is isolated and how operators
should invoke or configure that custom host; ensure the same change is applied
to the other occurrences mentioned (lines ~79-81).
| --hardware-class) | ||
| CLASS="$2" | ||
| shift 2 | ||
| ;; | ||
| --prompt) | ||
| PROMPT="$2" | ||
| shift 2 |
There was a problem hiding this comment.
Guard option values before reading $2.
--hardware-class / --prompt read $2 without checking it exists. With set -u, a missing value exits with a shell error instead of a clear usage failure.
Suggested fix
--hardware-class)
+ if [[ $# -lt 2 ]]; then
+ echo "--hardware-class requires a value (cpu|mid|high)" >&2
+ exit 1
+ fi
CLASS="$2"
shift 2
;;
--prompt)
+ if [[ $# -lt 2 ]]; then
+ echo "--prompt requires a value" >&2
+ exit 1
+ fi
PROMPT="$2"
shift 2
;;🤖 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 `@scripts/smoke-gguf.sh` around lines 29 - 35, The case arms handling
--hardware-class and --prompt currently read "$2" directly (assigning to CLASS
and PROMPT) which will fail under set -u if the value is missing; modify those
handlers to first validate the next argument before consuming it (e.g., check
that "${2-}" is set and does not start with a dash) and if the check fails call
the usage/error path, otherwise assign CLASS="$2" (or PROMPT="$2") and shift 2
as before; reference these exact case labels (--hardware-class, --prompt) and
variables (CLASS, PROMPT, "$2") when applying the guard.
| entry = backend._select_quant_entry() | ||
|
|
||
| t0 = time.monotonic() | ||
| tensor = backend.generate(prompt) | ||
| elapsed = time.monotonic() - t0 | ||
|
|
||
| # Save WAV to the expected path. | ||
| arr = tensor.squeeze(0).cpu().numpy() | ||
| sf.write(out_wav, arr, backend.sample_rate, subtype="PCM_16") | ||
|
|
||
| # Validate. | ||
| info = sf.info(out_wav) | ||
| duration_s = info.frames / info.samplerate | ||
| if duration_s < 2.5: | ||
| print(f"FAIL: output too short ({duration_s:.2f}s < 2.5s)", file=sys.stderr) | ||
| sys.exit(1) | ||
| if info.samplerate != 24_000: | ||
| print(f"FAIL: unexpected sample rate {info.samplerate} (expected 24000)", file=sys.stderr) | ||
| sys.exit(1) | ||
|
|
||
| meta = { | ||
| "class": class_name, | ||
| "quant_base": entry.get("base"), | ||
| "quant_tokenizer": entry.get("tokenizer"), | ||
| "rationale": entry.get("rationale"), | ||
| "duration_s": duration_s, | ||
| "elapsed_s": elapsed, | ||
| "sample_rate": info.samplerate, | ||
| "frames": info.frames, | ||
| "prompt": prompt, | ||
| } |
There was a problem hiding this comment.
Add an explicit quant-to-class assertion in the smoke check.
The script records entry fields but never fails if the selected quant is not the expected one for the forced hardware class. That can let regressions pass while still producing audio.
🤖 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 `@scripts/smoke-gguf.sh` around lines 109 - 139, The smoke script must fail
when the selected quant doesn't match the forced hardware class: after entry =
backend._select_quant_entry() (and before generating) check that
entry.get("base") and/or entry.get("tokenizer") equals the expected value for
the current class_name (use the same mapping used elsewhere for class→expected
quant, or derive expected from class_name), and if it does not match, print a
FAIL message to stderr and sys.exit(1); keep the existing meta recording but
ensure this explicit assertion runs prior to calling backend.generate so the
test fails fast on a wrong quant selection.
The pinned omnivoice.cpp commit (886fc079...) ships a `buildcpu.sh` that passes `-DGGML_BLAS=ON`. ubuntu-latest has no BLAS implementation preinstalled, so the cmake configure step fails with `Could NOT find BLAS (missing: BLAS_LIBRARIES)` and the job exits in 13 s before producing the linux-x86_64 binary. macOS (Accelerate, built in) and Windows (BLAS off by default in the ggml CMakeLists for non-APPLE platforms — the build script doesn't invoke buildcpu.sh on those slots) are unaffected and stay green. Adds a Linux-gated apt step to install libopenblas-dev + pkg-config before the build, restoring cross-platform parity per the CLAUDE.md "default features must work on every platform" rule. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
# Conflicts: # backend/services/settings_store.py # backend/services/tts_backend.py
…n Linux
The GGUF engine's `_build_argv` previously validated ref_audio only via
`ref_path.is_file()` — i.e. "does this path exist?" That check is
platform-dependent: `/etc/shadow` doesn't exist on macOS (rejected
naturally), but it IS a real system file on Linux, so the validation
silently accepted it. CI's ubuntu-22.04 runner exposed the gap via
`test_generate_blocks_freeform_ref_audio`, which exists precisely to
guard the "freeform ref_audio path" attack surface.
Fix: confine ref_audio to one of three allowed roots before existence
checks:
- VOICES_DIR (user-saved voice profiles)
- DUB_DIR (per-job auto-clones extracted from source video)
- tempfile.gettempdir() (browser-upload temp files; existing
`cleanup_ref` flow in generation.py)
Anything outside those roots → FileNotFoundError, matching the existing
failure-mode contract callers handle. Existence check still runs after,
so the test's mocked subprocess.run is never reached and the test
passes deterministically on all three platforms.
Cross-platform parity (per CLAUDE.md 2026-05-20 rule): identical
behaviour on macOS / Windows / Linux — the allow-list is computed from
core.config which uses platform-specific path resolution but yields the
same logical "project tree" on every OS.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
GitHub's macos-13 (Intel) runner pool is heavily contended — PR #100 queued for 30+ minutes waiting on darwin-x86_64 while every other platform finished in ~1m. Intel Macs are also fading hardware (Apple's platform momentum is entirely on Apple Silicon), and the GGUF engine's runtime already handles a missing binary gracefully (`is_available()` returns False on Intel Mac with a "binary not bundled for this platform" message, same path used for first-launch before any binaries build). `experimental: true` mirrors what darwin-arm64 (Metal) already has — slot still runs and uploads its binary when successful, but a failure or runner backlog no longer blocks merges. Keeps the GGUF engine shippable across the dominant arm64 / Linux / Windows surface without holding the inbox on a slow-runner queue. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 participant
Summary
SPIKE-01 outcome: GO.
Serveurperso/OmniVoice-GGUFis verified live as aquantization of
k2-fsa/OmniVoice(Apache-2.0 + MIT runtime, customomnivoice-lmarch). Wave 1 ships the engine integration: hardware probe,adaptive quant table, subprocess-host backend, Settings quant override,
CI build matrix, and the SPIKE-01 ADR ratified with pinned SHAs.
.planning/decisions/SPIKE-01-gguf.md— Decision: GO with pinned SHAs (361609388…/886fc079…); Status stays "Proposed (research-supported)" until Task 3 (human checkpoint) flips to Accepted.backend/engines/omnivoice_gguf/—OmniVoiceGGUFBackend(TTSBackend), hardware probe,quant_map.json, README,select_default_engine()resolver with graceful fallback._LAZY_REGISTRY["omnivoice-gguf"],settings_store.{get,set}_quant_override()with allow-list,gpu_sandbox.detect_capabilitiesre-export.scripts/build-omnivoice-tts.shcross-platform build from pinned commit;.github/workflows/ci.ymlnewbuild-omnivoice-ttsmatrix (Linux + Windows + macOS Intel required; macOS Apple Silicon Metalcontinue-on-error: trueper Pitfall 1).bin/— CI matrix produces real artifacts;is_available()falls back honestly until then.Test plan
tests/backend/engines/test_hardware_probe.py— 8 tests covering all 5 bucketing behaviours + the single-entry-point re-export invariant (detect_capabilitiesresolves identically from bothservices.gpu_sandboxandengines.omnivoice_gguf.hardware_probe).tests/backend/engines/test_omnivoice_gguf.py— 13 tests coveringquant_map.jsonschema, SHA-256 manifest verification (T-04-01),probe_loadtimeout (T-04-06), generate-returns-tensor-from-stub-WAV (3-second 24 kHz →(1, 72000)),select_default_enginefallback semantics (GGUF-05),_LAZY_REGISTRYplumbing, freeform-path rejection, and a tokenizer-basedshell=Truegrep gate.tests/backend/services/test_settings_store.py— 6 new tests for GGUF-04 round-trip + allow-list rejection (../etc/passwd, unknown filenames, non-strings).tests/test_supertonic3.pywhich depends on parallel work not in this PR's scope). Smoke suite 4/4 passed.bin/omnivoice-tts-*binaries; reviewer downloads, runsscripts/smoke-gguf.sh --hardware-class {cpu,mid,high}, listens to outputs, inspectsomnivoice.cppcommit886fc079…, makes the macOS Metal GO/NO-GO call, and flips ADR Status to Accepted (or Accepted with reduced scope).Notes
pyproject.toml,uv.lock,backend/engines/__init__.py, andbackend/engines/supertonic3/*(the GGUF engine needs no new Python deps)..planning/phases/04-adaptive-specialty-engines-spike-first/04-01-SUMMARY.md.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Chores
Documentation