fix(devin): preserve catalog supports_images as a tri-state - #4547
Conversation
parseCatalogBuffer had arms for ClientModelConfig fields 1, 4, 18 and 22 and no default, so field 5 (supports_images) was dropped by omission. Carry it on ModelCatalogEntry as an optional boolean: a present true asserts text+image support, a present false asserts text-only, and an omitted field stays unknown. It deliberately does not copy the disabled pattern, which defaults to false — collapsing unknown into text-only was the #1796 regression, and antigravity-models.ts already implements the same tri-state for its discovered catalog. The header schema comment gains the #5 row and the #18 row it never listed, and its verification claim now says which fields came from the bundled extension.js, which from a live catalog dump, and which from the public WindsurfAPI documentation. The owning structure doc records the catalog pre-flight contract. Propagation of the flag to the client catalog is a separate change.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 1 remains after this review. 📝 WalkthroughWalkthroughThe Devin cloud-direct catalog now parses ChangesDevin catalog image support
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~8 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to This narrowly scoped metadata change has no confirmed impact on client-visible routing or model capabilities. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
리뷰 · 우선순위 58 / 80이 PR은 Devin cloud-direct 카탈로그 파서 왜 중요하냐면, 이 모듈의 원래 목적은 이슈 #14 계열입니다. 계정 티어에 없는 삼상태를 고른 이유는 레포 안에서 이미 검증된 교훈입니다. 헤더 스키마 주석도 같이 고칩니다. 예전에는 #1/#4/#22만 적혀 있었고 #18은 코드에서만 파싱됐습니다. 이번엔 #5와 #18을 주석에 올리고, 검증 출처를 필드별로 나눕니다(#1/#4/#22는 번들 types.ts/config.ts 분할 캠페인과는 겹치지 않습니다. 닫을 중복도 아닙니다. #4501/#4511은 네이티브 행 eligibility 소비자 쪽이고, 이 PR은 Devin protobuf 파서 공급자 쪽이라 서로 대체 관계가 아닙니다.
스키마 주석의 #5 출처 - #18은 라이브 덤프인데 #5는 공개 WindsurfAPI 문서 교차확인입니다. Cognition CI - 로컬 스위트/설치를 안 돌린 라운드라, 머지 판단의 유일한 자동 증거는 호스트 CI입니다. 리뷰 작성 시점엔 checks가 pending이었습니다. 그린 나오기 전에 랜딩하지 마세요.
메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7f94c524bd
ℹ️ 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".
| } else if (sf.num === 5 && sf.wire === 0) { | ||
| // #5 = supportsImages (bool, varint 0/1). Absent stays unknown — see | ||
| // ModelCatalogEntry; do not default it like disabled. | ||
| supportsImages = sf.value === 1n; |
There was a problem hiding this comment.
This changes the shared src/adapters/ catalog contract, but inspection of the commit shows that only structure/adapters/registry.md was updated; the other documents mapped to src/adapters/ in structure/INDEX.md—including runtime.md, the three transport documents, data-planes/inbound-compat.md, providers/cursor.md, and providers/chat-compat.md—remain unchanged. Update every mapped document in this change, or correct the manifest ownership if those documents should not own this area, so the repository's source-to-document contract does not drift.
AGENTS.md reference: src/AGENTS.md:L11-L11
Useful? React with 👍 / 👎.
|
✅ Deterministic PR hygiene checks passed. |
|
Integrating through the maintainer self-integration path in MAINTAINERS.md and recording that choice here. Exact-head evidence: Cross-platform CI run 34775751844 completed success at the current head. Local product suite, typecheck, build and install NOT RUN; a fresh lane worktree has no On the change: This layer changes nothing client-visible on purpose. Propagation to the advertised catalog and the operator-override precedence are the round-2 layer and are not in this diff. |
1 participant
Summary
parseCatalogBufferinsrc/adapters/devin/cloud-direct/catalog.tshad protobuf arms forClientModelConfigfields 1, 4, 18, and 22 and no default, so field 5 (supports_images) was silently dropped. This PR adds the field-5 varint arm and carries it onModelCatalogEntryas an optional boolean tri-state: presenttrueasserts text+image, presentfalseasserts text-only, omitted stays unknown. That is deliberately not thedisabledpattern (defaultsfalse); collapsing unknown into text-only was the Routing capability evidence discards every catalog row (field-shape mismatch) #1796 regression, already implemented as a tri-state insrc/providers/antigravity-models.ts.extension.js, Streaming text duplicates and code-edit turns can stall before fileChange events under opencodex #18 from a live catalog dump, Fix stale Codex autostart shim repair #5 corroborated against public WindsurfAPIClientModelConfigdocs).structure/adapters/registry.mdrecords the catalog pre-flight contract (the AGENTS.md structure obligation).true, varint 0 ->toBe(false)(asserted text-only, not omission), omitted field -> entry exists andsupportsImagesistoBeUndefined().Verification
7f94c524bd8ac5cc62f06b4551c7e38a6ca327d1: run 34775751844 (pull_requestevent, the run attached to this PR's checks). An earlier explicitworkflow_dispatchrun (34775719256) at the same SHA was a duplicate of this run and was cancelled as redundant.bun run structure:checkpassed locally. The focused adapter test file could not run locally because dependencies are not installed in this worktree (install is out of scope for this round).Checklist
structure/adapters/registry.mdrecords the pre-flight contract; no user-facing release note — the flag is not yet propagated to the client catalog.)