feat(dashboard): expand Settings with a full config viewer, wider settable set, and search#346
Conversation
GitHub Actions
Inactive
|
Warning Review limit reached
Next review available in: 4 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: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (15)
WalkthroughChangesThe settings system now exposes grouped effective configuration metadata, supports typed persisted runtime overrides, and applies hot-changeable values after successful commits. The dashboard renders dynamic controls with validation, filtering, and startup-only indicators. Tests, API examples, and rebuilt assets were updated accordingly. Settings expansion
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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 Actions
Inactive
There was a problem hiding this comment.
Pull request overview
Expands the standalone admin dashboard “Settings” surface from a couple of toggles into a full effective-config viewer (grouped + searchable) and broadens the runtime-settable settings persisted via runtime_settings so operators can safely hot-change more gateway behavior.
Changes:
- Backend: adds a typed per-key runtime-settings spec/validation layer, expands the hot-changeable key set, and returns a grouped “full config” view from
GET /v1/settings. - Dashboard: rebuilds Settings UI from the backend config view, adds fuzzy search and “settable only” filtering, and implements per-type controls (toggle, select, number+save, nullable text+save).
- Tests/specs: adds unit coverage for runtime settings validation/serialization and settings endpoint shape, updates dashboard tests, and regenerates OpenAPI/Postman + dashboard bundle.
Reviewed changes
Copilot reviewed 14 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
web/src/pages/SettingsPage.tsx |
New grouped settings viewer UI with fuzzy search and per-type controls. |
web/src/pages/SettingsPage.test.tsx |
Adds tests for matching/filtering and each control type (bool/int/float/enum/nullable). |
web/src/pages/ProvidersPage.test.tsx |
Updates settings test fixture to include the new config field. |
web/src/pages/ModelsPage.test.tsx |
Updates settings test fixture to include the new config field. |
web/src/components/PricingWarning.test.tsx |
Updates settings test fixture to include the new config field. |
web/src/api/types.ts |
Adds ConfigField/GatewaySettings.config and expands UpdateSettingsRequest types. |
tests/unit/test_settings_endpoint.py |
Adds backend tests for full config view, redaction, PATCH semantics, and restart persistence. |
tests/unit/test_runtime_settings_service.py |
New unit tests for validation, serialization, and loading overrides from DB. |
src/gateway/static/dashboard/index.html |
Updates compiled dashboard asset references. |
src/gateway/services/runtime_settings_service.py |
Refactors runtime overrides into a typed spec registry with validation/parse/serialize. |
src/gateway/core/config.py |
Factors enum allowed-sets into shared constants used by validators + runtime settings. |
src/gateway/api/routes/settings.py |
Extends /v1/settings to return full config view and accept more settable fields. |
docs/public/otari.postman_collection.json |
Updates Settings PATCH example payload and description. |
docs/public/openapi.json |
Regenerates OpenAPI to reflect the new schemas and fields. |
| def _parse(key: str, raw: str) -> SettingValue: | ||
| spec = _SPECS[key] | ||
| if spec.type == "bool": | ||
| return raw.strip().lower() == "true" | ||
| if spec.type == "int": | ||
| return int(raw) | ||
| if spec.type == "float": | ||
| return float(raw) | ||
| if spec.nullable and raw == "": | ||
| return None | ||
| return raw |
| def _redact_url_password(value: str) -> str: | ||
| """Mask the password in a URL's userinfo, leaving the rest intact.""" | ||
| try: | ||
| parts = urlsplit(value) | ||
| except ValueError: | ||
| return value | ||
| if parts.password is None: | ||
| return value | ||
| host = parts.hostname or "" | ||
| if parts.port is not None: | ||
| host = f"{host}:{parts.port}" | ||
| user = parts.username or "" | ||
| netloc = f"{user}:***@{host}" if user else f":***@{host}" | ||
| return urlunsplit((parts.scheme, netloc, parts.path, parts.query, parts.fragment)) |
khaledosman
left a comment
khaledosman
left a comment
There was a problem hiding this comment.
Reviewed the read-only/settable partition closely and it holds: SETTABLE_KEYS derives from _SPECS, UpdateSettingsRequest declares exactly those 15 fields, and the write path is gated at three layers (schema ignores extras, key in SETTABLE_KEYS, validate_value), so there is no path to PATCH a startup-only or secret key. Every SSRF gate and secret is display-only. Null-clear via model_fields_set, commit-then-apply ordering, and fail-safe load_overrides are all correct, and every settable field is read fresh at runtime (verified the discovery-TTL and models.dev cases). Approving. Two Low notes inline and one design question below.
Design question (non-blocking): require_pricing and reject_user_mismatch are now runtime-mutable. Both keep fail-closed defaults and are master-key gated, so this is not an exploitable vuln, but by the docstring's own SSRF-gate rationale ("a security decision should not sit behind a dashboard toggle") these are the §0.4 metering and §0.1 tenant-isolation controls. Worth a docstring note on why they differ from the gates, or a UI confirm step for require_pricing.
Review by Claude Code (Opus 4.8), driven by @khaledosman.
| parts = urlsplit(value) | ||
| except ValueError: | ||
| return value | ||
| if parts.password is None: |
There was a problem hiding this comment.
Low: this masks only parts.password. A credential in the query string (https://sandbox.internal/?api-key=secret) or in the username position (https://TOKEN@host) is returned unredacted, which contradicts the "a secret is never echoed into the response or the browser's query cache" goal for sandbox_url/guardrails_url. database_url's user:pass@ form is fine. Consider masking the full userinfo and stripping/masking known credential query params for _REDACTED_URL_FIELDS.
|
|
||
| const parsed = Number(draft); | ||
| const wellFormed = draft.trim() !== "" && Number.isFinite(parsed) && (isFloat || Number.isInteger(parsed)); | ||
| const valid = wellFormed && parsed >= 0; |
There was a problem hiding this comment.
Low (UX): the Save gate is hardcoded parsed >= 0, but vision_describe_max_tokens and model_discovery_timeout_seconds are declared gt=0, so 0 enables Save then gets rejected with a 422 error banner instead of a disabled button. Thread the spec's gt/ge through ConfigField so the min/allowed-zero is per-field.
GitHub Actions
Inactive
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
web/src/pages/SettingsPage.tsx (1)
103-113: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse an
--otari-*design token instead of rawbg-whiteon the two new inputs. Both settable inputs hardcodebg-white, while the search input in the same file already usesbg-[var(--otari-bg)]. Per the design-token guideline, raw Tailwind palette classes are reserved for status surfaces (ErrorBanner/InfoBanner); everything else should draw fromweb/src/styles/globals.csstokens. Purely cosmetic and totally non-blocking — just keeps the surface colors consistent.
web/src/pages/SettingsPage.tsx#L103-L113: swapbg-whitein theNumberSettinginput'sclassNamefor the appropriate--otari-*surface token.web/src/pages/SettingsPage.tsx#L149-L157: apply the same swap in theTextSettinginput'sclassName.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/pages/SettingsPage.tsx` around lines 103 - 113, Replace the raw bg-white utility with the existing --otari-bg surface token in both NumberSetting (web/src/pages/SettingsPage.tsx lines 103-113) and TextSetting (web/src/pages/SettingsPage.tsx lines 149-157) input className values, matching the search input’s token-based styling.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@web/src/pages/SettingsPage.tsx`:
- Around line 103-113: Replace the raw bg-white utility with the existing
--otari-bg surface token in both NumberSetting (web/src/pages/SettingsPage.tsx
lines 103-113) and TextSetting (web/src/pages/SettingsPage.tsx lines 149-157)
input className values, matching the search input’s token-based styling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2a9d54a8-067e-4c2c-ba47-2cdd62492e42
⛔ Files ignored due to path filters (1)
docs/public/openapi.jsonis excluded by!docs/public/openapi.json
📒 Files selected for processing (15)
docs/public/otari.postman_collection.jsonsrc/gateway/api/routes/settings.pysrc/gateway/core/config.pysrc/gateway/services/runtime_settings_service.pysrc/gateway/static/dashboard/assets/index-Bfs2rd78.jssrc/gateway/static/dashboard/assets/index-DLlAEI3q.csssrc/gateway/static/dashboard/index.htmltests/unit/test_runtime_settings_service.pytests/unit/test_settings_endpoint.pyweb/src/api/types.tsweb/src/components/PricingWarning.test.tsxweb/src/pages/ModelsPage.test.tsxweb/src/pages/ProvidersPage.test.tsxweb/src/pages/SettingsPage.test.tsxweb/src/pages/SettingsPage.tsx
…table set, and search Expands the dashboard Settings page from two toggles into a full grouped view of every effective gateway setting, and widens the runtime-settable set to every field that can be safely hot-changed. Backend: - Rework runtime_settings_service around a per-key spec registry (bool / int / float / enum-string / nullable-string). Each key carries the constraints its GatewayConfig field declares. - Widen the settable set from 2 to 15 fields, each read fresh from config on the request path (or next discovery). Startup-only fields and the outbound network-safety gates stay read-only; require_pricing / reject_user_mismatch are settable with a documented rationale. - GET /v1/settings returns a grouped read-only full-config view; secrets and complex catalog fields are omitted, and URL fields have credentials redacted (userinfo password/token, IPv6-safe, all query values masked). - PATCH uses model_fields_set so an explicit null clears a nullable field. - Settable numeric fields expose ge/gt bounds so the dashboard gates input. Dashboard: - Rebuild the Settings page as grouped sections rendered from the config view, with per-type controls and a startup-only marker for read-only fields. - Add fuzzy search (key/description/group + key subsequence), a settable-only filter, per-group counts, and an empty state. Regenerated OpenAPI spec, Postman collection, and dashboard bundle. Fixes #307 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
94258dd to
3b31f02
Compare
GitHub Actions
Inactive
Description
Expands the dashboard Settings page from two toggles into a full, grouped view of every effective gateway setting, and widens the runtime-settable set to every field that can be safely hot-changed.
Backend
runtime_settings_servicearound a per-key spec registry (bool / int / float / enum-string / nullable-string), replacing the previous bool-only path. Each key carries the same constraints itsGatewayConfigfield declares, so a dashboard write cannot push the gateway into a state a fresh config load would reject.require_pricing,reject_user_mismatch,model_cache_ttl_seconds,stream_missing_usage_policy,model_discovery_timeout_seconds,model_discovery_negative_ttl_seconds,models_dev_metadata,models_dev_cache_ttl_seconds,file_understanding_enabled,vision_strategy,vision_describe_model,vision_describe_max_tokens,budget_estimate_default_output_tokens.host,port,database_url,mode,db_pool_*,rate_limit_rpm,cors_allow_origins,budget_strategy, docs/metrics, log writer), and the outbound network-safety gates (mcp_allow_*,web_search_allow_private_hosts, the*_urlfields), since opening an SSRF gate should not sit behind a dashboard toggle.rate_limit_rpmin particular is consumed once at startup to buildapp.state.rate_limiter, so mutating config alone would not change running behavior.GET /v1/settingsnow returns a grouped, read-only "full config" view of every non-secret effective setting, each marked settable or startup-only, with the field's description and (for enums) its allowed options sourced from the config schema. Secrets (master_key, provider credentials) and the complex catalog fields managed on their own pages (providers, pricing, aliases, model_capabilities, platform) are omitted.PATCHusesmodel_fields_set, so an explicitnullclears a nullable field (vision_describe_model) while an omitted field is left unchanged; invalid keys or values are rejected before any write.Dashboard
model_cache_ttl_seconds; "/" focuses it, Esc clears), a "settable only" filter, a visible-count line, per-group counts, and an empty state, so the ~47-field page stays navigable.Tests
apply_override; the GET full-view shape; PATCH apply for bool/int/float/enum plus nullable clear-to-null; ignore of startup-only fields; 422 on bad input; and a sqlite restart test that verifies the serialize to DB then parse then apply roundtrip for the new (non-bool) types, plusload_overridesskipping stale/invalid rows. The Postgres integration test that already covered "override survives a restart" for the bool flags continues to run in CI; the new sqlite test mirrors that promise locally and extends it to the new types.fieldMatcheshelper, each control type (including float decimals and clear-to-null), the search and settable-only filters, and the empty state.Regenerated OpenAPI spec and dashboard bundle are included.
PR Type
Relevant issues
Fixes #307 (part of #299).
Checklist
tests/unit,tests/integration).make lint,make typecheck,make test).uv run python scripts/generate_openapi.py).AI Usage
AI Model/Tool used: Claude Code (Opus 4.8)
Any additional AI details you'd like to share:
Written by Claude Code via back-and-forth with @njbrake; the design decisions (which fields are safe to hot-change, keeping SSRF gates and startup-wired fields read-only) are his, driven by review of each field's runtime read site.
NOTE:
When responding to reviewer questions, please respond yourself rather than copy/pasting reviewer comments into an AI and pasting back its answer. We want to discuss with you, not your AI :)
Summary