Fix/traefik routing and remote #118
Conversation
Ignore .code and add CI guard
Stabilize workflows and guard .code in PR checks
Implement Gemini API integration
Remove master from workflow filters
Guard against tracked .env and .code
Fix Traefik Docker API version mismatch
Automate VPS deploy via deploy.sh
📝 WalkthroughWalkthroughThis PR introduces Docker containerization, a comprehensive CI/CD pipeline with GitHub Actions workflows, automated VPS deployment via shell scripts, Gemini API integration for assistant chat, health/readiness endpoints for monitoring, and supporting configuration and documentation updates. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant App as AssistantChatSession
participant GeminiClient as gemini_client
participant GeminiAPI as Gemini API<br/>(OpenAI-compatible)
User->>App: send_message("user query")
rect rgba(100, 150, 200, 0.5)
Note over App: Provider check: Gemini configured?
end
App->>GeminiClient: stream_chat(message, system_prompt)
rect rgba(100, 150, 200, 0.5)
Note over GeminiClient: Build AsyncOpenAI client<br/>with API key & base URL
end
GeminiClient->>GeminiAPI: POST /v1beta/openai/chat/completions (stream=true)
GeminiAPI-->>GeminiClient: Streaming completion chunks
rect rgba(100, 150, 200, 0.5)
Note over GeminiClient: Iterate chunks → extract delta.content
end
GeminiClient-->>App: Yield text tokens (async generator)
App->>App: Accumulate full response
App-->>User: Stream response tokens
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In @.github/workflows/pr-check.yml:
- Around line 3-5: The workflow trigger is configured to use the non-existent
branch name "main" (see the on: pull_request: branches: [main] block); update
this across the three workflow files (pr-check.yml, deploy.yml, ci.yml) by
changing branches: [main] to branches: [master] so PR events target the
repository's default branch; ensure you update the same "on: pull_request:"
block in each file and commit the three modified workflow files.
In `@deploy.sh`:
- Around line 165-173: The cron file written by configure_duckdns contains the
DUCKDNS_TOKEN and is created world-readable (0644); change the file creation and
permissions so only root can read it: after writing the cron_file in
configure_duckdns, set owner to root:root and restrict mode to 0600 (or use
install/umask to create it with 0600) to prevent other local users from reading
DUCKDNS_TOKEN; reference symbols: configure_duckdns, cron_file, DUCKDNS_TOKEN.
In `@scripts/deploy.sh`:
- Around line 74-83: The DuckDNS update URL in configure_duckdns uses $DOMAIN
which may be a full hostname but DuckDNS expects only the subdomain; update
configure_duckdns to extract the subdomain before building the URL (e.g., strip
suffix like ".duckdns.org" from DOMAIN) and use that extracted variable in both
the cron entry and the immediate curl, referencing the configure_duckdns
function and variables DOMAIN and DUCKDNS_TOKEN; alternatively, update the user
prompt where DOMAIN is set to require just the subdomain and ensure any Traefik
logic derives the full domain from that subdomain consistently.
- Around line 13-25: The export in prompt_required() uses export "$var_name"
which exports the literal string instead of the named variable; remove the
ineffective export or change it to export "${var_name?}" (or use declare -x
"$var_name") after printf -v sets the variable so the actual variable named by
var_name is exported; update prompt_required to either omit the export (since
printf -v already sets the variable) or replace export "$var_name" with export
"${var_name?}" / declare -x "$var_name" to correctly export the named variable.
In `@server/gemini_client.py`:
- Around line 37-80: The streaming handler in stream_chat incorrectly assumes
delta.content is a list and iterates strings as sequences; update the delta
handling in the async for chunk loop (inside stream_chat) to detect the type of
delta.content and handle each correctly: if delta.content is a str yield it as a
single chunk; if it is a list iterate parts and extract part.text or
part.get("text") when a dict (or use getattr for objects); if it's a dict
extract its "text" similarly; ensure you avoid iterating over raw strings so you
don't yield single characters.
In `@server/main.py`:
- Around line 119-136: There are two inconsistent health endpoints—health()
returning {"status": "ok"} and the /api/health handler returning {"status":
"healthy"}—so reconcile them: decide whether /health is an infra liveness probe
and /api/health is an application-level readiness check or consolidate both to
the same semantic and payload; then update the handlers (health(), readiness(),
and the /api/health handler) to return a consistent status string/shape and add
a brief docstring to each explaining their distinct purpose (or merge routes so
only one handler serves both paths) so consumers receive a single,
well-documented contract.
🧹 Nitpick comments (12)
.gitignore (1)
9-23: Duplicate.code/entry in .gitignore.The
.code/directory is listed twice: at line 10 under "Repository-specific" and at line 23 under "Local Codex/Claude configuration". Consider consolidating into a single entry to avoid confusion.Suggested consolidation
-# Repository-specific -.code/ - # Browser profiles for parallel agent execution .browser-profiles/Keep only the more descriptive entry at lines 22-23.
Dockerfile (1)
12-25: Consider running the container as a non-root user for improved security.The runtime stage runs as root by default. For production deployments, consider adding a non-root user to reduce the attack surface if the container is compromised.
Suggested non-root user setup
FROM python:3.11-slim AS runtime ENV PYTHONUNBUFFERED=1 \ PYTHONDONTWRITEBYTECODE=1 WORKDIR /app + +# Create non-root user +RUN useradd --create-home --shell /bin/bash appuser + COPY requirements.txt . RUN pip install --no-cache-dir -r requirements.txt # Copy source code and built UI COPY . . COPY --from=ui-builder /app/ui/dist ./ui/dist +# Change ownership and switch to non-root user +RUN chown -R appuser:appuser /app +USER appuser + EXPOSE 8888 CMD ["uvicorn", "server.main:app", "--host", "0.0.0.0", "--port", "8888"]Note: If the application requires writing to
/root/.autocoder(as indicated by the volume mount in docker-compose.yml), you'll need to adjust the volume path accordingly.docker-compose.yml (2)
1-1: Theversionkey is obsolete in Docker Compose.The
versionfield is deprecated in the Compose Specification and is ignored by recent versions of Docker Compose. While harmless, it can be removed for cleanliness.Remove deprecated version key
-version: "3.9" - services:
21-21: Redundant command override.The
commandduplicates theCMDin the Dockerfile. This is harmless but can be removed unless you need the flexibility to override it differently in other compose files..github/workflows/pr-check.yml (1)
38-52: Consider adding type checking to the Python job.The workflow runs
ruff checkfor linting but the project includesmypy>=1.13.0in dev dependencies. Consider adding mypy for static type checking.Add mypy type checking
- name: Lint with ruff run: ruff check . + - name: Type check with mypy + run: mypy server/ - name: Run security tests run: python test_security.pyrequirements.txt (1)
13-13: Version 1.52.0 is valid; upper bound suggestion is overly restrictive.The
openai>=1.52.0dependency is confirmed to exist and has no known security vulnerabilities. However, the suggested upper bound<2.0.0is unnecessarily restrictive since version 2.15.0 is the current stable release. If version constraints are desired for consistency withclaude-agent-sdk<0.2.0andapscheduler<4.0.0, useopenai>=1.52.0,<3.0.0instead, or verify that version 2.x is compatible before removing the upper bound entirely.docker-compose.traefik.yml (2)
1-1: Consider removing the deprecatedversionfield.Docker Compose V2 ignores the
versionfield. Removing it avoids confusion and aligns with current best practices.♻️ Suggested change
-version: "3.9" - services:
5-11: Consider pinning the Traefik version and makingDOCKER_API_VERSIONconfigurable.Using
traefik:latestmay introduce breaking changes unexpectedly. Pinning to a specific version (e.g.,traefik:v3.2) improves reproducibility.The hardcoded
DOCKER_API_VERSION=1.53may cause issues on systems with older Docker daemons. Consider using a variable with a sensible default.♻️ Suggested change
traefik: - image: traefik:latest + image: traefik:v3.2 environment: # Force a modern Docker API version. Some VPS environments set # DOCKER_API_VERSION=1.24 globally, which breaks Traefik's Docker provider # when the daemon requires >= 1.44. # Use the server's current API version to avoid a too-old client default. - - DOCKER_API_VERSION=1.53 + - DOCKER_API_VERSION=${DOCKER_API_VERSION:-1.53}.github/workflows/ci.yml (1)
40-43: Consider running the new health endpoint tests in CI.The
pythonjob runstest_security.pybut nottest_health.py. Consider adding pytest or running the new health tests to ensure endpoint regressions are caught.♻️ Suggested addition
- name: Run security tests run: python test_security.py + - name: Run health tests + run: python -m pytest test_health.py -vThis would require adding
pytesttorequirements.txtif not already present.deploy.sh (1)
92-141: Consider defaultingDEPLOY_BRANCHto the repo’s default branch.
If the repo doesn’t have amainbranch, the default path fails. Consider deriving from remote HEAD after clone or aligning the default with the repo’s primary branch..github/workflows/deploy.yml (2)
33-40: Consider pinning the action to a commit SHA for enhanced supply chain security.While
v1.2.4is a pinned version, tags can be moved. For critical deployment workflows, pinning to a specific commit SHA provides stronger guarantees against supply chain attacks.🔒 Suggested improvement
- uses: appleboy/ssh-action@v1.2.4 + uses: appleboy/ssh-action@<full-commit-sha> # v1.2.4You can find the commit SHA for v1.2.4 on the action's releases page.
90-113: Health check implementation is solid.The retry logic with 12 attempts and 5-second intervals provides a reasonable 60-second window for each endpoint to become available. The
curlflags (-fsS --max-time 5) are appropriate for health checks.Consider extracting the retry logic into a shell function to reduce duplication:
♻️ Optional: Extract retry logic into a function
+ wait_for_endpoint() { + local endpoint=$1 + local retries=12 + until curl -fsS --max-time 5 "http://127.0.0.1:${APP_PORT}${endpoint}" >/dev/null; do + retries=$((retries - 1)) + if [ "$retries" -le 0 ]; then + echo "${endpoint} check failed after retries." + exit 1 + fi + echo "Waiting for ${endpoint}... ($retries retries left)" + sleep 5 + done + } + echo "Running smoke test on http://127.0.0.1:${APP_PORT}/health and /readiness ..." - retries=12 - until curl -fsS --max-time 5 "http://127.0.0.1:${APP_PORT}/health" >/dev/null; do - retries=$((retries - 1)) - if [ "$retries" -le 0 ]; then - echo "Health check failed after retries." - exit 1 - fi - echo "Waiting for health... ($retries retries left)" - sleep 5 - done - - retries=12 - until curl -fsS --max-time 5 "http://127.0.0.1:${APP_PORT}/readiness" >/dev/null; do - retries=$((retries - 1)) - if [ "$retries" -le 0 ]; then - echo "Readiness check failed after retries." - exit 1 - fi - echo "Waiting for readiness... ($retries retries left)" - sleep 5 - done + wait_for_endpoint "/health" + wait_for_endpoint "/readiness"
| on: | ||
| pull_request: | ||
| branches: [main] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the default branch and workflow branch triggers
echo "=== Default branch ==="
git remote show origin 2>/dev/null | grep 'HEAD branch' || git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's|refs/remotes/origin/||' || echo "Could not determine default branch"
echo ""
echo "=== Branches in the repository ==="
git branch -r | head -20
echo ""
echo "=== Branch triggers in all workflows ==="
rg -n "branches:" .github/workflows/ --type yaml || echo "No branch triggers found"
echo ""
echo "=== Content of pr-check.yml (first 20 lines) ==="
head -20 .github/workflows/pr-check.ymlRepository: leonvanzyl/autocoder
Length of output: 844
Critical: All workflows configured to trigger on non-existent main branch, but repository uses master.
The repository's default branch is master, but all three workflows (pr-check.yml, deploy.yml, ci.yml) are configured to trigger on branches: [main], which doesn't exist. These workflows won't run for any pull requests.
Update all workflow files to trigger on master:
Fix all workflows
on:
pull_request:
- branches: [main]
+ branches: [master]Apply this change to .github/workflows/pr-check.yml, .github/workflows/deploy.yml, and .github/workflows/ci.yml.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| on: | |
| pull_request: | |
| branches: [main] | |
| on: | |
| pull_request: | |
| branches: [master] |
🤖 Prompt for AI Agents
In @.github/workflows/pr-check.yml around lines 3 - 5, The workflow trigger is
configured to use the non-existent branch name "main" (see the on: pull_request:
branches: [main] block); update this across the three workflow files
(pr-check.yml, deploy.yml, ci.yml) by changing branches: [main] to branches:
[master] so PR events target the repository's default branch; ensure you update
the same "on: pull_request:" block in each file and commit the three modified
workflow files.
| configure_duckdns() { | ||
| echo | ||
| echo "==> Configuring DuckDNS..." | ||
| local cron_file="/etc/cron.d/duckdns" | ||
| cat > "${cron_file}" <<EOF | ||
| */5 * * * * root curl -fsS "https://www.duckdns.org/update?domains=${DUCKDNS_SUBDOMAIN}&token=${DUCKDNS_TOKEN}&ip=" >/var/log/duckdns.log 2>&1 | ||
| EOF | ||
| chmod 644 "${cron_file}" | ||
|
|
There was a problem hiding this comment.
Protect DuckDNS token from world-readable cron file.
The token is written to a 0644 cron file, making it readable by any local user. Restrict it to root to avoid leakage.
🔒 Proposed fix
- chmod 644 "${cron_file}"
+ chmod 600 "${cron_file}"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| configure_duckdns() { | |
| echo | |
| echo "==> Configuring DuckDNS..." | |
| local cron_file="/etc/cron.d/duckdns" | |
| cat > "${cron_file}" <<EOF | |
| */5 * * * * root curl -fsS "https://www.duckdns.org/update?domains=${DUCKDNS_SUBDOMAIN}&token=${DUCKDNS_TOKEN}&ip=" >/var/log/duckdns.log 2>&1 | |
| EOF | |
| chmod 644 "${cron_file}" | |
| configure_duckdns() { | |
| echo | |
| echo "==> Configuring DuckDNS..." | |
| local cron_file="/etc/cron.d/duckdns" | |
| cat > "${cron_file}" <<EOF | |
| */5 * * * * root curl -fsS "https://www.duckdns.org/update?domains=${DUCKDNS_SUBDOMAIN}&token=${DUCKDNS_TOKEN}&ip=" >/var/log/duckdns.log 2>&1 | |
| EOF | |
| chmod 600 "${cron_file}" | |
| } |
🤖 Prompt for AI Agents
In `@deploy.sh` around lines 165 - 173, The cron file written by configure_duckdns
contains the DUCKDNS_TOKEN and is created world-readable (0644); change the file
creation and permissions so only root can read it: after writing the cron_file
in configure_duckdns, set owner to root:root and restrict mode to 0600 (or use
install/umask to create it with 0600) to prevent other local users from reading
DUCKDNS_TOKEN; reference symbols: configure_duckdns, cron_file, DUCKDNS_TOKEN.
| prompt_required() { | ||
| local var_name="$1" prompt_msg="$2" | ||
| local value | ||
| while true; do | ||
| read -r -p "$prompt_msg: " value | ||
| if [[ -n "$value" ]]; then | ||
| printf -v "$var_name" '%s' "$value" | ||
| export "$var_name" | ||
| return | ||
| fi | ||
| echo "Value cannot be empty." | ||
| done | ||
| } |
There was a problem hiding this comment.
Fix the ineffective export statement (SC2163).
The export "$var_name" on line 20 exports the literal string value of $var_name, not the variable it names. The printf -v already sets the variable correctly, but if you need it exported, use export "${var_name?}" or declare it beforehand.
🔧 Suggested fix
prompt_required() {
local var_name="$1" prompt_msg="$2"
local value
while true; do
read -r -p "$prompt_msg: " value
if [[ -n "$value" ]]; then
printf -v "$var_name" '%s' "$value"
- export "$var_name"
+ export "${var_name?}"
return
fi
echo "Value cannot be empty."
done
}🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 20-20: This does not export 'var_name'. Remove
(SC2163)
🤖 Prompt for AI Agents
In `@scripts/deploy.sh` around lines 13 - 25, The export in prompt_required() uses
export "$var_name" which exports the literal string instead of the named
variable; remove the ineffective export or change it to export "${var_name?}"
(or use declare -x "$var_name") after printf -v sets the variable so the actual
variable named by var_name is exported; update prompt_required to either omit
the export (since printf -v already sets the variable) or replace export
"$var_name" with export "${var_name?}" / declare -x "$var_name" to correctly
export the named variable.
| configure_duckdns() { | ||
| echo "Configuring DuckDNS..." | ||
| local cron_file="/etc/cron.d/duckdns" | ||
| cat > "$cron_file" <<EOF | ||
| */5 * * * * root curl -fsS "https://www.duckdns.org/update?domains=$DOMAIN&token=$DUCKDNS_TOKEN&ip=" >/var/log/duckdns.log 2>&1 | ||
| EOF | ||
| chmod 644 "$cron_file" | ||
| # Run once immediately | ||
| curl -fsS "https://www.duckdns.org/update?domains=$DOMAIN&token=$DUCKDNS_TOKEN&ip=" >/var/log/duckdns.log 2>&1 || true | ||
| } |
There was a problem hiding this comment.
DuckDNS API expects the subdomain only, not the full domain.
The DuckDNS update URL on line 78 uses $DOMAIN directly, but the API expects just the subdomain (e.g., myapp), not the full domain (e.g., myapp.duckdns.org). If users enter the full domain as prompted, the update will fail.
🔧 Suggested fix - extract subdomain
configure_duckdns() {
echo "Configuring DuckDNS..."
+ # Extract subdomain from full domain (myapp.duckdns.org -> myapp)
+ local subdomain="${DOMAIN%%.*}"
local cron_file="/etc/cron.d/duckdns"
cat > "$cron_file" <<EOF
-*/5 * * * * root curl -fsS "https://www.duckdns.org/update?domains=$DOMAIN&token=$DUCKDNS_TOKEN&ip=" >/var/log/duckdns.log 2>&1
+*/5 * * * * root curl -fsS "https://www.duckdns.org/update?domains=$subdomain&token=$DUCKDNS_TOKEN&ip=" >/var/log/duckdns.log 2>&1
EOF
chmod 644 "$cron_file"
# Run once immediately
- curl -fsS "https://www.duckdns.org/update?domains=$DOMAIN&token=$DUCKDNS_TOKEN&ip=" >/var/log/duckdns.log 2>&1 || true
+ curl -fsS "https://www.duckdns.org/update?domains=$subdomain&token=$DUCKDNS_TOKEN&ip=" >/var/log/duckdns.log 2>&1 || true
}Alternatively, update the prompt on line 29 to clarify that users should enter the subdomain only, and derive the full domain for Traefik.
🤖 Prompt for AI Agents
In `@scripts/deploy.sh` around lines 74 - 83, The DuckDNS update URL in
configure_duckdns uses $DOMAIN which may be a full hostname but DuckDNS expects
only the subdomain; update configure_duckdns to extract the subdomain before
building the URL (e.g., strip suffix like ".duckdns.org" from DOMAIN) and use
that extracted variable in both the cron entry and the immediate curl,
referencing the configure_duckdns function and variables DOMAIN and
DUCKDNS_TOKEN; alternatively, update the user prompt where DOMAIN is set to
require just the subdomain and ensure any Traefik logic derives the full domain
from that subdomain consistently.
| async def stream_chat( | ||
| user_message: str, | ||
| *, | ||
| system_prompt: Optional[str] = None, | ||
| model: Optional[str] = None, | ||
| extra_messages: Optional[Iterable[dict]] = None, | ||
| ) -> AsyncGenerator[str, None]: | ||
| """ | ||
| Stream a chat completion from Gemini. | ||
|
|
||
| Args: | ||
| user_message: Primary user input | ||
| system_prompt: Optional system prompt to prepend | ||
| model: Optional model name; defaults to GEMINI_MODEL env or fallback constant | ||
| extra_messages: Optional prior messages (list of {"role","content"}) | ||
| Yields: | ||
| Text chunks as they arrive. | ||
| """ | ||
| client = _build_client() | ||
| messages = [] | ||
|
|
||
| if system_prompt: | ||
| messages.append({"role": "system", "content": system_prompt}) | ||
|
|
||
| if extra_messages: | ||
| messages.extend(extra_messages) | ||
|
|
||
| messages.append({"role": "user", "content": user_message}) | ||
|
|
||
| completion = await client.chat.completions.create( | ||
| model=model or DEFAULT_GEMINI_MODEL, | ||
| messages=messages, | ||
| stream=True, | ||
| ) | ||
|
|
||
| async for chunk in completion: | ||
| for choice in chunk.choices: | ||
| delta = choice.delta | ||
| if delta and delta.content: | ||
| # delta.content is a list of content parts | ||
| for part in delta.content: | ||
| text = getattr(part, "text", None) or part.get("text") if isinstance(part, dict) else None | ||
| if text: | ||
| yield text |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, examine the actual file to understand imports and context
head -40 server/gemini_client.pyRepository: leonvanzyl/autocoder
Length of output: 1243
🏁 Script executed:
# Check for any requirements or setup files that indicate SDK versions
find . -name "requirements*.txt" -o -name "pyproject.toml" -o -name "setup.py" | head -5Repository: leonvanzyl/autocoder
Length of output: 99
🏁 Script executed:
# Search for how delta.content is used elsewhere in the codebase
rg "delta\.content" --type py -B 2 -A 2Repository: leonvanzyl/autocoder
Length of output: 583
🌐 Web query:
OpenAI Python SDK 1.52+ chat completions streaming delta.content format
💡 Result:
In openai-python SDK 1.52+, client.chat.completions.create(..., stream=True) yields ChatCompletionChunk objects. Each chunk has:
chunk.choices[0].delta(a “delta” object), which may include:delta.role(typically appears once at the start)delta.content(string text increment, sometimes""/Noneon some chunks)delta.tool_calls(when the model is calling tools; in these chunksdelta.contentis often absent)delta.refusal(if refusing)
These fields are documented as part of the streamed “chat completion chunk” schema. [1]
Typical usage (text streaming):
stream = client.chat.completions.create(..., stream=True)
for chunk in stream:
delta = chunk.choices[0].delta
if delta.content:
print(delta.content, end="")This exact chunk.choices[0].delta.content access pattern is also shown in OpenAI’s streaming example. [2]
Notes you’ll see in real streams:
- The first chunk often includes
delta.role="assistant"anddelta.content="". [1] - If you set
stream_options={"include_usage": true}, the last chunk may havechoices=[](empty). [1]
If you meant the newer “Responses API” streaming: it does not use delta.content; you’ll instead receive events like response.output_text.delta with a delta string payload. [3]
Sources: [1] Chat Completions streaming chunk schema, [2] OpenAI Cookbook streaming example, [3] Responses streaming guide.
🌐 Web query:
Google Gemini OpenAI-compatible API endpoint delta.content structure
💡 Result:
Endpoint (Gemini API “OpenAI compatibility”)
Use the OpenAI Chat Completions path under the /openai/ prefix:
- Base URL:
https://generativelanguage.googleapis.com/v1beta/openai/ - Chat Completions:
POST https://generativelanguage.googleapis.com/v1beta/openai/chat/completions(ai.google.dev)
Streaming delta.content structure
In streaming mode ("stream": true / stream=True), Gemini returns OpenAI-style chat completion chunks where each SSE event contains (conceptually):
{
"choices": [
{
"delta": {
"content": "…partial text…"
}
}
]
}choices[].delta.contentis a string containing the next incremental piece of the assistant message (and can be omitted/null on chunks that carry other fields). (platform.openai.com)- The docs’ streaming examples access it exactly like OpenAI:
chunk.choices[0].delta/chunk.choices[0].delta.content. (ai.google.dev)
Known compatibility quirks (worth handling defensively)
rolemay be repeated in later streameddeltachunks (not typical in OpenAI streaming), per a reported/acknowledged issue. (discuss.ai.google.dev)- For tool calling + streaming, Gemini has been reported to emit
delta.tool_calls[]entries without the innertool_call.indexfield (some OpenAI parsers expect it). (discuss.ai.google.dev)
Citations:
- 1: https://ai.google.dev/gemini-api/docs/openai
- 2: https://platform.openai.com/docs/api-reference/chat-streaming?utm_source=openai
- 3: https://ai.google.dev/gemini-api/docs/openai
- 4: https://discuss.ai.google.dev/t/openai-compatible-endpoint-is-producing-corrupted-response-when-streaming/77205?utm_source=openai
- 5: https://discuss.ai.google.dev/t/gemini-openai-compatibility-issue-with-tool-call-streaming/59886?utm_source=openai
Handle string delta.content in streaming responses.
OpenAI Python SDK 1.52+ and Gemini's OpenAI-compatible endpoint return delta.content as a string, not a list. The current code iterates through it as a list, yielding individual characters instead of proper text chunks—a major inefficiency that defeats the purpose of streaming.
Proposed fix
async for chunk in completion:
for choice in chunk.choices:
delta = choice.delta
if delta and delta.content:
- # delta.content is a list of content parts
- for part in delta.content:
- text = getattr(part, "text", None) or part.get("text") if isinstance(part, dict) else None
- if text:
- yield text
+ if isinstance(delta.content, str):
+ yield delta.content
+ else:
+ # Fallback: handle list case for other endpoints
+ for part in delta.content:
+ text = getattr(part, "text", None) or (part.get("text") if isinstance(part, dict) else None)
+ if text:
+ yield text🤖 Prompt for AI Agents
In `@server/gemini_client.py` around lines 37 - 80, The streaming handler in
stream_chat incorrectly assumes delta.content is a list and iterates strings as
sequences; update the delta handling in the async for chunk loop (inside
stream_chat) to detect the type of delta.content and handle each correctly: if
delta.content is a str yield it as a single chunk; if it is a list iterate parts
and extract part.text or part.get("text") when a dict (or use getattr for
objects); if it's a dict extract its "text" similarly; ensure you avoid
iterating over raw strings so you don't yield single characters.
| # ============================================================================ | ||
| # Health Endpoint | ||
| # ============================================================================ | ||
|
|
||
| @app.get("/health") | ||
| async def health(): | ||
| """Lightweight liveness probe used by deploy smoke tests.""" | ||
| return {"status": "ok"} | ||
|
|
||
|
|
||
| @app.get("/readiness") | ||
| async def readiness(): | ||
| """ | ||
| Readiness probe placeholder. | ||
|
|
||
| Add dependency checks (DB, external APIs, queues) here when introduced. | ||
| """ | ||
| return {"status": "ready"} |
There was a problem hiding this comment.
Clarify the relationship between /health and /api/health endpoints.
There are now two health endpoints:
/health(Line 123) returns{"status": "ok"}/api/health(Line 187) returns{"status": "healthy"}
This inconsistency may confuse consumers. Consider consolidating them or documenting their distinct purposes (e.g., /health for infrastructure probes vs /api/health for application-level checks).
🤖 Prompt for AI Agents
In `@server/main.py` around lines 119 - 136, There are two inconsistent health
endpoints—health() returning {"status": "ok"} and the /api/health handler
returning {"status": "healthy"}—so reconcile them: decide whether /health is an
infra liveness probe and /api/health is an application-level readiness check or
consolidate both to the same semantic and payload; then update the handlers
(health(), readiness(), and the /api/health handler) to return a consistent
status string/shape and add a brief docstring to each explaining their distinct
purpose (or merge routes so only one handler serves both paths) so consumers
receive a single, well-documented contract.
- Fix workflows to trigger on master branch instead of main - Protect DuckDNS cron file with chmod 600 (security fix) - Fix gemini_client.py to handle string delta.content in streaming - Fix scripts/deploy.sh to extract DuckDNS subdomain properly
Create test_persistence_after_kernel_run() proving that after one HarnessKernel run, the DB contains AgentSpec, AgentRun, and AgentEvent records with correct foreign keys and ascending event sequences. - Creates AgentSpec and persists to in-memory SQLite - Executes via HarnessKernel with mocked turn_executor (2 turns) - Queries and verifies AgentSpec, AgentRun (FK), AgentEvent (FK + ordering) - All 43 tests in test_dspy_pipeline_e2e.py pass - Marked feature leonvanzyl#118 as passing Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
PR Review: Docker Deployment & Gemini IntegrationHey @heidi-dang! 👋 Thanks for this comprehensive PR - there's a lot of valuable work here including Docker containerization, CI/CD pipelines, VPS deployment automation, and Gemini API integration. Given the scope (1,178 additions across 20 files), I've done a thorough review and found a few issues that should be addressed before merging. 🚨 Critical Issues (Must Fix)1. Gemini Streaming Bug ( The code treats # Current (broken)
for part in delta.content:
text = getattr(part, "text", None) or ...
# Should be
if delta and delta.content:
yield delta.content # It's already a stringThis will cause character-by-character output or a TypeError at runtime. 2. CI Branch Mismatch ( The workflows target
Fix: Change to 3. DuckDNS Token Exposure ( The cron file is created with 644 permissions (world-readable), exposing the DuckDNS token to any user on the system: chmod 644 "${cron_file}" # Should be 600
|
| Issue | Suggestion |
|---|---|
| Duplicate health endpoints | /health returns "ok", /api/health returns "healthy" - consider consolidating |
| Duplicate deploy scripts | deploy.sh (371 lines) vs scripts/deploy.sh (133 lines) - consider removing the simpler one |
| No Gemini tests | test_health.py only has 2 tests - consider adding coverage for the new Gemini client |
| TypeScript breaking change | gemini: boolean should be optional (gemini?: boolean) for backwards compatibility |
| Hardcoded repo URL | deploy.sh defaults to heidi-dang/autocoder.git instead of the canonical repo |
| Pin Traefik version | Using traefik:latest may cause unexpected updates - consider pinning (e.g., traefik:v3.0) |
Summary
This is a substantial infrastructure improvement, but the critical issues above need to be addressed first. The streaming bug in particular will cause the Gemini chat feature to malfunction.
Happy to re-review once the critical items are fixed! Let me know if you have any questions about the feedback. 🙂
- Fix workflows to trigger on master branch instead of main - Protect DuckDNS cron file with chmod 600 (security fix) - Fix gemini_client.py to handle string delta.content in streaming - Fix scripts/deploy.sh to extract DuckDNS subdomain properly
2 participants
Summary by CodeRabbit
Release Notes
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.