chore: subdirectory routing, configurable domain, and deployment guidelines (PR #241 fixes)#242
chore: subdirectory routing, configurable domain, and deployment guidelines (PR #241 fixes)#242sheepdestroyer wants to merge 18 commits into
Conversation
…tions in AGENTS.md
…vironment variable
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
… single x570.vendeuvre.lan domain
…leak / TLS bypass
…up false failures
…al_host/port resolution, fix agy startup SSH commands
…uccess endpoints, nohup stdin redirect)
Reviewer's GuideImplements secure, domain-aware subdirectory routing for the dashboard and proxy, makes URLs/envs configurable for production behind HAProxy, syncs production pod/deployment wiring (paths, probes, qwen model, roster), and adds a concrete deployment checklist for the Sequence diagram for secure domain-aware dashboard routingsequenceDiagram
actor User
participant RouterApp
participant get_dashboard
participant LangfuseUI
participant LiteLLMAdmin
participant LlamaServer
User->>RouterApp: GET /llm-routing/dashboard
RouterApp->>get_dashboard: get_dashboard(request)
alt [external_host in ROUTING_DOMAIN]
get_dashboard-->>User: HTML with https://external_netloc/llm-routing/langfuse
get_dashboard-->>User: HTML with https://external_netloc/llm-routing/litellm/ui
get_dashboard-->>User: HTML with https://external_netloc/llm-routing/llama/
else [base_url in ROUTING_DOMAIN]
get_dashboard-->>User: HTML with {scheme}://{netloc}/llm-routing/langfuse
get_dashboard-->>User: HTML with {scheme}://{netloc}/llm-routing/litellm/ui
get_dashboard-->>User: HTML with {scheme}://{netloc}/llm-routing/llama/
else [untrusted domain]
get_dashboard-->>User: HTML with http://external_host:3001
get_dashboard-->>User: HTML with http://external_host:4000/ui
get_dashboard-->>User: HTML with http://external_host:8080
end
User->>LangfuseUI: Open langfuse_url link
User->>LiteLLMAdmin: Open litellm_url link
User->>LlamaServer: Open llama_url link
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Review limit reached
Next review available in: 23 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 (8)
✨ 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.
Hey - I've found 1 issue, and left some high level feedback:
- The logic for deriving base/public URLs and subdirectory paths is now spread across
start-stack.sh,router/main.pyandpod.yaml(e.g./llm-routing,/llm-routing/litellm); consider centralizing this into a single config/env convention so that changing the base path or domain only requires updating it in one place. - The updated
pod.yamlhardcodes host paths and user-specific IDs for theboyenvironment (e.g./mnt/DATA/boy/...,1002); if this pod needs to be reused in other environments, consider parameterizing these values (similar to the existing placeholders) rather than baking them directly into the manifest.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The logic for deriving base/public URLs and subdirectory paths is now spread across `start-stack.sh`, `router/main.py` and `pod.yaml` (e.g. `/llm-routing`, `/llm-routing/litellm`); consider centralizing this into a single config/env convention so that changing the base path or domain only requires updating it in one place.
- The updated `pod.yaml` hardcodes host paths and user-specific IDs for the `boy` environment (e.g. `/mnt/DATA/boy/...`, `1002`); if this pod needs to be reused in other environments, consider parameterizing these values (similar to the existing placeholders) rather than baking them directly into the manifest.
## Individual Comments
### Comment 1
<location path="start-stack.sh" line_range="612-616" />
<code_context>
echo "🎉 SUCCESS: LLM Triage Gateway restarted!"
- echo "📍 Entry endpoint : http://localhost:5000/v1"
- echo "⚙️ Dashboard URL : http://localhost:5000/dashboard"
+ echo "📍 Entry endpoint : ${PUBLIC_BASE_URL}/v1"
+ echo " (local) : ${LOCAL_BASE_URL}/v1"
+ echo "⚙️ Dashboard URL : ${PUBLIC_BASE_URL}/dashboard"
echo "🔑 Gateway API Key : gateway-pass"
- echo "🔐 LiteLLM Admin UI: http://localhost:4000/ui"
+ echo "🔐 LiteLLM Admin UI: ${PUBLIC_BASE_URL}/litellm/ui"
echo " Username: admin | Password: $LITELLM_MASTER_KEY"
echo "========================================================================="
</code_context>
<issue_to_address>
**nitpick:** Appending paths directly to PUBLIC_BASE_URL can produce double slashes if the base ends with `/`.
This assumes `PUBLIC_BASE_URL` has no trailing slash. If someone sets `PUBLIC_BASE_URL=https://example.com/llm-routing/`, these lines will output `//v1` and `/llm-routing//litellm/ui`. While most browsers tolerate this, you can avoid it by normalizing the value once (e.g. `PUBLIC_BASE_URL="${PUBLIC_BASE_URL%/}"`) before concatenation, or by trimming any trailing slash where the variable is initialized.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Code Review
This pull request introduces a production deployment checklist, re-enables the local Qwen model, updates the free models roster, and implements dynamic base URL resolution for the router dashboard. Feedback on these changes highlights critical issues: hardcoding paths in pod.yaml breaks the placeholder validation and dynamic deployment logic in start-stack.sh; PUBLIC_BASE_URL and LOCAL_BASE_URL are defined redundantly across different scopes in start-stack.sh; and the re-enabled local-qwen-3.6 model remains commented out in the fallback chains within litellm/config.yaml.
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.
| volumes: | ||
| - hostPath: | ||
| path: /home/gpav/Vrac/LAB/AI/LLM-Routing/valkey-data | ||
| path: /mnt/DATA/boy/LLM-Routing/valkey-data |
There was a problem hiding this comment.
Hardcoded paths break start-stack.sh placeholder validation and dynamic deployment
By replacing the placeholders /home/gpav/Vrac/LAB/AI/LLM-Routing, /home/gpav/, and /run/user/1000 with hardcoded paths (/mnt/DATA/boy/LLM-Routing, /mnt/DATA/boy/, and /run/user/1002), the placeholder validation in start-stack.sh (lines 545-548) will fail because it strictly checks for the presence of the original placeholders in pod.yaml:
placeholders = [
"/home/gpav/Vrac/LAB/AI/LLM-Routing",
"/home/gpav/",
"/run/user/1000",
...
]
for ph in placeholders:
if ph not in text:
sys.stderr.write(f"Error: Required placeholder '{ph}' not found in pod.yaml...")
sys.exit(1)Furthermore, start-stack.sh already dynamically replaces these placeholders with the current WORKDIR, HOME, and user uid at runtime. Hardcoding these paths is completely unnecessary and breaks multi-user/dynamic deployment.
Please revert all these paths in pod.yaml back to their original placeholder forms (e.g., /home/gpav/Vrac/LAB/AI/LLM-Routing/..., /home/gpav/..., and /run/user/1000/...).
path: /home/gpav/Vrac/LAB/AI/LLM-Routing/valkey-data|
|
||
| render_pod_yaml() { | ||
| export WORKDIR HOME LITELLM_MASTER_KEY POSTGRES_PASSWORD NEXTAUTH_SECRET SALT ENCRYPTION_KEY LANGFUSE_INIT_USER_PASSWORD MINIO_ROOT_USER MINIO_ROOT_PASSWORD OLLAMA_API_KEY LANGFUSE_PUBLIC_KEY LANGFUSE_SECRET_KEY CLASSIFIER_INPUT_MAX_CHARS REDIS_AUTH CLICKHOUSE_PASSWORD | ||
| PUBLIC_BASE_URL="${PUBLIC_BASE_URL:-${BASE_URL:-${BASEURL:-https://x570.vendeuvre.lan/llm-routing}}}" |
There was a problem hiding this comment.
Duplicate definition of PUBLIC_BASE_URL and LOCAL_BASE_URL
PUBLIC_BASE_URL is defined with a fallback value inside render_pod_yaml (line 514), and then redefined again on lines 606 and 634. Similarly, LOCAL_BASE_URL is defined twice on lines 607 and 635.
Since render_pod_yaml is executed in a pipeline (render_pod_yaml | podman play kube -), it runs in a subshell in many environments, meaning variables set inside it do not persist to the parent shell. To avoid duplication and ensure consistency, define these variables once at the top of the script (e.g., right after loading the .env file) and export them so they are available globally.
| # Re-enabled 2026-07-08 — llama.cpp on port 8081, alias local-qwen-3.6 | ||
| - litellm_params: | ||
| api_base: http://127.0.0.1:8081/v1 | ||
| api_key: local-token | ||
| model: openai/local-qwen-3.6 | ||
| request_timeout: 300 | ||
| model_name: local-qwen-3.6 |
There was a problem hiding this comment.
local-qwen-3.6 is still commented out in fallback chains
Although the local-qwen-3.6 model has been re-enabled in the model_list (lines 94-99), it remains commented out in the fallbacks configuration (e.g., lines 41, 48, 54, 63). If this model is intended to be used as a fallback when other backends fail, please uncomment it in the fallback chains.
…add fallbacks, avoid duplicate variables)
…PLACEHOLDER, HOME_PLACEHOLDER, and RUN_USER_PLACEHOLDER
…g subpath if missing
This PR resolves review comments from PR #241, syncs recent production changes from
boy, and introduces subdirectory proxy routing.Key Changes
/llm-routing/litellm,/llm-routing/langfuse, and/llm-routing/llama/subdirectories depending on request headers/domain.SERVER_ROOT_PATHand addedPROXY_BASE_URLbehind HAProxy.ROUTING_DOMAINenv variable inrouter/main.py(default:vendeuvre.lan).livenessProbeandreadinessProbeto use direct in-pod endpoints (/pingand/health/readiness) rather than the reverse proxy paths.boy:local-qwen-3.6model on port8081inlitellm/config.yaml.boy.ssh boyprefixes.Addressed PR #241 Review Fixes:
PUBLIC_BASE_URLandLOCAL_BASE_URLenv/config.urlparseimport to module scope.PROXY_BASE_URLdynamically configurable viaPROXY_BASE_URL_PLACEHOLDERreplacing the hardcoded URL./dev/nullfor SSHnohupcalls, and added missingssh boyprefix to the verification curl command.Summary by Sourcery
Introduce subdirectory-aware routing and configurable external URLs for the router dashboard and admin UIs, align pod configuration with the production host layout, and document the end-to-end deployment flow for the boy environment.
New Features:
Enhancements:
Documentation: