Skip to content

fix(devin): score family axes sparsely with own-key targets - #6324

Merged
lidge-jun merged 1 commit into
lidge-jun:devfrom
luvs01:transfer/681-devin-family-axis
Sep 30, 2026
Merged

lidge-jun merged 1 commit into
lidge-jun:devfrom
luvs01:transfer/681-devin-family-axis

Conversation

@luvs01

@luvs01 luvs01 commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Carry my original sparse family-axis scoring change, inspected source head 7034cbe76ecd6e9bf4ce0ac5a1e6aa21877e46ab.
  • Count the anchor's nonzero target axes once and inspect each member's own axes, avoiding the members-by-all-axes cross-product while retaining effort precedence and family confinement.
  • Address the remaining fork review finding: use an own-key target lookup so extra axes named toString, constructor or __proto__ cannot inherit Object.prototype values and incorrectly tie an exact match.
  • Add regressions for these extra axes and legitimate explicitly targeted prototype-named axes. Document the scoring contract.

Verification

  • Base: 592c5cfc043cd5b69e8aea0f12b9a0644cc50612. Published head: 63f460bfcef0bc73bc82dbf20ac104fa945b8142; tree: 902125ead1bfe51564f5241fb2ee2d8a17051b56.
  • The published tree and parent match the locally tested tree/parent byte-for-byte. Local test commit 86d58c78c80209ed7a41e313936d7899efda7edd has the same tree; the connected GitHub commit API supplied the publication metadata. All three remote blob hashes were checked against the local Git objects.
  • Linux x64, Bun 1.4.0: bun test tests/providers/devin-family-resolution.test.ts — 55 pass, 0 fail, 94 assertions.
  • Before the own-key correction, the added three cases reproduced the defect against the carried fork implementation: 0 pass, 3 fail, 52 filtered out. After the correction the entire focused file passed. The existing 12,000-member/12,000-anchor-axis regression remained within its unchanged five-second bound.
  • bun run typecheck, bun run structure:check, bun run privacy:scan, bun scripts/file-size-ratchet.ts and git diff --check passed. The first structure check exceeded the document's line budget; the final text was condensed and the unchanged 600-line gate passed.
  • bun run test:changed was attempted against origin/dev at the recorded base. The execution environment blocked continuation over an unapproved external SOCKS5 connection concern; no final aggregate test result was returned. This is blocked/incomplete, not a passing suite. No alternate network route or security-policy change was used, and no surviving owned test process remained.
  • The full local suite was not run. Under the documented test-scope exception, focused behavioral regression and static/structural checks are complete; broader import-connected and platform coverage remains for exact-head hosted CI.
  • Synthetic catalog fixtures only; no live model request or credential change. Local focused checks and exact-head hosted CI support review readiness. This is not maintainer approval or authorization to merge.

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.

Privacy and source checks passed. This change affects catalog ranking and test coverage; it does not alter authentication, account permissions or credential destinations. Maintainer review and all applicable merge requirements remain separate.

Review readiness

  • Focused validation passed, the blocked aggregate and remaining coverage are documented, and exact-head hosted CI passed.
  • Branch meets the recent-dev criterion at the recorded base.
  • The carried fork's own-key finding is addressed; no unresolved findings are present on this upstream PR.
  • Ready for review.

Ready checkpoint

  • Exact-head CI completed successfully for 63f460bfcef0bc73bc82dbf20ac104fa945b8142, including the aggregate, four test shards and applicable gates/smokes. Optional skipped jobs are not passing evidence. Target, hygiene and React Doctor checks passed.
  • The local aggregate remains blocked/incomplete as recorded above; hosted CI does not turn that local attempt into a pass.
  • Automatic review is expected after the normal Ready event and has not been counted as a completed review. New findings will be evaluated against the actual head. No manual review-bot invocation, merge or deployment is requested.

Summary by CodeRabbit

  • Bug Fixes
    • Improved Devin model selection when family members have different available options, including fast mode, extended context, and thinking settings.
    • Improved matching for missing or additional options, including options with special names.
  • Tests
    • Added coverage for selecting the best match across family members with differing options.

@coderabbitai

coderabbitai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

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: 9616b384-06bd-449f-8d09-a44a8b84c906

📥 Commits

Reviewing files that changed from the base of the PR and between 592c5cf and 63f460b.

📒 Files selected for processing (3)
  • src/adapters/devin/live-models.ts
  • structure/providers-and-adapters.md
  • tests/providers/devin-family-resolution.test.ts

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


📝 Walkthrough

Walkthrough

Devin family-member selection now derives request targets from the chosen anchor and scores candidates using their own axes. Tests cover large families, prototype-named axes, and exact, missing, or extra axis matches.

Changes

Devin family-member axis scoring

Layer / File(s) Summary
Build targets and score family members
src/adapters/devin/live-models.ts, tests/providers/devin-family-resolution.test.ts, structure/providers-and-adapters.md
live-models.ts counts nonzero targets once, then adjusts the score for each candidate’s axes. Missing axes have order zero; extra nonzero axes count as mismatches. Target selection uses the chosen anchor’s non-effort axes and assigns Fast Mode, 1M Context, and Thinking-off targets without checking whether those axes exist in the family. Tests cover a family with 12,000 anchor axes and 12,000 additional members, prototype-named axes, and candidate ranking. The documentation describes these scoring rules.

Priority: ⬇️ Low

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

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 63f46

The change makes Devin family-member selection scale with the size of each candidate's own axes and fixes prototype-named axes falsely tying exact matches. No actionable merge-blocking risk was found. The reported CI result for the exact head should still be relied on before merging.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ 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 accurately and concisely describes the main change: sparse scoring of Devin family axes using own-key targets.
Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

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

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 44 / 80

이 PR은 Devin 가족 모델 고르기(selectDevinFamilyMember)가 축을 세는 방식을 바꿉니다. 예전에는 가족 안 모든 멤버의 축 이름을 한데 모은 뒤, 멤버마다 그 전체 목록과 맞춰 보았습니다. 앵커 축이 많고 멤버도 많으면 멤버 수 × 축 수만큼 일이 커집니다. 지금은 앵커가 가리키는 목표 축의 “0이 아닌 개수”를 한 번만 세고, 각 후보는 자기 축만 봅니다. 빠진 목표 축은 기본값으로 남고, 후보에만 있는 여분 축은 점수를 깎습니다. 노력(effort) 순서와 “같은 가족 안에서만 고른다”는 규칙은 그대로입니다.

같이 고친 중요한 구멍이 있습니다. 축 이름이 toString, constructor, __proto__처럼 객체에 원래 있던 이름이면, 예전 방식으로 targets[축]을 읽으면 프로토타입 값이 섞일 수 있습니다. 그러면 여분 축이 있는 후보와 딱 맞는 후보가 같은 점수로 묶일 수 있습니다. 이번 코드는 Object.hasOwn으로 자기 키만 봅니다. 테스트가 그 이름들, 딱 맞음/빠짐/여분, 그리고 축 1만 2천 × 멤버 1만 2천 성능 상한(5초)을 넣었습니다. structure/providers-and-adapters.md에도 계약을 한 줄 적었습니다. 베이스는 dev이고, types/config 쪼개기로 닫을 무효·중복 PR은 없습니다.

라인 - src/adapters/devin/live-models.ts selectDevinFamilyMember · 예전에는 가족 안에 그 축이 있을 때만 fast / longContext / effort === "none" 목표를 넣었습니다. 지금은 요청만 있으면 무조건 targets에 넣습니다. 가족에 그 축이 하나도 없으면 모든 후보가 같은 만큼 어긋나서 순위는 보통 그대로지만, “축이 있을 때만 목표에 넣는다”는 예전 조건은 사라졌습니다.

라인 - tests/providers/devin-family-resolution.test.ts 성능 테스트 · 맞음/틀림을 벽시계 5초로 가릅니다. 의도(멤버×축 교차 vs 희소 합)는 분명하고, 보통은 훨씬 빨리 끝납니다. 아주 느린 러너에서는 드물게 흔들릴 여지는 있습니다.

라인 - PR 상태 · Draft이고 Review readiness 체크는 비어 있습니다. 작성자도 전체 스위트·test:changed는 막혔거나 안 돌렸다고 적었고, 포커스 테스트·정적 검사만 로컬에서 통과했다고 합니다. 제품 인증·권한 경로는 안 바꿉니다.

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

Draft를 풀고 CI(특히 이 테스트 파일·관련 import)가 초록인 뒤에만 머지할지 정해 주세요. fast / longContext / none 목표를 축 존재와 무관하게 넣는 새 계약을 그대로 둘지도 한 번만 확인하면 됩니다. 성능 테스트를 5초 벽시계로 둘지, 더 느슨하게 둘지는 취향입니다.

너의 추천

방향은 맞습니다. 희소 채점과 Object.hasOwn 수정이 리뷰 지적과 잘 맞고, 회귀 테스트도 그 구멍을 직접 칩니다. Draft 유지한 채 tip CI가 통과하면 Ready로 올린 뒤 머지해도 됩니다. 미리보기 배포 이야기는 이번 범위가 아닙니다.

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

@lidge-jun

Copy link
Copy Markdown
Owner

Maintainer integration into dev (MAINTAINERS.md, dev-only exception) by @lidge-jun.

Thanks @luvs01!

@lidge-jun
lidge-jun merged commit 8326838 into lidge-jun:dev Sep 30, 2026
46 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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants