Conversation
|
|
||
| critic_model: str = os.getenv("MODEL_NAME") | ||
| worker_model: str = os.getenv("MODEL_NAME") | ||
| critic_model: str = os.getenv("MODEL_NAME", "gemini-3.5-flash") |
There was a problem hiding this comment.
Avoid using hardcoded defaults on environment variable reads. There are two instances in this file; default values should be documented in .env.example instead.
|
|
||
| critic_model: str = os.getenv("MODEL_NAME") | ||
| worker_model: str = os.getenv("MODEL_NAME") | ||
| critic_model: str = os.getenv("MODEL_NAME", "gemini-3.5-flash") |
There was a problem hiding this comment.
Avoid hardcoding a default model name in the environment read. The model name should be loaded from the environment variable, and the default belongs in .env.example.
| critic_model: str = os.getenv("MODEL_NAME") | ||
| worker_model: str = os.getenv("MODEL_NAME") | ||
| critic_model: str = os.getenv("MODEL_NAME", "gemini-3.5-flash") | ||
| worker_model: str = os.getenv("MODEL_NAME", "gemini-3.5-flash") |
There was a problem hiding this comment.
Avoid hardcoding a default model name in the environment read. The model name should be loaded from the environment variable, and the default belongs in .env.example.
|
|
||
| critic_model: str = os.getenv("MODEL_NAME") | ||
| worker_model: str = os.getenv("MODEL_NAME") | ||
| critic_model: str = os.getenv("MODEL_NAME", "gemini-3.5-flash") |
There was a problem hiding this comment.
Avoid using hardcoded defaults for environment reads in source files. Document the default values in .env.example instead (there are 2 instances in this file).
| ```python | ||
| from pathlib import Path | ||
| from fastapi.staticfiles import StaticFiles | ||
| from fastapi.responses import FileResponse |
There was a problem hiding this comment.
The import of FileResponse is unused in this code snippet. It should be removed to keep the example clean.
| RUN npm run build | ||
|
|
||
| # Stage 2: Backend + bundled frontend | ||
| FROM python:3.11-slim |
There was a problem hiding this comment.
The Dockerfile example for the final stage lacks a USER directive, meaning the container runs as root by default. Add a USER instruction to run the container as a non-privileged user.
|
There is a problem with the Antigravity CLI Maintainability PR review. Please check the action logs for details. |
| WORKDIR /app | ||
| COPY --from=ghcr.io/astral-sh/uv:latest /uv /uvx /bin/ | ||
| COPY pyproject.toml uv.lock ./ | ||
| RUN uv sync --frozen --no-dev |
There was a problem hiding this comment.
Running uv sync before copying the source code will fail because uv expects the package source to be present to install the project by default. Add the --no-install-project flag to only sync dependencies first, or copy the source code before running the sync.
|
|
||
| frontend_dist = Path("frontend/dist") | ||
| if frontend_dist.exists(): | ||
| app.mount("/app", StaticFiles(directory=str(frontend_dist), html=True), name="frontend") |
There was a problem hiding this comment.
Mounting the frontend at /app will cause asset loading failures (404 errors for /assets/...) unless the Vite build is specifically configured with base: "/app/". Mount the static files at the root / to serve the frontend as the main interface while keeping /api/* routed to ADK.
There was a problem hiding this comment.
Automated House Rules review — 0 finding(s).
Also, on lines this PR does not change:
core/python/deep-search/frontend/vite.config.ts:1— [MINOR] no .ts file in this recipe carries the standard Apache header (0 have a shorter notice, 3 have none) (core/python/deep-search/frontend/vite.config.ts; core/python/deep-search/frontend/src/vite-env.d.ts; core/python/deep-search/frontend/src/utils.ts)
There was a problem hiding this comment.
Automated Security review — 0 finding(s).
.agents/skills/github-pr-review/scripts/existing_comments.py.agents/skills/github-pr-review/scripts/plan_review.py.agents/skills/github-pr-review/scripts/post_comments.py.agents/skills/github-pr-review/scripts/rejections.py.agents/skills/github-pr-review/scripts/verify_findings.py.agents/skills/github-pr-review/tests/conftest.py.agents/skills/github-pr-review/tests/fixtures/pr2373_outcomes.json.agents/skills/github-pr-review/tests/test_build_report.py.agents/skills/github-pr-review/tests/test_check_house_rules.py.agents/skills/github-pr-review/tests/test_house_rules_drift.py.agents/skills/github-pr-review/tests/test_plan_review.py.agents/skills/github-pr-review/tests/test_post_comments.py.agents/skills/github-pr-review/tests/test_regression_pr2373.py.agents/skills/github-pr-review/tests/test_rejections.py.agents/skills/github-pr-review/tests/test_verify_findings.py- …and 18 more
Splitting the PR up would get them reviewed.
Round 4 · 8 of this PR's 25 automated comments used · this round is capped at 1 across all reviewers · only Security and Correctness still run.
happyhuman
left a comment
There was a problem hiding this comment.
Please take a look at a few comments left by the bots, and address he failed workflow (all should be minor things).
What
Fixes #2575. Updates
core/python/deep-search/README.mdto resolve broken scaffolding and cloud deployment instructions, clarifies backend container vs custom React UI serving architecture, and removes unused imports in documentation code snippets.Why
agents-cli create -a adk@deep-searchfails withFileNotFoundErrorbecause the shorthand points to the legacy pathpython/agents/deep-search, which was removed in Delete old recipes which already have been copied to core or contrib #2618 when recipes moved tocore/.adk-samples/python/agents/deep-search.make deploy IAP=trueandmake deploy IAP=true PORT=5173).google/agents-clifor a non-existent "Deploy UI Guide", leaving users confused about how the React frontend communicates with the agent backend.How it works
Scaffold & Clone Commands (
core/python/deep-search/README.md):adk@deep-searchwith the full URL:https://github.com/google/adk-samples/tree/main/core/python/deep-search.adk-samples/python/agents/deep-searchtoadk-samples/core/python/deep-search.Cloud Deployment (
core/python/deep-search/README.md):make deploytargets withagents-cli deploy --iapandagents-cli deploy.Dockerfileshowing how to build the Vite/React frontend and bundle it with the FastAPI backend.app/fast_api_app.py.Testing
Verified locally with full test and validation suites: