Skip to content

fix(cli): select apply_patch from model family - #11701

Merged
chrarnoldus merged 5 commits into
mainfrom
fix/cli-apply-patch-model-family
Jul 2, 2026
Merged

fix(cli): select apply_patch from model family#11701
chrarnoldus merged 5 commits into
mainfrom
fix/cli-apply-patch-model-family

Conversation

@chrarnoldus

Copy link
Copy Markdown
Collaborator

Summary

Use the model family supplied by provider metadata when deciding whether GPT models receive apply_patch instead of edit.

Gateway-routed models can have opaque API IDs, so the previous ID-only heuristic missed GPT and Codex models even though their family identified them correctly. The existing ID fallback remains in place, while GPT-4, OSS, and image families stay excluded.

@chrarnoldus chrarnoldus self-assigned this Jun 25, 2026
Comment thread packages/opencode/src/tool/registry.ts
@kilo-code-bot

kilo-code-bot Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Full re-review of the current HEAD (aa6e84dfcd) — the branch was force-pushed since the last review, so the previous incremental base (63a65ce8) is no longer an ancestor and history was rewritten. Reviewed the complete PR diff instead.

The current diff cleanly implements family-aware apply_patch selection:

  • packages/opencode/src/kilocode/tool/registry.ts — new KiloToolRegistry.usePatch() helper accepts both modelID and optional family, with GPT-4/OSS/image exclusions checked against both.
  • packages/opencode/src/tool/registry.ts — shared file change is a single-line swap to call the new helper, well isolated behind kilocode_change markers.
  • packages/opencode/src/session/tools.ts and packages/opencode/src/server/routes/instance/httpapi/handlers/experimental.ts — both call sites now pass family through to registry.tools().
  • .changeset/use-gpt-family-patching.md — user-facing changeset present and appropriately worded.

Both previously flagged items remain resolved on this HEAD:

  • The apply_patch family-selection bypass in the experimental tool-list endpoint (fixed in c9c3881) — verified experimental.ts now resolves provider model metadata via provider.getModel(...).pipe(Effect.option) and passes family into registry.tools(), with a fallback test covering unknown models.
  • The "why no separate test file?" suggestion — resolved by moving the family-aware experimental-tools test into packages/opencode/test/kilocode/server/httpapi-experimental.test.ts (the corresponding entry was removed from the shared test/server/httpapi-experimental.test.ts, so that file has no net diff versus main).

No new issues found in the current diff.

Files Reviewed (7 files)
  • .changeset/use-gpt-family-patching.md
  • packages/opencode/src/kilocode/tool/registry.ts
  • packages/opencode/src/server/routes/instance/httpapi/handlers/experimental.ts
  • packages/opencode/src/session/tools.ts
  • packages/opencode/src/tool/registry.ts
  • packages/opencode/test/kilocode/server/httpapi-experimental.test.ts
  • packages/opencode/test/kilocode/tool-registry-apply-patch.test.ts
Previous Review Summaries (4 snapshots, latest commit 63a65ce)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 63a65ce)

Status: No Issues Found | Recommendation: Merge

Incremental review of the latest commit (63a65ce8, "test: stabilize flaky CI tests") since the previous review at 0c4f1ad6. This commit only touches two unrelated test files and does not change the PR's core apply_patch/model-family logic:

  • packages/core/test/util/effect-flock.test.ts - reduces stress-test concurrency/hold time on Windows to fix flakiness; logic remains correct.
  • packages/kilo-jetbrains/backend/src/test/kotlin/.../KiloBackendWorkspaceTest.kt - adds explicit withTimeout waits for workspace Ready state, consistent with the same pattern already used elsewhere in this file.

Both previously flagged items remain resolved on this HEAD:

  • The apply_patch family-selection bypass in the experimental tool-list endpoint (fixed in c9c3881).
  • The "why no separate test file?" question, resolved in 0c4f1ad by moving the family-aware experimental-tools test into the Kilo-owned test directory.
