build(tts): use slim CUDA 13.3.1 image - #31
Merged
Conversation
Reviewer's GuideReplaces the vLLM-derived monolithic TTS image with a slim, multi-stage CUDA 13.3.1 ARM64 Docker image backed by tightly enforced dependency and hash-lock contracts, plus tests that guard the new build, runtime toolchain, and Kaldi compatibility path against regressions. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The new helper functions and regex-based assertions in
test_dgx_deployment.pyare quite dense; consider extracting some of the more complex logic (e.g.,_assert_hash_locked, tooling gates) into a small non-test utility module to reduce test brittleness and improve readability. - Several tests assert exact multi-line Dockerfile snippets and full apt lock contents; using more targeted pattern checks (e.g., regex or subset assertions) would make these contracts less fragile to minor formatting or ordering changes while still enforcing the important guarantees.
- The long inline Python one-liners in Dockerfile
RUNsteps (especially the Kaldi compat extraction) are hard to read and modify; moving them into standalone scripts copied into the image would make future maintenance and debugging significantly easier.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new helper functions and regex-based assertions in `test_dgx_deployment.py` are quite dense; consider extracting some of the more complex logic (e.g., `_assert_hash_locked`, tooling gates) into a small non-test utility module to reduce test brittleness and improve readability.
- Several tests assert exact multi-line Dockerfile snippets and full apt lock contents; using more targeted pattern checks (e.g., regex or subset assertions) would make these contracts less fragile to minor formatting or ordering changes while still enforcing the important guarantees.
- The long inline Python one-liners in Dockerfile `RUN` steps (especially the Kaldi compat extraction) are hard to read and modify; moving them into standalone scripts copied into the image would make future maintenance and debugging significantly easier.
## Individual Comments
### Comment 1
<location path="dgx/tts/Dockerfile" line_range="36-38" />
<code_context>
+ && python3 -c "from pathlib import Path; from zipfile import ZipFile; import sys; wheel, root, license_root = sys.argv[1:]; archive = ZipFile(wheel); source = archive.read('torchaudio/compliance/kaldi.py').decode(); license_text = archive.read('torchaudio-2.9.1.dist-info/LICENSE'); archive.close(); assert source.count('import torchaudio\\n') == 1; provenance = '# Derived from torchaudio 2.9.1, torchaudio/compliance/kaldi.py.\\n# SPDX-License-Identifier: BSD-2-Clause\\n# Full license: /usr/share/licenses/torchaudio-kaldi-compat/LICENSE\\n# Compatibility contract: only fbank is supported; mfcc is outside this runtime contract.\\n\\n'; Path(root, 'core/tokenizer_25hz/vq/kaldi_compat.py').write_text(provenance + source.replace('import torchaudio\\n', ''), encoding='utf-8'); license_path = Path(license_root) / 'LICENSE'; license_path.parent.mkdir(parents=True, exist_ok=True); license_path.write_bytes(license_text)" \
</code_context>
<issue_to_address>
**suggestion:** The inlined wheel/kaldi-compat manipulation logic is quite dense and would benefit from being factored into a dedicated script for maintainability.
The `python3 -c` snippet that rewrites `kaldi_compat.py` tightly couples filesystem and archive manipulation into a single inline command, which makes it difficult to read, debug, and safely change (e.g., path updates or behavior tweaks). Moving this logic into a small, checked-in Python module that’s copied into the image and invoked from the Dockerfile would make the build more declarative and the transformation easier to test and evolve.
Suggested implementation:
```
COPY tts/torchaudio-kaldi-compat-arm64.lock /tmp/torchaudio-kaldi-compat-arm64.lock
COPY tts/extract_kaldi_compat.py /usr/local/bin/extract_kaldi_compat.py
RUN pip download --no-cache-dir --require-hashes --no-deps \
```
```
&& kaldi_wheel="$(find /tmp/torchaudio-kaldi -name 'torchaudio-*.whl' -print -quit)" \
&& python3 /usr/local/bin/extract_kaldi_compat.py \
"$kaldi_wheel" "$package_root" /opt/tts-licenses/torchaudio-kaldi-compat \
```
To fully implement this refactor, add a new checked-in Python module `tts/extract_kaldi_compat.py` with a `main()` that:
1. Parses `wheel_path`, `package_root`, and `license_root` from `sys.argv[1:]`.
2. Opens the wheel via `ZipFile(wheel_path)` and reads:
- `torchaudio/compliance/kaldi.py` into `source` (decoded as UTF-8).
- `torchaudio-2.9.1.dist-info/LICENSE` into `license_text` (bytes).
3. Asserts `source.count("import torchaudio\n") == 1` for safety.
4. Prepends the provenance header:
```python
PROVENANCE = (
"# Derived from torchaudio 2.9.1, torchaudio/compliance/kaldi.py.\n"
"# SPDX-License-Identifier: BSD-2-Clause\n"
"# Full license: /usr/share/licenses/torchaudio-kaldi-compat/LICENSE\n"
"# Compatibility contract: only fbank is supported; mfcc is outside this runtime contract.\n"
"\n"
)
```
5. Writes `PROVENANCE + source.replace("import torchaudio\n", "")` to
`Path(package_root) / "core/tokenizer_25hz/vq/kaldi_compat.py"` with UTF-8 encoding.
6. Ensures `license_root` exists (`Path(license_root).mkdir(parents=True, exist_ok=True)`) and writes `license_text` to `Path(license_root) / "LICENSE"`.
Include the usual `if __name__ == "__main__": main()` boilerplate so the script can be invoked as done in the Dockerfile.
</issue_to_address>
### Comment 2
<location path="tests/test_dgx_deployment.py" line_range="359-368" />
<code_context>
+def test_tts_runtime_stage_rejects_forbidden_tooling_instructions() -> None:
</code_context>
<issue_to_address>
**suggestion (testing):** Add a fixture where the tooling absence gate is present but a forbidden tool still appears in another runtime instruction.
One subtle case worth covering is a runtime stage that includes `_RUNTIME_TOOLING_ABSENCE_GATE` but also later runs a forbidden tooling command (e.g., `RUN make` or `apt-get install build-essential`) in the same stage. Because `_runtime_stage_uses_forbidden_tool` only skips the gate line, that fixture should still be rejected. Adding a test for this scenario would guard against future changes to `_runtime_stage_instructions` or the gate regex weakening this behavior.
Suggested implementation:
```python
def test_tts_runtime_stage_rejects_forbidden_tooling_instructions() -> None:
builder_only = """FROM base AS builder
RUN apt-get install -y build-essential make
FROM base AS runtime
RUN echo runtime-ready
"""
# Forbidden tooling in the builder stage should not cause the runtime stage to be rejected.
assert not _runtime_stage_uses_forbidden_tool(builder_only)
later_debug_stage = """FROM base AS builder
FROM base AS runtime
RUN echo runtime-ready
"""
assert not _runtime_stage_uses_forbidden_tool(later_debug_stage)
runtime_with_gate_and_forbidden_tooling = """FROM base AS builder
FROM base AS runtime
RUN _RUNTIME_TOOLING_ABSENCE_GATE
RUN make -j$(nproc)
"""
# Even with the tooling-absence gate present, a later forbidden tooling command
# in the same runtime stage must still cause rejection.
assert _runtime_stage_has_tooling_absence_gate(runtime_with_gate_and_forbidden_tooling)
assert _runtime_stage_uses_forbidden_tool(runtime_with_gate_and_forbidden_tooling)
```
1. If the actual gate instruction differs (e.g., it uses a shell wrapper or a different literal than `RUN _RUNTIME_TOOLING_ABSENCE_GATE`), update the `runtime_with_gate_and_forbidden_tooling` fixture to match the real gate line so that `_runtime_stage_has_tooling_absence_gate` returns `True`.
2. If your forbidden tooling matcher is keyed on specific commands (e.g., `apt-get install` rather than `make`), you may want to replace `RUN make -j$(nproc)` with a command known to be matched as forbidden in your implementation, such as `RUN apt-get install -y build-essential`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+36
to
+38
| && python3 -c "from pathlib import Path; from zipfile import ZipFile; import sys; wheel, root, license_root = sys.argv[1:]; archive = ZipFile(wheel); source = archive.read('torchaudio/compliance/kaldi.py').decode(); license_text = archive.read('torchaudio-2.9.1.dist-info/LICENSE'); archive.close(); assert source.count('import torchaudio\\n') == 1; provenance = '# Derived from torchaudio 2.9.1, torchaudio/compliance/kaldi.py.\\n# SPDX-License-Identifier: BSD-2-Clause\\n# Full license: /usr/share/licenses/torchaudio-kaldi-compat/LICENSE\\n# Compatibility contract: only fbank is supported; mfcc is outside this runtime contract.\\n\\n'; Path(root, 'core/tokenizer_25hz/vq/kaldi_compat.py').write_text(provenance + source.replace('import torchaudio\\n', ''), encoding='utf-8'); license_path = Path(license_root) / 'LICENSE'; license_path.parent.mkdir(parents=True, exist_ok=True); license_path.write_bytes(license_text)" \ | ||
| "$kaldi_wheel" "$package_root" /opt/tts-licenses/torchaudio-kaldi-compat \ | ||
| && python3 -c "from pathlib import Path; import sys; path = Path(sys.argv[1]) / 'core/tokenizer_25hz/vq/speech_vq.py'; source = path.read_text(encoding='utf-8'); updated = source.replace('import torchaudio.compliance.kaldi as kaldi', 'from . import kaldi_compat as kaldi'); assert updated != source; path.write_text(updated, encoding='utf-8')" \ |
There was a problem hiding this comment.
suggestion: The inlined wheel/kaldi-compat manipulation logic is quite dense and would benefit from being factored into a dedicated script for maintainability.
The python3 -c snippet that rewrites kaldi_compat.py tightly couples filesystem and archive manipulation into a single inline command, which makes it difficult to read, debug, and safely change (e.g., path updates or behavior tweaks). Moving this logic into a small, checked-in Python module that’s copied into the image and invoked from the Dockerfile would make the build more declarative and the transformation easier to test and evolve.
Suggested implementation:
COPY tts/torchaudio-kaldi-compat-arm64.lock /tmp/torchaudio-kaldi-compat-arm64.lock
COPY tts/extract_kaldi_compat.py /usr/local/bin/extract_kaldi_compat.py
RUN pip download --no-cache-dir --require-hashes --no-deps \
&& kaldi_wheel="$(find /tmp/torchaudio-kaldi -name 'torchaudio-*.whl' -print -quit)" \
&& python3 /usr/local/bin/extract_kaldi_compat.py \
"$kaldi_wheel" "$package_root" /opt/tts-licenses/torchaudio-kaldi-compat \
To fully implement this refactor, add a new checked-in Python module tts/extract_kaldi_compat.py with a main() that:
- Parses
wheel_path,package_root, andlicense_rootfromsys.argv[1:]. - Opens the wheel via
ZipFile(wheel_path)and reads:torchaudio/compliance/kaldi.pyintosource(decoded as UTF-8).torchaudio-2.9.1.dist-info/LICENSEintolicense_text(bytes).
- Asserts
source.count("import torchaudio\n") == 1for safety. - Prepends the provenance header:
PROVENANCE = ( "# Derived from torchaudio 2.9.1, torchaudio/compliance/kaldi.py.\n" "# SPDX-License-Identifier: BSD-2-Clause\n" "# Full license: /usr/share/licenses/torchaudio-kaldi-compat/LICENSE\n" "# Compatibility contract: only fbank is supported; mfcc is outside this runtime contract.\n" "\n" )
- Writes
PROVENANCE + source.replace("import torchaudio\n", "")to
Path(package_root) / "core/tokenizer_25hz/vq/kaldi_compat.py"with UTF-8 encoding. - Ensures
license_rootexists (Path(license_root).mkdir(parents=True, exist_ok=True)) and writeslicense_texttoPath(license_root) / "LICENSE".
Include the usual if __name__ == "__main__": main() boilerplate so the script can be invoked as done in the Dockerfile.
Comment on lines
+359
to
+368
| def test_tts_runtime_stage_rejects_forbidden_tooling_instructions() -> None: | ||
| builder_only = """FROM base AS builder | ||
| RUN apt-get install -y build-essential make | ||
| FROM base AS runtime | ||
| RUN echo runtime-ready | ||
| """ | ||
| assert not _runtime_stage_uses_forbidden_tool(builder_only) | ||
|
|
||
| later_debug_stage = """FROM base AS builder | ||
| FROM base AS runtime |
There was a problem hiding this comment.
suggestion (testing): Add a fixture where the tooling absence gate is present but a forbidden tool still appears in another runtime instruction.
One subtle case worth covering is a runtime stage that includes _RUNTIME_TOOLING_ABSENCE_GATE but also later runs a forbidden tooling command (e.g., RUN make or apt-get install build-essential) in the same stage. Because _runtime_stage_uses_forbidden_tool only skips the gate line, that fixture should still be rejected. Adding a test for this scenario would guard against future changes to _runtime_stage_instructions or the gate regex weakening this behavior.
Suggested implementation:
def test_tts_runtime_stage_rejects_forbidden_tooling_instructions() -> None:
builder_only = """FROM base AS builder
RUN apt-get install -y build-essential make
FROM base AS runtime
RUN echo runtime-ready
"""
# Forbidden tooling in the builder stage should not cause the runtime stage to be rejected.
assert not _runtime_stage_uses_forbidden_tool(builder_only)
later_debug_stage = """FROM base AS builder
FROM base AS runtime
RUN echo runtime-ready
"""
assert not _runtime_stage_uses_forbidden_tool(later_debug_stage)
runtime_with_gate_and_forbidden_tooling = """FROM base AS builder
FROM base AS runtime
RUN _RUNTIME_TOOLING_ABSENCE_GATE
RUN make -j$(nproc)
"""
# Even with the tooling-absence gate present, a later forbidden tooling command
# in the same runtime stage must still cause rejection.
assert _runtime_stage_has_tooling_absence_gate(runtime_with_gate_and_forbidden_tooling)
assert _runtime_stage_uses_forbidden_tool(runtime_with_gate_and_forbidden_tooling)- If the actual gate instruction differs (e.g., it uses a shell wrapper or a different literal than
RUN _RUNTIME_TOOLING_ABSENCE_GATE), update theruntime_with_gate_and_forbidden_toolingfixture to match the real gate line so that_runtime_stage_has_tooling_absence_gatereturnsTrue. - If your forbidden tooling matcher is keyed on specific commands (e.g.,
apt-get installrather thanmake), you may want to replaceRUN make -j$(nproc)with a command known to be matched as forbidden in your implementation, such asRUN apt-get install -y build-essential.
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.
Summary
Verification
Deployment
Production has already been switched through Portainer to
dgx-qwen3-tts:cuda1331-cu132-36181daand is healthy with zero restarts. The previousdgx-qwen3-tts:admission-08c6162image remains available for rollback.Remaining non-blocking risks
Summary by Sourcery
Switch the TTS service to a slim, multi-stage CUDA 13.3.1 ARM64 image with a self-contained Python 3.14 runtime and strictly locked dependencies while preserving the existing Qwen3-TTS behavior and contracts.
New Features:
Enhancements:
Tests: