Skip to content

fix(vision): honor modelCapabilities text-only on native rows (#4501) - #4511

Merged
lidge-jun merged 1 commit into
lidge-jun:devfrom
maoxin1234:fix/vision-native-model-capabilities
Sep 13, 2026
Merged

fix(vision): honor modelCapabilities text-only on native rows (#4501)#4511
lidge-jun merged 1 commit into
lidge-jun:devfrom
maoxin1234:fix/vision-native-model-capabilities

Conversation

@maoxin1234

@maoxin1234 maoxin1234 commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Fixes Vision describer eligibility ignores operator modelCapabilities on native rows #4501.
  • In src/vision/eligibility.ts (modelAcceptsImageInputWithCache), pass nativeProvider directly to isModelVisionSidecarConsumer instead of an object literal that omits modelCapabilities.
  • Check nativeProvider.modelCapabilities?.[candidate.id]?.inputModalities on native rows before falling back to nativeInputModalities(candidate.id) ?? true.
  • When an operator configures a native model text-only via modelCapabilities[id].inputModalities = ["text"], the model is properly disqualified from vision describer eligibility.

Verification

  • bun test tests/vision/vision-eligibility.test.ts — 20 passed, 0 failed.
  • bun test tests/vision/sidecar-settings-vision-filter.test.ts — 14 passed, 0 failed.
  • bun run typecheck — passed with 0 errors.
  • bun run privacy:scan — passed.
  • git diff --check — passed.
  • Added regression test cases 15 & 16 covering native candidate disqualification and baseline option dropping under text-only modelCapabilities.

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.

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.

@github-actions

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Sep 13, 2026
@github-actions

github-actions Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

✅ READY

  • all PR quality gates passed; the review readiness checklist is complete.

Review readiness checklist

  • ✅ 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.

4/4 boxes ticked.

This pull request is already Ready for Review.
The review-ready label marks this PR as ready; review automation runs independently.
Maintainers notified: @lidge-jun @Ingwannu

@github-actions
github-actions Bot marked this pull request as draft September 13, 2026 12:50
@coderabbitai

coderabbitai Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 233ec1de-0aae-4807-9555-6aacbd00423b

📥 Commits

Reviewing files that changed from the base of the PR and between 8e6c996 and 6d92624.

📒 Files selected for processing (2)
  • src/vision/eligibility.ts
  • tests/vision/vision-eligibility.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

Changes

Vision eligibility

Layer / File(s) Summary
Capability-aware native model eligibility
src/vision/eligibility.ts, tests/vision/vision-eligibility.test.ts
At lines 159–162, native eligibility passes the enriched provider to isModelVisionSidecarConsumer. Explicit modelCapabilities input modalities now override nativeInputModalities. Tests 15 and 16 verify text-only native and baseline models are excluded from vision eligibility.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 6d926

Text-only native models are no longer eligible as vision describers when explicitly configured that way. No actionable merge risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR meets the coding requirements in #4501. In src/vision/eligibility.ts, modelAcceptsImageInputWithCache enriches the candidate provider, passes that provider to isModelVisionSidecarConsumer
Out of Scope Changes check ✅ Passed The reviewed changes stay within #4501. The source change is limited to native vision eligibility and capability-override precedence in src/vision/eligibility.ts. The added tests in `tests/vision/vi…
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: honoring modelCapabilities text-only settings for native vision model rows.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@maoxin1234
maoxin1234 marked this pull request as ready for review September 13, 2026 12:53
@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 71 / 80

이 PR은 vision describer 자격 심사에서 native 모델의 modelCapabilities text-only 선언을 무시하던 구멍(#4501)을 막습니다. 지금 devsrc/vision/eligibility.ts modelAcceptsImageInputWithCache native 분기를 보면, isModelVisionSidecarConsumer{ noVisionModels, modelInputModalities } 객체 리터럴만 넘깁니다. 그 리터럴에는 modelCapabilities가 없어서, 함수 안의 provider.modelCapabilities?.[id].inputModalities 검사가 native 경로에서는 항상 건너뜁니다. 그 다음엔 nativeInputModalities(candidate.id) ?? true로 떨어져, 카탈로그 기본값이 이미지를 허용하면 운영자가 text-only로 막아도 describer 후보로 남습니다.

PR은 두 가지를 합니다. 첫째, nativeProvider 전체를 isModelVisionSidecarConsumer에 넘겨 sidecar consumer 판정이 capabilities를 보게 합니다. 둘째, native 행에서 modelCapabilities[id].inputModalities가 명시되어 있으면 그 선언이 image를 포함하는지로 바로 판정하고, 없을 때만 기존 nativeInputModalities 폴백을 씁니다. 테스트 15/16이 native 후보 탈락과 baseline 옵션 제거를 고정합니다.

비-native 경로는 원래부터 modelCapabilities를 보고 있었습니다. 이번 수정은 native/openai slug 경로를 같은 계약으로 맞추는 정렬입니다. types/config 분할 캠페인과는 무관하고, 파일도 eligibility + 테스트 두 개뿐입니다.

라인 - src/vision/eligibility.ts native 분기 Object.hasOwn(...modelCapabilities...) - 선언이 있으면 image 포함 여부로 확정한다. inputModalities: [] 빈 배열도 text-only로 떨어져 false가 된다. 의도와 맞는지 확인.
라인 - isModelVisionSidecarConsumer(nativeProvider, ...) - Pick 타입이 이미 modelCapabilities를 포함하므로 시그니처는 맞다. nativeProvider가 undefined일 때 짧은 회로도 유지된다.
경로/심볼 - tests/vision/vision-eligibility.test.ts case 15/16 - gpt-5.4-mini / baseline openai text-only 시나리오는 #4501 재현에 충분하다. DeepSeek Flash vision(#4500) 같은 최근 vision 방향과 충돌하지 않는지 tip에서 한 번만 보면 된다.

메인테이너의 판단이 필요한 지점

  • native 기본값 ?? true(메타데이터 없으면 이미지 허용)를 계속 둘지, 더 보수적으로 바꿀지(이 PR 범위 밖)
  • 운영자 문서에 "native 행도 modelCapabilities가 describer 자격을 이긴다"를 한 줄 넣을지

너의 추천
merge. 회귀 테스트가 핵심을 잠그고 변경량이 작다. #4501과 같이 닫으면 된다.

이 댓글은 grok-bot이 작성했습니다

@lidge-jun lidge-jun left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Approving on exact-head evidence.

Cross-platform CI run 34758154482 completed success at 6d926244101cb6234b66634464007268a06b1d89, which is the current head of this PR. That run had never executed before today: the fork workflow sat at action_required since it was opened, so this is the first time the change has been verified by repository CI rather than by author report.

On the change itself: the native arm of modelAcceptsImageInputWithCache now consults exact modelCapabilities[id].inputModalities before falling back to nativeInputModalities, which mirrors the ordering the non-native arm already used. An operator who declares a native row text-only through ocx provider edit --text-only is now honored for describer eligibility instead of having to reach for noVisionModels.

One forward-looking note rather than a change request: a separate lane is adding live inputModalities to Devin catalog rows from the provider catalog. The two compose — discovered modalities land on the row, and this change keeps the operator declaration ahead of them — so no precedence conflict is expected.

@lidge-jun
lidge-jun merged commit 10d61fc into lidge-jun:dev Sep 13, 2026
36 of 38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working review-ready

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants