Skip to content

docs: QIP — reorganize the repo into a symmetric multi-engine speech layout#94

Open
ogad-tether wants to merge 2 commits into
masterfrom
feat/speech-repo-reorg-qip
Open

docs: QIP — reorganize the repo into a symmetric multi-engine speech layout#94
ogad-tether wants to merge 2 commits into
masterfrom
feat/speech-repo-reorg-qip

Conversation

@ogad-tether

Copy link
Copy Markdown

What

The reorg proposal (docs/QIP-speech-repo-reorg.md) — this PR is the approval vehicle: review the plan, not code. No source or build changes here.

Proposed end state:

CMakeLists.txt                 # umbrella superbuild (feature-gated); ours
third_party/whisper.cpp/       # upstream via git subtree, minimally divergent + PATCHES.md
engines/
  whisper/                     # thin adapter over third_party
  parakeet/                    # git mv from parakeet-cpp/
  tts/                         # git mv from tts-cpp/
  qwen-asr/                    # future

Key points, from a repo audit (all verified against the tree):

  • "Pristine, never hand-edited" vendoring is not possible: we carry ~374 lines of patches on upstream files vs v1.9.1 (QVAC-21623 QKV fusion + logits slice in src/whisper.cpp, ggml backend-reg/vulkan). Policy amended to upstream-tracking, minimally divergent: deltas declared in PATCHES.md, enforced by a CI divergence-guard job; git subtree pull then conflicts only on the ~3 files we touch instead of today's whole-tree squashed re-apply.
  • ggml divergence is banned in-repo: canonical ggml patches live in qvac-ext-ggml@speech (the ggml-speech port source); the umbrella build forces system ggml so the vendored ggml/ is never compiled.
  • Concrete subtree conversion recipe (git subtree add can't target an existing prefix), phased PR plan (CI first → renames → vendor conversion → sync runbook), registry-port coordination, and upstream's new parakeet (C API) gated off vs our engines/parakeet (C++).
  • CI/testing scaffold: per-engine build+test lanes (already implemented and validated in ci: per-engine CI lanes for parakeet-cpp and tts-cpp (QIP PR 0) #93, which lands independently of this approval), GPU lane stubs, port-smoke follow-up, and the path to enabling the parity suites (hosted GGUF fixtures).

Approvals needed

Speech team lead · infra/registry owner · downstream (qvac) owner — table stubbed at the bottom of the doc.

🤖 Generated with Claude Code

…layout

Proposal: engines/{whisper,parakeet,tts} + upstream whisper.cpp vendored
as a minimally-divergent git subtree under third_party/whisper.cpp, with
an umbrella feature-gated CMake superbuild and in-repo per-engine CI.

Key deviations from the original sketch, based on a repo audit:
- "pristine, never hand-edited" vendoring is not possible — we carry
  ~374 lines of patches on upstream files (QKV fusion, logits slice,
  ggml backend-reg/vulkan); policy amended to upstream-tracking +
  PATCHES.md manifest + CI divergence guard
- ggml divergence moves entirely to qvac-ext-ggml@speech (the
  ggml-speech port source); the umbrella build forces system ggml
- concrete subtree conversion recipe (git subtree add can't target an
  existing prefix), phased PR plan, and upstream-sync runbook

CI scaffold (PR 0, #93) lands independently of this approval.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@ogad-tether
ogad-tether requested review from a team as code owners July 17, 2026 09:03
@github-actions

Copy link
Copy Markdown

Review Status

Current Status: ❌ PENDING
Approvals so far: none

Pending reviews: Needs 1 Management or Team Lead, and 1 more from Management, Team Lead, or Member.

ogad-tether added a commit that referenced this pull request Jul 17, 2026
… QIP to #94

First run of the new tts CI lane caught a real cross-platform bug:
test-campplus and test-campplus-backward-parity compile src/campplus.cpp
(which calls tts_cpp::detail::init_cpu_backend()) without
src/backend_selection.cpp, where that symbol lives. Apple's toolchain
happened to optimize the reference away; GNU ld on Linux fails with an
undefined reference — and Linux-with-tests was never built by any CI
until now. Mirror the source list test-voice-embedding already uses.

Also drop docs/QIP-speech-repo-reorg.md from this branch — the QIP now
has its own approval PR (#94); this PR is CI + fixes only.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@ogad-tether ogad-tether self-assigned this Jul 17, 2026
ogad-tether added a commit that referenced this pull request Jul 17, 2026
…e engines

engines-cross-ci.yml: engine x platform matrix complementing the
linux/mac lanes —
- windows-2022: MSVC, static system ggml, full build + non-GPU ctest
- android: NDK arm64-v8a compile smoke (no device to run tests on)
- ios: device arm64 compile smoke, Metal embedded (flags mirror
  build-xcframework.sh)

iOS recipe validated locally (ggml + both engines compile clean against
the iphoneos 26.2 SDK). On-device e2e remains downstream in
tetherto/qvac; these lanes front-load cross-compile/link breaks ahead
of the repo-reorg moves (QIP #94).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Downstream validation policy: in-repo ctests are not sufficient to
  approve engine changes — a qvac overlay-port draft PR exercising all
  platforms/addons (desktop + Device Farm mobile e2e) is a required
  gate before the registry pin bump. Documents the exact recipe
  (overlay portfiles + vcpkg-configuration, label mechanics) and
  references the executed example (qvac#3310). Replaces the weaker
  port-smoke idea.
- PATCHES.md policy: manifest rows must be self-contained (what + why),
  no reliance on tracker tickets.
- CI table: reflect the shipped Windows build+test and Android/iOS
  compile-smoke lanes (engines-cross-ci.yml, green in PR 0).
- PR 2 deliverables: full root README (engines/models, install + run,
  cross-platform perf) — delivered on the PR 2 branch.
- Fix stale `ctest -L cpu` reference.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant