perf(build): slim Docker image 1.63GB → 0.81GB (-50.31%) - #999
Conversation
WalkthroughThe PR replaces the single-stage Docker build with separate dependency, model-fetch, and runtime stages. It adds GCS-based AIC model retrieval, narrows runtime dependencies, updates Python dependencies, and expands Docker ignore rules. ChangesContainer build pipeline
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Pull request overview
This PR rewrites the container build to a multi-stage Dockerfile and trims Python dependencies to significantly reduce the shipped runtime image size while keeping the application runtime behavior the same.
Changes:
- Replaced the single-stage Docker build with a 3-stage build (builder → model-fetch → runtime) and removed build toolchains from the runtime image.
- Replaced
google-cloud-sdk/gcloud storage cpmodel fetching with a lightweight Python-based GCS download script. - Trimmed dependency extras (
pipecat-aiandredis) and regenerateduv.lock; tightened Docker context via.dockerignore.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Dockerfile | Multi-stage build; venv + model fetch separated from runtime; runtime now runs python run.py. |
| scripts/fetch_aic_models.py | New GCS model download helper used during build. |
| pyproject.toml | Removes unused extras (pipecat-ai azure/silero; redis[hiredis]). |
| uv.lock | Lockfile regeneration reflecting dependency trimming. |
| .dockerignore | Excludes large/unneeded build context paths; whitelists the new fetch script. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| VENV_PATH=/opt/venv \ | ||
| PATH="/opt/venv/bin:${PATH}" | ||
|
|
||
| # Runtime-only apt — no compilers, no headers. Skip ffmpeg므 X11/mesa/audio |
| RUN apt-get update && DEBIAN_FRONTEND=noninteractive apt-get install -y \ | ||
| --no-install-recommends \ | ||
| --ignore-missing \ | ||
| ffmpeg \ | ||
| libportaudio2 \ |
| def get_credentials(): | ||
| if os.path.exists(TOKEN_FILE) and os.path.getsize(TOKEN_FILE) > 0: | ||
| token = open(TOKEN_FILE).read().strip() | ||
| print("Auth: BuildKit secret gcp_token") | ||
| return Credentials(token=token), None |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@scripts/fetch_aic_models.py`:
- Line 22: Update the get_credentials function signature with a return type
annotation representing optional Google credentials together with an optional
project ID, using the appropriate typing constructs and existing credential type
imports.
- Around line 64-71: Update the model-fetch loop around the download try/except
to collect each failed model name when credentials are present, while retaining
the existing warning output. After all models are processed, exit non-zero if
any downloads failed; preserve the separate no-credentials AWS exit path.
- Around line 25-32: Replace the authentication diagnostic print calls in the
credential-loading flow with contextual Loguru logger calls. In the exception
handlers around ADC and related credential attempts, use
logger.opt(exception=e).warning(...) without interpolating e into the message;
preserve the existing diagnostic context in each structured message.
- Around line 41-57: Move the GOOGLE_CLOUD_PROJECT/GCP_PROJECT fallback,
AIC_BUCKET_PATH, and AIC_MODELS definitions into app/core/config/static.py,
exposing the required settings through get_required_env(). Update the
model-fetch script to import and use those static configuration values instead
of direct os.environ.get calls, while preserving existing defaults or mandatory
behavior as defined by the configuration. Update the Docker model-fetch stage to
copy the static configuration module before importing it.
- Around line 23-24: Make the script entry point asynchronous and move all
blocking filesystem and Google Cloud operations off the event loop: wrap open,
os.path.exists, os.makedirs, os.path.getsize, blob.download_to_filename, and ADC
lookup in awaitable calls, using asyncio.to_thread for synchronous APIs
including Google Cloud Storage downloads. Update callers to await the async
entry point and preserve the existing token and download behavior.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 70345d55-fe7c-48a3-b1fc-293925f08738
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
.dockerignoreDockerfilepyproject.tomlscripts/fetch_aic_models.py
| DEST_DIR = "/models" | ||
|
|
||
|
|
||
| def get_credentials(): |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add the required return type annotation.
get_credentials has no type annotation. Add a return type that represents optional Google credentials and an optional project ID.
As per coding guidelines, “Include required type hints on all function signatures.”
🤖 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/fetch_aic_models.py` at line 22, Update the get_credentials function
signature with a return type annotation representing optional Google credentials
together with an optional project ID, using the appropriate typing constructs
and existing credential type imports.
Source: Coding guidelines
| if os.path.exists(TOKEN_FILE) and os.path.getsize(TOKEN_FILE) > 0: | ||
| token = open(TOKEN_FILE).read().strip() |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- target file outline ---'
ast-grep outline scripts/fetch_aic_models.py || true
printf '%s\n' '--- target file ---'
cat -n scripts/fetch_aic_models.py
printf '%s\n' '--- related configuration and imports ---'
rg -n --glob '*.py' 'fetch_aic_models|get_required_env|TOKEN_FILE|google\.cloud|storage\.Client|download_to_filename|download_as_bytes|open\(' app scriptsRepository: juspay/clairvoyance
Length of output: 5866
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- callers and build context ---'
rg -n --glob '!scripts/fetch_aic_models.py' 'fetch_aic_models\.py|model-fetch|AIC_BUCKET_PATH|AIC_MODELS|/models' .
printf '%s\n' '--- project/runtime configuration ---'
fd -i 'pyproject.toml|Dockerfile*|static.py' . -t f -x sh -c 'echo "--- $1"; sed -n "1,220p" "$1"' sh {}
printf '%s\n' '--- synchronous I/O calls in target file (AST) ---'
python3 - <<'PY'
import ast
from pathlib import Path
path = Path("scripts/fetch_aic_models.py")
tree = ast.parse(path.read_text())
for node in ast.walk(tree):
if isinstance(node, ast.Call):
callee = ast.unparse(node.func)
if any(term in callee for term in (
"open", "os.path.exists", "os.path.getsize", "os.makedirs",
"download_to_filename", "storage.Client", "adc_default",
)):
print(f"line {node.lineno}: {callee}")
PYRepository: juspay/clairvoyance
Length of output: 29281
Move blocking I/O behind async calls.
Make the entry point asynchronous. Await filesystem operations and google-cloud-storage downloads. Use asyncio.to_thread for the synchronous client methods until an async GCS client is available. Cover open, os.path.exists, os.makedirs, os.path.getsize, blob.download_to_filename, and the ADC lookup.
🧰 Tools
🪛 ast-grep (0.45.1)
[warning] 23-23: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(TOKEN_FILE)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
🤖 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/fetch_aic_models.py` around lines 23 - 24, Make the script entry
point asynchronous and move all blocking filesystem and Google Cloud operations
off the event loop: wrap open, os.path.exists, os.makedirs, os.path.getsize,
blob.download_to_filename, and ADC lookup in awaitable calls, using
asyncio.to_thread for synchronous APIs including Google Cloud Storage downloads.
Update callers to await the async entry point and preserve the existing token
and download behavior.
Source: Coding guidelines
| print("Auth: BuildKit secret gcp_token") | ||
| return Credentials(token=token), None | ||
| try: | ||
| print("Auth: attempting ADC (metadata server)") | ||
| creds, project = adc_default() | ||
| return creds, project | ||
| except Exception as e: | ||
| print(f"Auth: no credentials found ({e})") |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Use structured Loguru logging for build diagnostics.
Replace print calls with contextual Loguru records. In the exception handlers, use logger.opt(exception=e).warning(...) and do not interpolate e into the message string.
As per coding guidelines, “Use Loguru logging with contextvars for structured logging; send traces to Langfuse for observability.” Based on learnings, exception objects must not be interpolated into Loguru messages.
Also applies to: 69-71
🧰 Tools
🪛 Ruff (0.16.1)
[warning] 31-31: Do not catch blind exception: Exception
(BLE001)
🤖 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/fetch_aic_models.py` around lines 25 - 32, Replace the authentication
diagnostic print calls in the credential-loading flow with contextual Loguru
logger calls. In the exception handlers around ADC and related credential
attempts, use logger.opt(exception=e).warning(...) without interpolating e into
the message; preserve the existing diagnostic context in each structured
message.
Sources: Coding guidelines, Learnings
| project = ( | ||
| os.environ.get("GOOGLE_CLOUD_PROJECT") | ||
| or os.environ.get("GCP_PROJECT") | ||
| or detected_project | ||
| ) | ||
| client = storage.Client(credentials=creds, project=project) | ||
|
|
||
| bucket_path = os.environ.get("AIC_BUCKET_PATH", "gs://breeze-clairvoyance-models/aic") | ||
| path = bucket_path.replace("gs://", "", 1) | ||
| parts = path.split("/", 1) | ||
| bucket_name = parts[0] | ||
| prefix = parts[1] if len(parts) > 1 else "" | ||
|
|
||
| models = os.environ.get( | ||
| "AIC_MODELS", | ||
| "quail_l_8khz.aicmodel quail_l_16khz.aicmodel quail_vf_2_1_l_16khz.aicmodel", | ||
| ).split() |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Move model-fetch configuration into static configuration.
These direct os.environ.get calls bypass app/core/config/static.py. Define the project, bucket, and model-list settings in static configuration, then load mandatory settings with get_required_env().
The Docker model-fetch stage must copy the required static configuration module before this script imports it.
As per coding guidelines, “Load ALL configuration from app/core/config/static.py using get_required_env() for mandatory variables; never import directly from os.environ elsewhere.”
🤖 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/fetch_aic_models.py` around lines 41 - 57, Move the
GOOGLE_CLOUD_PROJECT/GCP_PROJECT fallback, AIC_BUCKET_PATH, and AIC_MODELS
definitions into app/core/config/static.py, exposing the required settings
through get_required_env(). Update the model-fetch script to import and use
those static configuration values instead of direct os.environ.get calls, while
preserving existing defaults or mandatory behavior as defined by the
configuration. Update the Docker model-fetch stage to copy the static
configuration module before importing it.
Source: Coding guidelines
| try: | ||
| bucket = client.bucket(bucket_name) | ||
| blob = bucket.blob(blob_path) | ||
| blob.download_to_filename(dest) | ||
| size = os.path.getsize(dest) | ||
| print(f"Fetched {model_name}: {size} bytes") | ||
| except Exception as e: | ||
| print(f"Warning: failed to fetch {model_name}: {e}") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fail the build when authenticated model downloads fail.
Line 70 catches every download error and allows the script to exit successfully. The runtime code then detects the missing artifact and disables the AIC filter. A GCP build with invalid permissions, an invalid bucket, or a missing model can therefore publish an image without the configured noise filter.
Keep the no-credentials AWS exit path. If credentials exist, collect failed model names and exit non-zero after the loop.
🧰 Tools
🪛 Ruff (0.16.1)
[warning] 70-70: Do not catch blind exception: Exception
(BLE001)
🤖 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/fetch_aic_models.py` around lines 64 - 71, Update the model-fetch
loop around the download try/except to collect each failed model name when
credentials are present, while retaining the existing warning output. After all
models are processed, exit non-zero if any downloads failed; preserve the
separate no-credentials AWS exit path.
- Replace google-cloud-sdk install (~845MB unpacked) with plain curl + Bearer token against GCS JSON API (same BuildKit gcp_token contract, curl already installed in image's apt step) - useradd and 'COPY --chown=appuser:appuser' in one step instead of 'chown -R /app' in a later layer (eliminates 1.82GB duplicate layer) - Delete uv cache inside the same layer that fills it (rm -rf /app/.uv-cache after both 'uv sync' and 'nltk.downloader' steps) because layers are additive - 'uv run --no-sync' as CMD: keeps the already-built venv immutable and lets us skip a 'chown -R /app/.venv' without giving up dependency determinism Verified on Cloud Build E2_HIGHCPU_8 + GKE registry: - image 1,705 MB → 857 MB (-50%) - build time unchanged (~5 min) - AIC models downloaded into /app/models/voice/aic (89MB) - /health returns 200; pipecat services import cleanly; subprocess-spawned voice bot (daily.py pattern) boots Co-authored-by: harsh.tiwari <harsh.tiwari@juspay.in>
44ae98e to
cb49323
Compare
cmd-err
changed the title
cmd-err
changed the title
Summary
Minimal-change Dockerfile refactor. Drops registry image size 1,705 MB → 857 MB (-50%) on identical Cloud Build hardware, with zero changes to
pyproject.toml,uv.lock, app code, runtime CMD semantics, or external build contracts.Change surface
One file:
Dockerfile. Nopyproject.toml/uv.lock/.dockerignore/ workflow / app-code changes.What changed
Replace
google-cloud-sdkinstall with plaincurl+ Bearer token. Old code installed the full GCloud SDK (~845 MB unpacked / ~170 MB compressed) just to rungcloud storage cpto pull 3 AIC model files. New code hitshttps://storage.googleapis.com/<bucket>/<path>withAuthorization: Bearer $TOKENwhere the token comes from the samegcp_tokenBuildKit secret.curlis already in the apt layer, so this is a net -845 MB of disk with zero added bytes.Move
useraddbefore the bigCOPY, thenCOPY --chown=appuser:appuser . .. Old code didRUN chown -R appuser:appuser /appafterCOPY . ., which forced Docker to duplicate every byte in/app(1.82 GB unpacked / ~500 MB compressed) into a second layer. New code never duplicates the app dir.Delete the
uvcache inside the same layer that fills it. Two places: afteruv syncand after thenltk.downloaderstep. Layers are additive — removing the cache in a later layer just adds a whiteout while the bytes still ship in the build layer. Net: -861 MB unpacked from theuv synclayer alone.uv run --no-sync python run.pyas CMD. Without--no-sync,uv runre-resolves the project on every container start, tries to reinstall the project wheel + dev dependencies, and fails writing into the root-owned/app/.venv(which is intentionally not chown'd — the venv only needs read+execute). With--no-syncit runs the already-built environment as-is, so we can keep/app/.venvroot-owned and still avoid a 1.8 GBchown -Rlayer.Verified head-to-head on identical Cloud Build (E2_HIGHCPU_8, same source tree)
voice-agent:test-baselinevoice-agent:test-chown-fixedgcloud artifacts ... --include-tagsSIZE/app/models/voice/aic//root/google-cloud-sdk//healthreturns 200Backward compatibility — everything unchanged
gcp_token(same contract as original).gcloud auth print-access-tokenpassed via--secret.AIC_BUCKET_PATHandAIC_MODELSenv/ARG plumbing.CMDshape (uv run python run.py); only added--no-sync. No process supervision change.uv.lockuntouched..dockerignorechange.COPY . .still copies the same context.gcp_tokensecret → models step is a no-op.appuser,/usr/local/nltk_datastill owned byappuser,/app/.uv-cachewritable byappuser.Files
Dockerfile(+49 / -23)Out of scope (intentionally)
build-essentialetc. out of runtime) — needs careful confirmation that no transitive Python package re-compiles at runtime.pipecat[azure],redis[hiredis],pytorch) — unrelated, deferred.Co-authored-by: Claude noreply@anthropic.com