Skip to content

fix(devin): apply explicit SWE-2 effort before model suffix - #4420

Closed
Smartnewb wants to merge 1 commit into
lidge-jun:devfrom
Smartnewb:fix/swe2-shared-effort-current
Closed

fix(devin): apply explicit SWE-2 effort before model suffix#4420
Smartnewb wants to merge 1 commit into
lidge-jun:devfrom
Smartnewb:fix/swe2-shared-effort-current

Conversation

@Smartnewb

@Smartnewb Smartnewb commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

Summary

Fix SWE-2 effort selection in the shared Devin adapter: requesting swe-2-high with explicit medium reasoning currently returns swe-2-high immediately because the existing suffix wins. Resolve explicit SWE-2 effort to its native medium/high/max UID before that suffix shortcut.

This follows the maintainer's guidance on #4416 after #4415 removed ACP. It changes src/adapters/devin.ts, shared by devin and devin-cli, and does not restore any ACP implementation. Omitted or unknown effort preserves an explicit variant; other model families retain their existing suffix precedence. Lower-than-medium labels map to Medium and xhigh/ultra map to Max.

Verification

  • bun test tests/providers/devin-adapter.test.ts tests/providers/devin-hardening.test.ts: 37 passed, 0 failed, 144 assertions. Covers each explicit effort against base/suffixed ids, aliases, omission, and other model families.
  • bun run typecheck: passed.
  • bun run structure:check and bun run privacy:scan: passed.
  • cd docs-site && bun install --frozen-lockfile && bun run build: passed, 425 pages.
  • Read the current account's cloud model catalog through the existing catalog client: swe-2-medium, swe-2-high, and swe-2-max are present and enabled. Only these model facts are reported; no credential or private catalog dump is committed. This verifies available UIDs, not a live inference run of the modified shared adapter.
  • git diff --check: passed. No full-suite success claim; submitted as a focused draft. AI-assisted implementation and verification.

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.

@coderabbitai

coderabbitai Bot commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

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.

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

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@github-actions

Copy link
Copy Markdown
Contributor

⏳ DRAFT

  • review readiness checklist open (0/4 boxes ticked).

What to do

  • Tick all four boxes in the PR description once you're done (currently 0/4).

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.

0/4 boxes ticked.

This PR stays in draft until every box above is ticked.

@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 64 / 80

이 PR은 #4416에서 하려던 SWE-2 effort 고정을, ACP가 사라진 뒤의 공유 Devin 어댑터(src/adapters/devin.ts)로 옮긴 수정입니다. 지금 dev HEAD는 1e28e6236(#4419)이고, #4415가 devin-cli ACP 스택을 제거한 뒤라 effort 선택은 cloud-direct/공유 resolveWireModelUid 한곳입니다. 현재 코드는 normalizeDevinModelId 직후 hasEffortSuffix(modelId)이면 곧바로 반환합니다. 그래서 피커가 swe-2-high인데 클라이언트가 reasoning medium을내도 접미사가 이겨 swe-2-high가 그대로 나갑니다. 이번 변경은 SWE-2(및 medium/high/max 변형)일 때 명시 effort를 네이티브 UID(swe-2-medium|high|max)로 먼저 바꾼 다음, 기존 접미사 단축을 타게 합니다. 생략·未知 effort는 변형을 보존하고, 다른 모델 패밀리는 예전처럼 접미사 우선을 유지합니다. none/off/minimal/low → medium, xhigh/ultra → max 매핑과 단위 테스트(tests/providers/devin-adapter.test.ts)가 따라옵니다. #4416 ACP 경로 수정은 이미 코드베이스에서 무효였고, 이 PR이 그 후속 착지로 읽힙니다. 다만 아직 draft이고 readiness 체크리스트가 비어 있습니다.

라인 (~95–110, src/adapters/devin.ts resolveWireModelUid) - SWE-2 전용 매핑이 함수 안에 인라인으로 들어갑니다. EFFORT_SUFFIXES에는 ultra/off/minimal이 없고, 새 맵이 그 별칭을 담당합니다. 의도는 맞지만, 나중에 effort 별칭이 늘면 카탈로그 경로의 EFFORT_SUFFIXES.has(reasoningEffort)와 이중 관리가 됩니다. 공유 맵 하나로 모을지 정하면 좋습니다.

라인 (테스트의 resolveWireModelUid export) - 테스트를 위해 함수를 export합니다. 프로덕션 공개 API로 굳힐지, ForTests 이름을 쓸지 한 줄만 정해 두면 이후 리팩터에 덜 흔들립니다.