Files Reviewed (2 files, incremental)
  • packages/core/test/util/effect-flock.test.ts
  • packages/kilo-jetbrains/backend/src/test/kotlin/ai/kilocode/backend/workspace/KiloBackendWorkspaceTest.kt

Previous review (commit 0c4f1ad)

Status: No Issues Found | Recommendation: Merge

Both previously open items are resolved on this HEAD:

  • The apply_patch family-selection bypass in the experimental tool-list endpoint was fixed in c9c3881 (endpoint now resolves provider model metadata and passes canonical id + family into tool selection).
  • The "why no separate test file?" question is resolved in 0c4f1ad, which moves the family-aware experimental-tools test into the Kilo-owned packages/opencode/test/kilocode/server/httpapi-experimental.test.ts and reverts the shared upstream packages/opencode/test/server/httpapi-experimental.test.ts back to match main, removing the kilocode_change block and the now-unneeded extended timeout.
Files Reviewed (7 files)
  • .changeset/use-gpt-family-patching.md
  • packages/opencode/src/kilocode/tool/registry.ts
  • packages/opencode/src/server/routes/instance/httpapi/handlers/experimental.ts
  • packages/opencode/src/session/tools.ts
  • packages/opencode/src/tool/registry.ts
  • packages/opencode/test/kilocode/server/httpapi-experimental.test.ts
  • packages/opencode/test/kilocode/tool-registry-apply-patch.test.ts

Previous review (commit c9c3881)

Status: No Issues Found | Recommendation: Merge

Files Reviewed (7 files)
  • .changeset/use-gpt-family-patching.md
  • packages/opencode/src/kilocode/tool/registry.ts
  • packages/opencode/src/server/routes/instance/httpapi/handlers/experimental.ts
  • packages/opencode/src/session/tools.ts
  • packages/opencode/src/tool/registry.ts
  • packages/opencode/test/kilocode/tool-registry-apply-patch.test.ts
  • packages/opencode/test/server/httpapi-experimental.test.ts

Previous review (commit da2ea37)

Status: 1 Issues Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 0

Fix these issues in Kilo Cloud

Issue Details (click to expand)

WARNING

File Line Issue
packages/opencode/src/tool/registry.ts 370 Family-based apply_patch selection is still bypassed in the experimental tool-list API
Files Reviewed (5 files)
  • .changeset/use-gpt-family-patching.md - 0 issues
  • packages/opencode/src/kilocode/tool/registry.ts - 0 issues
  • packages/opencode/src/session/tools.ts - 0 issues
  • packages/opencode/src/tool/registry.ts - 1 issue
  • packages/opencode/test/kilocode/tool-registry-apply-patch.test.ts - 0 issues

Reviewed by claude-sonnet-5-20260630 · Input: 7.4K · Output: 15.7K · Cached: 2.1M

Review guidance: REVIEW.md from base branch main

15_000, // kilocode_change
)

// kilocode_change start

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why no separate test file?

@markijbema

Copy link
Copy Markdown
Contributor

I'd prefer not to include test fixes in unrelated prs

chrarnoldus and others added 3 commits July 1, 2026 14:10
Co-authored-by: kiloconnect[bot] <240665456+kiloconnect[bot]@users.noreply.github.com>
Co-authored-by: kiloconnect[bot] <240665456+kiloconnect[bot]@users.noreply.github.com>
Co-authored-by: kiloconnect[bot] <240665456+kiloconnect[bot]@users.noreply.github.com>
@chrarnoldus
chrarnoldus force-pushed the fix/cli-apply-patch-model-family branch from 63a65ce to aa6e84d Compare July 1, 2026 12:10
@chrarnoldus
chrarnoldus merged commit 2992e3e into main Jul 2, 2026
26 checks passed
@chrarnoldus
chrarnoldus deleted the fix/cli-apply-patch-model-family branch July 2, 2026 08:36
t7tran pushed a commit to t7tran/kilocode that referenced this pull request Aug 14, 2026
…odel-family

fix(cli): select apply_patch from model family
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants