Skip to content

feat(combos): enroll text-only members in vision sidecar (carry #4932) - #7162

Merged
lidge-jun merged 7 commits into
devfrom
carry/4932-vision-enrollment
Oct 15, 2026
Merged

lidge-jun merged 7 commits into
devfrom
carry/4932-vision-enrollment

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Oct 15, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • Carry @x3M3x's feat(combos): enroll text-only members in vision sidecar from the UI #4932 from source head c9cda1b5a95292bc280ce1ec7dd942820c08cba1 onto dev at 7640a942b35044c974e534b84d5a8049f4a8ae9a, following the maintainer carry direction and the author's authoritative consumer-flag correction.
  • A mixed combo can now turn on Image / multimodal when its non-image members are known text models. On save, those members are declared text-only (modelCapabilities[model].inputModalities = ["text"]) so the Vision Sidecar describes their images. The request field is visionSidecarTargets on PUT /api/combos; it is validated against the submitted combo's exact targets and is never stored on the combo.
  • Existing declarations are never overwritten. A target that already accepts images, or whose capability axis, custom row, or provider modelInputModalities hint lacks text (for example audio-only), is rejected with 400. Targets the sidecar already covers are skipped. Unknown or audio-only members keep the switch off and are named in the hint; while any member blocks images, the dashboard sends no enrollment at all.
  • The combo write, rename reference migrations, disabled-selector rewrite, and capability patches commit in one provider-patch transaction. A failure before publication restores all of them; a failure after publication keeps live and disk in agreement and is reported as an error.
  • /api/models rows carry the runtime's visionSidecarConsumer verdict, so the GUI classification and the disabled-sidecar warning survive a reload. The listing shares one registry enrichment per provider, keeping a 2000-row listing at about 1 ms for this field. The warning (amber: requests continue) says images become omission notes while the sidecar is off. All 11 GUI locales, the combos and sidecars guides, and the structure docs are updated; the core/Lab boundary is unchanged.

Co-authored-by: x3M3x 98298256+x3M3x@users.noreply.github.com

Verification

Exact head c09715c43eedeffd5725a892bd108408e4cd8752. Full cross-platform CI was dispatched on the branch: https://github.com/lidge-jun/opencodex/actions/runs/38716102808 (queued at handoff; no CI pass claimed). PR fast subset: https://github.com/lidge-jun/opencodex/actions/runs/38716111918.

Every test and proxy ran with throwaway OPENCODEX_HOME and CODEX_HOME (screenshot proxy also isolated CLAUDE_CONFIG_DIR, non-default ports 19592/19594).

  • bun run test:changed (OCX_TEST_NO_QUEUE=1) at 061542ea4 (review fixes, before the listing-cache commit): 8623 pass, 32 skip, 0 fail across 382 files. At 4fec400fc (pre-review head) the same run had 1 failure: tests/providers/minimax-combo-image-input.test.ts still expected a text-only member to block images, which this PR deliberately changes; the test now asserts sidecar coverage and that an audio-only member still blocks.
  • bun run test:changed at final head c09715c43 (the eligibility module widens the graph to 1331 files): 29965 pass, 39 skip, 11 fail, 4 errors. Attribution: tests/cli/sibling-home-client-sync.test.ts fails identically on clean origin/dev (32/1 on both). tests/service/service.test.ts (211/0), tests/service/service-sqlite-home.test.ts (5/0), tests/cli/cli-access-audio-live.test.ts (36/0) and tests/server/server-live-realtime-fixtures.test.ts (8/0) pass standalone on both this branch and clean origin/dev; their failures were timeouts under concurrent host load. None touch combo or vision code.
  • Focused files, each in its own process: combo enrollment, combo management API, combo workspace data, model settings API, MiniMax combo image input, core/Lab boundary, test layout (both), file-size ratchet, and every test importing src/vision/eligibility or listManagementModelRows: 722 pass, 0 fail.
  • The new provider-hint regression in tests/routing/combo-vision-enrollment.test.ts fails without the route change (9 pass / 1 fail) and passes with it. The earlier custom-audio regression was likewise observed failing before its fix.
  • Listing cost: 2000 isVisionSidecarConsumer row lookups took ~170 ms; the shared-cache classifier takes ~0.7 ms with identical verdicts.
  • bun run typecheck, bun run lint:gui, bun run lint:i18n (from gui/), bun run build:gui, bun run privacy:scan, bun run structure:check, git diff --check — all passed.
  • Docs: from docs-site/, bun install --frozen-lockfile then bun run build passed at 4fec400fc (577 pages, 81,202 internal links); review commits do not touch docs-site/.
  • Real GUI: a sandboxed proxy served the built GUI with a local fixture provider (vision + text model) and the Vision Sidecar off. With Chrome Headless Shell 154 driven by Playwright Core (installed /Applications/Google Chrome.app stalls at launch on this host), the mixed combo shows the switch enabled, the enrollment hint naming demo/text-model, and the disabled-sidecar warning in the amber token (computed rgb(154, 74, 8)). Save/reload enrollment against the real PUT /api/combos was exercised by the earlier smoke at 4fec400fc; the review commits change only the blocked-member gate and styling on the GUI side, both covered by unit tests above.
  • Management API: no new routes; PUT /api/combos keeps its existing authentication and origin checks. Enrollment validates exact combo membership and existing declarations before any mutation. No credentials, dependencies, workflow permissions, or release behavior changed.

Before (origin/dev, real GUI build): the text-only member keeps the image switch unavailable.

Before: mixed text-only combo has an unavailable image switch

After (this branch at c09715c43, real GUI build): the switch is on, the hint names the member to enroll, and the amber warning explains the disabled sidecar.

After: image switch enables sidecar enrollment with an amber disabled-sidecar warning

Screenshots live only on pr-assets, outside this PR branch.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Summary by CodeRabbit

  • New Features
    • Combos with image input can now use the Vision Sidecar for eligible text-only targets. The Dashboard identifies targets that cannot be covered and explains why.
    • When the sidecar is disabled, requests continue without images for covered targets, with an omission note.
    • Sidecar declarations remain in place when a target is removed from a combo.
  • Documentation
    • Updated combo, sidecar, catalog, and configuration guides with enrollment requirements and image-handling behavior.

Follow-up on b7393ffe4 (CI react-doctor + stale GUI tests)

  • cf6000364: React Doctor (PR gate, blocking=warning) flagged two lines this PR added: the sidecar-disabled hint used an in-page <a href="#dashboard"> (anchor-target-exists) and the modality parse looped twice (js-flatmap-filter). The hint now uses a ghost button with navigateHash("dashboard"), like other dashboard links, and the parse is a single flatMap. Local react-doctor@0.9.11 --scope changed --blocking warning: both findings gone.
  • b7393ffe4: gui/tests/combo-capability-hidden-targets.test.tsx still asserted the pre-PR rule that text-only members force the Image switch off (4 failures at c09715c43, 0 on dev). Updated to the intended behavior: text-only and mixed image/text members stay on through the Vision Sidecar; unknown, audio-only and text+audio-only members still force it off (new cases); cached capability rows include visionSidecarConsumer.
  • cd gui && bun run test: 3330 pass / 0 fail (342 files). GUI typecheck and bun run lint clean.
  • Cross-platform CI on the new head: https://github.com/lidge-jun/opencodex/actions/runs/38730701255

Port c9cda1b onto current dev, preserving atomic enrollment, runtime consumer classification and rename rollback. Keep new regressions in a registered sibling file.

Co-authored-by: x3M3x <98298256+x3M3x@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: cdbd90b1-8a33-4ace-88a1-5d71c23e6106
📥 Commits

Reviewing files that changed from the base of the PR and between c09715c and 7eff9c1.

📒 Files selected for processing: 5
  • gui/src/components/combo-workspace-controls.tsx
  • gui/src/pages/Combos.tsx
  • gui/tests/combo-capability-hidden-targets.test.tsx
  • src/server/management/combo-routes.ts
  • tests/routing/combo-vision-enrollment.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.

📝 Walkthrough

The Dashboard now identifies text-only combo targets that the Vision Sidecar can cover and submits eligible targets during combo saves. The management API validates enrollment requests and commits text-only declarations with combo changes. Model rows expose the Sidecar consumer classification.

Changes: Combo Vision Sidecar enrollment

Layer / File(s) Summary
Classify Vision Sidecar consumers
src/vision/eligibility.ts, src/server/management/model-rows.ts, tests/routing/combo-vision-enrollment.test.ts
A shared classifier marks Sidecar consumers in management model rows. Tests cover legacy and registry-seeded consumers.
Validate and commit enrollment
src/server/management/combo-routes.ts, tests/routing/combo-vision-enrollment.test.ts, structure/config.md, structure/catalog.md, structure/gui-and-management-api.md, scripts/test-layout/layout.json, tests/fixtures/test-layout-expected.json
PUT /api/combos validates request-only enrollment targets and commits text-only declarations with combo and reference changes. Tests cover validation, persistence, and save outcomes.
Select and submit eligible targets
gui/src/combo-capabilities.ts, gui/src/combo-workspace-data.ts, gui/src/components/combo-workspace-*, gui/src/pages/Combos.tsx, gui/src/i18n/*, gui/src/styles-combos-workspace.css, tests/gui/combo-workspace-data.test.ts, tests/providers/minimax-combo-image-input.test.ts, gui/tests/combo-capability-hidden-targets.test.tsx, docs-site/src/content/docs/guides/*, structure/dashboard-and-usage.md
The Dashboard classifies targets by modality and Sidecar status, names targets that block image input, and submits eligible targets when image input is enabled. The interface reports Sidecar state. Tests and guides describe enrollment and image handling.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant Dashboard
  participant ComboRoute
  participant commitProviderPatch
  participant Config
  Dashboard->>ComboRoute: PUT combo and visionSidecarTargets
  ComboRoute->>commitProviderPatch: Submit combo and text-only capability patches
  commitProviderPatch->>Config: Publish combo and provider changes
Loading

Possibly related PRs

Suggested reviewers: wibias

Merge Risk: ⚪ Minimal · up to 7eff9

No confirmed issue prevents merging after normal checks.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 45.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 20 functions across 25 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the primary change: enrolling text-only combo members in the Vision Sidecar.
  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

GOAT and others added 2 commits October 15, 2026 18:44
- Send no visionSidecarTargets while a blocked member forces the image
  switch off, so a save never declares the remaining members text-only.
- Reject enrollment of a model whose provider modelInputModalities hint
  lacks text instead of overriding it with a text-only declaration.
- Update the MiniMax combo picker test to the sidecar-coverage contract.
- Align pt/vi unavailable hints with English, revert an unrelated fr
  apostrophe change, and style the disabled-sidecar warning with the
  amber warning token.

Co-authored-by: x3M3x <98298256+x3M3x@users.noreply.github.com>
GET /api/models classified every row with isVisionSidecarConsumer, which
rebuilds the registry-enriched provider per call (~0.1 ms/row, ~170 ms for
a 2000-row listing). A per-listing classifier reuses the eligibility
module's existing enrichment cache, so verdicts are unchanged.

Co-authored-by: x3M3x <98298256+x3M3x@users.noreply.github.com>
@lidge-jun
lidge-jun marked this pull request as ready for review October 15, 2026 09:59
@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner October 15, 2026 09:59
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-15T10:03:40.945328Z c09715c Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c09715c43e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/server/management/combo-routes.ts
@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

GOAT and others added 3 commits October 15, 2026 20:51
…ngle modality pass

Co-authored-by: x3M3x <x3M3x@users.noreply.github.com>
…h on

Text-only and mixed image/text members are covered by the Vision Sidecar, so the switch stays enabled; unknown and audio-only members still force it off. The cached capability rows now carry visionSidecarConsumer.

Co-authored-by: x3M3x <x3M3x@users.noreply.github.com>
…ng sidecar members

A provider already at the modelCapabilities cap accepted a valid one-entry patch and persisted an oversized map that load-time sanitization then truncated, dropping the new text-only declaration. The route now validates the merged map (400, nothing written) and re-checks it under the config lock.

Co-authored-by: x3M3x <x3M3x@users.noreply.github.com>
@github-actions github-actions Bot added the enhancement New feature or request label Oct 15, 2026
@lidge-jun

Copy link
Copy Markdown
Owner Author

Maintainer integration into dev (MAINTAINERS.md dev maintainer-integration policy; batch integration authorized by @lidge-jun on 2026-10-15).

  • Head: ab7f851581edb199b17a5a301de1d0bc3670e23a (tree 2bd0e2a8e9dcc9fc02f05bb8f6d131588bdc0e10) = this branch merged with origin/dev 318e4833f3.
  • Exact-head evidence: GitHub-hosted Actions for this account have not been assigning runners since 2026-10-15 13:30 UTC, so by owner decision the evidence is a full bun run test (Bun 1.4.3 test runner, root and GUI dependencies installed, GUI built) of this exact tree on a maintainer host (macOS arm64): exit 1, 10 min, 4 failing test(s), each also failing in the same host's baseline run of dev 60bf4c7 (environment-only): a secondary ensure parent preserves shared Grok, Codex and Claude agent bytes, connected Desktop uses hub IDs across a cold restart (stored profile=false), live audio readiness and bounded closure > native open and non-readiness events cannot cla, update stops the running proxy before replacing files > npm launcher restarts the stopped .
  • Local gates on the same tree: bun run typecheck, file-size ratchet, test-layout and structure:check.
  • Hosted CI checks for this head may still be queued; they are not claimed as evidence.

@lidge-jun
lidge-jun merged commit b538531 into dev Oct 15, 2026
2 checks passed
@lidge-jun
lidge-jun deleted the carry/4932-vision-enrollment branch October 15, 2026 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant