docs: refresh Mintlify docs and dashboard UI#233
Merged
SantiagoDePolonia merged 6 commits intomainfrom Apr 14, 2026
Merged
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 45 minutes and 44 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThis PR performs a comprehensive rebranding of the project from "GOModel" to "GoModel" across documentation, environment templates, and code comments. Concurrently, it modernizes the admin dashboard frontend by integrating Lucide icons, refactoring module resolution and rendering logic, and restructuring sidebar/template components. The Makefile is enhanced to generate OpenAPI specifications alongside Swagger documentation, and new feature documentation for aliases and user-path scoping is introduced. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/advanced/configuration.mdx (1)
237-254:⚠️ Potential issue | 🟡 MinorRe-add the provider filtering caveat in auto-discovery docs.
Line 237 says providers are auto-registered, but runtime still filters entries with missing/unresolved credentials (except API-keyless providers like Ollama). Please restore that caveat here to prevent operator confusion.
✏️ Proposed doc patch
The simplest way to add providers. GoModel checks for well-known API key environment variables and automatically registers providers: @@ GoModel also works with additional OpenAI-compatible providers out of the box through YAML provider blocks. + +<Note> + Providers with missing or unresolved credentials are filtered out at startup. + API-keyless providers (for example, Ollama) are kept. +</Note>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/advanced/configuration.mdx` around lines 237 - 254, Restore the caveat in the auto-discovery section that, although GoModel detects well-known API key env vars and will attempt to auto-register providers (the examples block showing OPENAI_API_KEY, ANTHROPIC_API_KEY, OLLAMA_BASE_URL, etc.), the runtime filters out provider entries with missing or unresolved credentials so they won't be active at runtime; explicitly call out that API-keyless providers like Ollama are exceptions and remain usable without an API key. Update the surrounding text near the provider examples to mention this filtering behavior and highlight the Ollama exception so operators aren’t confused.internal/admin/dashboard/templates/index.html (1)
967-969:⚠️ Potential issue | 🟡 MinorThe
user_pathshint is narrower than the actual matching rules.This copy says matching uses the managed API key
user_path, but the effective requestuser_pathcan also come fromX-GoModel-User-Pathwhen no key-bound path is set. That wording will mislead operators when testing header-based access rules.📝 Suggested wording
- The selector uses <code>/</code> for all providers and models, <code>{provider_name}/</code> for one provider, or <code>{provider_name}/{model}</code> for one model. <code>user_paths</code> is matched against the effective managed API key <code>user_path</code>. + The selector uses <code>/</code> for all providers and models, <code>{provider_name}/</code> for one provider, or <code>{provider_name}/{model}</code> for one model. <code>user_paths</code> is matched against the effective request <code>user_path</code>.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/admin/dashboard/templates/index.html` around lines 967 - 969, Update the help text in the alias-form-hint paragraph: clarify that matching against user_paths uses the effective request user_path which is taken from the managed API key's user_path when present, but if the key has no bound path the server will use the X-GoModel-User-Path request header instead; mention both sources (managed API key user_path and X-GoModel-User-Path header) and keep the examples of selector formats ({provider_name}/, {provider_name}/{model}, /) intact so operators understand header-based access rules.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/about/benchmarks.mdx`:
- Around line 66-69: The four bullets that each start with "GoModel" (lines
containing "GoModel posted lower p95 latency...", "GoModel reached higher
throughput...", "GoModel stayed near `45-46 MB` RSS...", and "GoModel also used
less CPU...") are repetitive; replace them by either a single summary sentence
that lists those outcomes for GoModel or reword the following three bullets to
drop the repeated subject (e.g., "Posted lower p95 latency at every tested
concurrency level," "Reached higher throughput across the benchmark matrix,"
"Stayed near `45-46 MB` RSS while LiteLLM stayed near `320-321 MB`," "Used less
CPU in these runs"), so the section reads more concise and scans faster.
In `@docs/about/values.mdx`:
- Around line 16-25: The "Quality" section repeats sentence openings beginning
with "We" across multiple adjacent sentences (e.g., "We do not believe an AI
gateway...", "We chose Go...", "We try to put the big stones in first.");
rephrase to vary sentence openings and tighten flow by combining or
restructuring those lines—for example, merge the rationale about language and
maintainability into one sentence, turn "We chose Go because..." into "Go was
chosen for runtime behavior, deployment shape, and long-term maintainability,"
and reword the closing "We try to put the big stones in first" to a concise
active phrase like "We put the big stones in first" or "Big stones come first"
to avoid repeated "We" starts while preserving meaning.
In `@internal/admin/dashboard/static/js/dashboard.js`:
- Around line 224-226: The code stores a high-privilege admin API key in
localStorage via localStorage.getItem("gomodel_api_key") when initializing
this.apiKey (and similarly around the other block flagged); stop persisting the
raw key to localStorage: remove reads/writes to localStorage for
"gomodel_api_key" and instead keep the key only in memory (assign to this.apiKey
after this.normalizeApiKey) or exchange it for a backend-issued HttpOnly session
token. Locate and update usages around the this.apiKey initialization and any
methods that set or clear the key (e.g., places that call normalizeApiKey or
write "gomodel_api_key") so they no longer call localStorage.getItem/set/remove
and instead rely on an in-memory property or server session handshake.
- Around line 513-521: Set the runtimeRefreshReport and mark the refresh as
successful immediately after parsing the POST response (referencing
runtimeRefreshReport and runtimeRefreshSucceeded()/runtimeRefreshNotice), then
call refreshDashboardDataAfterRuntimeRefresh() in its own try/catch so any
rejection only updates runtimeRefreshNotice/runtimeRefreshError without
overwriting the successful runtimeRefreshReport/state; specifically, move the
success assignment (this.runtimeRefreshReport = payload;
this.runtimeRefreshNotice = this.runtimeRefreshSummary(); etc.) out of the
try/catch that waits for refreshDashboardDataAfterRuntimeRefresh(), and wrap
refreshDashboardDataAfterRuntimeRefresh() in a separate try/catch to handle UI
reload failures by setting this.runtimeRefreshNotice and
this.runtimeRefreshError only on that catch.
In `@internal/admin/dashboard/templates/sidebar.html`:
- Around line 82-85: The sidebar collapse handle is currently a non-focusable
div using `@mousedown`; change the element with class "sidebar-toggle" to a real
<button> (type="button") and switch the event handler from `@mousedown` to `@click`,
keep using the toggleSidebar() method and the :class binding with
sidebarCollapsed, and add accessible attributes such as
:aria-pressed="sidebarCollapsed" or :aria-expanded="!sidebarCollapsed" and a
meaningful title/aria-label so keyboard and screen-reader users can operate the
control.
- Around line 49-60: The three theme icon buttons (class "theme-btn", handlers
setTheme('light'|'system'|'dark')) are currently unreachable due to
tabindex="-1" and lack accessible names; remove tabindex="-1" (or set to 0) so
they are focusable and add explicit accessible names via aria-label (e.g.,
aria-label="Light theme", "System theme", "Dark theme") matching their title
text; also make the mobile toggle (toggleTheme) consistent by removing
tabindex="-1" and adding an aria-label that mirrors its computed title so
keyboard users can focus and operate it.
In `@Makefile`:
- Around line 95-104: The docs-openapi Makefile target assumes Node tooling is
present; add early preflight checks to abort with clear messages if node or npx
are missing before running the rest of the recipe. Update the docs-openapi
target to first test for node and npx (e.g., using command -v or which) and
print actionable errors like "node is required to build docs; install from
https://nodejs.org" or "npx is required; install npm (includes npx)". Keep these
checks at the top of the docs-openapi recipe (before the go run and npx
swagger2openapi invocation) so missing tools fail fast and the rest of the
commands (tmp_dir, go run, npx, node -e) only run when both are available.
---
Outside diff comments:
In `@docs/advanced/configuration.mdx`:
- Around line 237-254: Restore the caveat in the auto-discovery section that,
although GoModel detects well-known API key env vars and will attempt to
auto-register providers (the examples block showing OPENAI_API_KEY,
ANTHROPIC_API_KEY, OLLAMA_BASE_URL, etc.), the runtime filters out provider
entries with missing or unresolved credentials so they won't be active at
runtime; explicitly call out that API-keyless providers like Ollama are
exceptions and remain usable without an API key. Update the surrounding text
near the provider examples to mention this filtering behavior and highlight the
Ollama exception so operators aren’t confused.
In `@internal/admin/dashboard/templates/index.html`:
- Around line 967-969: Update the help text in the alias-form-hint paragraph:
clarify that matching against user_paths uses the effective request user_path
which is taken from the managed API key's user_path when present, but if the key
has no bound path the server will use the X-GoModel-User-Path request header
instead; mention both sources (managed API key user_path and X-GoModel-User-Path
header) and keep the examples of selector formats ({provider_name}/,
{provider_name}/{model}, /) intact so operators understand header-based access
rules.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: dc102948-aaf6-4aa0-98de-b1e05d77fb10
⛔ Files ignored due to path filters (4)
docs/about/jar-and-stones.jpgis excluded by!**/*.jpgdocs/features/aliases.pngis excluded by!**/*.pngdocs/wordmark-dark.svgis excluded by!**/*.svgdocs/wordmark-light.svgis excluded by!**/*.svg
📒 Files selected for processing (59)
.env.template2026-03-16_ARCHITECTURE_SNAPSHOT.mdAGENTS.mdCLAUDE.mdGETTING_STARTED.mdMETRICS_CONFIGURATION.mdMakefilePROMETHEUS_IMPLEMENTATION.mdREADME.mdcmd/gomodel/docs/docs.gocmd/gomodel/docs/swagger.jsoncmd/gomodel/main.goconfig/config.example.yamldocs/2026-04-09_CODEBASE_SNAPSHOT.mddocs/TESTING_STRATEGY.mddocs/about/benchmarks.mdxdocs/about/license.mdxdocs/about/technical-philosophy.mdxdocs/about/values.mdxdocs/about/who-we-are.mdxdocs/adr/0001-explicit-provider-registration.mddocs/adr/0002-ingress-frame-and-semantic-envelope.mddocs/adr/0003-policy-resolved-workflow.mddocs/adr/0005-provider-qualified-model-selectors.mddocs/adr/0006-semantic-response-cache.mddocs/advanced/admin-endpoints.mdxdocs/advanced/config-yaml.mdxdocs/advanced/configuration.mdxdocs/advanced/guardrails.mdxdocs/advanced/workflows.mdxdocs/docs.jsondocs/features/aliases.mdxdocs/features/cache.mdxdocs/features/user-path.mdxdocs/getting-started/quickstart.mdxdocs/guides/claude-code.mdxdocs/guides/codex.mdxdocs/guides/multiple-ollama.mdxdocs/guides/openclaw.mdxdocs/guides/opencode-and-other-agents.mdxdocs/guides/oracle.mdxdocs/openapi.jsonhelm/Chart.yamlhelm/README.mdhelm/values.schema.jsoninternal/admin/dashboard/dashboard.gointernal/admin/dashboard/static/css/dashboard.cssinternal/admin/dashboard/static/js/dashboard.jsinternal/admin/dashboard/static/js/modules/dashboard-layout.test.jsinternal/admin/dashboard/static/js/modules/icons.jsinternal/admin/dashboard/static/js/modules/timezone-layout.test.jsinternal/admin/dashboard/static/js/modules/workflows-layout.test.jsinternal/admin/dashboard/templates/index.htmlinternal/admin/dashboard/templates/layout.htmlinternal/admin/dashboard/templates/sidebar.htmlinternal/admin/handler.gointernal/app/app.gointernal/providers/openrouter/openrouter.gointernal/server/http_test.go
Comment on lines
+66
to
+69
| - GoModel posted lower p95 latency at every tested concurrency level. | ||
| - GoModel reached higher throughput across the benchmark matrix. | ||
| - GoModel stayed near `45-46 MB` RSS, while LiteLLM stayed near `320-321 MB`. | ||
| - GoModel also used less CPU in these runs. |
Contributor
There was a problem hiding this comment.
Reduce repeated bullet starters for faster scanning.
Line 66 through Line 69 repeat “GoModel” at the start of successive bullets, which makes this section feel mechanical.
✍️ Suggested wording tweak
-- GoModel posted lower p95 latency at every tested concurrency level.
-- GoModel reached higher throughput across the benchmark matrix.
-- GoModel stayed near `45-46 MB` RSS, while LiteLLM stayed near `320-321 MB`.
-- GoModel also used less CPU in these runs.
+- GoModel posted lower p95 latency at every tested concurrency level.
+- Throughput was also higher across the benchmark matrix.
+- RSS stayed near `45-46 MB`, while LiteLLM stayed near `320-321 MB`.
+- CPU usage was lower in these runs as well.📝 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.
Suggested change
| - GoModel posted lower p95 latency at every tested concurrency level. | |
| - GoModel reached higher throughput across the benchmark matrix. | |
| - GoModel stayed near `45-46 MB` RSS, while LiteLLM stayed near `320-321 MB`. | |
| - GoModel also used less CPU in these runs. | |
| - GoModel posted lower p95 latency at every tested concurrency level. | |
| - Throughput was also higher across the benchmark matrix. | |
| - RSS stayed near `45-46 MB`, while LiteLLM stayed near `320-321 MB`. | |
| - CPU usage was lower in these runs as well. |
🧰 Tools
🪛 LanguageTool
[style] ~68-~68: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...roughput across the benchmark matrix. - GoModel stayed near 45-46 MB RSS, while LiteL...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~69-~69: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ile LiteLLM stayed near 320-321 MB. - GoModel also used less CPU in these runs. At t...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/about/benchmarks.mdx` around lines 66 - 69, The four bullets that each
start with "GoModel" (lines containing "GoModel posted lower p95 latency...",
"GoModel reached higher throughput...", "GoModel stayed near `45-46 MB` RSS...",
and "GoModel also used less CPU...") are repetitive; replace them by either a
single summary sentence that lists those outcomes for GoModel or reword the
following three bullets to drop the repeated subject (e.g., "Posted lower p95
latency at every tested concurrency level," "Reached higher throughput across
the benchmark matrix," "Stayed near `45-46 MB` RSS while LiteLLM stayed near
`320-321 MB`," "Used less CPU in these runs"), so the section reads more concise
and scans faster.
Comment on lines
+16
to
+25
| We are here to outdo LiteLLM, and quality is how we get there. | ||
|
|
||
| We do not believe an AI gateway has to be written in Python. We chose Go | ||
| because runtime behavior, deployment shape, and long-term maintainability matter. | ||
|
|
||
| The jar is a useful reminder: if you fill it with sand first, the larger stones | ||
| will not fit later. In GoModel, the big stones are experience, programming language and architecture. | ||
| The sand is the smaller work: features, functions, tests, and fixes. | ||
|
|
||
| We try to put the big stones in first. |
Contributor
There was a problem hiding this comment.
Tighten repeated “We …” openings in the Quality section.
Line 16, Line 18, and Line 25 stack similar sentence starts; a small rewrite will read more naturally.
✍️ Suggested wording tweak
-We are here to outdo LiteLLM, and quality is how we get there.
+We are here to outdo LiteLLM, and quality is how we get there.
-We do not believe an AI gateway has to be written in Python. We chose Go
+An AI gateway does not have to be written in Python. We chose Go
because runtime behavior, deployment shape, and long-term maintainability matter.
-We try to put the big stones in first.
+So we put the big stones in first.📝 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.
Suggested change
| We are here to outdo LiteLLM, and quality is how we get there. | |
| We do not believe an AI gateway has to be written in Python. We chose Go | |
| because runtime behavior, deployment shape, and long-term maintainability matter. | |
| The jar is a useful reminder: if you fill it with sand first, the larger stones | |
| will not fit later. In GoModel, the big stones are experience, programming language and architecture. | |
| The sand is the smaller work: features, functions, tests, and fixes. | |
| We try to put the big stones in first. | |
| We are here to outdo LiteLLM, and quality is how we get there. | |
| An AI gateway does not have to be written in Python. We chose Go | |
| because runtime behavior, deployment shape, and long-term maintainability matter. | |
| The jar is a useful reminder: if you fill it with sand first, the larger stones | |
| will not fit later. In GoModel, the big stones are experience, programming language and architecture. | |
| The sand is the smaller work: features, functions, tests, and fixes. | |
| So we put the big stones in first. |
🧰 Tools
🪛 LanguageTool
[style] ~18-~18: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...AI gateway has to be written in Python. We chose Go because runtime behavior, depl...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/about/values.mdx` around lines 16 - 25, The "Quality" section repeats
sentence openings beginning with "We" across multiple adjacent sentences (e.g.,
"We do not believe an AI gateway...", "We chose Go...", "We try to put the big
stones in first."); rephrase to vary sentence openings and tighten flow by
combining or restructuring those lines—for example, merge the rationale about
language and maintainability into one sentence, turn "We chose Go because..."
into "Go was chosen for runtime behavior, deployment shape, and long-term
maintainability," and reword the closing "We try to put the big stones in first"
to a concise active phrase like "We put the big stones in first" or "Big stones
come first" to avoid repeated "We" starts while preserving meaning.
Comment on lines
+513
to
+521
| const payload = await res.json(); | ||
| this.runtimeRefreshReport = | ||
| payload && typeof payload === "object" ? payload : null; | ||
| this.runtimeRefreshNotice = this.runtimeRefreshSummary(); | ||
| await this.refreshDashboardDataAfterRuntimeRefresh(); | ||
| } catch (e) { | ||
| console.error("Failed to refresh runtime:", e); | ||
| this.runtimeRefreshNotice = "Runtime refresh failed."; | ||
| this.runtimeRefreshError = this.runtimeRefreshNotice; |
Contributor
There was a problem hiding this comment.
Separate a successful runtime refresh from a failed UI reload.
Once runtimeRefreshReport is set, any rejection from refreshDashboardDataAfterRuntimeRefresh() drops into the same catch path and rewrites the notice/error while runtimeRefreshSucceeded() still stays true. That can leave the page rendering a success-styled “Runtime refresh failed.” state after a successful POST.
🩹 Suggested fix
const payload = await res.json();
this.runtimeRefreshReport =
payload && typeof payload === "object" ? payload : null;
this.runtimeRefreshNotice = this.runtimeRefreshSummary();
- await this.refreshDashboardDataAfterRuntimeRefresh();
+ try {
+ await this.refreshDashboardDataAfterRuntimeRefresh();
+ } catch (e) {
+ console.error(
+ "Runtime refresh succeeded, but dashboard data reload failed:",
+ e,
+ );
+ this.runtimeRefreshError =
+ "Runtime refreshed, but some dashboard data could not be reloaded.";
+ }
} catch (e) {
console.error("Failed to refresh runtime:", e);
this.runtimeRefreshNotice = "Runtime refresh failed.";
this.runtimeRefreshError = this.runtimeRefreshNotice;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/admin/dashboard/static/js/dashboard.js` around lines 513 - 521, Set
the runtimeRefreshReport and mark the refresh as successful immediately after
parsing the POST response (referencing runtimeRefreshReport and
runtimeRefreshSucceeded()/runtimeRefreshNotice), then call
refreshDashboardDataAfterRuntimeRefresh() in its own try/catch so any rejection
only updates runtimeRefreshNotice/runtimeRefreshError without overwriting the
successful runtimeRefreshReport/state; specifically, move the success assignment
(this.runtimeRefreshReport = payload; this.runtimeRefreshNotice =
this.runtimeRefreshSummary(); etc.) out of the try/catch that waits for
refreshDashboardDataAfterRuntimeRefresh(), and wrap
refreshDashboardDataAfterRuntimeRefresh() in a separate try/catch to handle UI
reload failures by setting this.runtimeRefreshNotice and
this.runtimeRefreshError only on that catch.
Comment on lines
+49
to
+60
| <div class="theme-toggle"> | ||
| <button class="theme-btn" :class="{ active: theme === 'light' }" @click="setTheme('light')" title="Light" tabindex="-1"> | ||
| <i data-lucide="sun" class="theme-icon" aria-hidden="true"></i> | ||
| </button> | ||
| <button class="theme-btn" :class="{ active: theme === 'system' }" @click="setTheme('system')" title="System" tabindex="-1"> | ||
| <i data-lucide="monitor" class="theme-icon" aria-hidden="true"></i> | ||
| </button> | ||
| <button class="theme-btn" :class="{ active: theme === 'dark' }" @click="setTheme('dark')" title="Dark" tabindex="-1"> | ||
| <i data-lucide="moon" class="theme-icon" aria-hidden="true"></i> | ||
| </button> | ||
| </div> | ||
| <button class="theme-toggle-mobile" @click="toggleTheme()" :title="theme === 'light' ? 'Light' : theme === 'dark' ? 'Dark' : 'System'" tabindex="-1"> |
Contributor
There was a problem hiding this comment.
Theme switching is unreachable from keyboard.
tabindex="-1" removes all four theme buttons from the tab order, and the icon-only controls rely on title instead of an accessible name. Keyboard users currently can't change the theme from the sidebar.
♿ Suggested fix
- <button class="theme-btn" :class="{ active: theme === 'light' }" `@click`="setTheme('light')" title="Light" tabindex="-1">
+ <button type="button"
+ class="theme-btn"
+ :class="{ active: theme === 'light' }"
+ :aria-pressed="theme === 'light' ? 'true' : 'false'"
+ aria-label="Light theme"
+ `@click`="setTheme('light')"
+ title="Light">
<i data-lucide="sun" class="theme-icon" aria-hidden="true"></i>
</button>
@@
- <button class="theme-toggle-mobile" `@click`="toggleTheme()" :title="theme === 'light' ? 'Light' : theme === 'dark' ? 'Dark' : 'System'" tabindex="-1">
+ <button type="button"
+ class="theme-toggle-mobile"
+ :aria-label="theme === 'light' ? 'Light theme' : theme === 'dark' ? 'Dark theme' : 'System theme'"
+ `@click`="toggleTheme()"
+ :title="theme === 'light' ? 'Light' : theme === 'dark' ? 'Dark' : 'System'">Apply the same pattern to the light/system/dark buttons.
🧰 Tools
🪛 HTMLHint (1.9.2)
[warning] 50-50: The type attribute must be present on elements.
(button-type-require)
[warning] 53-53: The type attribute must be present on
elements.(button-type-require)
[warning] 56-56: The type attribute must be present on
elements.(button-type-require)
[warning] 60-60: The type attribute must be present on
elements.(button-type-require)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/admin/dashboard/templates/sidebar.html` around lines 49 - 60, The
three theme icon buttons (class "theme-btn", handlers
setTheme('light'|'system'|'dark')) are currently unreachable due to
tabindex="-1" and lack accessible names; remove tabindex="-1" (or set to 0) so
they are focusable and add explicit accessible names via aria-label (e.g.,
aria-label="Light theme", "System theme", "Dark theme") matching their title
text; also make the mobile toggle (toggleTheme) consistent by removing
tabindex="-1" and adding an aria-label that mirrors its computed title so
keyboard users can focus and operate it.
Comment on lines
+82
to
+85
| <div class="sidebar-toggle" | ||
| @mousedown="toggleSidebar()" | ||
| :class="{ collapsed: sidebarCollapsed }" | ||
| :title="sidebarCollapsed ? 'Expand sidebar' : 'Collapse sidebar'"></div> |
Contributor
There was a problem hiding this comment.
The sidebar collapse handle is mouse-only.
A bare <div> with @mousedown can't receive focus or be activated via keyboard/screen readers. Use a real button and @click so the control is operable without a mouse.
♿ Suggested fix
-<div class="sidebar-toggle"
- `@mousedown`="toggleSidebar()"
- :class="{ collapsed: sidebarCollapsed }"
- :title="sidebarCollapsed ? 'Expand sidebar' : 'Collapse sidebar'"></div>
+<button type="button"
+ class="sidebar-toggle"
+ `@click`="toggleSidebar()"
+ :class="{ collapsed: sidebarCollapsed }"
+ :aria-label="sidebarCollapsed ? 'Expand sidebar' : 'Collapse sidebar'"
+ :title="sidebarCollapsed ? 'Expand sidebar' : 'Collapse sidebar'"></button>📝 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.
Suggested change
| <div class="sidebar-toggle" | |
| @mousedown="toggleSidebar()" | |
| :class="{ collapsed: sidebarCollapsed }" | |
| :title="sidebarCollapsed ? 'Expand sidebar' : 'Collapse sidebar'"></div> | |
| <button type="button" | |
| class="sidebar-toggle" | |
| `@click`="toggleSidebar()" | |
| :class="{ collapsed: sidebarCollapsed }" | |
| :aria-label="sidebarCollapsed ? 'Expand sidebar' : 'Collapse sidebar'" | |
| :title="sidebarCollapsed ? 'Expand sidebar' : 'Collapse sidebar'"></button> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/admin/dashboard/templates/sidebar.html` around lines 82 - 85, The
sidebar collapse handle is currently a non-focusable div using `@mousedown`;
change the element with class "sidebar-toggle" to a real <button>
(type="button") and switch the event handler from `@mousedown` to `@click`, keep
using the toggleSidebar() method and the :class binding with sidebarCollapsed,
and add accessible attributes such as :aria-pressed="sidebarCollapsed" or
:aria-expanded="!sidebarCollapsed" and a meaningful title/aria-label so keyboard
and screen-reader users can operate the control.
Comment on lines
+95
to
+104
| docs-openapi: | ||
| @tmp_dir=$$(mktemp -d); \ | ||
| trap 'rm -rf "$$tmp_dir"' EXIT; \ | ||
| go run github.com/swaggo/swag/cmd/swag init --quiet --generalInfo main.go \ | ||
| --dir cmd/gomodel,internal \ | ||
| --output "$$tmp_dir" \ | ||
| --outputTypes json \ | ||
| --parseDependency; \ | ||
| npx -y swagger2openapi@7.0.8 --patch -o docs/openapi.json "$$tmp_dir/swagger.json"; \ | ||
| DOCS_API_SERVERS="$(DOCS_API_SERVERS)" node -e 'const fs = require("fs"); const file = "docs/openapi.json"; const urls = (process.env.DOCS_API_SERVERS || "").split(",").map((url) => url.trim()).filter(Boolean); if (!urls.length) throw new Error("DOCS_API_SERVERS must include at least one URL"); const spec = JSON.parse(fs.readFileSync(file, "utf8")); spec.servers = urls.map((url) => ({ url, description: /(^https?:\/\/)?(localhost|127\.0\.0\.1)(:|\/|$$)/.test(url) ? "Local GoModel" : "GoModel" })); fs.writeFileSync(file, JSON.stringify(spec, null, 2) + "\n");' |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Checking tool availability required by docs-openapi..."
command -v node >/dev/null && node --version
command -v npx >/dev/null && npx --version
echo
echo "Confirming target wiring in Makefile..."
rg -n '^(swagger:|docs-openapi:)|swagger2openapi|DOCS_API_SERVERS' Makefile -n -C2Repository: ENTERPILOT/GoModel
Length of output: 1405
Add explicit node and npx preflight checks to docs-openapi target.
Lines 95–104 implicitly require Node tooling; on Go-only environments, missing tools fail late with unclear errors. Add early preflight checks to fail fast with actionable messages.
Suggested fix
docs-openapi:
+ `@command` -v node >/dev/null 2>&1 || (echo "node is required for docs-openapi" && exit 1)
+ `@command` -v npx >/dev/null 2>&1 || (echo "npx is required for docs-openapi" && exit 1)
`@tmp_dir`=$$(mktemp -d); \
trap 'rm -rf "$$tmp_dir"' EXIT; \
go run github.com/swaggo/swag/cmd/swag init --quiet --generalInfo main.go \📝 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.
Suggested change
| docs-openapi: | |
| @tmp_dir=$$(mktemp -d); \ | |
| trap 'rm -rf "$$tmp_dir"' EXIT; \ | |
| go run github.com/swaggo/swag/cmd/swag init --quiet --generalInfo main.go \ | |
| --dir cmd/gomodel,internal \ | |
| --output "$$tmp_dir" \ | |
| --outputTypes json \ | |
| --parseDependency; \ | |
| npx -y swagger2openapi@7.0.8 --patch -o docs/openapi.json "$$tmp_dir/swagger.json"; \ | |
| DOCS_API_SERVERS="$(DOCS_API_SERVERS)" node -e 'const fs = require("fs"); const file = "docs/openapi.json"; const urls = (process.env.DOCS_API_SERVERS || "").split(",").map((url) => url.trim()).filter(Boolean); if (!urls.length) throw new Error("DOCS_API_SERVERS must include at least one URL"); const spec = JSON.parse(fs.readFileSync(file, "utf8")); spec.servers = urls.map((url) => ({ url, description: /(^https?:\/\/)?(localhost|127\.0\.0\.1)(:|\/|$$)/.test(url) ? "Local GoModel" : "GoModel" })); fs.writeFileSync(file, JSON.stringify(spec, null, 2) + "\n");' | |
| docs-openapi: | |
| `@command` -v node >/dev/null 2>&1 || (echo "node is required for docs-openapi" && exit 1) | |
| `@command` -v npx >/dev/null 2>&1 || (echo "npx is required for docs-openapi" && exit 1) | |
| `@tmp_dir`=$$(mktemp -d); \ | |
| trap 'rm -rf "$$tmp_dir"' EXIT; \ | |
| go run github.com/swaggo/swag/cmd/swag init --quiet --generalInfo main.go \ | |
| --dir cmd/gomodel,internal \ | |
| --output "$$tmp_dir" \ | |
| --outputTypes json \ | |
| --parseDependency; \ | |
| npx -y swagger2openapi@7.0.8 --patch -o docs/openapi.json "$$tmp_dir/swagger.json"; \ | |
| DOCS_API_SERVERS="$(DOCS_API_SERVERS)" node -e 'const fs = require("fs"); const file = "docs/openapi.json"; const urls = (process.env.DOCS_API_SERVERS || "").split(",").map((url) => url.trim()).filter(Boolean); if (!urls.length) throw new Error("DOCS_API_SERVERS must include at least one URL"); const spec = JSON.parse(fs.readFileSync(file, "utf8")); spec.servers = urls.map((url) => ({ url, description: /(^https?:\/\/)?(localhost|127\.0\.0\.1)(:|\/|$$)/.test(url) ? "Local GoModel" : "GoModel" })); fs.writeFileSync(file, JSON.stringify(spec, null, 2) + "\n");' |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Makefile` around lines 95 - 104, The docs-openapi Makefile target assumes
Node tooling is present; add early preflight checks to abort with clear messages
if node or npx are missing before running the rest of the recipe. Update the
docs-openapi target to first test for node and npx (e.g., using command -v or
which) and print actionable errors like "node is required to build docs; install
from https://nodejs.org" or "npx is required; install npm (includes npx)". Keep
these checks at the top of the docs-openapi recipe (before the go run and npx
swagger2openapi invocation) so missing tools fail fast and the rest of the
commands (tmp_dir, go run, npx, node -e) only run when both are available.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Verification
node --test internal/admin/dashboard/static/js/modules/*.test.jsgo test ./internal/admin/dashboardnode --check internal/admin/dashboard/static/js/modules/icons.jsjq . docs/docs.jsongit diff --checkSummary by CodeRabbit
New Features
Documentation