Skip to content

refactor(vscode): share context request bookkeeping - #13643

Merged
marius-kilocode merged 2 commits into
mainfrom
dedupe-b-context-requests
Sep 1, 2026
Merged

refactor(vscode): share context request bookkeeping#13643
marius-kilocode merged 2 commits into
mainfrom
dedupe-b-context-requests

Conversation

@marius-kilocode

@marius-kilocode marius-kilocode commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

What Problem This Solves

Git-change and terminal context hooks maintain the same per-instance request map, IDs, timeout settlement, and disposal logic.

Why This Change Was Made

Share only that bookkeeping in a small browser-only helper and remove the now-redundant request wrappers. Message routing, mention detection, attachments, Git scope and no-Git guards, and embedded-terminal fallback remain local. Preserve the 15-second Git timeout, 10-second terminal timeout, known-ID-only settlement, synchronous replies, and the existing cleanup difference: Git resets pending, terminal does not. Add ES2024.Promise type declarations without changing the emitted target.

User Impact

No intended behavior change. No release note is needed for this internal refactor.

Evidence

  • Fresh duplication report: 35 to 34 pairs, 684 to 669 duplicated lines, 4,688 to 4,583 duplicated tokens. Remove only fingerprint 3474d577ea4814b0 from the allowlist.
  • Production hooks plus helper: 186 to 166 lines, a net reduction of 20 lines. Whole PR: 95 additions and 105 deletions, 10 fewer lines including the test and allowlist change.
  • Keep one 22-line regression test for synchronous/out-of-order settlement, late IDs, timeout, and both cleanup modes. Together with existing terminal utility/architecture tests: 7 tests passed, 33 assertions. No large hook harness.
  • Package compile/build:check (including host/webview typecheck and lint), knip, formatting, and duplication/annotation guards passed. Initial dependency errors were resolved with the frozen-lockfile install.
  • Before the final wrapper simplification, visible fresh isolated VS Code on a disposable Git fixture rendered both @git-changes and @Terminal in an unsent prompt. Cropped screenshot inspected. Attachment capture and pending transitions occur only on submission, so those UI flows were not exercised. No prompt or model call was sent; no real credentials were used. stop-vscode --cleanup true and stop completed.

@kilo-code-bot

kilo-code-bot Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (6 files)
  • packages/kilo-vscode/tests/unit/context-requests.test.ts
  • packages/kilo-vscode/webview-ui/src/hooks/context-requests.ts
  • packages/kilo-vscode/webview-ui/src/hooks/useGitChangesContext.ts
  • packages/kilo-vscode/webview-ui/src/hooks/useTerminalContext.ts
  • packages/kilo-vscode/webview-ui/tsconfig.json
  • script/kilocode-duplication-allowlist.json
Previous Review Summary (commit f1d3f9f)

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

Previous review (commit f1d3f9f)

Status: No Issues Found | Recommendation: Merge

Files Reviewed (7 files)
  • packages/kilo-vscode/tests/unit/context-requests.test.ts
  • packages/kilo-vscode/tests/unit/git-changes-context.test.ts
  • packages/kilo-vscode/webview-ui/src/hooks/context-requests.ts
  • packages/kilo-vscode/webview-ui/src/hooks/useGitChangesContext.ts
  • packages/kilo-vscode/webview-ui/src/hooks/useTerminalContext.ts
  • packages/kilo-vscode/webview-ui/tsconfig.json
  • script/kilocode-duplication-allowlist.json

Reviewed by grok-4.6 · Input: 69.2K · Output: 9.1K · Cached: 139.3K

Review guidance: REVIEW.md from base branch main

@marius-kilocode
marius-kilocode merged commit 018c5e5 into main Sep 1, 2026
33 checks passed
@marius-kilocode
marius-kilocode deleted the dedupe-b-context-requests branch September 1, 2026 09:39
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