structure/* 여러 파일 · docs-site adapters.md - 같은 문장(“공유 Devin 어댑터가 SWE-2 model effort selection을 소유한다”)이 registry 외에 inbound-compat·chat-compat·cursor·runtime·transports 등 관련 薄い 문서에도 복붙됩니다. structure 게이트를 통과하려는 흔적으로 보이지만, 실제 계약 문서는 structure/adapters/registry.md(+ adapters.md)면 충분해 보입니다. 나머지 복붙은 노이즈입니다.

draft readiness 체크리스트 - CI 로컬 전부·최신 dev 리베이스·Codex/CodeRabbit 반영·ready 박스가 모두 비어 있습니다. 기능 자체와 별개로 머지 전에 채워야 합니다.

정규식 /^swe-2(?:-(?:medium|high|max))?$/ - swe-2.high는 앞선 normalizeDevinModelIdswe-2-high로 바꾼 뒤 매칭되므로 테스트의 dotted 케이스는 맞습니다. swe-20-high·swe-1-7-high는 의도적으로 제외됩니다.

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

  • structure 문서 복붙을 registry(+ docs-site)만 남기고 줄일지
  • draft를 ready로 올리기 전에 작성자가 체크리스트를 채울지, 메인테이너가 범위만 보고 진행할지
  • effort 별칭 맵을 EFFORT_SUFFIXES 근처로 끌어올려 한곳에서 관리할지
  • #4416은 이 PR 머지 후 landed-via로 닫을지

너의 추천
방향은 맞습니다. ACP가 사라진 뒤 고쳐야 할 진짜 지점(공유 resolveWireModelUid)을 짚었고 테스트도 핵심 케이스를 덮습니다. structure 복붙을 줄이고 draft 체크리스트·CI를 채운 뒤 머지하는 쪽을 추천합니다. 우선순위는 중상(Devin SWE-2 선택 버그)이지만, #4424 revert·#4419 계열보다는 급하지 않습니다. types/config 무효화 대상은 아닙니다.

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

lidge-jun added a commit that referenced this pull request Sep 13, 2026
Carry #4420 from 6a456fb onto 7ca00ff.

Cognition spells SWE-2 effort as the model id rather than a separate field,
and resolveWireModelUid returned early whenever the id already carried an
effort suffix. Asking for swe-2-high at medium therefore stayed swe-2-high
and the caller was silently ignored. Resolve an explicit SWE-2 effort to the
native medium/high/max UID before that shortcut runs. none, off, minimal and
low select Medium, since SWE-2 has no lane below it and rounding down to
nothing would quietly disable its reasoning; xhigh, ultra and max select Max.
Omitted or unknown effort preserves the variant, and other model families
keep their existing suffix precedence.

Two deliberate differences from the source PR. The effort map is a named
table instead of an inline branch, because EFFORT_SUFFIXES does not carry
ultra, off or minimal and the two would drift apart silently. And matching is
case-insensitive, so a caller sending HIGH reaches the same lane as high.

The resolver is exposed to tests through resolveWireModelUidForTests rather
than by exporting its bare name, mirroring
sanitizeToolDescriptionForCognitionForTests: it is an async helper that
reaches the catalog and does not belong in the adapter public API.

Ownership is recorded in structure/adapters/registry.md and the user-facing
adapters page. The source PR also pasted the same sentence into six unrelated
structure files; those hunks are omitted, and the structure gate checks path
mentions rather than behavioral prose, so the omission is safe.

This is the post-ACP landing of closed #4416 and does not restore ACP.

Local product tests, typecheck, build and install: NOT RUN.
Hosted exact-head CI on this PR is the merge proof.

Co-authored-by: Smartnewb <159137930+Smartnewb@users.noreply.github.com>
lidge-jun added a commit that referenced this pull request Sep 13, 2026
Carry #4420 from 6a456fb onto 7ca00ff.

Cognition spells SWE-2 effort as the model id rather than a separate field,
and resolveWireModelUid returned early whenever the id already carried an
effort suffix. Asking for swe-2-high at medium therefore stayed swe-2-high
and the caller was silently ignored. Resolve an explicit SWE-2 effort to the
native medium/high/max UID before that shortcut runs. none, off, minimal and
low select Medium, since SWE-2 has no lane below it and rounding down to
nothing would quietly disable its reasoning; xhigh, ultra and max select Max.
Omitted or unknown effort preserves the variant, and other model families
keep their existing suffix precedence.

Two deliberate differences from the source PR. The effort map is a named
table instead of an inline branch, because EFFORT_SUFFIXES does not carry
ultra, off or minimal and the two would drift apart silently. And matching is
case-insensitive, so a caller sending HIGH reaches the same lane as high.

The resolver is exposed to tests through resolveWireModelUidForTests rather
than by exporting its bare name, mirroring
sanitizeToolDescriptionForCognitionForTests: it is an async helper that
reaches the catalog and does not belong in the adapter public API.

Ownership is recorded in structure/adapters/registry.md and the user-facing
adapters page. The source PR also pasted the same sentence into six unrelated
structure files; those hunks are omitted, and the structure gate checks path
mentions rather than behavioral prose, so the omission is safe.

This is the post-ACP landing of closed #4416 and does not restore ACP.

Local product tests, typecheck, build and install: NOT RUN.
Hosted exact-head CI on this PR is the merge proof.

Co-authored-by: Smartnewb <159137930+Smartnewb@users.noreply.github.com>
@lidge-jun

Copy link
Copy Markdown
Owner

Landed on dev as eee8fd8 via #4445, carried onto the current tip with your Co-authored-by trailer on the squashed commit. Two deliberate changes from this PR: the effort map is a named table rather than an inline branch, because EFFORT_SUFFIXES does not carry ultra/off/minimal and the two would drift; and matching is case-insensitive. The resolver is exposed to tests through resolveWireModelUidForTests instead of a bare export, and the six duplicated structure hunks were omitted since the ownership gate checks path mentions rather than repeated prose. Thanks for the fix. Closing as superseded.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants