feat(dashboard): expose remaining vision sidecar controls - #1561
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (16)
📝 WalkthroughWalkthroughThe PR adds Dashboard controls for Vision sidecar enablement, description limits, and request timeouts. It extends management API serialization, validation, and persistence, centralizes runtime timeout bounds, adds GUI and API tests, and updates localized documentation. ChangesVision sidecar controls
Estimated code review effort: 4 (Complex) | ~45 minutes Mergeability Score: 🟡 Moderate · up to The PR adds Vision sidecar controls without a reported product-impact defect, but the full test suite previously failed and the branch is not confirmed up to date with dev, so merge readiness remains unproven until those checks are resolved or explicitly accepted. Sequence Diagram(s)sequenceDiagram
participant Dashboard
participant SidecarSettingsAPI
participant ConfigStore
participant VisionRuntime
Dashboard->>SidecarSettingsAPI: Submit Vision settings patch
SidecarSettingsAPI->>SidecarSettingsAPI: Validate fields and timeout bounds
SidecarSettingsAPI->>ConfigStore: Persist enabled state and timeout
SidecarSettingsAPI-->>Dashboard: Return resolved Vision settings
VisionRuntime->>ConfigStore: Resolve Vision timeout
ConfigStore-->>VisionRuntime: Return bounded timeout
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
|
✅ Deterministic PR hygiene checks passed. |
✅ READY
Review readiness checklist
✅ 4/4 boxes ticked. This pull request is already Ready for Review. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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.
Inline comments:
In `@docs-site/src/content/docs/ru/guides/sidecars.md`:
- Around line 146-147: Update the preserved-settings sentence in the Russian
sidecars documentation to replace “прежние модель, бэкенд, reasoning, таймаут и
лимит” with the grammatically correct “прежние значения модели, бэкенда,
reasoning, таймаута и лимита”.
In `@gui/src/pages/dashboard-shared.ts`:
- Around line 213-220: Export DEFAULT_MAX_DESCRIPTIONS_PER_TURN from the runtime
vision module and replace the hardcoded 8 assigned to
VISION_MAX_DESCRIPTIONS_DEFAULT in dashboard-shared.ts with that exported
runtime default, keeping the GUI and runtime values synchronized.
In `@gui/tests/vision-sidecar-dashboard.test.tsx`:
- Around line 24-38: The hydration test fixture and assertions use values
identical to the defaults, so they cannot prove server values are applied.
Update initialSidecar and the test “Dashboard hydrates enabled, max
descriptions, and timeout from the server” to use distinct
maxDescriptionsPerTurn and timeoutMs values such as 12 and 30_000, then assert
those rendered inputs show the fixture values; optionally add a separate
undefined-values case to verify default fallback.
In `@structure/05_gui-and-management-api.md`:
- Line 111: Update the Sidecar/shadow-call settings entry to include
vision.reasoning in the PUT-accepted fields and add the vision reasoning value
to the reported GET and PUT-response fields, matching
publicVisionSidecarSettings and the config-routes handler.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8d19152e-d04a-4353-8fab-9e60e4adfbcf
⛔ Files ignored due to path filters (5)
docs/pr-assets/1201-vision-1280.pngis excluded by!**/*.pngdocs/pr-assets/1201-vision-1440.pngis excluded by!**/*.pngdocs/pr-assets/1201-vision-420.pngis excluded by!**/*.pngdocs/pr-assets/1201-vision-768.pngis excluded by!**/*.pngdocs/pr-assets/1201-vision-sidecar-controls.pngis excluded by!**/*.png
📒 Files selected for processing (33)
docs-site/src/content/docs/guides/sidecars.mddocs-site/src/content/docs/ja/guides/sidecars.mddocs-site/src/content/docs/ja/reference/configuration/server.mddocs-site/src/content/docs/ko/guides/sidecars.mddocs-site/src/content/docs/ko/reference/configuration/server.mddocs-site/src/content/docs/reference/configuration/server.mddocs-site/src/content/docs/ru/guides/sidecars.mddocs-site/src/content/docs/ru/reference/configuration/server.mddocs-site/src/content/docs/zh-cn/guides/sidecars.mddocs-site/src/content/docs/zh-cn/reference/configuration/server.mddocs-site/src/content/docs/zh-tw/guides/sidecars.mddocs-site/src/content/docs/zh-tw/reference/configuration/server.mdgui/src/i18n/de.tsgui/src/i18n/en.tsgui/src/i18n/ja.tsgui/src/i18n/ko.tsgui/src/i18n/ru.tsgui/src/i18n/tr.tsgui/src/i18n/zh-TW.tsgui/src/i18n/zh.tsgui/src/pages/dashboard-overview-sections.tsxgui/src/pages/dashboard-shared.tsgui/src/styles-dashboard-workspace.cssgui/tests/vision-sidecar-controls.test.tsgui/tests/vision-sidecar-dashboard.test.tsxsrc/server/management/config-routes.tssrc/vision/index.tssrc/vision/timeout-bounds.tsstructure/05_gui-and-management-api.mdtests/sidecar-settings-vision-controls.test.tstests/vision-anthropic.test.tstests/vision-cache.test.tstests/vision-sidecar-timeout-bounds.test.ts
|
Addressed the CodeRabbit review on
Follow-up commit: |
Wibias
left a comment
Wibias
left a comment
There was a problem hiding this comment.
Re-review on exact head d9cd5baebbfe537c835769a9b394aa6827b08025: I do not see a remaining code blocker in the Vision-sidecar changes.
The earlier CodeRabbit points are addressed on current code: the hydration test now uses distinct server values (12 / 30000), the Russian wording and vision.reasoning documentation are corrected, and the mirrored GUI max-description default is protected by a cross-layer contract test against DEFAULT_MAX_DESCRIPTIONS_PER_TURN rather than importing the full Vision runtime into the GUI bundle. The management API also preserves partial-update semantics, validates enabled/limit/timeout strictly, and returns resolved defaults consistently.
I am not approving this stale draft head yet: it is currently 15 commits behind dev (6c14e343), and Cross-platform CI is still in progress. Rebase onto current dev, rerun exact-head CI, then this is ready for final approval if the rebase stays clean.
Let operators toggle Vision sidecar, cap descriptions per turn, and set timeout from the Dashboard without clobbering model, backend, or reasoning on unrelated saves. Co-authored-by: Cursor <cursoragent@cursor.com>
…ontrols The two number fields made the control group wider than the card, and a zero flex basis collapsed the title into a one-glyph column. Stack the card when that happens, import timeout bounds from one runtime module, and drive the model/reasoning GUI test through real Select interaction. Co-authored-by: Cursor <cursoragent@cursor.com>
Use non-default fixture values so hydration is actually proven, pin the description-cap default to the runtime export, document vision.reasoning on the management API, and fix Russian agreement in the sidecars guide. Co-authored-by: Cursor <cursoragent@cursor.com>
53bd0d2 to
938a517
Compare
|
Tip For best results, initiate chat on the files or code changes.
The PR is marked Ready for Review. The 🧠 Learnings usedYou are interacting with an AI system. |
|
Tip For best results, initiate chat on the files or code changes.
🧠 Learnings usedYou are interacting with an AI system. |
2 similar comments
|
Tip For best results, initiate chat on the files or code changes.
🧠 Learnings usedYou are interacting with an AI system. |
|
Tip For best results, initiate chat on the files or code changes.
🧠 Learnings usedYou are interacting with an AI system. |
|
Thanks @LeoWang331 for the work on this! This is genuinely useful because it exposes the remaining Vision sidecar controls directly in the Dashboard instead of forcing users to edit config manually. In particular, being able to disable Vision without losing the selected model/settings, plus having the per-turn description limit and timeout available in the UI, makes the sidecar much easier and safer to tune in normal use. The partial-update behavior and regression coverage are also nice additions. |
2 participants
Summary
Dashboard and
GET/PUT /api/sidecar-settingsnow expose the remaining Vision sidecar controls from #1201:enabled,maxDescriptionsPerTurn, andtimeoutMs, alongside the existing model, backend, and reasoning fields.enabledmust be a boolean;maxDescriptionsPerTurnmust be a positive integer;timeoutMsuses the single runtime contract insrc/vision/timeout-bounds.ts(1to2147483647). Invalid input returns{ error }with HTTP 400.gui/src/pages/dashboard-shared.ts.VISION_MAX_DESCRIPTIONS_DEFAULTstill mirrors the runtime default and is pinned by the same contract test.tests/vision-sidecar-timeout-bounds.test.tspins both.Layout follow-up: a real Dashboard at 1440 / 1280 / 768 / 420 showed the extra number fields collapsing the title into a one-glyph column. The card now wraps to a stacked copy-then-controls layout instead of shrinking the hint.
Closes #1201
1440:
Verification
Targeted management API (pass, ~3.6s):
bun test tests/sidecar-settings-vision-controls.test.ts tests/vision-sidecar-timeout-bounds.test.ts tests/vision-anthropic.test.ts tests/vision-cache.test.ts-- 32 passGUI (pass):
cd gui && bun test tests/vision-sidecar-controls.test.ts tests/vision-sidecar-dashboard.test.tsx-- 8 pass (model/reasoning coverage clicks the real Select controls; hydration uses non-default 12 / 30000)cd gui && bun test tests-- 781 pass, 0 fail, ~59s (run on the layout commitd1941dadb; not re-run after the CodeRabbit follow-up)cd gui && bun run lint-- passcd gui && bun run lint:i18n-- passcd gui && bun run build-- passRepo:
bun run typecheck-- passbun run privacy:scan-- passgit diff --check-- pass (no whitespace errors)Docs:
cd docs-site && bun install --frozen-lockfile-- passcd docs-site && bun run build-- pass (265 pages)bun run test(full suite, one earlier attempt): did not pass. Exit code 3 after ~1028s. About 169(fail)lines, then Bun 1.3.14 crashed duringtests/codex-native-residue.test.ts. Not re-run. The local-CI checklist box stays unchecked.CodeRabbit (ASSERTIVE) posted 4 comments on
d1941dadb. Three were applied (Russian agreement, hydration fixture using non-default values,vision.reasoningon the management-API doc). The fourth asked the Dashboard to importDEFAULT_MAX_DESCRIPTIONS_PER_TURNfromsrc/vision/index.ts; that would pull the vision runtime into the GUI bundle, so the GUI still mirrors8and the existing contract test now pins it to the runtime export.Fork CI (
Cross-platform CI,React Doctor) isaction_requireduntil a maintainer approves the workflows. This PR stays Draft.Checklist
Review readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
All CI tests are green on my local testing.
I pushed my PR to the latest dev commit.
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.
Summary by CodeRabbit
New Features
Documentation