fix(speaker-recognition): restore green integration CI - #343
Merged
Conversation
Three independent problems kept the speaker-recognition workflow red on
every run since 2026-07-15.
1. BuildKit disabled but required (the actual CI failure)
run-test.sh exported DOCKER_BUILDKIT=0, forcing the legacy builder,
while the Dockerfile uses `RUN --mount=type=cache` for the uv cache:
Step 10/21 : RUN --mount=type=cache,target=/root/.cache/uv uv sync ...
the --mount option requires BuildKit.
At the last green commit (087a106) the Dockerfile had a plain
`RUN uv sync ...` with no --mount, and DOCKER_BUILDKIT=0 was already
present. The cache mount was re-added later without dropping the
export, so `docker compose up` died ~90s in on every run.
Swap DOCKER_BUILDKIT=0 for BUILDKIT_PROGRESS=plain: the cache mount
works and we keep the flat, greppable build log that disabling
BuildKit was presumably meant to preserve. The workflow already runs
docker/setup-buildx-action, so BuildKit was set up and then discarded.
2. `uv run` discarded the CPU torch selection
run-test.sh synced `--extra cpu`, then bare `uv run` resynced to the
default extra set, pulling PyPI CUDA torch plus 15 nvidia wheels on
top of the CPU build - ~40s and ~12GB of pure waste per run. Pin the
invocation to `uv run --extra cpu --group test`.
3. Phase 7 asserted a contract that never existed
Phase 7 required a `words` field on every /diarize-and-identify
segment, but that endpoint only diarizes and identifies - it has never
transcribed, on main or dev. Phases 7-8 were added after the last green
run (which had only Phases 1-6), so Phase 7 has never passed once.
Point it at the structure the endpoint actually guarantees rather than
bolting transcription onto a diarization endpoint. Word-level coverage
is not lost: Phase 8 still validates it against
/v1/diarize-identify-match, which is handed a transcript to match.
Verified locally against the real service in a container with real
HF/Deepgram credentials: 1 passed in 110.92s, with Phase 8 executing for
the first time. The BuildKit change is read directly from the CI error
and cannot be exercised locally (docker here is a podman shim that
accepts --mount regardless), so it needs a CI run to confirm.
The workflow gated both its push and pull_request triggers on [main, develop], but develop was retired in favour of dev years ago and no longer exists on the remote (origin/HEAD -> origin/dev). The practical effect was silent: PRs targeting dev never ran the speaker recognition suite at all, so the only runs came from PRs into main. Point both triggers at dev so the tests actually gate the branch we merge into. ios-ipa-build.yml and android-apk-build.yml carry the same stale reference and are left alone here to keep this change scoped.
Contributor
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
The iOS and Android workflows had two overlapping gaps: push: branches: [main, develop] # develop no longer exists pull_request: branches: [main] # dev never included at all develop was retired in favour of dev, so the push trigger's second entry was dead. Worse, the pull_request trigger only ever covered main, so app changes merged into dev were never built - not by the stale entry, and not by the PR gate either. Four merged PRs touched app/** with base=dev and got no build: #319, #315, #311, #233. Point both triggers at [main, dev].
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The Speaker Recognition Tests workflow has been red on every run since 2026-07-15. Four independent problems, three of which had to be fixed before the suite could go green.
1. BuildKit disabled but required — the actual CI failure
run-test.shexportedDOCKER_BUILDKIT=0, forcing the legacy builder, while the Dockerfile usesRUN --mount=type=cachefor the uv cache. The legacy builder rejects that flag outright, sodocker compose updied ~90s in on every run.At the last green commit (
087a1060) the Dockerfile had a plainRUN uv sync ...with no--mount, andDOCKER_BUILDKIT=0was already present. The cache mount was re-added later without dropping the export.Swapped for
BUILDKIT_PROGRESS=plain: the cache mount works, and we keep the flat greppable build log that disabling BuildKit was presumably meant to preserve. The workflow already runsdocker/setup-buildx-action, so BuildKit was being set up and then discarded.2.
uv rundiscarded the CPU torch selectionrun-test.shsynced--extra cpu, then bareuv runresynced to the default extra set. The CI log showstorch==2.9.0+cpuinstalling at 06:58:39, immediately followed byDownloading nvidia-cudnn-cu12 (674.0MiB),nvidia-cublas-cu12 (566.8MiB)and 13 more — roughly 40s and ~12GB of pure waste per run.Pinned to
uv run --extra cpu --group test.3. Phase 7 asserted a contract that never existed
Phase 7 required a
wordsfield on every/diarize-and-identifysegment, but that endpoint only diarizes and identifies — it has never transcribed, onmainordev. Phases 7-8 were added after the last green run (which had only Phases 1-6), so Phase 7 has never passed once.Repointed at the structure the endpoint actually guarantees, rather than bolting transcription onto a diarization endpoint — that would change production behaviour and add a per-call Deepgram cost to satisfy a test. Word-level coverage is not lost: Phase 8 still validates it against
/v1/diarize-identify-match, which is handed a transcript to match.4. Workflow gated on a branch that no longer exists
Both triggers were
[main, develop], butdevelopwas retired in favour ofdevand is gone from the remote. PRs targetingdevnever ran this suite at all — only PRs intomaindid. Now[main, dev].ios-ipa-build.ymlandandroid-apk-build.ymlcarry the same stale reference; left alone to keep this scoped.Verification
Run locally against the real service in a container with real HF/Deepgram credentials — not mocks:
Phase 8 executed for the first time in this run. The 4
test_enrollment_*.pytests also pass (12s), though no workflow runs them — worth wiring up separately.Caveat: the BuildKit change is read directly off the CI error and could not be exercised locally (
dockeron my machine is a podman shim that accepts--mountregardless). This PR's own run is the first real test of it.