Conversation
WalkthroughThis change replaces the legacy server-rendered HTML UI with a new React-based frontend served under Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant GoServer
participant ReactFS
Browser->>GoServer: GET /ui/
GoServer->>ReactFS: Serve static files (index.html, assets)
ReactFS-->>GoServer: index.html or asset
GoServer-->>Browser: index.html or asset
Browser->>GoServer: GET /api/models
GoServer->>GoServer: Gather model status
GoServer-->>Browser: JSON [{id, state}, ...]
Browser->>GoServer: POST /api/models/unload
GoServer->>GoServer: Stop all model processes
GoServer-->>Browser: JSON {ok: true}
Browser->>GoServer: GET /api/modelsSSE
GoServer-->>Browser: SSE stream with model status updates
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (15)
ui/tsconfig.json (1)
1-7:files: []is fine, but consider adding anincludearray to avoid accidental global compilationWith an empty
filesarray and noinclude,tsc -b uican sometimes pick up stray.d.tsfiles that live outside the project reference graph (especially if developers runtscfrom the repo root). An explicit"include": ["src/**/*"](or similar) reduces that risk and makes the intent unambiguous.
ui/src/main.tsx (1)
7-13: Minor: pull the non-null assertion into a variable for readability-createRoot(document.getElementById("root")!).render( +const rootEl = document.getElementById("root") as HTMLElement; +createRoot(rootEl).render(Makes the “it really exists” intent explicit and eases future refactors/testing.
ui/vite.config.ts (1)
14-18: Hard-coding the backend origin will break non-local or containerised dev setups
"http://localhost:8080"is fine for single-machine work, but fails inside Docker, on remote dev-containers, or when the backend port changes.Expose the target via env vars that Vite already supports:
- "/api": "http://localhost:8080", + "/api": process.env.VITE_BACKEND_URL ?? "http://localhost:8080",Developers can then run
VITE_BACKEND_URL=http://backend:8080 pnpm dev.ui/README.md (1)
12-23: Add note about the monorepo build orderThe README explains linting in depth but omits how the UI integrates with the Go build (
make ui-> embed).
A short section like### Building with the backend make ui # builds React app into proxy/ui_dist make # builds Go binary that embeds the fileswould save newcomers a round-trip.
ui/src/App.tsx (1)
24-26: Emoji toggle lacks accessible labelScreen-reader users won’t know what the 🌙/☀️ button does.
- <button className="btn btn--sm" onClick={theme.toggleTheme}> - {theme.isDarkMode ? "🌙" : "☀️"} - </button> + <button + className="btn btn--sm" + aria-label="Toggle dark mode" + onClick={theme.toggleTheme} + > + {theme.isDarkMode ? "🌙" : "☀️"} + </button>ui/eslint.config.js (1)
8-11:node_modulesshould be ignored explicitlyESLint already skips
node_modules, but adding it prevents accidental overrides by editor configs.- { ignores: ['dist'] }, + { ignores: ['dist', 'node_modules'] },ui/src/contexts/ThemeProvider.tsx (1)
18-20: Avoid first-paint flash by applying the theme earlier
useEffectruns after the first paint; users will briefly see the wrong theme. Use
useLayoutEffect(client-only) or inject the attribute via a small inline script
inindex.htmlto setdata-themebefore React hydration.-import { createContext, useContext, useEffect, type ReactNode } from "react"; +import { createContext, useContext, useLayoutEffect as useEffect, type ReactNode } from "react";(Or keep
useEffectbut move the logic toindex.html.)ui/src/hooks/usePersistentState.ts (1)
15-36: LocalStorage write happens twice – can drop theuseEffect
setPersistentStatealready writes tolocalStorage; the additional
useEffectduplicates the work and serialises even when state is unchanged
(e.g. on component re-mount). Removing it simplifies the hook and saves one
synchronous write per update.- useEffect(() => { - try { - localStorage.setItem(key, JSON.stringify(state)); - } catch (e) { - console.error(`Error saving value for ${key}`, e); - } - }, [key, state]); + // side-effect already handled inside setPersistentStatePurely optional but tightens performance.
Makefile (1)
79-79: Add the missing phony targets
testandtest-allare real targets but not listed in.PHONY, triggering the linter warning and risking name/file clashes.-.PHONY: all clean ui mac linux windows simple-responder +.PHONY: all clean ui mac linux windows simple-responder test test-allui/src/index.css (1)
155-164: Redefining built-in utilities can cause surprises
ml-2andmy-8already exist in Tailwind. Overriding them here changes semantics globally and may break third-party components. Prefer unique class names or extend viatheme.spacinginstead.ui/src/pages/LogViewer.tsx (1)
84-94: Avoid recompiling the RegExp on every keystrokeFor large logs this can be expensive. Memoise the
RegExpobject instead:const regex = useMemo(() => { try { return filterRegex ? new RegExp(filterRegex, "i") : null; } catch { return null; } }, [filterRegex]); const filteredLogs = useMemo(() => { if (!regex) return logData; return logData.split("\n").filter(line => regex.test(line)).join("\n"); }, [logData, regex]);proxy/proxymanager.go (1)
188-211: Consider caching headers for SPA assetsStatic UI files are immutable build artifacts; setting
Cache-Control: public,max-age=31536000,immutableon them (exceptindex.html) will greatly improve load performance.proxy/proxymanager_api.go (3)
11-14: Prefer canonicalIDacronym to avoid Go vet warningsGo’s naming convention treats “ID” as an acronym. Using
Idwill tripgo vet’sallcapslinter and is inconsistent with the rest of the ecosystem.-type Model struct { - Id string `json:"id"` - State string `json:"state"` -} +type Model struct { + ID string `json:"id"` + State string `json:"state"` +} ... - Id: modelID, + ID: modelID,Also applies to: 68-70
16-24: Re-evaluate REST semantics & endpoint naming
/modelsSSEand/models/unloadbreak with common REST/resource naming patterns (/models/streamandDELETE /modelsorPOST /models:unloadare more orthodox). While not functionally wrong, cleaner URIs improve UX and future maintenance.
90-100: Replacetime.Afterin tight loop withtime.TickerCreating a new timer each iteration allocates continuously. A
time.Tickeris cheaper and expresses intent better:- for { - select { - case <-notify: - return - default: - models := pm.getModelStatus() - c.SSEvent("message", models) - c.Writer.Flush() - <-time.After(1 * time.Second) - } - } + ticker := time.NewTicker(1 * time.Second) + defer ticker.Stop() + for { + select { + case <-notify: + return + case <-ticker.C: + models := pm.getModelStatus() + c.SSEvent("message", models) + c.Writer.Flush() + } + }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
proxy/html/favicon.icois excluded by!**/*.icoui/package-lock.jsonis excluded by!**/package-lock.jsonui/public/favicon.icois excluded by!**/*.icoui/src/assets/logo.pngis excluded by!**/*.pngui/src/assets/react.svgis excluded by!**/*.svg
📒 Files selected for processing (29)
Makefile(2 hunks)config.example.yaml(1 hunks)proxy/.gitignore(1 hunks)proxy/html/index.html(0 hunks)proxy/html/logs.html(0 hunks)proxy/html_files.go(0 hunks)proxy/proxymanager.go(1 hunks)proxy/proxymanager_api.go(1 hunks)proxy/proxymanager_loghandlers.go(1 hunks)proxy/ui_embed.go(1 hunks)ui/.gitignore(1 hunks)ui/README.md(1 hunks)ui/eslint.config.js(1 hunks)ui/index.html(1 hunks)ui/package.json(1 hunks)ui/src/App.css(1 hunks)ui/src/App.tsx(1 hunks)ui/src/contexts/APIProvider.tsx(1 hunks)ui/src/contexts/ThemeProvider.tsx(1 hunks)ui/src/hooks/usePersistentState.ts(1 hunks)ui/src/index.css(1 hunks)ui/src/main.tsx(1 hunks)ui/src/pages/LogViewer.tsx(1 hunks)ui/src/pages/Models.tsx(1 hunks)ui/src/vite-env.d.ts(1 hunks)ui/tsconfig.app.json(1 hunks)ui/tsconfig.json(1 hunks)ui/tsconfig.node.json(1 hunks)ui/vite.config.ts(1 hunks)
💤 Files with no reviewable changes (3)
- proxy/html_files.go
- proxy/html/index.html
- proxy/html/logs.html
🧰 Additional context used
🧬 Code Graph Analysis (5)
ui/src/main.tsx (1)
ui/src/contexts/ThemeProvider.tsx (1)
ThemeProvider(15-25)
ui/src/App.tsx (2)
ui/src/contexts/ThemeProvider.tsx (1)
useTheme(27-33)ui/src/contexts/APIProvider.tsx (1)
APIProvider(27-150)
ui/src/contexts/ThemeProvider.tsx (1)
ui/src/hooks/usePersistentState.ts (1)
usePersistentState(3-39)
proxy/proxymanager.go (1)
proxy/ui_embed.go (1)
GetReactFS(13-19)
ui/src/contexts/APIProvider.tsx (1)
proxy/proxymanager_api.go (1)
Model(11-14)
🪛 Biome (1.9.4)
ui/tsconfig.node.json
[error] 9-9: JSON standard does not allow comments.
(parse)
[error] 10-10: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 10-10: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 10-10: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 10-10: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 11-11: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 11-11: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 11-11: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 11-11: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 13-13: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 13-13: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 13-13: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 13-13: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-16: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 16-16: JSON standard does not allow comments.
(parse)
[error] 17-17: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 17-17: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 17-17: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 17-17: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 18-18: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 18-18: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 18-18: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 18-18: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 19-19: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 19-19: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 19-19: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 19-19: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 20-20: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 20-20: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 20-20: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 20-20: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 22-22: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 22-22: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 22-22: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 24-24: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 24-24: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 24-24: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 24-25: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
ui/tsconfig.app.json
[error] 10-10: JSON standard does not allow comments.
(parse)
[error] 11-11: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 11-11: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 11-11: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 11-11: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 12-12: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 13-13: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 13-13: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 13-13: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 13-13: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 14-14: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 15-15: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 15-15: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 15-15: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 15-15: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 16-16: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 16-16: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 16-16: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 16-18: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 18-18: JSON standard does not allow comments.
(parse)
[error] 19-19: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 19-19: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 19-19: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 19-19: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 20-20: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 20-20: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 20-20: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 20-20: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 21-21: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 22-22: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 22-22: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 22-22: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 22-22: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 23-23: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 24-24: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 24-24: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 24-24: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 25-25: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 26-26: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 26-26: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 26-26: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 26-27: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
🪛 golangci-lint (1.64.8)
proxy/ui_embed.go
9-9: pattern ui_dist: no matching files found
(typecheck)
🪛 checkmake (0.2.2)
Makefile
[warning] 79-79: Missing required phony target "test"
(minphony)
🔇 Additional comments (12)
proxy/.gitignore (1)
1-1: Ignore React build output
Theui_dist/*entry correctly prevents committing generated React build artifacts underproxy/ui_dist.ui/tsconfig.node.json (1)
1-25: Approve Node-specific TSConfig
The JSONC comments are supported in TSConfig, and the compiler/lint settings properly target the Node environment forvite.config.ts.ui/src/vite-env.d.ts (1)
1-1: Include Vite client types
The triple-slash directive ensures TypeScript picks up Vite’s environment typings.ui/src/App.css (1)
1-6: Constrain and center root container
The CSS sets a max-width, centers horizontally withmargin: 0 auto, adds padding, and centers text for the#rootelement.ui/index.html (2)
6-7: Favicon path may 404 when served under/ui/baseBecause the SPA is embedded under
/ui/,/favicon.icomaps to the server root. Unless the backend serves the favicon at that exact path, prefer a relative reference:-<link rel="icon" type="image/png" href="/favicon.ico" /> +<link rel="icon" type="image/png" href="./favicon.ico" />Same applies to any other root-absolute asset links.
11-12: Dev-only script src is fine; build will rewrite – just confirm Vitebaseis set
/src/main.tsxworks in dev, but in prod Vite rewrites to/ui/assets/.... Ensurevite.config.tssets
base: "/ui/"(looks like it does) so the compiled path matches the backend route.ui/.gitignore (1)
17-19: Nice touch keeping.vscode/extensions.jsoncommittedGood compromise between team tooling consistency and keeping editor cruft out of the repo.
ui/package.json (1)
15-18: Verify availability of React 19 and React-Router 7React 19 and
react-router-dom7 are still in pre-release. Unless you’re intentionally targeting the canary channel, npm install will fail for most users.Please confirm that these versions exist on the public registry or pin to the latest stable (
react@18,react-router-dom@6).ui/tsconfig.app.json (1)
1-27:tsconfig.app.jsonis JSONC, but the filename is.json– tools complainTypeScript accepts comments inside tsconfig files, but many JSON tools (Biome in the log, jq, IDE JSON validators, CI linters, etc.) interpret the file as strict JSON and throw errors.
Two quick remedies:-// keep the name as-is -"tsBuildInfoFile": "./node_modules/.tmp/tsconfig.app.tsbuildinfo", +// 1) rename the file to `tsconfig.app.jsonc`, OR +// 2) add `"//": "jsonc"` to ESLint / editorconfig so non‐TS tools skip validationEither option stops the noise without touching the actual compiler pipeline.
ui/src/pages/Models.tsx (1)
18-30: Stale closure inuseCallback
unloadAllModelsshould be included in the dependency list; otherwise the
callback might capture an outdated reference.- }, []); + }, [unloadAllModels]);ui/src/index.css (1)
1-7: Verify Tailwind plugins for@custom-variant/@themeThese at-rules are not recognised by core Tailwind.
Ensure the build pipeline includes the appropriate PostCSS/Tailwind plugin (e.g.tailwindcss-variant-groupor a custom theme plugin) or the compile will fail.proxy/proxymanager_api.go (1)
31-39: Potential concurrent map read/write onpm.config.Models
rangeover a map without a lock can panic if any goroutine mutatespm.config.Modelsconcurrently (e.g., hot-reloading models). Verify thatpm.config.Modelsis immutable after startup or wrap accesses in an RLock.
config.example.yaml
Outdated
| groups: | ||
| coding: | ||
| swap: false | ||
| swap: true |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update documentation for swap setting
Ensure the README or config docs are updated to reflect the new default swap: true for the coding group and explain its impact on model ordering.
🤖 Prompt for AI Agents
In config.example.yaml at line 11, the swap setting default has been changed to
true. Update the README or configuration documentation to reflect this new
default value for the coding group. Additionally, add an explanation of how
setting swap to true affects model ordering to ensure users understand the
impact of this configuration change.
| accept := c.GetHeader("Accept") | ||
| if strings.Contains(accept, "text/html") { | ||
| // Set the Content-Type header to text/html | ||
| c.Header("Content-Type", "text/html") | ||
|
|
||
| // Write the embedded HTML content to the response | ||
| logsHTML, err := getHTMLFile("logs.html") | ||
| if err != nil { | ||
| c.String(http.StatusInternalServerError, err.Error()) | ||
| return | ||
| } | ||
| _, err = c.Writer.Write(logsHTML) | ||
| if err != nil { | ||
| c.String(http.StatusInternalServerError, fmt.Sprintf("failed to write response: %v", err)) | ||
| return | ||
| } | ||
| c.Redirect(http.StatusFound, "/ui/") | ||
| } else { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Redirect logic: make header check case-insensitive and use SeeOther for clarity
- The
Acceptheader is case-insensitive; usestrings.Contains(strings.ToLower(accept), "text/html"). - For GET requests a 303
SeeOtherbetter expresses “redirect to GET at/ui/”, avoiding surprises if the method ever changes.
- accept := c.GetHeader("Accept")
- if strings.Contains(accept, "text/html") {
- c.Redirect(http.StatusFound, "/ui/")
+ accept := strings.ToLower(c.GetHeader("Accept"))
+ if strings.Contains(accept, "text/html") {
+ c.Redirect(http.StatusSeeOther, "/ui/")
}📝 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.
| accept := c.GetHeader("Accept") | |
| if strings.Contains(accept, "text/html") { | |
| // Set the Content-Type header to text/html | |
| c.Header("Content-Type", "text/html") | |
| // Write the embedded HTML content to the response | |
| logsHTML, err := getHTMLFile("logs.html") | |
| if err != nil { | |
| c.String(http.StatusInternalServerError, err.Error()) | |
| return | |
| } | |
| _, err = c.Writer.Write(logsHTML) | |
| if err != nil { | |
| c.String(http.StatusInternalServerError, fmt.Sprintf("failed to write response: %v", err)) | |
| return | |
| } | |
| c.Redirect(http.StatusFound, "/ui/") | |
| } else { | |
| accept := strings.ToLower(c.GetHeader("Accept")) | |
| if strings.Contains(accept, "text/html") { | |
| c.Redirect(http.StatusSeeOther, "/ui/") | |
| } else { |
🤖 Prompt for AI Agents
In proxy/proxymanager_loghandlers.go around lines 12 to 15, update the redirect
logic to make the Accept header check case-insensitive by converting the header
value to lowercase before checking if it contains "text/html". Also, change the
HTTP status code from StatusFound (302) to StatusSeeOther (303) to better
indicate a redirect to a GET request at "/ui/".
| import { StrictMode } from "react"; | ||
| import { createRoot } from "react-dom/client"; | ||
| import "./index.css"; | ||
| import App from "./App.tsx"; | ||
| import { ThemeProvider } from "./contexts/ThemeProvider.tsx"; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Drop explicit .tsx extensions in imports – improves tooling & build portability
Vite/TS resolves extensions automatically. Keeping them:
- breaks
isolatedModules/babel settings that refuse non-URL file extensions - complicates Jest/ts-node runners
- causes noisy diffs if you ever switch to bare imports
-import App from "./App.tsx";
-import { ThemeProvider } from "./contexts/ThemeProvider.tsx";
+import App from "./App";
+import { ThemeProvider } from "./contexts/ThemeProvider";📝 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.
| import { StrictMode } from "react"; | |
| import { createRoot } from "react-dom/client"; | |
| import "./index.css"; | |
| import App from "./App.tsx"; | |
| import { ThemeProvider } from "./contexts/ThemeProvider.tsx"; | |
| import { StrictMode } from "react"; | |
| import { createRoot } from "react-dom/client"; | |
| import "./index.css"; | |
| import App from "./App"; | |
| import { ThemeProvider } from "./contexts/ThemeProvider"; |
🤖 Prompt for AI Agents
In ui/src/main.tsx lines 1 to 6, remove the explicit .tsx extensions from the
imports of App and ThemeProvider. Change the import statements to import from
"./App" and "./contexts/ThemeProvider" without the file extensions to improve
tooling compatibility and build portability.
| outDir: "../proxy/ui_dist", | ||
| assetsDir: "assets", | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
outDir escapes the project root – consider keeping build artifacts inside ui/
Writing build output to ../proxy/ui_dist means Vite will emit files one directory above the ui package.
While this matches the Go embed path today, it couples two packages and makes ui impossible to build in isolation or publish independently.
- outDir: "../proxy/ui_dist",
+ // keep output local and copy/embed from proxy build step instead
+ outDir: "dist",A small cp in the Makefile (or go:embed with a relative path) keeps boundaries clean.
🤖 Prompt for AI Agents
In ui/vite.config.ts around lines 10 to 12, the build output directory `outDir`
is set to "../proxy/ui_dist", which places build artifacts outside the `ui`
project root and couples it with another package. To fix this, change `outDir`
to a directory inside the `ui` folder (e.g., "./dist") so the build output stays
within the project boundary. Then, handle copying or embedding the build
artifacts externally via a Makefile or Go embed with relative paths to maintain
clean package separation.
| //go:embed ui_dist | ||
| var reactStaticFS embed.FS | ||
|
|
There was a problem hiding this comment.
Build fails if ui_dist is absent – add //go:build ui guard or dummy file
//go:embed ui_dist requires the directory to exist (even empty) at compile
time; otherwise go generate ./... && go build will error when the UI hasn’t
been built yet. Consider:
//go:build ui
// +build ui
//go:embed ui_dist/*or commit an empty .keep file into ui_dist so the embed finds at least one
path.
🧰 Tools
🪛 golangci-lint (1.64.8)
9-9: pattern ui_dist: no matching files found
(typecheck)
🤖 Prompt for AI Agents
In proxy/ui_embed.go around lines 9 to 11, the build fails if the ui_dist
directory is missing because //go:embed requires the directory to exist at
compile time. Fix this by adding a build constraint like //go:build ui and //
+build ui above the embed directive to conditionally include this code only when
the UI is built, or alternatively, add a dummy file such as an empty .keep file
inside ui_dist so the embed directive always finds at least one file.
There was a problem hiding this comment.
explain the go:build ui part a bit more
There was a problem hiding this comment.
@mostlygeek, the //go:build ui is a build constraint (also called build tags) in Go that enables conditional compilation. Here's how it works:
Build Constraints Explained
When you add //go:build ui at the top of a file, Go will only compile that file when you explicitly specify the ui tag during build:
//go:build ui
// +build ui
package proxy
//go:embed ui_dist
var reactStaticFS embed.FSHow It Solves Your Problem
Without build constraint (current situation):
go build # ❌ FAILS - ui_dist doesn't exist
make ui && go build # ✅ Works - ui_dist exists after make uiWith build constraint:
go build # ✅ Works - ui_embed.go is ignored completely
go build -tags ui # ❌ FAILS if ui_dist missing
make ui && go build -tags ui # ✅ Works - builds with UI embeddedPractical Workflow
Your Makefile targets would become:
# Development build (no UI)
dev:
go build
# Production build (with UI)
mac: ui
GOOS=darwin go build -tags ui -o bin/llama-swap-mac
linux: ui
GOOS=linux go build -tags ui -o bin/llama-swap-linuxThis way developers can build and test the Go backend without needing to build the React UI first, while production builds properly embed the UI assets.
The alternative approach (adding a dummy .keep file in ui_dist) is simpler but means the embed code is always compiled, even when not needed.
| # build react UI | ||
| ui: | ||
| cd ui && npm run build |
There was a problem hiding this comment.
🛠️ Refactor suggestion
ui target should install dependencies first
If node_modules is absent in clean CI environments the build fails.
Consider a sentinel dependency to run npm ci once, e.g.:
ui: ui/node_modules.stamp
cd ui && npm run build
ui/node_modules.stamp:
cd ui && npm ci
touch $@🤖 Prompt for AI Agents
In the Makefile around lines 32 to 34, the `ui` target currently only runs the
build command without ensuring dependencies are installed, which causes failures
in clean CI environments. Modify the `ui` target to depend on a sentinel file
like `ui/node_modules.stamp` that triggers `npm ci` to install dependencies if
needed. Add a new target `ui/node_modules.stamp` that runs `npm ci` inside the
`ui` directory and then creates the stamp file to mark completion. This ensures
dependencies are installed before building.
| const enableProxyLogs = useCallback( | ||
| (enabled: boolean) => { | ||
| if (enabled) { | ||
| const eventSource = new EventSource("/logs/streamSSE/proxy"); | ||
| eventSource.onmessage = handleProxyMessage; | ||
| proxyEventSource.current = eventSource; | ||
| } else { | ||
| proxyEventSource.current?.close(); | ||
| proxyEventSource.current = null; | ||
| } | ||
| }, | ||
| [handleProxyMessage] | ||
| ); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Handle EventSource errors & double-enable gracefully
- Add
onerrorto close/retry when the connection drops. - Guard against multiple enables:
if (enabled && !proxyEventSource.current) { ... }Otherwise repeated enableProxyLogs(true) would create orphaned streams.
🤖 Prompt for AI Agents
In ui/src/contexts/APIProvider.tsx around lines 57 to 70, the enableProxyLogs
function should be updated to handle EventSource errors by adding an onerror
handler that closes or retries the connection when it drops. Additionally, add a
guard condition to prevent multiple EventSource instances by checking if
proxyEventSource.current is already set before creating a new one, i.e., only
create a new EventSource if enabled is true and proxyEventSource.current is
null.
| func (pm *ProxyManager) apiUnloadAllModels(c *gin.Context) { | ||
| pm.StopProcesses(StopImmediately) | ||
| c.JSON(http.StatusOK, gin.H{"msg": "ok"}) | ||
| } |
There was a problem hiding this comment.
Handle possible StopProcesses errors
StopProcesses likely returns an error (or could in the future). Swallowing it masks operational failures.
-func (pm *ProxyManager) apiUnloadAllModels(c *gin.Context) {
- pm.StopProcesses(StopImmediately)
- c.JSON(http.StatusOK, gin.H{"msg": "ok"})
+func (pm *ProxyManager) apiUnloadAllModels(c *gin.Context) {
+ if err := pm.StopProcesses(StopImmediately); err != nil {
+ c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
+ return
+ }
+ c.JSON(http.StatusOK, gin.H{"msg": "ok"})
}📝 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.
| func (pm *ProxyManager) apiUnloadAllModels(c *gin.Context) { | |
| pm.StopProcesses(StopImmediately) | |
| c.JSON(http.StatusOK, gin.H{"msg": "ok"}) | |
| } | |
| func (pm *ProxyManager) apiUnloadAllModels(c *gin.Context) { | |
| if err := pm.StopProcesses(StopImmediately); err != nil { | |
| c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) | |
| return | |
| } | |
| c.JSON(http.StatusOK, gin.H{"msg": "ok"}) | |
| } |
🤖 Prompt for AI Agents
In proxy/proxymanager_api.go around lines 26 to 29, the call to
pm.StopProcesses(StopImmediately) does not handle any returned error, which
could hide operational issues. Modify the code to capture the error returned by
StopProcesses, check if it is non-nil, and if so, respond with an appropriate
HTTP error status and message instead of always returning "ok". This ensures any
failure in stopping processes is properly reported to the client.
- Add a react UI to replace the plain HTML one. - Serve as a foundation for better GUI interactions
1 participant
Closes #157
Was a pain converting the AI generated designs into react but coming along...
Summary by CodeRabbit
New Features
Improvements
/uipath.Chores & Tooling
.gitignorefiles to exclude build artifacts and dependencies from version control.Documentation