From 3c65ab8a12469d87088479be5828ca82070f4e12 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 10 Jun 2026 08:20:14 +1000 Subject: [PATCH 01/39] chore(porch): 789 init pir --- .../status.yaml | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 codev/projects/789-vscode-inject-file-hunk-refere/status.yaml diff --git a/codev/projects/789-vscode-inject-file-hunk-refere/status.yaml b/codev/projects/789-vscode-inject-file-hunk-refere/status.yaml new file mode 100644 index 000000000..af0d0b2b5 --- /dev/null +++ b/codev/projects/789-vscode-inject-file-hunk-refere/status.yaml @@ -0,0 +1,18 @@ +id: '789' +title: vscode-inject-file-hunk-refere +protocol: pir +phase: plan +plan_phases: [] +current_plan_phase: null +gates: + plan-approval: + status: pending + dev-approval: + status: pending + pr: + status: pending +iteration: 1 +build_complete: false +history: [] +started_at: '2026-06-09T22:20:14.795Z' +updated_at: '2026-06-09T22:20:14.795Z' From e1a66b31328812ba79b92df1f1cbb206af98553b Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 10 Jun 2026 08:25:56 +1000 Subject: [PATCH 02/39] [PIR #789] Plan draft --- .../789-vscode-inject-file-hunk-refere.md | 80 +++++++++++++++++++ codev/state/pir-789_thread.md | 17 ++++ 2 files changed, 97 insertions(+) create mode 100644 codev/plans/789-vscode-inject-file-hunk-refere.md create mode 100644 codev/state/pir-789_thread.md diff --git a/codev/plans/789-vscode-inject-file-hunk-refere.md b/codev/plans/789-vscode-inject-file-hunk-refere.md new file mode 100644 index 000000000..56144d2ba --- /dev/null +++ b/codev/plans/789-vscode-inject-file-hunk-refere.md @@ -0,0 +1,80 @@ +# PIR Plan: Inject file/hunk reference into builder PTY from the unified-diff editor (codelens) + +## Understanding + +Reviewing a builder's changes is a per-file retype loop: open `codev.viewDiff`, read a file, switch to the builder terminal, manually retype the file path (and line numbers), add feedback, submit. The path-retyping is the bottleneck. + +The fix mirrors the proven `codev.referenceIssueInArchitect` pattern (`extension.ts:785`): an inline action injects a reference token into a terminal's prompt buffer **without** pressing Enter, then the human types their feedback and submits. That command targets the *architect* terminal with `# `; we want the same mechanic targeting the *builder* terminal with a file/hunk reference, surfaced as CodeLenses inside the diff editor `codev.viewDiff` opens. + +Key facts established from the code: + +- `codev.viewDiff` (`commands/view-diff.ts:301`) opens VSCode's multi-file diff editor via `vscode.changes`. For each changed file the **right** (modified) side is a plain `file:` URI at `/` (added/modified/renamed); deleted/binary right sides are `codev-diff:` placeholders. The right side carries the new-side line numbers we need. +- `getBuilderChanges(wt)` (`view-diff.ts:261`) already resolves `defaultBranch`, the merge-base `baseRef`, the `ChangeEntry[]`, and the binary-path set — everything except the per-file hunk line ranges. +- `terminal-manager.ts` has `injectArchitectText(text)` (`:146`): look up `architect:` terminal, `show()`, `sendText(text, false)` (no newline). There is no builder equivalent yet. `openBuilderByRoleOrId(roleOrId, focus)` (`:192`) resolves a builder and opens/reveals its terminal, but returns `void` and resolves `roleOrId` → a *canonical* `builder.id` that may differ from the input (e.g. `153` → `builder-spir-153`); the terminal is keyed `builder-`. +- `buildArchitectReferenceInjection` (`architect-reference-injection.ts`) is the pure, unit-tested string builder precedent — palette-hidden command, no `vscode` import, tested in `__tests__/architect-reference-injection.test.ts`. +- Palette suppression: a command is kept out of the Command Palette either by a `commandPalette` `when:"false"` entry (e.g. `referenceIssueInArchitect`, `openBuilderFileDiff`) **or** by not declaring it in `contributes.commands` at all. CodeLens-backing commands are invoked programmatically and need no package.json declaration; no test enforces registered-vs-declared parity. + +## Proposed Change + +Add a `CodeLensProvider` that renders inject actions on the **right-side `file:` documents** shown in the multi-file diff editor, plus the builder-terminal injection plumbing. + +**1. Pure helpers (new file `packages/vscode/src/diff-inject-ref.ts`, no `vscode` import — mirrors `architect-reference-injection.ts`):** +- `parseHunkRanges(patch: string): Array<{ newStart: number; newEnd: number }>` — parse `@@ -a,b +c,d @@` headers from a single file's unified diff. New-side start = `c`; length = `d` (absent → 1). `newEnd = newStart + max(length, 1) - 1`. A pure-deletion hunk (`+c,0`) clamps to a single anchor line at `c` (or `c+1`), so a click still references a sane location. +- `buildBuilderFileRef(relPath): string` → `` `${relPath} ` `` (trailing space, no Enter). +- `buildBuilderHunkRef(relPath, start, end): string` → `` `${relPath}:L${start}-L${end} ` ``. + +**2. CodeLens provider (new file `packages/vscode/src/diff-inject-codelens.ts`):** +- `DiffInjectCodeLensProvider implements vscode.CodeLensProvider`, holding a registry `Map` and an `onDidChangeCodeLenses` emitter. +- `provideCodeLenses(document)`: look up `document.uri.fsPath`. If present, emit one **file-level** lens (range at line 0) titled `Send to builder PTY` and one lens **per hunk** (range at `newStart-1`) titled `Send to builder PTY (lines -)`. Each lens's `command` is `codev.injectBuilderFileRef` with args `[builderId, refText]`. +- `setSession(entries)` replaces the registry (one active diff session per `viewDiff` invocation) and fires the change event; provider registered once at activation for `{ scheme: 'file' }`. + +**3. `viewDiff` populates the registry:** after `getBuilderChanges`, run one `git -C diff -M --unified=3 ` (full multi-file patch), split it per file on `diff --git`, map each file's hunks to its new path, and call `provider.setSession(...)` keyed by the right-side fs path (`path.join(wt, resourcePath)`) before opening the editor. Deleted/binary files are skipped (no right-side file document). This is one extra git call alongside the two `getBuilderChanges` already runs. + +**4. `terminal-manager.ts`:** +- Add `injectBuilderText(builderId, text): boolean` — look up `builder-`, `show()`, `sendText(text, false)`; return false if absent (exact mirror of `injectArchitectText`). +- Change `openBuilderByRoleOrId` to **return** the resolved canonical `builder.id` (`Promise`); existing callers ignore the return (non-breaking). This closes the id-mismatch gap so the inject targets the same terminal key that was opened. + +**5. `extension.ts`:** register `codev.injectBuilderFileRef` via the existing `reg(...)` helper, **without** declaring it in `contributes.commands` (palette-hidden by omission). Handler: +```ts +reg('codev.injectBuilderFileRef', async (builderId: string, text: string) => { + const resolvedId = await terminalManager?.openBuilderByRoleOrId(builderId, true); // open + focus, fallback flow + const ok = resolvedId ? terminalManager?.injectBuilderText(resolvedId, text) : false; + if (!ok) { vscode.window.showWarningMessage('Codev: Builder terminal not available'); } +}); +``` +`openBuilderByRoleOrId` already contains the "no active terminal" recovery flow (`promptNoTerminalRecovery`), satisfying the fallback acceptance criterion. Wire provider registration through a new `activateDiffInjectCodeLens(context)` called alongside `activateDiffView(context)`. + +## Files to Change + +- `packages/vscode/src/diff-inject-ref.ts` — **new**: pure helpers `parseHunkRanges`, `buildBuilderFileRef`, `buildBuilderHunkRef`. +- `packages/vscode/src/diff-inject-codelens.ts` — **new**: `DiffInjectCodeLensProvider` + `activateDiffInjectCodeLens(context)` returning the provider instance (so `viewDiff` can populate it). +- `packages/vscode/src/commands/view-diff.ts:261` / `:301` — fetch + parse the per-file patch in `getBuilderChanges` (or a sibling helper) and call `provider.setSession(...)` in `viewDiff` before `vscode.changes`. +- `packages/vscode/src/terminal-manager.ts:146` (add `injectBuilderText`), `:192` (return resolved id from `openBuilderByRoleOrId`). +- `packages/vscode/src/extension.ts:806` area — register `codev.injectBuilderFileRef`; call `activateDiffInjectCodeLens` near `activateDiffView` (`:884`); hold the provider so `viewDiff` reaches it (pass via the command registration / a module singleton, matching `view-diff`'s existing module-level `provider`). +- `packages/vscode/src/__tests__/diff-inject-ref.test.ts` — **new**: unit tests for hunk parsing + ref builders. + +## Risks & Alternatives Considered + +- **Primary risk — CodeLenses may not render inside `vscode.changes` (the multi-file diff editor).** CodeLens providers are document-scoped, and the multi-diff editor embeds standard diff editors; lenses normally render on the modified side, but multi-diff embedding has historically had gaps. **This is exactly what the `dev-approval` gate validates** — the reviewer runs the worktree and confirms lenses appear. Fallback if they don't render in the multi-diff editor: surface the same lenses in the per-file `vscode.diff` editor (`codev.openBuilderFileDiff`), which definitively renders CodeLenses — same provider, same registry, narrower entry point. I'll flag the outcome explicitly at the gate. +- **Over-trigger:** because the provider matches `{ scheme: 'file' }`, lenses also appear if the reviewer opens that exact worktree file *normally* (outside the diff) after running `viewDiff`. Accepted as benign — the injected reference is still correct and useful. The registry is replaced per `viewDiff` run, so stale sessions don't accumulate. +- **Id mismatch (builderId in registry vs terminal key):** resolved by returning the canonical id from `openBuilderByRoleOrId` and injecting against *that*, not the raw arg. +- **Deleted files:** no right-side `file:` document, so no lenses — you can't reference new lines of a deleted file. Acceptable and out of the issue's intent. +- **Alternative rejected — render lenses on the left `codev-diff:` side** (fully scoped to the diff editor, never over-triggers): rejected because the left side holds *old* content with old-side line numbers and is empty for added files, so it can't carry new-side hunk ranges. +- **Alternative rejected — replace the side-by-side editor with a single unified-diff text document** (file/hunk headers literally in text): rejected as a UX regression — reviewers rely on the side-by-side multi-file editor, and the issue asks to add lenses *to the existing `viewDiff` editor*, not replace it. + +## Test Plan + +**Unit (`diff-inject-ref.test.ts`, vitest — `pnpm --filter codev-vscode test:unit`):** +- `parseHunkRanges` on a multi-hunk patch → correct new-side `{newStart,newEnd}` per hunk; single-line hunk (`@@ -10 +11 @@`) → `{11,11}`; pure-deletion hunk (`+c,0`) clamps to one line; multiple files isolated correctly after the per-file split. +- `buildBuilderFileRef('a/b.ts')` → `'a/b.ts '`; `buildBuilderHunkRef('a/b.ts', 10, 20)` → `'a/b.ts:L10-L20 '`. + +**Manual (at the `dev-approval` gate — the reviewer runs the worktree):** +1. `pnpm build` the vscode package; launch the Extension Development Host (or sideload). +2. With a builder that has changes, run **Codev: View Diff**. +3. Confirm a `Send to builder PTY` lens appears above each file and a `Send to builder PTY (lines N-M)` lens above each hunk. **(Validates the primary risk.)** +4. Click a file lens → builder terminal is revealed/focused and ` ` is typed into the prompt with **no Enter**. +5. Click a hunk lens → `:L-L ` typed, no Enter; line range matches the hunk's new-side range. +6. Confirm `codev.injectBuilderFileRef` does **not** appear in the Command Palette. +7. Close the builder terminal, click a lens → the no-terminal recovery flow opens the terminal, then injects. + +**Build/CI:** `pnpm --filter codev-vscode check-types`, `pnpm --filter codev-vscode lint`, and `pnpm --filter codev-vscode test:unit` must pass. diff --git a/codev/state/pir-789_thread.md b/codev/state/pir-789_thread.md new file mode 100644 index 000000000..50dcb4db6 --- /dev/null +++ b/codev/state/pir-789_thread.md @@ -0,0 +1,17 @@ +# PIR #789 — inject file/hunk ref into builder PTY from the unified-diff editor (codelens) + +## Plan phase (iteration 1) + +Investigated the vscode extension. Key findings: +- `codev.viewDiff` opens VSCode's multi-file diff editor (`vscode.changes`); right side of each file is a plain `file:` URI at `/` — carries new-side line numbers. +- `injectArchitectText` (`terminal-manager.ts:146`) is the inject-without-Enter precedent; no builder equivalent exists yet. +- `openBuilderByRoleOrId` opens/reveals a builder terminal but returns void and resolves roleOrId → a *canonical* id that can differ from the input → terminal key mismatch risk. Plan returns the resolved id to fix this. +- Palette suppression: just don't declare the codelens-backing command in `contributes.commands`. + +### Design chosen +CodeLensProvider on the right-side `file:` documents. New pure helpers (`diff-inject-ref.ts`) for hunk parsing + ref strings; provider (`diff-inject-codelens.ts`) reads a per-`viewDiff`-run registry; `viewDiff` parses one extra `git diff --unified` to populate hunk ranges; `terminal-manager` gets `injectBuilderText` + id-returning `openBuilderByRoleOrId`; `extension.ts` registers palette-hidden `codev.injectBuilderFileRef`. + +### Primary risk (to validate at dev-approval gate) +CodeLenses may not render inside the `vscode.changes` multi-diff editor (document-scoped lenses + multi-diff embedding has historical gaps). Fallback if so: same provider/registry on the per-file `vscode.diff` (`openBuilderFileDiff`). This is exactly what PIR's dev-approval gate (run the worktree) is for. + +Plan written to `codev/plans/789-vscode-inject-file-hunk-refere.md`, committed. Awaiting `plan-approval`. From d87ef59e63eae9fc0ed60d60403531f2966ce9ef Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 10 Jun 2026 08:26:05 +1000 Subject: [PATCH 03/39] chore(porch): 789 plan-approval gate-requested --- codev/projects/789-vscode-inject-file-hunk-refere/status.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/codev/projects/789-vscode-inject-file-hunk-refere/status.yaml b/codev/projects/789-vscode-inject-file-hunk-refere/status.yaml index af0d0b2b5..5ef16c9a7 100644 --- a/codev/projects/789-vscode-inject-file-hunk-refere/status.yaml +++ b/codev/projects/789-vscode-inject-file-hunk-refere/status.yaml @@ -7,6 +7,7 @@ current_plan_phase: null gates: plan-approval: status: pending + requested_at: '2026-06-09T22:26:05.749Z' dev-approval: status: pending pr: @@ -15,4 +16,4 @@ iteration: 1 build_complete: false history: [] started_at: '2026-06-09T22:20:14.795Z' -updated_at: '2026-06-09T22:20:14.795Z' +updated_at: '2026-06-09T22:26:05.749Z' From edaeac9e53c594c7fd9d7043c716885f3cb91551 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 10 Jun 2026 08:41:51 +1000 Subject: [PATCH 04/39] [PIR #789] Plan revised: add coexistence/no-collision note --- codev/plans/789-vscode-inject-file-hunk-refere.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/codev/plans/789-vscode-inject-file-hunk-refere.md b/codev/plans/789-vscode-inject-file-hunk-refere.md index 56144d2ba..c65978689 100644 --- a/codev/plans/789-vscode-inject-file-hunk-refere.md +++ b/codev/plans/789-vscode-inject-file-hunk-refere.md @@ -56,7 +56,8 @@ reg('codev.injectBuilderFileRef', async (builderId: string, text: string) => { ## Risks & Alternatives Considered - **Primary risk — CodeLenses may not render inside `vscode.changes` (the multi-file diff editor).** CodeLens providers are document-scoped, and the multi-diff editor embeds standard diff editors; lenses normally render on the modified side, but multi-diff embedding has historically had gaps. **This is exactly what the `dev-approval` gate validates** — the reviewer runs the worktree and confirms lenses appear. Fallback if they don't render in the multi-diff editor: surface the same lenses in the per-file `vscode.diff` editor (`codev.openBuilderFileDiff`), which definitively renders CodeLenses — same provider, same registry, narrower entry point. I'll flag the outcome explicitly at the gate. -- **Over-trigger:** because the provider matches `{ scheme: 'file' }`, lenses also appear if the reviewer opens that exact worktree file *normally* (outside the diff) after running `viewDiff`. Accepted as benign — the injected reference is still correct and useful. The registry is replaced per `viewDiff` run, so stale sessions don't accumulate. +- **Over-trigger:** because the provider matches `{ scheme: 'file' }`, lenses also appear if the reviewer opens that exact worktree file *normally* (outside the diff) after running `viewDiff`. Accepted as benign — the injected reference is still correct and useful. The registry is replaced per `viewDiff` run, so stale sessions don't accumulate. (Optional tightening: scope the selector to a worktree glob so `provideCodeLenses` isn't even invoked for unrelated `file:` documents — functionally inert either way since non-registry paths return `[]`.) +- **Coexistence with existing editor chrome — no collision, purely additive.** The extension registers *no* `CodeLensProvider` today, and the `viewDiff` editor shows zero codelenses by default (left side is the no-language `codev-diff:` scheme; right side is a `file:` source doc, and TS reference/implementation lenses are off by default and unset in this repo). Ours are the first lenses in that editor. They render as *viewzone* lines anchored above a code line, so they (a) do **not** remove, reorder, or override the multi-diff editor's built-in chrome — the per-file header (filename, collapse/expand, open-file) and the diff gutter are untouched; (b) do **not** shift line numbers or desync hunk mapping (viewzones aren't real document lines; the editor inserts a matching filler on the opposite side to keep left/right aligned); and (c) coexist with any future language-service lenses — VSCode joins multiple providers' lenses on the same anchor with ` | ` separators rather than suppressing either. The only real cost is added vertical space (one lens line per file + per hunk), beyond the primary multi-diff-rendering risk above. The inline plan-review Comments API (`activateReviewComments`) is scoped to `codev/plans|specs|reviews/*.md` documents, not the diffed source files, so it's unaffected. - **Id mismatch (builderId in registry vs terminal key):** resolved by returning the canonical id from `openBuilderByRoleOrId` and injecting against *that*, not the raw arg. - **Deleted files:** no right-side `file:` document, so no lenses — you can't reference new lines of a deleted file. Acceptable and out of the issue's intent. - **Alternative rejected — render lenses on the left `codev-diff:` side** (fully scoped to the diff editor, never over-triggers): rejected because the left side holds *old* content with old-side line numbers and is empty for added files, so it can't carry new-side hunk ranges. From 74b9d3267b565b34bf6a981141e39e5a60f71cd4 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 10 Jun 2026 08:58:51 +1000 Subject: [PATCH 05/39] chore(porch): 789 plan-approval gate-approved --- .../projects/789-vscode-inject-file-hunk-refere/status.yaml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/codev/projects/789-vscode-inject-file-hunk-refere/status.yaml b/codev/projects/789-vscode-inject-file-hunk-refere/status.yaml index 5ef16c9a7..dcf4ddfa6 100644 --- a/codev/projects/789-vscode-inject-file-hunk-refere/status.yaml +++ b/codev/projects/789-vscode-inject-file-hunk-refere/status.yaml @@ -6,8 +6,9 @@ plan_phases: [] current_plan_phase: null gates: plan-approval: - status: pending + status: approved requested_at: '2026-06-09T22:26:05.749Z' + approved_at: '2026-06-09T22:58:51.188Z' dev-approval: status: pending pr: @@ -16,4 +17,4 @@ iteration: 1 build_complete: false history: [] started_at: '2026-06-09T22:20:14.795Z' -updated_at: '2026-06-09T22:26:05.749Z' +updated_at: '2026-06-09T22:58:51.189Z' From efecc71783b3cb283a0e63cb65cfe70fbba15c84 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 10 Jun 2026 08:58:57 +1000 Subject: [PATCH 06/39] chore(porch): 789 implement phase-transition --- codev/projects/789-vscode-inject-file-hunk-refere/status.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codev/projects/789-vscode-inject-file-hunk-refere/status.yaml b/codev/projects/789-vscode-inject-file-hunk-refere/status.yaml index dcf4ddfa6..aacba346d 100644 --- a/codev/projects/789-vscode-inject-file-hunk-refere/status.yaml +++ b/codev/projects/789-vscode-inject-file-hunk-refere/status.yaml @@ -1,7 +1,7 @@ id: '789' title: vscode-inject-file-hunk-refere protocol: pir -phase: plan +phase: implement plan_phases: [] current_plan_phase: null gates: @@ -17,4 +17,4 @@ iteration: 1 build_complete: false history: [] started_at: '2026-06-09T22:20:14.795Z' -updated_at: '2026-06-09T22:58:51.189Z' +updated_at: '2026-06-09T22:58:57.235Z' From 7ee8068aac5b63b21603acdf7f856a2be531a069 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 10 Jun 2026 09:03:40 +1000 Subject: [PATCH 07/39] [PIR #789] Add diff-inject ref helpers + CodeLens provider --- .../src/__tests__/diff-inject-ref.test.ts | 131 ++++++++++++++++++ packages/vscode/src/diff-inject-codelens.ts | 83 +++++++++++ packages/vscode/src/diff-inject-ref.ts | 120 ++++++++++++++++ 3 files changed, 334 insertions(+) create mode 100644 packages/vscode/src/__tests__/diff-inject-ref.test.ts create mode 100644 packages/vscode/src/diff-inject-codelens.ts create mode 100644 packages/vscode/src/diff-inject-ref.ts diff --git a/packages/vscode/src/__tests__/diff-inject-ref.test.ts b/packages/vscode/src/__tests__/diff-inject-ref.test.ts new file mode 100644 index 000000000..5692a6219 --- /dev/null +++ b/packages/vscode/src/__tests__/diff-inject-ref.test.ts @@ -0,0 +1,131 @@ +/** + * Unit tests for the pure diff/ref helpers behind the "Send to builder PTY" + * CodeLens actions (#789). No `vscode` dependency, so the live implementation + * is imported directly (same pattern as `architect-reference-injection.test.ts`). + */ + +import { describe, it, expect } from 'vitest'; +import { + parseHunkRanges, + parseUnifiedDiff, + buildBuilderFileRef, + buildBuilderHunkRef, + buildLensDescriptors, +} from '../diff-inject-ref.js'; + +describe('parseHunkRanges', () => { + it('reads the new-side start and length from each hunk header', () => { + const patch = [ + '@@ -1,4 +1,6 @@', + ' a', + '+b', + '@@ -20,3 +22,10 @@ func()', + ' c', + ].join('\n'); + expect(parseHunkRanges(patch)).toEqual([ + { newStart: 1, newEnd: 6 }, + { newStart: 22, newEnd: 31 }, + ]); + }); + + it('treats an absent new-side length as a single line', () => { + expect(parseHunkRanges('@@ -10 +11 @@')).toEqual([{ newStart: 11, newEnd: 11 }]); + }); + + it('clamps a pure-deletion hunk (+c,0) to a single anchor line', () => { + expect(parseHunkRanges('@@ -5,3 +4,0 @@')).toEqual([{ newStart: 4, newEnd: 4 }]); + }); + + it('ignores non-hunk lines, including content that looks like @@', () => { + const patch = ['+const x = "@@ not a header";', '@@ -1,1 +1,2 @@'].join('\n'); + expect(parseHunkRanges(patch)).toEqual([{ newStart: 1, newEnd: 2 }]); + }); +}); + +describe('parseUnifiedDiff', () => { + it('maps each file new-path to its hunk ranges', () => { + const patch = [ + 'diff --git a/src/a.ts b/src/a.ts', + 'index 111..222 100644', + '--- a/src/a.ts', + '+++ b/src/a.ts', + '@@ -1,2 +1,3 @@', + ' x', + '+y', + 'diff --git a/src/b.ts b/src/b.ts', + 'index 333..444 100644', + '--- a/src/b.ts', + '+++ b/src/b.ts', + '@@ -10,0 +11,2 @@', + '+p', + '+q', + ].join('\n'); + const map = parseUnifiedDiff(patch); + expect(map.get('src/a.ts')).toEqual([{ newStart: 1, newEnd: 3 }]); + expect(map.get('src/b.ts')).toEqual([{ newStart: 11, newEnd: 12 }]); + }); + + it('uses the new path for a rename (+++ b/)', () => { + const patch = [ + 'diff --git a/old/name.ts b/new/name.ts', + 'similarity index 90%', + 'rename from old/name.ts', + 'rename to new/name.ts', + '--- a/old/name.ts', + '+++ b/new/name.ts', + '@@ -3,1 +3,2 @@', + '+added', + ].join('\n'); + const map = parseUnifiedDiff(patch); + expect([...map.keys()]).toEqual(['new/name.ts']); + expect(map.get('new/name.ts')).toEqual([{ newStart: 3, newEnd: 4 }]); + }); + + it('omits deleted files (new side is /dev/null)', () => { + const patch = [ + 'diff --git a/gone.ts b/gone.ts', + 'deleted file mode 100644', + '--- a/gone.ts', + '+++ /dev/null', + '@@ -1,3 +0,0 @@', + '-x', + ].join('\n'); + expect(parseUnifiedDiff(patch).size).toBe(0); + }); +}); + +describe('ref builders', () => { + it('builds a file ref with a trailing space and no newline', () => { + expect(buildBuilderFileRef('packages/vscode/src/extension.ts')) + .toBe('packages/vscode/src/extension.ts '); + }); + + it('builds a hunk ref with the L-L range', () => { + expect(buildBuilderHunkRef('a/b.ts', 10, 20)).toBe('a/b.ts:L10-L20 '); + }); +}); + +describe('buildLensDescriptors', () => { + it('emits a file-level lens at line 0 plus one lens per hunk', () => { + const lenses = buildLensDescriptors('a/b.ts', [ + { newStart: 5, newEnd: 9 }, + { newStart: 30, newEnd: 30 }, + ]); + expect(lenses).toEqual([ + { line: 0, title: 'Send to builder PTY', refText: 'a/b.ts ' }, + { line: 4, title: 'Send to builder PTY (lines 5-9)', refText: 'a/b.ts:L5-L9 ' }, + { line: 29, title: 'Send to builder PTY (lines 30-30)', refText: 'a/b.ts:L30-L30 ' }, + ]); + }); + + it('clamps a hunk anchored at line 1 to a non-negative index', () => { + const lenses = buildLensDescriptors('a/b.ts', [{ newStart: 1, newEnd: 1 }]); + expect(lenses[1]!.line).toBe(0); + }); + + it('emits just the file-level lens when there are no hunks', () => { + expect(buildLensDescriptors('a/b.ts', [])).toEqual([ + { line: 0, title: 'Send to builder PTY', refText: 'a/b.ts ' }, + ]); + }); +}); diff --git a/packages/vscode/src/diff-inject-codelens.ts b/packages/vscode/src/diff-inject-codelens.ts new file mode 100644 index 000000000..2a6f3981a --- /dev/null +++ b/packages/vscode/src/diff-inject-codelens.ts @@ -0,0 +1,83 @@ +/** + * CodeLens provider backing the "Send to builder PTY" actions in the + * `codev.viewDiff` multi-file diff editor (issue #789). + * + * The right side of each file in that editor is a plain `file:` document at + * `/`, so we register against `{ scheme: 'file' }` + * and emit lenses only for documents whose fs path is in the *active diff + * session* registry. `viewDiff` replaces that registry (via + * `setDiffInjectSession`) on each invocation, so lenses are scoped to the + * files the reviewer just opened for diffing; any other `file:` document + * returns `[]` and is unaffected. + * + * Clicking a lens runs `codev.injectBuilderFileRef` (a palette-hidden command + * registered in `extension.ts`), which opens/reveals the builder terminal and + * types the reference into its prompt without pressing Enter — mirroring the + * architect-reference pattern (`codev.referenceIssueInArchitect`). + */ + +import * as vscode from 'vscode'; +import { buildLensDescriptors, type HunkRange } from './diff-inject-ref.js'; + +/** Command id the lenses invoke. Registered in `extension.ts`, NOT declared in + * `contributes.commands`, so it never appears in the Command Palette. */ +export const INJECT_BUILDER_FILE_REF = 'codev.injectBuilderFileRef'; + +/** One changed file in the active diff session, keyed by its right-side fs path. */ +export interface DiffInjectSessionEntry { + /** Absolute fs path of the right-side (worktree) document. */ + fsPath: string; + /** Builder that owns this diff (canonical id used for the terminal lookup). */ + builderId: string; + /** Repo-relative path injected into the prompt. */ + relPath: string; + /** New-side hunk ranges for the per-hunk lenses. */ + hunks: HunkRange[]; +} + +class DiffInjectCodeLensProvider implements vscode.CodeLensProvider { + private registry = new Map(); + private readonly _onDidChangeCodeLenses = new vscode.EventEmitter(); + readonly onDidChangeCodeLenses = this._onDidChangeCodeLenses.event; + + setSession(entries: DiffInjectSessionEntry[]): void { + this.registry = new Map(entries.map(e => [e.fsPath, e])); + this._onDidChangeCodeLenses.fire(); + } + + provideCodeLenses(document: vscode.TextDocument): vscode.CodeLens[] { + const entry = this.registry.get(document.uri.fsPath); + if (!entry) { return []; } + const lastLine = Math.max(document.lineCount - 1, 0); + return buildLensDescriptors(entry.relPath, entry.hunks).map(d => { + const line = Math.min(Math.max(d.line, 0), lastLine); + const range = new vscode.Range(line, 0, line, 0); + return new vscode.CodeLens(range, { + title: d.title, + command: INJECT_BUILDER_FILE_REF, + arguments: [entry.builderId, d.refText], + }); + }); + } + + dispose(): void { + this._onDidChangeCodeLenses.dispose(); + } +} + +const provider = new DiffInjectCodeLensProvider(); + +/** Register the provider for `file:` documents. Called once at activation, + * alongside `activateDiffView`. */ +export function activateDiffInjectCodeLens(context: vscode.ExtensionContext): void { + context.subscriptions.push( + vscode.languages.registerCodeLensProvider({ scheme: 'file' }, provider), + provider, + ); +} + +/** Replace the active diff session — called by `viewDiff` before it opens the + * multi-file diff editor. */ +export function setDiffInjectSession(entries: DiffInjectSessionEntry[]): void { + provider.setSession(entries); +} diff --git a/packages/vscode/src/diff-inject-ref.ts b/packages/vscode/src/diff-inject-ref.ts new file mode 100644 index 000000000..a049f3aa6 --- /dev/null +++ b/packages/vscode/src/diff-inject-ref.ts @@ -0,0 +1,120 @@ +/** + * Pure helpers for the "Send to builder PTY" CodeLens actions in the + * `codev.viewDiff` editor (issue #789). No `vscode` import — same precedent + * as `architect-reference-injection.ts`, so the parsing/string logic is + * unit-tested directly without mocking the editor API. + * + * The provider (`diff-inject-codelens.ts`) is thin glue over these: it turns + * `LensDescriptor`s into `vscode.CodeLens` objects and wires the inject + * command. All the line-math and ref-string logic lives here. + */ + +/** New-side line range of one diff hunk (1-based, inclusive). */ +export interface HunkRange { + newStart: number; + newEnd: number; +} + +/** + * Editor-agnostic description of one CodeLens to render: the 0-based line to + * anchor it on, the label, and the text to inject into the builder prompt. + */ +export interface LensDescriptor { + /** 0-based anchor line (the provider clamps to the document bounds). */ + line: number; + title: string; + /** Text typed into the builder terminal — always ends with a space, no Enter. */ + refText: string; +} + +const HUNK_HEADER = /^@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@/; + +/** + * Parse the `@@ -a,b +c,d @@` hunk headers from a single file's unified diff + * and return each hunk's new-side line range. + * + * - New-side length is `d` (absent → 1, matching git's shorthand for a + * single added/changed line). + * - A pure-deletion hunk (`+c,0`) has no new-side lines; we clamp it to a + * single anchor at `c` (or line 1 if `c` is 0) so a click still references + * a sane location near the change. + */ +export function parseHunkRanges(patch: string): HunkRange[] { + const ranges: HunkRange[] = []; + for (const line of patch.split('\n')) { + const m = HUNK_HEADER.exec(line); + if (!m) { continue; } + const newStart = Number(m[1]); + const len = m[2] === undefined ? 1 : Number(m[2]); + if (len <= 0) { + const anchor = Math.max(newStart, 1); + ranges.push({ newStart: anchor, newEnd: anchor }); + } else { + ranges.push({ newStart, newEnd: newStart + len - 1 }); + } + } + return ranges; +} + +/** + * Split a multi-file unified diff (`git diff -M --unified=N `) into a + * map from each file's **new** path to its hunk ranges. The new path is read + * from the `+++ b/` line (the canonical new-side path, correct for + * renames); files whose new side is `/dev/null` (deletions) are omitted — + * they have no right-side document to host a lens. + */ +export function parseUnifiedDiff(patch: string): Map { + const out = new Map(); + // Split on the per-file boundary; the first chunk before any `diff --git` + // is empty/preamble and parses to no path, so it's harmless. + const sections = patch.split(/^diff --git .*$/m).slice(1); + // `split` drops the delimiter lines, but the `+++`/`@@` lines we need live + // in the body after each boundary, so re-walk the raw text per section. + for (const section of sections) { + const newPath = newPathFromSection(section); + if (!newPath) { continue; } + out.set(newPath, parseHunkRanges(section)); + } + return out; +} + +/** Extract the new-side path from a single file section's `+++ b/` line. */ +function newPathFromSection(section: string): string | null { + for (const line of section.split('\n')) { + if (!line.startsWith('+++ ')) { continue; } + const target = line.slice(4).trim(); + if (target === '/dev/null') { return null; } + // Strip the conventional `b/` prefix git prepends to the new path. + return target.startsWith('b/') ? target.slice(2) : target; + } + return null; +} + +/** Text injected by the file-level lens: ` ` (no Enter). */ +export function buildBuilderFileRef(relPath: string): string { + return `${relPath} `; +} + +/** Text injected by a hunk lens: `:L-L ` (no Enter). */ +export function buildBuilderHunkRef(relPath: string, start: number, end: number): string { + return `${relPath}:L${start}-L${end} `; +} + +/** + * Build the full set of lens descriptors for one changed file: a file-level + * lens at the top, plus one lens per hunk anchored at the hunk's new-side + * start line (converted to 0-based). + */ +export function buildLensDescriptors(relPath: string, hunks: HunkRange[]): LensDescriptor[] { + const lenses: LensDescriptor[] = [ + { line: 0, title: 'Send to builder PTY', refText: buildBuilderFileRef(relPath) }, + ]; + for (const h of hunks) { + lenses.push({ + line: Math.max(h.newStart - 1, 0), + title: `Send to builder PTY (lines ${h.newStart}-${h.newEnd})`, + refText: buildBuilderHunkRef(relPath, h.newStart, h.newEnd), + }); + } + return lenses; +} From 4c6af3ee7ea8db5b9b5bcf2fdf8e3f660b69488c Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 10 Jun 2026 09:03:40 +1000 Subject: [PATCH 08/39] [PIR #789] terminal-manager: injectBuilderText + return resolved id from openBuilderByRoleOrId --- packages/vscode/src/terminal-manager.ts | 32 +++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/packages/vscode/src/terminal-manager.ts b/packages/vscode/src/terminal-manager.ts index 827afa0ad..136a6ff6a 100644 --- a/packages/vscode/src/terminal-manager.ts +++ b/packages/vscode/src/terminal-manager.ts @@ -152,6 +152,22 @@ export class TerminalManager { return true; } + /** + * Type `text` into a builder terminal's input *without* a trailing newline + * (no submit) — the builder-side analogue of `injectArchitectText`, backing + * the "Send to builder PTY" CodeLens (#789). Returns false if no terminal is + * registered for `builderId`; callers ensure it's open first (via + * `openBuilderByRoleOrId`, whose resolved id keys the lookup). + */ + injectBuilderText(builderId: string, text: string): boolean { + const key = `builder-${builderId}`; + const entry = this.terminals.get(key); + if (!entry) { return false; } + entry.terminal.show(); + entry.terminal.sendText(text, false); + return true; + } + /** * Open a builder terminal. If a terminal already exists for this builder * but points at a different (stale) Tower session, dispose it before @@ -188,13 +204,19 @@ export class TerminalManager { * Tower's `/api/state` can momentarily omit a live builder while its session * registry rehydrates/reconnects, and that transient miss self-heals on the * next call (PIR #982). Only a persistent miss surfaces the recovery toast. + * + * Returns the resolved **canonical** builder id on success (the key under + * which the terminal is registered), or `undefined` when the builder can't + * be opened. The "Send to builder PTY" inject path (#789) uses this to target + * the same terminal that was opened, even when called with a bare numeric id; + * other callers ignore the return value. */ - async openBuilderByRoleOrId(roleOrId: string, focus = false): Promise { + async openBuilderByRoleOrId(roleOrId: string, focus = false): Promise { const client = this.connectionManager.getClient(); const workspacePath = this.connectionManager.getWorkspacePath(); if (!client || !workspacePath) { vscode.window.showErrorMessage('Codev: Not connected to Tower'); - return; + return undefined; } try { // resolveBuilderTerminal uses resolveAgentName internally so the bare @@ -212,17 +234,19 @@ export class TerminalManager { vscode.window.showWarningMessage( `Codev: Multiple builders match "${roleOrId}": ${outcome.matches.map(b => b.name).join(', ')}`, ); - return; + return undefined; } if (outcome.kind === 'missing') { await this.promptNoTerminalRecovery(roleOrId, workspacePath, focus); - return; + return undefined; } const { builder, terminalId } = outcome; await this.openBuilder(terminalId, builder.id, `Codev: ${builder.name}`, focus); + return builder.id; } catch (err) { this.log('ERROR', `Failed to open builder ${roleOrId}: ${(err as Error).message}`); vscode.window.showErrorMessage(`Codev: Failed to open ${roleOrId}`); + return undefined; } } From 2777acc4a1bb27020823c83cf7ae2bf8bb5e7fe8 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 10 Jun 2026 09:03:40 +1000 Subject: [PATCH 09/39] [PIR #789] Wire viewDiff registry + register palette-hidden inject command --- packages/vscode/src/commands/view-diff.ts | 34 ++++++++++++++++++++--- packages/vscode/src/extension.ts | 21 ++++++++++++++ 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/packages/vscode/src/commands/view-diff.ts b/packages/vscode/src/commands/view-diff.ts index 7d865c53e..4358c9234 100644 --- a/packages/vscode/src/commands/view-diff.ts +++ b/packages/vscode/src/commands/view-diff.ts @@ -28,6 +28,8 @@ import { execFile } from 'node:child_process'; import { promisify } from 'node:util'; import * as path from 'node:path'; import type { ConnectionManager } from '../connection-manager.js'; +import { parseUnifiedDiff } from '../diff-inject-ref.js'; +import { setDiffInjectSession, type DiffInjectSessionEntry } from '../diff-inject-codelens.js'; const execFileAsync = promisify(execFile); @@ -348,10 +350,9 @@ export async function viewDiff( return; } - const resources: Array<[vscode.Uri, vscode.Uri, vscode.Uri]> = planResources( - changes, - binaryPaths, - ).map(plan => { + const plans = planResources(changes, binaryPaths); + + const resources: Array<[vscode.Uri, vscode.Uri, vscode.Uri]> = plans.map(plan => { const { left, right } = diffUrisForChange(plan, { wt, ref: baseRef }); return [ vscode.Uri.file(path.join(wt, plan.resourcePath)), @@ -360,6 +361,31 @@ export async function viewDiff( ]; }); + // CodeLens "Send to builder PTY" actions (#789): give the provider the + // right-side fs paths and new-side hunk ranges for the files just opened. + // One extra git call here keeps `getBuilderChanges` (shared with the + // Builders tree) untouched. A patch failure is non-fatal — the diff still + // opens, just without lenses. + try { + const { stdout: patch } = await execFileAsync( + 'git', + ['-C', wt, 'diff', '-M', '--unified=3', baseRef], + { maxBuffer: 64 * 1024 * 1024 }, + ); + const hunksByPath = parseUnifiedDiff(patch); + const entries: DiffInjectSessionEntry[] = plans + .filter(plan => plan.right.kind === 'file') + .map(plan => ({ + fsPath: path.join(wt, plan.resourcePath), + builderId: builder.id, + relPath: plan.resourcePath, + hunks: hunksByPath.get(plan.resourcePath) ?? [], + })); + setDiffInjectSession(entries); + } catch { + setDiffInjectSession([]); + } + await vscode.commands.executeCommand( 'vscode.changes', `Reviewing #${builder.issueId ?? builder.id} (${defaultBranch} ↔ HEAD)`, diff --git a/packages/vscode/src/extension.ts b/packages/vscode/src/extension.ts index 0e6d40e2c..622f5cd5c 100644 --- a/packages/vscode/src/extension.ts +++ b/packages/vscode/src/extension.ts @@ -8,6 +8,7 @@ import { approveGate } from './commands/approve.js'; import { cleanupBuilder } from './commands/cleanup.js'; import { openWorktreeWindow } from './commands/open-worktree-window.js'; import { viewDiff, activateDiffView, diffUrisForChange } from './commands/view-diff.js'; +import { activateDiffInjectCodeLens } from './diff-inject-codelens.js'; import { runWorktreeDev } from './commands/run-worktree-dev.js'; import { stopWorktreeDev } from './commands/stop-worktree-dev.js'; import { runWorkspaceDev, stopWorkspaceDev } from './commands/run-workspace-dev.js'; @@ -805,6 +806,20 @@ export async function activate(context: vscode.ExtensionContext) { openWorktreeWindow(connectionManager!, extractBuilderId(arg))), reg('codev.viewDiff', (arg: vscode.TreeItem | string | undefined) => viewDiff(connectionManager!, extractBuilderId(arg))), + // CodeLens-only inject (#789): open + focus the builder terminal, then + // type the file/hunk reference into its prompt without submitting, so + // the reviewer keeps typing feedback before hitting Enter. Mirrors + // `codev.referenceIssueInArchitect`. Not declared in + // `contributes.commands` → never appears in the Command Palette. + reg('codev.injectBuilderFileRef', async (builderId: string, text: string) => { + // openBuilderByRoleOrId resolves to the canonical id and runs the + // no-terminal recovery flow on a miss; inject against that id so the + // terminal lookup hits the same key that was just opened. + const resolvedId = await terminalManager?.openBuilderByRoleOrId(builderId, true); + if (resolvedId && !terminalManager?.injectBuilderText(resolvedId, text)) { + vscode.window.showWarningMessage('Codev: Builder terminal not available'); + } + }), reg('codev.openBuilderFileDiff', async (arg: unknown) => { if (!(arg instanceof BuilderFileTreeItem)) { return; } const { left, right } = diffUrisForChange(arg.plan, { wt: arg.worktreePath, ref: arg.baseRef }); @@ -883,6 +898,12 @@ export async function activate(context: vscode.ExtensionContext) { // without relying on the Git extension's worktree discovery. activateDiffView(context); + // CodeLens "Send to builder PTY" actions inside the View Diff editor (#789). + // The backing command `codev.injectBuilderFileRef` is registered below and + // deliberately NOT declared in `contributes.commands`, so it stays out of + // the Command Palette (codelens-only entry point). + activateDiffInjectCodeLens(context); + // Review comment decorations activateReviewDecorations(context); From cc70aa8f267d001f80e96fcd70cfbcb6fb8d63f9 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 10 Jun 2026 09:03:56 +1000 Subject: [PATCH 10/39] [PIR #789] Update thread: implement phase complete --- codev/state/pir-789_thread.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/codev/state/pir-789_thread.md b/codev/state/pir-789_thread.md index 50dcb4db6..b93cec6b1 100644 --- a/codev/state/pir-789_thread.md +++ b/codev/state/pir-789_thread.md @@ -15,3 +15,16 @@ CodeLensProvider on the right-side `file:` documents. New pure helpers (`diff-in CodeLenses may not render inside the `vscode.changes` multi-diff editor (document-scoped lenses + multi-diff embedding has historical gaps). Fallback if so: same provider/registry on the per-file `vscode.diff` (`openBuilderFileDiff`). This is exactly what PIR's dev-approval gate (run the worktree) is for. Plan written to `codev/plans/789-vscode-inject-file-hunk-refere.md`, committed. Awaiting `plan-approval`. + +## Implement phase (iteration 1) + +plan-approval approved. Implemented per plan, 3 commits: +- `diff-inject-ref.ts` (pure helpers: parseHunkRanges/parseUnifiedDiff/buildBuilderFileRef/buildBuilderHunkRef/buildLensDescriptors) + `diff-inject-codelens.ts` (provider, `{scheme:'file'}`, per-viewDiff-run registry) + `diff-inject-ref.test.ts` (12 tests). +- `terminal-manager.ts`: added `injectBuilderText`; `openBuilderByRoleOrId` now returns the resolved canonical id (non-breaking). +- `view-diff.ts`: one extra `git diff -M --unified=3` parsed into the registry before `vscode.changes` (patch failure non-fatal). `extension.ts`: palette-hidden `codev.injectBuilderFileRef` + `activateDiffInjectCodeLens`. + +Validation: `pnpm test:unit` 372 pass (had to build core+types dist first — the 7 import-resolution failures were pre-existing unbuilt-dep, not my diff), `check-types` clean, `lint` clean, esbuild bundles. + +**Primary risk still unvalidated**: whether CodeLenses render inside the `vscode.changes` multi-diff editor. Needs the human to run the worktree at the dev-approval gate. Fallback documented in plan if they don't render. + +Awaiting `dev-approval`. From 8089b78db895220a0285eb21d08b03802143ce65 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 10 Jun 2026 09:04:34 +1000 Subject: [PATCH 11/39] chore(porch): 789 dev-approval gate-requested --- codev/projects/789-vscode-inject-file-hunk-refere/status.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/codev/projects/789-vscode-inject-file-hunk-refere/status.yaml b/codev/projects/789-vscode-inject-file-hunk-refere/status.yaml index aacba346d..7e8b33521 100644 --- a/codev/projects/789-vscode-inject-file-hunk-refere/status.yaml +++ b/codev/projects/789-vscode-inject-file-hunk-refere/status.yaml @@ -11,10 +11,11 @@ gates: approved_at: '2026-06-09T22:58:51.188Z' dev-approval: status: pending + requested_at: '2026-06-09T23:04:34.034Z' pr: status: pending iteration: 1 build_complete: false history: [] started_at: '2026-06-09T22:20:14.795Z' -updated_at: '2026-06-09T22:58:57.235Z' +updated_at: '2026-06-09T23:04:34.035Z' From fecb9658ed59a22a43e6f1754a66f9895da8ec5c Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 10 Jun 2026 09:39:33 +1000 Subject: [PATCH 12/39] [PIR #789] Option A: lenses on per-file diff + diffEditor.codeLens prompt The multi-file vscode.changes editor does not render CodeLens (VS Code hides CodeLens in diff editors; the multi-diff editor ignores the diffEditor.codeLens setting entirely). Surface the inject lenses on the single-file vscode.diff opened from the Builders tree instead, which honors diffEditor.codeLens. openBuilderFileDiff now populates the registry for its file and prompts to enable the setting when it is off. --- packages/vscode/src/commands/view-diff.ts | 40 +++++++++++++++++++-- packages/vscode/src/diff-inject-codelens.ts | 14 ++++++++ packages/vscode/src/ensure-diff-codelens.ts | 34 ++++++++++++++++++ packages/vscode/src/extension.ts | 14 +++++++- 4 files changed, 99 insertions(+), 3 deletions(-) create mode 100644 packages/vscode/src/ensure-diff-codelens.ts diff --git a/packages/vscode/src/commands/view-diff.ts b/packages/vscode/src/commands/view-diff.ts index 4358c9234..16036828e 100644 --- a/packages/vscode/src/commands/view-diff.ts +++ b/packages/vscode/src/commands/view-diff.ts @@ -28,8 +28,8 @@ import { execFile } from 'node:child_process'; import { promisify } from 'node:util'; import * as path from 'node:path'; import type { ConnectionManager } from '../connection-manager.js'; -import { parseUnifiedDiff } from '../diff-inject-ref.js'; -import { setDiffInjectSession, type DiffInjectSessionEntry } from '../diff-inject-codelens.js'; +import { parseUnifiedDiff, parseHunkRanges, type HunkRange } from '../diff-inject-ref.js'; +import { setDiffInjectSession, upsertDiffInjectEntry, type DiffInjectSessionEntry } from '../diff-inject-codelens.js'; const execFileAsync = promisify(execFile); @@ -393,6 +393,42 @@ export async function viewDiff( ); } +/** + * Populate the inject-codelens registry for a single changed file — the + * per-file `vscode.diff` opened from the Builders tree (`openBuilderFileDiff`). + * Mirrors what `viewDiff` does for the whole delta, but for one file, so the + * "Send to builder PTY" lenses appear even when the reviewer opens a file diff + * without first running View Diff. Non-fatal on git failure (the file-level + * lens still shows; only the per-hunk lenses are dropped). + */ +export async function registerFileInjectSession(args: { + worktreePath: string; + baseRef: string; + builderId: string; + plan: ResourcePlan; +}): Promise { + // Deleted/binary files have no right-side `file:` document to host a lens. + if (args.plan.right.kind !== 'file') { return; } + const relPath = args.plan.resourcePath; + let hunks: HunkRange[] = []; + try { + const { stdout } = await execFileAsync( + 'git', + ['-C', args.worktreePath, 'diff', '-M', '--unified=3', args.baseRef, '--', relPath], + { maxBuffer: 64 * 1024 * 1024 }, + ); + hunks = parseHunkRanges(stdout); + } catch { + // Leave hunks empty — the file-level lens is still useful. + } + upsertDiffInjectEntry({ + fsPath: path.join(args.worktreePath, relPath), + builderId: args.builderId, + relPath, + hunks, + }); +} + interface BuilderLike { id: string; issueId: string | null; diff --git a/packages/vscode/src/diff-inject-codelens.ts b/packages/vscode/src/diff-inject-codelens.ts index 2a6f3981a..d6397e0ba 100644 --- a/packages/vscode/src/diff-inject-codelens.ts +++ b/packages/vscode/src/diff-inject-codelens.ts @@ -45,6 +45,14 @@ class DiffInjectCodeLensProvider implements vscode.CodeLensProvider { this._onDidChangeCodeLenses.fire(); } + /** Add/replace a single file's entry without clearing the rest — used by the + * per-file diff path (`codev.openBuilderFileDiff`), which can be opened + * without a prior `viewDiff` run. */ + upsert(entry: DiffInjectSessionEntry): void { + this.registry.set(entry.fsPath, entry); + this._onDidChangeCodeLenses.fire(); + } + provideCodeLenses(document: vscode.TextDocument): vscode.CodeLens[] { const entry = this.registry.get(document.uri.fsPath); if (!entry) { return []; } @@ -81,3 +89,9 @@ export function activateDiffInjectCodeLens(context: vscode.ExtensionContext): vo export function setDiffInjectSession(entries: DiffInjectSessionEntry[]): void { provider.setSession(entries); } + +/** Add/replace one file's entry without clearing the rest — called by the + * per-file diff path so its lenses appear without a prior `viewDiff` run. */ +export function upsertDiffInjectEntry(entry: DiffInjectSessionEntry): void { + provider.upsert(entry); +} diff --git a/packages/vscode/src/ensure-diff-codelens.ts b/packages/vscode/src/ensure-diff-codelens.ts new file mode 100644 index 000000000..c0cfae17d --- /dev/null +++ b/packages/vscode/src/ensure-diff-codelens.ts @@ -0,0 +1,34 @@ +/** + * The "Send to builder PTY" CodeLens actions (#789) only render in a diff + * editor when `diffEditor.codeLens` is on — VS Code hides CodeLens in diff + * editors by default (microsoft/vscode#97640). When a reviewer opens a builder + * file diff with the setting off, offer a one-click enable rather than letting + * the feature look silently broken. A "Don't ask again" choice is remembered in + * globalState so it never nags. + */ + +import * as vscode from 'vscode'; + +const DISMISS_KEY = 'codev.diffCodeLensPromptDismissed'; + +export async function ensureDiffEditorCodeLens( + context: vscode.ExtensionContext, +): Promise { + const cfg = vscode.workspace.getConfiguration('diffEditor'); + if (cfg.get('codeLens')) { return; } + if (context.globalState.get(DISMISS_KEY)) { return; } + + const ENABLE = 'Enable'; + const DISMISS = "Don't ask again"; + const choice = await vscode.window.showInformationMessage( + 'Codev’s "Send to builder PTY" actions need CodeLens in diff editors, ' + + 'which VS Code hides by default. Enable it?', + ENABLE, + DISMISS, + ); + if (choice === ENABLE) { + await cfg.update('codeLens', true, vscode.ConfigurationTarget.Global); + } else if (choice === DISMISS) { + await context.globalState.update(DISMISS_KEY, true); + } +} diff --git a/packages/vscode/src/extension.ts b/packages/vscode/src/extension.ts index 622f5cd5c..0651e152d 100644 --- a/packages/vscode/src/extension.ts +++ b/packages/vscode/src/extension.ts @@ -7,8 +7,9 @@ import { sendMessage } from './commands/send.js'; import { approveGate } from './commands/approve.js'; import { cleanupBuilder } from './commands/cleanup.js'; import { openWorktreeWindow } from './commands/open-worktree-window.js'; -import { viewDiff, activateDiffView, diffUrisForChange } from './commands/view-diff.js'; +import { viewDiff, activateDiffView, diffUrisForChange, registerFileInjectSession } from './commands/view-diff.js'; import { activateDiffInjectCodeLens } from './diff-inject-codelens.js'; +import { ensureDiffEditorCodeLens } from './ensure-diff-codelens.js'; import { runWorktreeDev } from './commands/run-worktree-dev.js'; import { stopWorktreeDev } from './commands/stop-worktree-dev.js'; import { runWorkspaceDev, stopWorkspaceDev } from './commands/run-workspace-dev.js'; @@ -822,6 +823,17 @@ export async function activate(context: vscode.ExtensionContext) { }), reg('codev.openBuilderFileDiff', async (arg: unknown) => { if (!(arg instanceof BuilderFileTreeItem)) { return; } + // Populate the inject-codelens registry for this file so the + // "Send to builder PTY" lenses (#789) render on the diff without a + // prior View Diff run, then offer to enable diffEditor.codeLens (off + // by default — VS Code hides CodeLens in diff editors). + await registerFileInjectSession({ + worktreePath: arg.worktreePath, + baseRef: arg.baseRef, + builderId: arg.builderId, + plan: arg.plan, + }); + await ensureDiffEditorCodeLens(context); const { left, right } = diffUrisForChange(arg.plan, { wt: arg.worktreePath, ref: arg.baseRef }); const title = `${arg.plan.resourcePath} (#${arg.builderId})`; await vscode.commands.executeCommand('vscode.diff', left, right, title); From 443f9da8158cfb434f14409094afb6baf21b53b3 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 10 Jun 2026 09:39:52 +1000 Subject: [PATCH 13/39] [PIR #789] Thread: pivot to Option A (per-file diff) --- codev/state/pir-789_thread.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/codev/state/pir-789_thread.md b/codev/state/pir-789_thread.md index b93cec6b1..8a5742cbd 100644 --- a/codev/state/pir-789_thread.md +++ b/codev/state/pir-789_thread.md @@ -28,3 +28,18 @@ Validation: `pnpm test:unit` 372 pass (had to build core+types dist first — th **Primary risk still unvalidated**: whether CodeLenses render inside the `vscode.changes` multi-diff editor. Needs the human to run the worktree at the dev-approval gate. Fallback documented in plan if they don't render. Awaiting `dev-approval`. + +## Dev-approval gate — primary risk materialized, pivoted to Option A + +Human tested in the Extension Dev Host. Lenses did NOT appear in the `codev.viewDiff` multi-file editor, even with `diffEditor.codeLens` enabled. Confirmed via VSCode issues #97640 / #156707: CodeLens is hidden in diff editors by default; the setting re-enables it for the **single-file** diff editor, but the **multi-file `vscode.changes` editor doesn't render CodeLens at all**. So the issue's literal surface (lenses in the viewDiff multi-file editor) is infeasible. + +**Provider proven correct**: opening a changed file in a plain editor tab shows the lens on line 1. So registration + fsPath matching + rendering all work — only the multi-diff host is the blocker. + +**Pivot (human chose Option A)**: surface the lenses on the **single-file `vscode.diff`** opened from the Builders tree (`codev.openBuilderFileDiff`), which honors `diffEditor.codeLens`. Changes: +- `openBuilderFileDiff` now populates the registry for its file (new `registerFileInjectSession` in view-diff.ts, `upsert` on the provider) so lenses appear without a prior View Diff run. +- New `ensure-diff-codelens.ts`: when opening a file diff with `diffEditor.codeLens` off, one-click prompt to enable it (with "Don't ask again" in globalState). Avoids shipping a feature that silently needs a hidden setting. +- viewDiff still populates the registry (harmless; future-proofs if multi-diff ever supports lenses; also powers the normal-tab case). + +Validation: check-types/lint/test:unit (372) all green; esbuild bundles. **Needs human to re-test Option A**: F5 relaunch → click a changed file row in the Builders tree → single-file diff → lenses render (prompt offers to enable the setting first). + +Note for review file: the spec'd surface was infeasible; should reflect the per-file-diff entry point back to the issue. From 2722d26b0baa393adc7b002574c852f25cd0f2cc Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 10 Jun 2026 09:48:53 +1000 Subject: [PATCH 14/39] [PIR #789] Anchor hunk lenses on the first changed line, not the context start git --unified=3 puts each hunk's new-side start up to 3 context lines above the first real change, so a lens could render on the wrong function's signature near a boundary. Walk the hunk body to find the first/last added (+) new-side lines and anchor + label the lens there (changeStart/changeEnd) instead of the header span. --- .../src/__tests__/diff-inject-ref.test.ts | 76 ++++++++++++++----- packages/vscode/src/diff-inject-ref.ts | 64 +++++++++++++--- 2 files changed, 111 insertions(+), 29 deletions(-) diff --git a/packages/vscode/src/__tests__/diff-inject-ref.test.ts b/packages/vscode/src/__tests__/diff-inject-ref.test.ts index 5692a6219..632ca73f6 100644 --- a/packages/vscode/src/__tests__/diff-inject-ref.test.ts +++ b/packages/vscode/src/__tests__/diff-inject-ref.test.ts @@ -14,31 +14,63 @@ import { } from '../diff-inject-ref.js'; describe('parseHunkRanges', () => { - it('reads the new-side start and length from each hunk header', () => { + it('reads the header span and the first/last actually-changed new-side lines', () => { const patch = [ '@@ -1,4 +1,6 @@', - ' a', - '+b', + ' a', // new line 1 (context) + '+b', // new line 2 (added) ← first/last change + ' c', // new line 3 (context) '@@ -20,3 +22,10 @@ func()', - ' c', + ' x', // new line 22 (context) + ' y', // new line 23 (context) + '+z1', // new line 24 (added) ← first change + '+z2', // new line 25 (added) ← last change + ' w', // new line 26 (context) ].join('\n'); expect(parseHunkRanges(patch)).toEqual([ - { newStart: 1, newEnd: 6 }, - { newStart: 22, newEnd: 31 }, + { newStart: 1, newEnd: 6, changeStart: 2, changeEnd: 2 }, + { newStart: 22, newEnd: 31, changeStart: 24, changeEnd: 25 }, + ]); + }); + + it('does not let deleted (-) lines advance the new-side line counter', () => { + const patch = [ + '@@ -10,4 +10,3 @@', + ' ctx', // new line 10 + '-gone1', // old-side only + '-gone2', // old-side only + '+added', // new line 11 ← the change + ' tail', // new line 12 + ].join('\n'); + expect(parseHunkRanges(patch)).toEqual([ + { newStart: 10, newEnd: 12, changeStart: 11, changeEnd: 11 }, ]); }); it('treats an absent new-side length as a single line', () => { - expect(parseHunkRanges('@@ -10 +11 @@')).toEqual([{ newStart: 11, newEnd: 11 }]); + expect(parseHunkRanges('@@ -10 +11 @@\n+added')).toEqual([ + { newStart: 11, newEnd: 11, changeStart: 11, changeEnd: 11 }, + ]); }); - it('clamps a pure-deletion hunk (+c,0) to a single anchor line', () => { - expect(parseHunkRanges('@@ -5,3 +4,0 @@')).toEqual([{ newStart: 4, newEnd: 4 }]); + it('falls back to the header start for a pure-deletion hunk (no + lines)', () => { + expect(parseHunkRanges('@@ -5,3 +4,0 @@\n-gone')).toEqual([ + { newStart: 4, newEnd: 4, changeStart: 4, changeEnd: 4 }, + ]); + }); + + it('ignores the "\\ No newline at end of file" marker', () => { + const patch = ['@@ -1,1 +1,2 @@', ' a', '+b', '\\ No newline at end of file'].join('\n'); + expect(parseHunkRanges(patch)).toEqual([ + { newStart: 1, newEnd: 2, changeStart: 2, changeEnd: 2 }, + ]); }); it('ignores non-hunk lines, including content that looks like @@', () => { - const patch = ['+const x = "@@ not a header";', '@@ -1,1 +1,2 @@'].join('\n'); - expect(parseHunkRanges(patch)).toEqual([{ newStart: 1, newEnd: 2 }]); + const patch = ['+const x = "@@ not a header";', '@@ -1,1 +1,2 @@', '+real'].join('\n'); + expect(parseHunkRanges(patch)).toEqual([ + { newStart: 1, newEnd: 2, changeStart: 1, changeEnd: 1 }, + ]); }); }); @@ -61,8 +93,12 @@ describe('parseUnifiedDiff', () => { '+q', ].join('\n'); const map = parseUnifiedDiff(patch); - expect(map.get('src/a.ts')).toEqual([{ newStart: 1, newEnd: 3 }]); - expect(map.get('src/b.ts')).toEqual([{ newStart: 11, newEnd: 12 }]); + expect(map.get('src/a.ts')).toEqual([ + { newStart: 1, newEnd: 3, changeStart: 2, changeEnd: 2 }, + ]); + expect(map.get('src/b.ts')).toEqual([ + { newStart: 11, newEnd: 12, changeStart: 11, changeEnd: 12 }, + ]); }); it('uses the new path for a rename (+++ b/)', () => { @@ -78,7 +114,9 @@ describe('parseUnifiedDiff', () => { ].join('\n'); const map = parseUnifiedDiff(patch); expect([...map.keys()]).toEqual(['new/name.ts']); - expect(map.get('new/name.ts')).toEqual([{ newStart: 3, newEnd: 4 }]); + expect(map.get('new/name.ts')).toEqual([ + { newStart: 3, newEnd: 4, changeStart: 3, changeEnd: 3 }, + ]); }); it('omits deleted files (new side is /dev/null)', () => { @@ -106,10 +144,10 @@ describe('ref builders', () => { }); describe('buildLensDescriptors', () => { - it('emits a file-level lens at line 0 plus one lens per hunk', () => { + it('emits a file-level lens at line 0 plus one lens per hunk, anchored on the change', () => { const lenses = buildLensDescriptors('a/b.ts', [ - { newStart: 5, newEnd: 9 }, - { newStart: 30, newEnd: 30 }, + { newStart: 2, newEnd: 12, changeStart: 5, changeEnd: 9 }, + { newStart: 28, newEnd: 32, changeStart: 30, changeEnd: 30 }, ]); expect(lenses).toEqual([ { line: 0, title: 'Send to builder PTY', refText: 'a/b.ts ' }, @@ -119,7 +157,9 @@ describe('buildLensDescriptors', () => { }); it('clamps a hunk anchored at line 1 to a non-negative index', () => { - const lenses = buildLensDescriptors('a/b.ts', [{ newStart: 1, newEnd: 1 }]); + const lenses = buildLensDescriptors('a/b.ts', [ + { newStart: 1, newEnd: 1, changeStart: 1, changeEnd: 1 }, + ]); expect(lenses[1]!.line).toBe(0); }); diff --git a/packages/vscode/src/diff-inject-ref.ts b/packages/vscode/src/diff-inject-ref.ts index a049f3aa6..a6ea374a9 100644 --- a/packages/vscode/src/diff-inject-ref.ts +++ b/packages/vscode/src/diff-inject-ref.ts @@ -9,10 +9,21 @@ * command. All the line-math and ref-string logic lives here. */ -/** New-side line range of one diff hunk (1-based, inclusive). */ +/** + * New-side line numbers of one diff hunk (1-based, inclusive). + * + * `newStart`/`newEnd` are the hunk header's full new-side span (context + * included — what `@@ … +c,d @@` reports). `changeStart`/`changeEnd` are the + * first and last lines that are *actually added* on the new side (the `+` + * lines), so a lens anchored there sits on the real change rather than on the + * up-to-3 lines of leading context git emits with `--unified=3`. For a + * pure-deletion hunk (no `+` lines) both change fields collapse to `newStart`. + */ export interface HunkRange { newStart: number; newEnd: number; + changeStart: number; + changeEnd: number; } /** @@ -40,18 +51,45 @@ const HUNK_HEADER = /^@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@/; * a sane location near the change. */ export function parseHunkRanges(patch: string): HunkRange[] { + const lines = patch.split('\n'); const ranges: HunkRange[] = []; - for (const line of patch.split('\n')) { - const m = HUNK_HEADER.exec(line); + for (let i = 0; i < lines.length; i++) { + const m = HUNK_HEADER.exec(lines[i]!); if (!m) { continue; } const newStart = Number(m[1]); const len = m[2] === undefined ? 1 : Number(m[2]); - if (len <= 0) { - const anchor = Math.max(newStart, 1); - ranges.push({ newStart: anchor, newEnd: anchor }); - } else { - ranges.push({ newStart, newEnd: newStart + len - 1 }); + const newEnd = len <= 0 ? Math.max(newStart, 1) : newStart + len - 1; + + // Walk the hunk body to find the first/last lines actually added on the + // new side. `cur` tracks the new-side line number: context (' ') and + // additions ('+') advance it; deletions ('-') are old-side only; the + // "\ No newline at end of file" marker advances nothing. + let cur = newStart; + let firstChange = -1; + let lastChange = -1; + for (let j = i + 1; j < lines.length; j++) { + const l = lines[j]!; + if (HUNK_HEADER.test(l) || l.startsWith('diff --git')) { break; } + if (l.startsWith('\\')) { continue; } + if (l.startsWith('+')) { + if (firstChange === -1) { firstChange = cur; } + lastChange = cur; + cur++; + } else if (l.startsWith('-')) { + // old-side only — does not advance the new-side counter + } else { + cur++; // context line + } } + + const changeStart = firstChange === -1 ? Math.max(newStart, 1) : firstChange; + const changeEnd = lastChange === -1 ? changeStart : lastChange; + ranges.push({ + newStart: len <= 0 ? Math.max(newStart, 1) : newStart, + newEnd, + changeStart, + changeEnd, + }); } return ranges; } @@ -110,10 +148,14 @@ export function buildLensDescriptors(relPath: string, hunks: HunkRange[]): LensD { line: 0, title: 'Send to builder PTY', refText: buildBuilderFileRef(relPath) }, ]; for (const h of hunks) { + // Anchor and label on the first/last *changed* new-side lines, not the + // hunk header's context-inclusive span — so the lens sits on the real edit + // (e.g. inside the right function) rather than on the up-to-3 leading + // context lines git emits with --unified=3. lenses.push({ - line: Math.max(h.newStart - 1, 0), - title: `Send to builder PTY (lines ${h.newStart}-${h.newEnd})`, - refText: buildBuilderHunkRef(relPath, h.newStart, h.newEnd), + line: Math.max(h.changeStart - 1, 0), + title: `Send to builder PTY (lines ${h.changeStart}-${h.changeEnd})`, + refText: buildBuilderHunkRef(relPath, h.changeStart, h.changeEnd), }); } return lenses; From ff7df2afbd7a23cb544b25076d8cd594de90a7a8 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 10 Jun 2026 09:49:05 +1000 Subject: [PATCH 15/39] [PIR #789] Thread: hunk-lens placement fix --- codev/state/pir-789_thread.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/codev/state/pir-789_thread.md b/codev/state/pir-789_thread.md index 8a5742cbd..250b35a6a 100644 --- a/codev/state/pir-789_thread.md +++ b/codev/state/pir-789_thread.md @@ -43,3 +43,7 @@ Human tested in the Extension Dev Host. Lenses did NOT appear in the `codev.view Validation: check-types/lint/test:unit (372) all green; esbuild bundles. **Needs human to re-test Option A**: F5 relaunch → click a changed file row in the Builders tree → single-file diff → lenses render (prompt offers to enable the setting first). Note for review file: the spec'd surface was infeasible; should reflect the per-file-diff entry point back to the issue. + +## Dev-approval iteration: hunk-lens placement fix + +Human observed hunk lenses landing on the wrong function's signature. Root cause: `--unified=3` reports each hunk's new-side start up to 3 *context* lines above the first real change, so near a function boundary the anchor lands on the preceding `def`/`return`. Fixed: `parseHunkRanges` now walks the hunk body to find the first/last actually-added (`+`) new-side lines (`changeStart`/`changeEnd`); `buildLensDescriptors` anchors AND labels on those instead of the header span. Deviation from issue's literal "hunk header range" — intentional, more accurate. 374 tests green. Needs human re-test after F5 relaunch. From 6c0884806375da032b355529671b92e8ab7074c5 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 10 Jun 2026 11:50:03 +1000 Subject: [PATCH 16/39] [PIR #789] Rename CodeLens label to 'Forward to Builder' --- packages/vscode/src/__tests__/diff-inject-ref.test.ts | 10 +++++----- packages/vscode/src/commands/view-diff.ts | 4 ++-- packages/vscode/src/diff-inject-codelens.ts | 2 +- packages/vscode/src/diff-inject-ref.ts | 6 +++--- packages/vscode/src/ensure-diff-codelens.ts | 4 ++-- packages/vscode/src/extension.ts | 4 ++-- packages/vscode/src/terminal-manager.ts | 4 ++-- 7 files changed, 17 insertions(+), 17 deletions(-) diff --git a/packages/vscode/src/__tests__/diff-inject-ref.test.ts b/packages/vscode/src/__tests__/diff-inject-ref.test.ts index 632ca73f6..00821ab56 100644 --- a/packages/vscode/src/__tests__/diff-inject-ref.test.ts +++ b/packages/vscode/src/__tests__/diff-inject-ref.test.ts @@ -1,5 +1,5 @@ /** - * Unit tests for the pure diff/ref helpers behind the "Send to builder PTY" + * Unit tests for the pure diff/ref helpers behind the "Forward to Builder" * CodeLens actions (#789). No `vscode` dependency, so the live implementation * is imported directly (same pattern as `architect-reference-injection.test.ts`). */ @@ -150,9 +150,9 @@ describe('buildLensDescriptors', () => { { newStart: 28, newEnd: 32, changeStart: 30, changeEnd: 30 }, ]); expect(lenses).toEqual([ - { line: 0, title: 'Send to builder PTY', refText: 'a/b.ts ' }, - { line: 4, title: 'Send to builder PTY (lines 5-9)', refText: 'a/b.ts:L5-L9 ' }, - { line: 29, title: 'Send to builder PTY (lines 30-30)', refText: 'a/b.ts:L30-L30 ' }, + { line: 0, title: 'Forward to Builder', refText: 'a/b.ts ' }, + { line: 4, title: 'Forward to Builder (lines 5-9)', refText: 'a/b.ts:L5-L9 ' }, + { line: 29, title: 'Forward to Builder (lines 30-30)', refText: 'a/b.ts:L30-L30 ' }, ]); }); @@ -165,7 +165,7 @@ describe('buildLensDescriptors', () => { it('emits just the file-level lens when there are no hunks', () => { expect(buildLensDescriptors('a/b.ts', [])).toEqual([ - { line: 0, title: 'Send to builder PTY', refText: 'a/b.ts ' }, + { line: 0, title: 'Forward to Builder', refText: 'a/b.ts ' }, ]); }); }); diff --git a/packages/vscode/src/commands/view-diff.ts b/packages/vscode/src/commands/view-diff.ts index 16036828e..2256f8709 100644 --- a/packages/vscode/src/commands/view-diff.ts +++ b/packages/vscode/src/commands/view-diff.ts @@ -361,7 +361,7 @@ export async function viewDiff( ]; }); - // CodeLens "Send to builder PTY" actions (#789): give the provider the + // CodeLens "Forward to Builder" actions (#789): give the provider the // right-side fs paths and new-side hunk ranges for the files just opened. // One extra git call here keeps `getBuilderChanges` (shared with the // Builders tree) untouched. A patch failure is non-fatal — the diff still @@ -397,7 +397,7 @@ export async function viewDiff( * Populate the inject-codelens registry for a single changed file — the * per-file `vscode.diff` opened from the Builders tree (`openBuilderFileDiff`). * Mirrors what `viewDiff` does for the whole delta, but for one file, so the - * "Send to builder PTY" lenses appear even when the reviewer opens a file diff + * "Forward to Builder" lenses appear even when the reviewer opens a file diff * without first running View Diff. Non-fatal on git failure (the file-level * lens still shows; only the per-hunk lenses are dropped). */ diff --git a/packages/vscode/src/diff-inject-codelens.ts b/packages/vscode/src/diff-inject-codelens.ts index d6397e0ba..686a114b1 100644 --- a/packages/vscode/src/diff-inject-codelens.ts +++ b/packages/vscode/src/diff-inject-codelens.ts @@ -1,5 +1,5 @@ /** - * CodeLens provider backing the "Send to builder PTY" actions in the + * CodeLens provider backing the "Forward to Builder" actions in the * `codev.viewDiff` multi-file diff editor (issue #789). * * The right side of each file in that editor is a plain `file:` document at diff --git a/packages/vscode/src/diff-inject-ref.ts b/packages/vscode/src/diff-inject-ref.ts index a6ea374a9..f9ea41116 100644 --- a/packages/vscode/src/diff-inject-ref.ts +++ b/packages/vscode/src/diff-inject-ref.ts @@ -1,5 +1,5 @@ /** - * Pure helpers for the "Send to builder PTY" CodeLens actions in the + * Pure helpers for the "Forward to Builder" CodeLens actions in the * `codev.viewDiff` editor (issue #789). No `vscode` import — same precedent * as `architect-reference-injection.ts`, so the parsing/string logic is * unit-tested directly without mocking the editor API. @@ -145,7 +145,7 @@ export function buildBuilderHunkRef(relPath: string, start: number, end: number) */ export function buildLensDescriptors(relPath: string, hunks: HunkRange[]): LensDescriptor[] { const lenses: LensDescriptor[] = [ - { line: 0, title: 'Send to builder PTY', refText: buildBuilderFileRef(relPath) }, + { line: 0, title: 'Forward to Builder', refText: buildBuilderFileRef(relPath) }, ]; for (const h of hunks) { // Anchor and label on the first/last *changed* new-side lines, not the @@ -154,7 +154,7 @@ export function buildLensDescriptors(relPath: string, hunks: HunkRange[]): LensD // context lines git emits with --unified=3. lenses.push({ line: Math.max(h.changeStart - 1, 0), - title: `Send to builder PTY (lines ${h.changeStart}-${h.changeEnd})`, + title: `Forward to Builder (lines ${h.changeStart}-${h.changeEnd})`, refText: buildBuilderHunkRef(relPath, h.changeStart, h.changeEnd), }); } diff --git a/packages/vscode/src/ensure-diff-codelens.ts b/packages/vscode/src/ensure-diff-codelens.ts index c0cfae17d..66444f088 100644 --- a/packages/vscode/src/ensure-diff-codelens.ts +++ b/packages/vscode/src/ensure-diff-codelens.ts @@ -1,5 +1,5 @@ /** - * The "Send to builder PTY" CodeLens actions (#789) only render in a diff + * The "Forward to Builder" CodeLens actions (#789) only render in a diff * editor when `diffEditor.codeLens` is on — VS Code hides CodeLens in diff * editors by default (microsoft/vscode#97640). When a reviewer opens a builder * file diff with the setting off, offer a one-click enable rather than letting @@ -21,7 +21,7 @@ export async function ensureDiffEditorCodeLens( const ENABLE = 'Enable'; const DISMISS = "Don't ask again"; const choice = await vscode.window.showInformationMessage( - 'Codev’s "Send to builder PTY" actions need CodeLens in diff editors, ' + + 'Codev’s "Forward to Builder" actions need CodeLens in diff editors, ' + 'which VS Code hides by default. Enable it?', ENABLE, DISMISS, diff --git a/packages/vscode/src/extension.ts b/packages/vscode/src/extension.ts index 0651e152d..35e25cc71 100644 --- a/packages/vscode/src/extension.ts +++ b/packages/vscode/src/extension.ts @@ -824,7 +824,7 @@ export async function activate(context: vscode.ExtensionContext) { reg('codev.openBuilderFileDiff', async (arg: unknown) => { if (!(arg instanceof BuilderFileTreeItem)) { return; } // Populate the inject-codelens registry for this file so the - // "Send to builder PTY" lenses (#789) render on the diff without a + // "Forward to Builder" lenses (#789) render on the diff without a // prior View Diff run, then offer to enable diffEditor.codeLens (off // by default — VS Code hides CodeLens in diff editors). await registerFileInjectSession({ @@ -910,7 +910,7 @@ export async function activate(context: vscode.ExtensionContext) { // without relying on the Git extension's worktree discovery. activateDiffView(context); - // CodeLens "Send to builder PTY" actions inside the View Diff editor (#789). + // CodeLens "Forward to Builder" actions inside the View Diff editor (#789). // The backing command `codev.injectBuilderFileRef` is registered below and // deliberately NOT declared in `contributes.commands`, so it stays out of // the Command Palette (codelens-only entry point). diff --git a/packages/vscode/src/terminal-manager.ts b/packages/vscode/src/terminal-manager.ts index 136a6ff6a..dce37976f 100644 --- a/packages/vscode/src/terminal-manager.ts +++ b/packages/vscode/src/terminal-manager.ts @@ -155,7 +155,7 @@ export class TerminalManager { /** * Type `text` into a builder terminal's input *without* a trailing newline * (no submit) — the builder-side analogue of `injectArchitectText`, backing - * the "Send to builder PTY" CodeLens (#789). Returns false if no terminal is + * the "Forward to Builder" CodeLens (#789). Returns false if no terminal is * registered for `builderId`; callers ensure it's open first (via * `openBuilderByRoleOrId`, whose resolved id keys the lookup). */ @@ -207,7 +207,7 @@ export class TerminalManager { * * Returns the resolved **canonical** builder id on success (the key under * which the terminal is registered), or `undefined` when the builder can't - * be opened. The "Send to builder PTY" inject path (#789) uses this to target + * be opened. The "Forward to Builder" inject path (#789) uses this to target * the same terminal that was opened, even when called with a bare numeric id; * other callers ignore the return value. */ From 4a319a2cdbc94de09e398512c06e200889d5754f Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 10 Jun 2026 11:51:10 +1000 Subject: [PATCH 17/39] [PIR #789] Rename command id to codev.forwardToBuilder --- packages/vscode/src/diff-inject-codelens.ts | 6 +++--- packages/vscode/src/extension.ts | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/vscode/src/diff-inject-codelens.ts b/packages/vscode/src/diff-inject-codelens.ts index 686a114b1..d2203b0d1 100644 --- a/packages/vscode/src/diff-inject-codelens.ts +++ b/packages/vscode/src/diff-inject-codelens.ts @@ -10,7 +10,7 @@ * files the reviewer just opened for diffing; any other `file:` document * returns `[]` and is unaffected. * - * Clicking a lens runs `codev.injectBuilderFileRef` (a palette-hidden command + * Clicking a lens runs `codev.forwardToBuilder` (a palette-hidden command * registered in `extension.ts`), which opens/reveals the builder terminal and * types the reference into its prompt without pressing Enter — mirroring the * architect-reference pattern (`codev.referenceIssueInArchitect`). @@ -21,7 +21,7 @@ import { buildLensDescriptors, type HunkRange } from './diff-inject-ref.js'; /** Command id the lenses invoke. Registered in `extension.ts`, NOT declared in * `contributes.commands`, so it never appears in the Command Palette. */ -export const INJECT_BUILDER_FILE_REF = 'codev.injectBuilderFileRef'; +export const FORWARD_TO_BUILDER_COMMAND = 'codev.forwardToBuilder'; /** One changed file in the active diff session, keyed by its right-side fs path. */ export interface DiffInjectSessionEntry { @@ -62,7 +62,7 @@ class DiffInjectCodeLensProvider implements vscode.CodeLensProvider { const range = new vscode.Range(line, 0, line, 0); return new vscode.CodeLens(range, { title: d.title, - command: INJECT_BUILDER_FILE_REF, + command: FORWARD_TO_BUILDER_COMMAND, arguments: [entry.builderId, d.refText], }); }); diff --git a/packages/vscode/src/extension.ts b/packages/vscode/src/extension.ts index 35e25cc71..eba051112 100644 --- a/packages/vscode/src/extension.ts +++ b/packages/vscode/src/extension.ts @@ -812,7 +812,7 @@ export async function activate(context: vscode.ExtensionContext) { // the reviewer keeps typing feedback before hitting Enter. Mirrors // `codev.referenceIssueInArchitect`. Not declared in // `contributes.commands` → never appears in the Command Palette. - reg('codev.injectBuilderFileRef', async (builderId: string, text: string) => { + reg('codev.forwardToBuilder', async (builderId: string, text: string) => { // openBuilderByRoleOrId resolves to the canonical id and runs the // no-terminal recovery flow on a miss; inject against that id so the // terminal lookup hits the same key that was just opened. @@ -911,7 +911,7 @@ export async function activate(context: vscode.ExtensionContext) { activateDiffView(context); // CodeLens "Forward to Builder" actions inside the View Diff editor (#789). - // The backing command `codev.injectBuilderFileRef` is registered below and + // The backing command `codev.forwardToBuilder` is registered below and // deliberately NOT declared in `contributes.commands`, so it stays out of // the Command Palette (codelens-only entry point). activateDiffInjectCodeLens(context); From 67ba46a3598368814dcaace17dc32e3f7217c7dc Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 10 Jun 2026 14:32:10 +1000 Subject: [PATCH 18/39] [PIR #789] Skip hunk lens that collides with the file-level lens at line 0 A newly-added file is a single whole-file hunk starting at line 1, so its hunk lens anchored at line 0 stacked a second 'Forward to Builder' on the file-level lens. Skip any hunk lens that anchors on line 0. --- .../src/__tests__/diff-inject-ref.test.ts | 22 ++++++++++++++++--- packages/vscode/src/diff-inject-ref.ts | 8 ++++++- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/packages/vscode/src/__tests__/diff-inject-ref.test.ts b/packages/vscode/src/__tests__/diff-inject-ref.test.ts index 00821ab56..7183c8dea 100644 --- a/packages/vscode/src/__tests__/diff-inject-ref.test.ts +++ b/packages/vscode/src/__tests__/diff-inject-ref.test.ts @@ -156,11 +156,27 @@ describe('buildLensDescriptors', () => { ]); }); - it('clamps a hunk anchored at line 1 to a non-negative index', () => { + it('skips a hunk that anchors on the file-level lens line (added file / first-line change)', () => { + // A newly-added file is one whole-file hunk starting at line 1, so its + // hunk lens would anchor at line 0 and stack on the file-level lens. Only + // the file-level lens should remain. const lenses = buildLensDescriptors('a/b.ts', [ - { newStart: 1, newEnd: 1, changeStart: 1, changeEnd: 1 }, + { newStart: 1, newEnd: 17, changeStart: 1, changeEnd: 17 }, + ]); + expect(lenses).toEqual([ + { line: 0, title: 'Forward to Builder', refText: 'a/b.ts ' }, + ]); + }); + + it('keeps hunks below line 1 even when an earlier hunk was skipped at line 0', () => { + const lenses = buildLensDescriptors('a/b.ts', [ + { newStart: 1, newEnd: 3, changeStart: 1, changeEnd: 3 }, // anchors line 0 → skipped + { newStart: 48, newEnd: 52, changeStart: 50, changeEnd: 52 }, // anchors line 49 → kept + ]); + expect(lenses).toEqual([ + { line: 0, title: 'Forward to Builder', refText: 'a/b.ts ' }, + { line: 49, title: 'Forward to Builder (lines 50-52)', refText: 'a/b.ts:L50-L52 ' }, ]); - expect(lenses[1]!.line).toBe(0); }); it('emits just the file-level lens when there are no hunks', () => { diff --git a/packages/vscode/src/diff-inject-ref.ts b/packages/vscode/src/diff-inject-ref.ts index f9ea41116..5ebd9df5e 100644 --- a/packages/vscode/src/diff-inject-ref.ts +++ b/packages/vscode/src/diff-inject-ref.ts @@ -152,8 +152,14 @@ export function buildLensDescriptors(relPath: string, hunks: HunkRange[]): LensD // hunk header's context-inclusive span — so the lens sits on the real edit // (e.g. inside the right function) rather than on the up-to-3 leading // context lines git emits with --unified=3. + const line = Math.max(h.changeStart - 1, 0); + // The file-level lens already occupies line 0. A hunk that anchors there — + // a newly-added file (one whole-file hunk from line 1) or a change to the + // very first line — would stack a second "Forward to Builder" on the same + // line. Skip it; the file-level lens covers that spot. + if (line === 0) { continue; } lenses.push({ - line: Math.max(h.changeStart - 1, 0), + line, title: `Forward to Builder (lines ${h.changeStart}-${h.changeEnd})`, refText: buildBuilderHunkRef(relPath, h.changeStart, h.changeEnd), }); From 9984f8c4cdf0900485c7d268ae3ffc6bd9800392 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 10 Jun 2026 14:51:25 +1000 Subject: [PATCH 19/39] [PIR #789] Drive lenses off document symbols, not git hunks Hunk-driven lenses are unusable on new files (one whole-file hunk = one lens). Replace with symbol-driven lenses: a file-level lens plus one per top-level Function/Class/Interface/Enum/Struct/Namespace/Module (and multi-line top-level Variable/Constant), descending one level into Class/Struct for Method/Constructor. Provider resolves symbols via vscode.executeDocumentSymbolProvider. --- .../src/__tests__/diff-inject-ref.test.ts | 217 ++++++------------ packages/vscode/src/diff-inject-codelens.ts | 91 ++++++-- packages/vscode/src/diff-inject-ref.ts | 209 +++++++---------- 3 files changed, 226 insertions(+), 291 deletions(-) diff --git a/packages/vscode/src/__tests__/diff-inject-ref.test.ts b/packages/vscode/src/__tests__/diff-inject-ref.test.ts index 7183c8dea..75a13e720 100644 --- a/packages/vscode/src/__tests__/diff-inject-ref.test.ts +++ b/packages/vscode/src/__tests__/diff-inject-ref.test.ts @@ -1,136 +1,33 @@ /** - * Unit tests for the pure diff/ref helpers behind the "Forward to Builder" + * Unit tests for the pure symbol/ref helpers behind the "Forward to Builder" * CodeLens actions (#789). No `vscode` dependency, so the live implementation * is imported directly (same pattern as `architect-reference-injection.test.ts`). */ import { describe, it, expect } from 'vitest'; import { - parseHunkRanges, - parseUnifiedDiff, buildBuilderFileRef, - buildBuilderHunkRef, - buildLensDescriptors, + buildBuilderRangeRef, + buildSymbolLensDescriptors, + type SymbolNode, } from '../diff-inject-ref.js'; -describe('parseHunkRanges', () => { - it('reads the header span and the first/last actually-changed new-side lines', () => { - const patch = [ - '@@ -1,4 +1,6 @@', - ' a', // new line 1 (context) - '+b', // new line 2 (added) ← first/last change - ' c', // new line 3 (context) - '@@ -20,3 +22,10 @@ func()', - ' x', // new line 22 (context) - ' y', // new line 23 (context) - '+z1', // new line 24 (added) ← first change - '+z2', // new line 25 (added) ← last change - ' w', // new line 26 (context) - ].join('\n'); - expect(parseHunkRanges(patch)).toEqual([ - { newStart: 1, newEnd: 6, changeStart: 2, changeEnd: 2 }, - { newStart: 22, newEnd: 31, changeStart: 24, changeEnd: 25 }, - ]); - }); - - it('does not let deleted (-) lines advance the new-side line counter', () => { - const patch = [ - '@@ -10,4 +10,3 @@', - ' ctx', // new line 10 - '-gone1', // old-side only - '-gone2', // old-side only - '+added', // new line 11 ← the change - ' tail', // new line 12 - ].join('\n'); - expect(parseHunkRanges(patch)).toEqual([ - { newStart: 10, newEnd: 12, changeStart: 11, changeEnd: 11 }, - ]); - }); - - it('treats an absent new-side length as a single line', () => { - expect(parseHunkRanges('@@ -10 +11 @@\n+added')).toEqual([ - { newStart: 11, newEnd: 11, changeStart: 11, changeEnd: 11 }, - ]); - }); - - it('falls back to the header start for a pure-deletion hunk (no + lines)', () => { - expect(parseHunkRanges('@@ -5,3 +4,0 @@\n-gone')).toEqual([ - { newStart: 4, newEnd: 4, changeStart: 4, changeEnd: 4 }, - ]); - }); - - it('ignores the "\\ No newline at end of file" marker', () => { - const patch = ['@@ -1,1 +1,2 @@', ' a', '+b', '\\ No newline at end of file'].join('\n'); - expect(parseHunkRanges(patch)).toEqual([ - { newStart: 1, newEnd: 2, changeStart: 2, changeEnd: 2 }, - ]); - }); - - it('ignores non-hunk lines, including content that looks like @@', () => { - const patch = ['+const x = "@@ not a header";', '@@ -1,1 +1,2 @@', '+real'].join('\n'); - expect(parseHunkRanges(patch)).toEqual([ - { newStart: 1, newEnd: 2, changeStart: 1, changeEnd: 1 }, - ]); - }); -}); - -describe('parseUnifiedDiff', () => { - it('maps each file new-path to its hunk ranges', () => { - const patch = [ - 'diff --git a/src/a.ts b/src/a.ts', - 'index 111..222 100644', - '--- a/src/a.ts', - '+++ b/src/a.ts', - '@@ -1,2 +1,3 @@', - ' x', - '+y', - 'diff --git a/src/b.ts b/src/b.ts', - 'index 333..444 100644', - '--- a/src/b.ts', - '+++ b/src/b.ts', - '@@ -10,0 +11,2 @@', - '+p', - '+q', - ].join('\n'); - const map = parseUnifiedDiff(patch); - expect(map.get('src/a.ts')).toEqual([ - { newStart: 1, newEnd: 3, changeStart: 2, changeEnd: 2 }, - ]); - expect(map.get('src/b.ts')).toEqual([ - { newStart: 11, newEnd: 12, changeStart: 11, changeEnd: 12 }, - ]); - }); - - it('uses the new path for a rename (+++ b/)', () => { - const patch = [ - 'diff --git a/old/name.ts b/new/name.ts', - 'similarity index 90%', - 'rename from old/name.ts', - 'rename to new/name.ts', - '--- a/old/name.ts', - '+++ b/new/name.ts', - '@@ -3,1 +3,2 @@', - '+added', - ].join('\n'); - const map = parseUnifiedDiff(patch); - expect([...map.keys()]).toEqual(['new/name.ts']); - expect(map.get('new/name.ts')).toEqual([ - { newStart: 3, newEnd: 4, changeStart: 3, changeEnd: 3 }, - ]); - }); - - it('omits deleted files (new side is /dev/null)', () => { - const patch = [ - 'diff --git a/gone.ts b/gone.ts', - 'deleted file mode 100644', - '--- a/gone.ts', - '+++ /dev/null', - '@@ -1,3 +0,0 @@', - '-x', - ].join('\n'); - expect(parseUnifiedDiff(patch).size).toBe(0); - }); -}); +// Numeric vscode.SymbolKind values used by the tests. +const K = { + Class: 4, + Method: 5, + Property: 6, + Constructor: 8, + Enum: 9, + Interface: 10, + Function: 11, + Variable: 12, + Constant: 13, +} as const; + +function sym(kind: number, startLine: number, endLine: number, children: SymbolNode[] = []): SymbolNode { + return { kind, startLine, endLine, children }; +} describe('ref builders', () => { it('builds a file ref with a trailing space and no newline', () => { @@ -138,50 +35,74 @@ describe('ref builders', () => { .toBe('packages/vscode/src/extension.ts '); }); - it('builds a hunk ref with the L-L range', () => { - expect(buildBuilderHunkRef('a/b.ts', 10, 20)).toBe('a/b.ts:L10-L20 '); + it('builds a range ref with the L-L range', () => { + expect(buildBuilderRangeRef('a/b.ts', 10, 20)).toBe('a/b.ts:L10-L20 '); }); }); -describe('buildLensDescriptors', () => { - it('emits a file-level lens at line 0 plus one lens per hunk, anchored on the change', () => { - const lenses = buildLensDescriptors('a/b.ts', [ - { newStart: 2, newEnd: 12, changeStart: 5, changeEnd: 9 }, - { newStart: 28, newEnd: 32, changeStart: 30, changeEnd: 30 }, +describe('buildSymbolLensDescriptors', () => { + it('always emits a file-level lens at line 0', () => { + expect(buildSymbolLensDescriptors('a/b.ts', [])).toEqual([ + { line: 0, title: 'Forward to Builder', refText: 'a/b.ts ' }, ]); - expect(lenses).toEqual([ + }); + + it('lenses top-level structural declarations with their full range', () => { + const symbols = [ + sym(K.Function, 4, 9), // function → line 4, L5-L10 + sym(K.Interface, 12, 18), // interface → line 12, L13-L19 + sym(K.Enum, 20, 24), // enum → line 20, L21-L25 + ]; + expect(buildSymbolLensDescriptors('a/b.ts', symbols)).toEqual([ { line: 0, title: 'Forward to Builder', refText: 'a/b.ts ' }, - { line: 4, title: 'Forward to Builder (lines 5-9)', refText: 'a/b.ts:L5-L9 ' }, - { line: 29, title: 'Forward to Builder (lines 30-30)', refText: 'a/b.ts:L30-L30 ' }, + { line: 4, title: 'Forward to Builder', refText: 'a/b.ts:L5-L10 ' }, + { line: 12, title: 'Forward to Builder', refText: 'a/b.ts:L13-L19 ' }, + { line: 20, title: 'Forward to Builder', refText: 'a/b.ts:L21-L25 ' }, ]); }); - it('skips a hunk that anchors on the file-level lens line (added file / first-line change)', () => { - // A newly-added file is one whole-file hunk starting at line 1, so its - // hunk lens would anchor at line 0 and stack on the file-level lens. Only - // the file-level lens should remain. - const lenses = buildLensDescriptors('a/b.ts', [ - { newStart: 1, newEnd: 17, changeStart: 1, changeEnd: 17 }, + it('descends one level into a class for methods and the constructor', () => { + const cls = sym(K.Class, 3, 40, [ + sym(K.Constructor, 5, 8), + sym(K.Method, 10, 20), + sym(K.Property, 22, 22), // excluded ]); - expect(lenses).toEqual([ + expect(buildSymbolLensDescriptors('a/b.ts', [cls])).toEqual([ { line: 0, title: 'Forward to Builder', refText: 'a/b.ts ' }, + { line: 3, title: 'Forward to Builder', refText: 'a/b.ts:L4-L41 ' }, // class + { line: 5, title: 'Forward to Builder', refText: 'a/b.ts:L6-L9 ' }, // constructor + { line: 10, title: 'Forward to Builder', refText: 'a/b.ts:L11-L21 ' }, // method ]); }); - it('keeps hunks below line 1 even when an earlier hunk was skipped at line 0', () => { - const lenses = buildLensDescriptors('a/b.ts', [ - { newStart: 1, newEnd: 3, changeStart: 1, changeEnd: 3 }, // anchors line 0 → skipped - { newStart: 48, newEnd: 52, changeStart: 50, changeEnd: 52 }, // anchors line 49 → kept + it('lenses a top-level multi-line Variable/Constant but skips one-line ones', () => { + const symbols = [ + sym(K.Variable, 4, 12), // multi-line const (e.g. arrow component) → lensed + sym(K.Constant, 14, 14), // one-line scalar → skipped + ]; + expect(buildSymbolLensDescriptors('a/b.ts', symbols)).toEqual([ + { line: 0, title: 'Forward to Builder', refText: 'a/b.ts ' }, + { line: 4, title: 'Forward to Builder', refText: 'a/b.ts:L5-L13 ' }, ]); - expect(lenses).toEqual([ + }); + + it('skips a symbol that anchors on line 0 (collides with the file-level lens)', () => { + // A file whose first declaration starts at line 0. + const symbols = [sym(K.Function, 0, 30)]; + expect(buildSymbolLensDescriptors('a/b.ts', symbols)).toEqual([ { line: 0, title: 'Forward to Builder', refText: 'a/b.ts ' }, - { line: 49, title: 'Forward to Builder (lines 50-52)', refText: 'a/b.ts:L50-L52 ' }, ]); }); - it('emits just the file-level lens when there are no hunks', () => { - expect(buildLensDescriptors('a/b.ts', [])).toEqual([ + it('does not lens excluded top-level kinds (Property) or recurse past one level', () => { + const cls = sym(K.Class, 2, 50, [ + sym(K.Class, 10, 40, [ // nested class: not lensed, not recursed + sym(K.Method, 12, 20), + ]), + ]); + expect(buildSymbolLensDescriptors('a/b.ts', [cls])).toEqual([ { line: 0, title: 'Forward to Builder', refText: 'a/b.ts ' }, + { line: 2, title: 'Forward to Builder', refText: 'a/b.ts:L3-L51 ' }, ]); }); }); diff --git a/packages/vscode/src/diff-inject-codelens.ts b/packages/vscode/src/diff-inject-codelens.ts index d2203b0d1..0519c7c5c 100644 --- a/packages/vscode/src/diff-inject-codelens.ts +++ b/packages/vscode/src/diff-inject-codelens.ts @@ -1,28 +1,42 @@ /** - * CodeLens provider backing the "Forward to Builder" actions in the - * `codev.viewDiff` multi-file diff editor (issue #789). + * CodeLens provider backing the "Forward to Builder" actions in the builder + * diff (#789). * - * The right side of each file in that editor is a plain `file:` document at - * `/`, so we register against `{ scheme: 'file' }` - * and emit lenses only for documents whose fs path is in the *active diff - * session* registry. `viewDiff` replaces that registry (via - * `setDiffInjectSession`) on each invocation, so lenses are scoped to the - * files the reviewer just opened for diffing; any other `file:` document - * returns `[]` and is unaffected. + * Lenses are driven by **document symbols** (functions/classes/interfaces/ + * methods), not git hunks, so granularity follows the code and a brand-new + * file is as forwardable as a modified one. The right side of each file in the + * diff is a plain `file:` document at `/`, so we + * register against `{ scheme: 'file' }` and emit lenses only for documents + * whose fs path is in the *active diff session* registry. `viewDiff` and the + * per-file diff replace/extend that registry, so lenses are scoped to the + * files the reviewer opened for diffing; any other `file:` document returns + * `[]`. * * Clicking a lens runs `codev.forwardToBuilder` (a palette-hidden command * registered in `extension.ts`), which opens/reveals the builder terminal and - * types the reference into its prompt without pressing Enter — mirroring the - * architect-reference pattern (`codev.referenceIssueInArchitect`). + * types the reference into its prompt without pressing Enter. + * + * The registry also backs the `codev.activeEditorIsBuilderFile` context key + * (set on active-editor change) that scopes the right-click "Forward Selection + * to Builder" menu item to tracked builder-diff files. + * + * NOTE: CodeLens is suppressed in the multi-file `vscode.changes` editor, so + * these lenses render in the per-file `vscode.diff` and normal editor tabs + * (with `diffEditor.codeLens` enabled). The selection context-menu action is + * the path that works inside the multi-file editor. */ import * as vscode from 'vscode'; -import { buildLensDescriptors, type HunkRange } from './diff-inject-ref.js'; +import { buildSymbolLensDescriptors, type SymbolNode } from './diff-inject-ref.js'; /** Command id the lenses invoke. Registered in `extension.ts`, NOT declared in * `contributes.commands`, so it never appears in the Command Palette. */ export const FORWARD_TO_BUILDER_COMMAND = 'codev.forwardToBuilder'; +/** Context key (true when the active editor is a tracked builder-diff file) that + * scopes the `editor/context` "Forward Selection to Builder" item. */ +export const BUILDER_FILE_CONTEXT_KEY = 'codev.activeEditorIsBuilderFile'; + /** One changed file in the active diff session, keyed by its right-side fs path. */ export interface DiffInjectSessionEntry { /** Absolute fs path of the right-side (worktree) document. */ @@ -31,8 +45,16 @@ export interface DiffInjectSessionEntry { builderId: string; /** Repo-relative path injected into the prompt. */ relPath: string; - /** New-side hunk ranges for the per-hunk lenses. */ - hunks: HunkRange[]; +} + +/** Map a `vscode.DocumentSymbol` tree to the pure `SymbolNode` shape. */ +function toSymbolNode(s: vscode.DocumentSymbol): SymbolNode { + return { + kind: s.kind as number, + startLine: s.range.start.line, + endLine: s.range.end.line, + children: s.children?.map(toSymbolNode) ?? [], + }; } class DiffInjectCodeLensProvider implements vscode.CodeLensProvider { @@ -53,11 +75,32 @@ class DiffInjectCodeLensProvider implements vscode.CodeLensProvider { this._onDidChangeCodeLenses.fire(); } - provideCodeLenses(document: vscode.TextDocument): vscode.CodeLens[] { + get(fsPath: string): DiffInjectSessionEntry | undefined { + return this.registry.get(fsPath); + } + + async provideCodeLenses( + document: vscode.TextDocument, + token: vscode.CancellationToken, + ): Promise { const entry = this.registry.get(document.uri.fsPath); if (!entry) { return []; } + + let symbols: vscode.DocumentSymbol[] = []; + try { + symbols = + (await vscode.commands.executeCommand( + 'vscode.executeDocumentSymbolProvider', + document.uri, + )) ?? []; + } catch { + symbols = []; + } + if (token.isCancellationRequested) { return []; } + + const nodes = symbols.map(toSymbolNode); const lastLine = Math.max(document.lineCount - 1, 0); - return buildLensDescriptors(entry.relPath, entry.hunks).map(d => { + return buildSymbolLensDescriptors(entry.relPath, nodes).map(d => { const line = Math.min(Math.max(d.line, 0), lastLine); const range = new vscode.Range(line, 0, line, 0); return new vscode.CodeLens(range, { @@ -75,11 +118,19 @@ class DiffInjectCodeLensProvider implements vscode.CodeLensProvider { const provider = new DiffInjectCodeLensProvider(); -/** Register the provider for `file:` documents. Called once at activation, +/** Register the provider for `file:` documents and keep the builder-file + * context key in sync with the active editor. Called once at activation, * alongside `activateDiffView`. */ export function activateDiffInjectCodeLens(context: vscode.ExtensionContext): void { + const syncContextKey = (editor: vscode.TextEditor | undefined): void => { + const isBuilderFile = !!editor && provider.get(editor.document.uri.fsPath) !== undefined; + void vscode.commands.executeCommand('setContext', BUILDER_FILE_CONTEXT_KEY, isBuilderFile); + }; + syncContextKey(vscode.window.activeTextEditor); + context.subscriptions.push( vscode.languages.registerCodeLensProvider({ scheme: 'file' }, provider), + vscode.window.onDidChangeActiveTextEditor(syncContextKey), provider, ); } @@ -95,3 +146,9 @@ export function setDiffInjectSession(entries: DiffInjectSessionEntry[]): void { export function upsertDiffInjectEntry(entry: DiffInjectSessionEntry): void { provider.upsert(entry); } + +/** Look up the tracked builder-diff entry for a file path (the selection + * command uses this to resolve the owning builder + repo-relative path). */ +export function getDiffInjectEntry(fsPath: string): DiffInjectSessionEntry | undefined { + return provider.get(fsPath); +} diff --git a/packages/vscode/src/diff-inject-ref.ts b/packages/vscode/src/diff-inject-ref.ts index 5ebd9df5e..b959867e1 100644 --- a/packages/vscode/src/diff-inject-ref.ts +++ b/packages/vscode/src/diff-inject-ref.ts @@ -1,31 +1,16 @@ /** - * Pure helpers for the "Forward to Builder" CodeLens actions in the - * `codev.viewDiff` editor (issue #789). No `vscode` import — same precedent - * as `architect-reference-injection.ts`, so the parsing/string logic is - * unit-tested directly without mocking the editor API. + * Pure helpers for the "Forward to Builder" CodeLens actions in the builder + * diff (#789). No `vscode` import — same precedent as + * `architect-reference-injection.ts`, so the symbol-selection and ref-string + * logic is unit-tested directly without mocking the editor API. * - * The provider (`diff-inject-codelens.ts`) is thin glue over these: it turns - * `LensDescriptor`s into `vscode.CodeLens` objects and wires the inject - * command. All the line-math and ref-string logic lives here. + * Lenses are driven by **document symbols**, not git hunks: granularity follows + * the code (functions/classes/interfaces/methods), so a brand-new file is just + * as forwardable as a modified one. The provider + * (`diff-inject-codelens.ts`) resolves symbols via VSCode and adapts them to + * the `SymbolNode` shape below; all selection/anchor logic lives here. */ -/** - * New-side line numbers of one diff hunk (1-based, inclusive). - * - * `newStart`/`newEnd` are the hunk header's full new-side span (context - * included — what `@@ … +c,d @@` reports). `changeStart`/`changeEnd` are the - * first and last lines that are *actually added* on the new side (the `+` - * lines), so a lens anchored there sits on the real change rather than on the - * up-to-3 lines of leading context git emits with `--unified=3`. For a - * pure-deletion hunk (no `+` lines) both change fields collapse to `newStart`. - */ -export interface HunkRange { - newStart: number; - newEnd: number; - changeStart: number; - changeEnd: number; -} - /** * Editor-agnostic description of one CodeLens to render: the 0-based line to * anchor it on, the label, and the text to inject into the builder prompt. @@ -38,131 +23,103 @@ export interface LensDescriptor { refText: string; } -const HUNK_HEADER = /^@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@/; - /** - * Parse the `@@ -a,b +c,d @@` hunk headers from a single file's unified diff - * and return each hunk's new-side line range. - * - * - New-side length is `d` (absent → 1, matching git's shorthand for a - * single added/changed line). - * - A pure-deletion hunk (`+c,0`) has no new-side lines; we clamp it to a - * single anchor at `c` (or line 1 if `c` is 0) so a click still references - * a sane location near the change. + * Minimal, `vscode`-free projection of `vscode.DocumentSymbol` — just the + * fields the selection logic needs. `kind` is the numeric `vscode.SymbolKind` + * value; `startLine`/`endLine` are the symbol's full range (0-based). */ -export function parseHunkRanges(patch: string): HunkRange[] { - const lines = patch.split('\n'); - const ranges: HunkRange[] = []; - for (let i = 0; i < lines.length; i++) { - const m = HUNK_HEADER.exec(lines[i]!); - if (!m) { continue; } - const newStart = Number(m[1]); - const len = m[2] === undefined ? 1 : Number(m[2]); - const newEnd = len <= 0 ? Math.max(newStart, 1) : newStart + len - 1; - - // Walk the hunk body to find the first/last lines actually added on the - // new side. `cur` tracks the new-side line number: context (' ') and - // additions ('+') advance it; deletions ('-') are old-side only; the - // "\ No newline at end of file" marker advances nothing. - let cur = newStart; - let firstChange = -1; - let lastChange = -1; - for (let j = i + 1; j < lines.length; j++) { - const l = lines[j]!; - if (HUNK_HEADER.test(l) || l.startsWith('diff --git')) { break; } - if (l.startsWith('\\')) { continue; } - if (l.startsWith('+')) { - if (firstChange === -1) { firstChange = cur; } - lastChange = cur; - cur++; - } else if (l.startsWith('-')) { - // old-side only — does not advance the new-side counter - } else { - cur++; // context line - } - } - - const changeStart = firstChange === -1 ? Math.max(newStart, 1) : firstChange; - const changeEnd = lastChange === -1 ? changeStart : lastChange; - ranges.push({ - newStart: len <= 0 ? Math.max(newStart, 1) : newStart, - newEnd, - changeStart, - changeEnd, - }); - } - return ranges; +export interface SymbolNode { + kind: number; + startLine: number; + endLine: number; + children: SymbolNode[]; } /** - * Split a multi-file unified diff (`git diff -M --unified=N `) into a - * map from each file's **new** path to its hunk ranges. The new path is read - * from the `+++ b/` line (the canonical new-side path, correct for - * renames); files whose new side is `/dev/null` (deletions) are omitted — - * they have no right-side document to host a lens. + * Numeric `vscode.SymbolKind` values (stable API enum). Kept here so the pure + * module needs no `vscode` import; the provider passes `symbol.kind` straight + * through. */ -export function parseUnifiedDiff(patch: string): Map { - const out = new Map(); - // Split on the per-file boundary; the first chunk before any `diff --git` - // is empty/preamble and parses to no path, so it's harmless. - const sections = patch.split(/^diff --git .*$/m).slice(1); - // `split` drops the delimiter lines, but the `+++`/`@@` lines we need live - // in the body after each boundary, so re-walk the raw text per section. - for (const section of sections) { - const newPath = newPathFromSection(section); - if (!newPath) { continue; } - out.set(newPath, parseHunkRanges(section)); - } - return out; -} +const KIND = { + Module: 1, + Namespace: 2, + Class: 4, + Method: 5, + Constructor: 8, + Enum: 9, + Interface: 10, + Function: 11, + Variable: 12, + Constant: 13, + Struct: 22, +} as const; -/** Extract the new-side path from a single file section's `+++ b/` line. */ -function newPathFromSection(section: string): string | null { - for (const line of section.split('\n')) { - if (!line.startsWith('+++ ')) { continue; } - const target = line.slice(4).trim(); - if (target === '/dev/null') { return null; } - // Strip the conventional `b/` prefix git prepends to the new path. - return target.startsWith('b/') ? target.slice(2) : target; - } - return null; -} +/** Top-level structural declarations that always get a lens. */ +const TOP_LEVEL_KINDS = new Set([ + KIND.Function, + KIND.Class, + KIND.Interface, + KIND.Enum, + KIND.Struct, + KIND.Namespace, + KIND.Module, +]); -/** Text injected by the file-level lens: ` ` (no Enter). */ +/** The whole file: ` ` (trailing space, no Enter). */ export function buildBuilderFileRef(relPath: string): string { return `${relPath} `; } -/** Text injected by a hunk lens: `:L-L ` (no Enter). */ -export function buildBuilderHunkRef(relPath: string, start: number, end: number): string { +/** A line range: `:L-L ` (trailing space, no Enter). */ +export function buildBuilderRangeRef(relPath: string, start: number, end: number): string { return `${relPath}:L${start}-L${end} `; } /** - * Build the full set of lens descriptors for one changed file: a file-level - * lens at the top, plus one lens per hunk anchored at the hunk's new-side - * start line (converted to 0-based). + * Build the lens descriptors for a file from its document symbols: + * + * - A **file-level** lens at line 0 (forward the whole file). + * - A lens on each **top-level** structural declaration (the allowlist above), + * plus **top-level multi-line** Variable/Constant (catches arrow-function + * components/handlers reported as Variable, while skipping scalar consts). + * - One level into **Class/Struct** for Method/Constructor lenses, so a single + * method can be forwarded. No deeper recursion. + * + * A symbol lens that would anchor on line 0 is skipped — the file-level lens + * already occupies that line. */ -export function buildLensDescriptors(relPath: string, hunks: HunkRange[]): LensDescriptor[] { +export function buildSymbolLensDescriptors(relPath: string, symbols: SymbolNode[]): LensDescriptor[] { const lenses: LensDescriptor[] = [ { line: 0, title: 'Forward to Builder', refText: buildBuilderFileRef(relPath) }, ]; - for (const h of hunks) { - // Anchor and label on the first/last *changed* new-side lines, not the - // hunk header's context-inclusive span — so the lens sits on the real edit - // (e.g. inside the right function) rather than on the up-to-3 leading - // context lines git emits with --unified=3. - const line = Math.max(h.changeStart - 1, 0); - // The file-level lens already occupies line 0. A hunk that anchors there — - // a newly-added file (one whole-file hunk from line 1) or a change to the - // very first line — would stack a second "Forward to Builder" on the same - // line. Skip it; the file-level lens covers that spot. - if (line === 0) { continue; } + + const addLens = (s: SymbolNode): void => { + const line = Math.max(s.startLine, 0); + if (line === 0) { return; } // collides with the file-level lens lenses.push({ line, - title: `Forward to Builder (lines ${h.changeStart}-${h.changeEnd})`, - refText: buildBuilderHunkRef(relPath, h.changeStart, h.changeEnd), + title: 'Forward to Builder', + refText: buildBuilderRangeRef(relPath, s.startLine + 1, s.endLine + 1), }); + }; + + for (const s of symbols) { + if (TOP_LEVEL_KINDS.has(s.kind)) { + addLens(s); + if (s.kind === KIND.Class || s.kind === KIND.Struct) { + for (const child of s.children) { + if (child.kind === KIND.Method || child.kind === KIND.Constructor) { + addLens(child); + } + } + } + } else if ( + (s.kind === KIND.Variable || s.kind === KIND.Constant) && + s.endLine > s.startLine + ) { + addLens(s); + } } + return lenses; } From 0139177ace67dcd6bfc5b3e6da7644f40c6d38b6 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 10 Jun 2026 14:51:25 +1000 Subject: [PATCH 20/39] [PIR #789] Drop git hunk fetch from viewDiff registry population (symbols replace it) --- packages/vscode/src/commands/view-diff.ts | 65 +++++++---------------- 1 file changed, 19 insertions(+), 46 deletions(-) diff --git a/packages/vscode/src/commands/view-diff.ts b/packages/vscode/src/commands/view-diff.ts index 2256f8709..6da215159 100644 --- a/packages/vscode/src/commands/view-diff.ts +++ b/packages/vscode/src/commands/view-diff.ts @@ -28,7 +28,6 @@ import { execFile } from 'node:child_process'; import { promisify } from 'node:util'; import * as path from 'node:path'; import type { ConnectionManager } from '../connection-manager.js'; -import { parseUnifiedDiff, parseHunkRanges, type HunkRange } from '../diff-inject-ref.js'; import { setDiffInjectSession, upsertDiffInjectEntry, type DiffInjectSessionEntry } from '../diff-inject-codelens.js'; const execFileAsync = promisify(execFile); @@ -361,30 +360,17 @@ export async function viewDiff( ]; }); - // CodeLens "Forward to Builder" actions (#789): give the provider the - // right-side fs paths and new-side hunk ranges for the files just opened. - // One extra git call here keeps `getBuilderChanges` (shared with the - // Builders tree) untouched. A patch failure is non-fatal — the diff still - // opens, just without lenses. - try { - const { stdout: patch } = await execFileAsync( - 'git', - ['-C', wt, 'diff', '-M', '--unified=3', baseRef], - { maxBuffer: 64 * 1024 * 1024 }, - ); - const hunksByPath = parseUnifiedDiff(patch); - const entries: DiffInjectSessionEntry[] = plans - .filter(plan => plan.right.kind === 'file') - .map(plan => ({ - fsPath: path.join(wt, plan.resourcePath), - builderId: builder.id, - relPath: plan.resourcePath, - hunks: hunksByPath.get(plan.resourcePath) ?? [], - })); - setDiffInjectSession(entries); - } catch { - setDiffInjectSession([]); - } + // CodeLens "Forward to Builder" actions (#789): register the right-side fs + // paths + owning builder for the files just opened. The lenses themselves + // come from document symbols, resolved lazily by the provider — no git here. + const entries: DiffInjectSessionEntry[] = plans + .filter(plan => plan.right.kind === 'file') + .map(plan => ({ + fsPath: path.join(wt, plan.resourcePath), + builderId: builder.id, + relPath: plan.resourcePath, + })); + setDiffInjectSession(entries); await vscode.commands.executeCommand( 'vscode.changes', @@ -394,38 +380,25 @@ export async function viewDiff( } /** - * Populate the inject-codelens registry for a single changed file — the - * per-file `vscode.diff` opened from the Builders tree (`openBuilderFileDiff`). - * Mirrors what `viewDiff` does for the whole delta, but for one file, so the - * "Forward to Builder" lenses appear even when the reviewer opens a file diff - * without first running View Diff. Non-fatal on git failure (the file-level - * lens still shows; only the per-hunk lenses are dropped). + * Register the inject-codelens entry for a single changed file — the per-file + * `vscode.diff` opened from the Builders tree (`openBuilderFileDiff`). Mirrors + * what `viewDiff` does for the whole delta, but for one file, so the "Forward + * to Builder" lenses appear even when the reviewer opens a file diff without + * first running View Diff. The lenses come from document symbols, so no git is + * needed here — just the path → builder mapping. */ -export async function registerFileInjectSession(args: { +export function registerFileInjectSession(args: { worktreePath: string; - baseRef: string; builderId: string; plan: ResourcePlan; -}): Promise { +}): void { // Deleted/binary files have no right-side `file:` document to host a lens. if (args.plan.right.kind !== 'file') { return; } const relPath = args.plan.resourcePath; - let hunks: HunkRange[] = []; - try { - const { stdout } = await execFileAsync( - 'git', - ['-C', args.worktreePath, 'diff', '-M', '--unified=3', args.baseRef, '--', relPath], - { maxBuffer: 64 * 1024 * 1024 }, - ); - hunks = parseHunkRanges(stdout); - } catch { - // Leave hunks empty — the file-level lens is still useful. - } upsertDiffInjectEntry({ fsPath: path.join(args.worktreePath, relPath), builderId: args.builderId, relPath, - hunks, }); } From 75c03eb57aac3ef22431900b34bd5b9e9126726f Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 10 Jun 2026 14:51:25 +1000 Subject: [PATCH 21/39] [PIR #789] Add right-click 'Forward Selection to Builder' editor/context action Forwards an arbitrary selected line range; works inside the multi-file View Diff editor (context menus aren't suppressed there, unlike CodeLens). Scoped via codev.activeEditorIsBuilderFile context key + editorHasSelection. --- packages/vscode/package.json | 15 +++++++++++++ packages/vscode/src/extension.ts | 37 ++++++++++++++++++++++++++------ 2 files changed, 45 insertions(+), 7 deletions(-) diff --git a/packages/vscode/package.json b/packages/vscode/package.json index bc933aff2..41c309614 100644 --- a/packages/vscode/package.json +++ b/packages/vscode/package.json @@ -50,6 +50,10 @@ "command": "codev.helloWorld", "title": "Codev: Show Connection State" }, + { + "command": "codev.forwardSelectionToBuilder", + "title": "Forward Selection to Builder" + }, { "command": "codev.openArchitectTerminal", "title": "Codev: Open Architect Terminal" @@ -295,6 +299,10 @@ ], "menus": { "commandPalette": [ + { + "command": "codev.forwardSelectionToBuilder", + "when": "false" + }, { "command": "codev.openBuilderById", "when": "false" @@ -388,6 +396,13 @@ "when": "false" } ], + "editor/context": [ + { + "command": "codev.forwardSelectionToBuilder", + "when": "codev.activeEditorIsBuilderFile && editorHasSelection", + "group": "codev@1" + } + ], "view/item/context": [ { "command": "codev.recheckCli", diff --git a/packages/vscode/src/extension.ts b/packages/vscode/src/extension.ts index eba051112..791a50a63 100644 --- a/packages/vscode/src/extension.ts +++ b/packages/vscode/src/extension.ts @@ -8,8 +8,9 @@ import { approveGate } from './commands/approve.js'; import { cleanupBuilder } from './commands/cleanup.js'; import { openWorktreeWindow } from './commands/open-worktree-window.js'; import { viewDiff, activateDiffView, diffUrisForChange, registerFileInjectSession } from './commands/view-diff.js'; -import { activateDiffInjectCodeLens } from './diff-inject-codelens.js'; +import { activateDiffInjectCodeLens, getDiffInjectEntry } from './diff-inject-codelens.js'; import { ensureDiffEditorCodeLens } from './ensure-diff-codelens.js'; +import { buildBuilderRangeRef } from './diff-inject-ref.js'; import { runWorktreeDev } from './commands/run-worktree-dev.js'; import { stopWorktreeDev } from './commands/stop-worktree-dev.js'; import { runWorkspaceDev, stopWorkspaceDev } from './commands/run-workspace-dev.js'; @@ -821,15 +822,37 @@ export async function activate(context: vscode.ExtensionContext) { vscode.window.showWarningMessage('Codev: Builder terminal not available'); } }), + // Right-click "Forward Selection to Builder" (#789): forward an arbitrary + // selected range when symbol/file lenses aren't granular enough. Unlike + // the CodeLens, a context-menu action works inside the multi-file View + // Diff editor too. Scoped via the `codev.activeEditorIsBuilderFile` + // context key + the built-in `editorHasSelection` in its `when` clause. + reg('codev.forwardSelectionToBuilder', async () => { + const editor = vscode.window.activeTextEditor; + if (!editor) { return; } + const entry = getDiffInjectEntry(editor.document.uri.fsPath); + if (!entry) { return; } + const sel = editor.selection; + if (sel.isEmpty) { return; } + const start = sel.start.line + 1; + // A selection ending at column 0 of a line doesn't include that line. + const end = sel.end.character === 0 && sel.end.line > sel.start.line + ? sel.end.line + : sel.end.line + 1; + const text = buildBuilderRangeRef(entry.relPath, start, end); + const resolvedId = await terminalManager?.openBuilderByRoleOrId(entry.builderId, true); + if (resolvedId && !terminalManager?.injectBuilderText(resolvedId, text)) { + vscode.window.showWarningMessage('Codev: Builder terminal not available'); + } + }), reg('codev.openBuilderFileDiff', async (arg: unknown) => { if (!(arg instanceof BuilderFileTreeItem)) { return; } - // Populate the inject-codelens registry for this file so the - // "Forward to Builder" lenses (#789) render on the diff without a - // prior View Diff run, then offer to enable diffEditor.codeLens (off - // by default — VS Code hides CodeLens in diff editors). - await registerFileInjectSession({ + // Register the inject-codelens entry for this file so the "Forward + // to Builder" lenses (#789) render on the diff without a prior View + // Diff run, then offer to enable diffEditor.codeLens (off by default + // — VS Code hides CodeLens in diff editors). + registerFileInjectSession({ worktreePath: arg.worktreePath, - baseRef: arg.baseRef, builderId: arg.builderId, plan: arg.plan, }); From 4f93915bedeff049d2172bd870a0fa5413fde3e2 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 10 Jun 2026 14:51:45 +1000 Subject: [PATCH 22/39] [PIR #789] Plan + thread: record pivot to symbol lenses + selection forward --- .../789-vscode-inject-file-hunk-refere.md | 19 +++++++++++++++++++ codev/state/pir-789_thread.md | 13 +++++++++++++ 2 files changed, 32 insertions(+) diff --git a/codev/plans/789-vscode-inject-file-hunk-refere.md b/codev/plans/789-vscode-inject-file-hunk-refere.md index c65978689..dc534af8d 100644 --- a/codev/plans/789-vscode-inject-file-hunk-refere.md +++ b/codev/plans/789-vscode-inject-file-hunk-refere.md @@ -63,6 +63,25 @@ reg('codev.injectBuilderFileRef', async (builderId: string, text: string) => { - **Alternative rejected — render lenses on the left `codev-diff:` side** (fully scoped to the diff editor, never over-triggers): rejected because the left side holds *old* content with old-side line numbers and is empty for added files, so it can't carry new-side hunk ranges. - **Alternative rejected — replace the side-by-side editor with a single unified-diff text document** (file/hunk headers literally in text): rejected as a UX regression — reviewers rely on the side-by-side multi-file editor, and the issue asks to add lenses *to the existing `viewDiff` editor*, not replace it. +## Revision (dev-approval gate): pivot from hunk lenses to symbol lenses + selection forward + +Testing at the `dev-approval` gate showed the approved hunk-driven approach is **not usable**: a newly-created file is a single whole-file git hunk, so it gets exactly one lens regardless of size — you can't forward a specific function/interface in a new file. Hunk granularity is hostage to where the *edit* is, not where you want to *comment*. (Also confirmed earlier: CodeLens doesn't render in the multi-file `vscode.changes` editor, so lenses live on the per-file `vscode.diff` / normal tab with `diffEditor.codeLens` enabled.) + +Revised approach (architect-directed at the gate): + +1. **Symbol-level lenses** — drive lenses off VSCode's Document Symbol provider (`vscode.executeDocumentSymbolProvider`) instead of git hunks, so granularity follows the *code*, not the diff. Level-2 depth + a kind allowlist: + - **Depth 0 (top-level):** Function, Class, Interface, Enum, Struct, Namespace, Module; plus **multi-line** top-level Variable/Constant (catches arrow-function components/handlers reported as Variable). + - **Depth 1 (into Class/Struct only):** Method, Constructor. + - Excluded: Property, Field, EnumMember, TypeParameter, Package, value kinds, one-line/non-top-level Variable/Constant. + - A symbol lens that anchors on line 0 is skipped (the file-level lens covers it). +2. **File-level lens** retained (whole file). +3. **Hunk lenses removed** — the git-diff hunk parsing (`parseHunkRanges`/`parseUnifiedDiff`) is no longer used for lenses. +4. **Right-click "Forward Selection to Builder"** — an `editor/context` command on a non-empty selection injects `:L-L ` for the exact selected range. Context menus are *not* suppressed in the multi-file View Diff editor, so this is the granular path that works in the scan view. Scoped via a `codev.activeEditorIsBuilderFile` context key (set when the active editor is a tracked builder-diff file) + the built-in `editorHasSelection`. + +Surfaces: symbol/file lenses → per-file diff + normal tab (CodeLens limitation). Selection context-menu → works in the multi-file View Diff editor too (to be validated at the gate). + +Deferred (future issue): hover-triggered forward action; deeper symbol nesting; per-kind density settings. + ## Test Plan **Unit (`diff-inject-ref.test.ts`, vitest — `pnpm --filter codev-vscode test:unit`):** diff --git a/codev/state/pir-789_thread.md b/codev/state/pir-789_thread.md index 250b35a6a..2e9775eb1 100644 --- a/codev/state/pir-789_thread.md +++ b/codev/state/pir-789_thread.md @@ -44,6 +44,19 @@ Validation: check-types/lint/test:unit (372) all green; esbuild bundles. **Needs Note for review file: the spec'd surface was infeasible; should reflect the per-file-diff entry point back to the issue. +## Major pivot (dev-approval): symbol lenses + selection forward, hunk lenses removed + +Hunk-driven lenses proved unusable: a new file is one whole-file hunk → one lens regardless of size, no way to forward a specific function/interface. Architect directed a redesign (captured in the plan's "Revision" section): +- **Lenses now driven by document symbols** (`vscode.executeDocumentSymbolProvider`), not git hunks. Level-2 policy: file-level lens + top-level Function/Class/Interface/Enum/Struct/Namespace/Module + multi-line top-level Variable/Constant; descend one level into Class/Struct for Method/Constructor. Line-0 collision with the file lens skipped. +- **Hunk parsing removed** (`parseHunkRanges`/`parseUnifiedDiff`/`HunkRange` gone); registry entry simplified to `{fsPath, builderId, relPath}`; viewDiff/registerFileInjectSession no longer touch git for lenses. +- **Right-click "Forward Selection to Builder"** (`editor/context`) added for arbitrary ranges — context menus DO work in the multi-file View Diff editor (unlike CodeLens), so this is the in-scan-view path. Scoped via `codev.activeEditorIsBuilderFile` context key + `editorHasSelection`. + +Label still "Forward to Builder" (lenses) / "Forward Selection to Builder" (menu). Command ids: `codev.forwardToBuilder` (lens, palette-hidden) + `codev.forwardSelectionToBuilder` (menu, palette-hidden, declared for the context menu). + +Validation: 368 unit tests, check-types, lint, bundle all green. **Needs human re-test (F5)**: symbol lenses on per-file diff/normal tab; right-click selection in View Diff editor. Surfaces caveat unchanged: CodeLens not in multi-file editor; selection menu is. + +Deferred to future issue(s): hover-triggered forward; deeper nesting; per-kind density settings. + ## Dev-approval iteration: hunk-lens placement fix Human observed hunk lenses landing on the wrong function's signature. Root cause: `--unified=3` reports each hunk's new-side start up to 3 *context* lines above the first real change, so near a function boundary the anchor lands on the preceding `def`/`return`. Fixed: `parseHunkRanges` now walks the hunk body to find the first/last actually-added (`+`) new-side lines (`changeStart`/`changeEnd`); `buildLensDescriptors` anchors AND labels on those instead of the header span. Deviation from issue's literal "hunk header range" — intentional, more accurate. 374 tests green. Needs human re-test after F5 relaunch. From 9ab74af4188eabafd00b081681017348e3d981e8 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 10 Jun 2026 16:56:26 +1000 Subject: [PATCH 23/39] [PIR #789] Add Cmd/Ctrl+K B shortcut for Forward Selection; prefix menu title with 'Codev:' --- packages/vscode/package.json | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/vscode/package.json b/packages/vscode/package.json index 41c309614..e547da4e6 100644 --- a/packages/vscode/package.json +++ b/packages/vscode/package.json @@ -52,7 +52,7 @@ }, { "command": "codev.forwardSelectionToBuilder", - "title": "Forward Selection to Builder" + "title": "Codev: Forward Selection to Builder" }, { "command": "codev.openArchitectTerminal", @@ -649,6 +649,12 @@ "key": "ctrl+k g", "mac": "cmd+k g" }, + { + "command": "codev.forwardSelectionToBuilder", + "key": "ctrl+k b", + "mac": "cmd+k b", + "when": "codev.activeEditorIsBuilderFile && editorHasSelection" + }, { "command": "workbench.view.extension.codev", "key": "ctrl+alt+c", From aea085f434f3c4db5374e6e4a8006a75d42bb5d0 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 10 Jun 2026 17:02:49 +1000 Subject: [PATCH 24/39] [PIR #789] Restore per-hunk lenses alongside symbol lenses (option B) Layer hunk lenses ('Forward to Builder (lines N-M)') back on top of the symbol/file lenses via buildAllLensDescriptors. A hunk lens is skipped when its anchor collides with a symbol/file lens (declaration-line change shows the structural lens; body change shows the hunk lens), so nothing stacks. --- .../src/__tests__/diff-inject-ref.test.ts | 59 +++++++++ packages/vscode/src/commands/view-diff.ts | 40 +++++- packages/vscode/src/diff-inject-codelens.ts | 6 +- packages/vscode/src/diff-inject-ref.ts | 115 ++++++++++++++++++ packages/vscode/src/extension.ts | 3 +- 5 files changed, 214 insertions(+), 9 deletions(-) diff --git a/packages/vscode/src/__tests__/diff-inject-ref.test.ts b/packages/vscode/src/__tests__/diff-inject-ref.test.ts index 75a13e720..f99ef5d8f 100644 --- a/packages/vscode/src/__tests__/diff-inject-ref.test.ts +++ b/packages/vscode/src/__tests__/diff-inject-ref.test.ts @@ -9,6 +9,8 @@ import { buildBuilderFileRef, buildBuilderRangeRef, buildSymbolLensDescriptors, + buildAllLensDescriptors, + parseHunkRanges, type SymbolNode, } from '../diff-inject-ref.js'; @@ -106,3 +108,60 @@ describe('buildSymbolLensDescriptors', () => { ]); }); }); + +describe('parseHunkRanges', () => { + it('reads the header span and the first/last added new-side lines', () => { + const patch = [ + '@@ -1,4 +1,6 @@', + ' a', // new line 1 (context) + '+b', // new line 2 (added) + ' c', + '@@ -20,3 +22,10 @@ func()', + ' x', // 22 + ' y', // 23 + '+z1', // 24 (first change) + '+z2', // 25 (last change) + ' w', + ].join('\n'); + expect(parseHunkRanges(patch)).toEqual([ + { newStart: 1, newEnd: 6, changeStart: 2, changeEnd: 2 }, + { newStart: 22, newEnd: 31, changeStart: 24, changeEnd: 25 }, + ]); + }); + + it('does not let deleted (-) lines advance the new-side counter', () => { + const patch = ['@@ -10,4 +10,3 @@', ' ctx', '-gone', '+added', ' tail'].join('\n'); + expect(parseHunkRanges(patch)).toEqual([ + { newStart: 10, newEnd: 12, changeStart: 11, changeEnd: 11 }, + ]); + }); +}); + +describe('buildAllLensDescriptors (symbol + hunk lenses)', () => { + it('adds per-hunk lenses below the symbol/file lenses', () => { + const symbols = [sym(K.Function, 4, 30)]; // function lens at line 4 + const hunks = [{ newStart: 8, newEnd: 12, changeStart: 10, changeEnd: 12 }]; + expect(buildAllLensDescriptors('a/b.ts', symbols, hunks)).toEqual([ + { line: 0, title: 'Forward to Builder', refText: 'a/b.ts ' }, + { line: 4, title: 'Forward to Builder', refText: 'a/b.ts:L5-L31 ' }, + { line: 9, title: 'Forward to Builder (lines 10-12)', refText: 'a/b.ts:L10-L12 ' }, + ]); + }); + + it('skips a hunk lens that collides with a symbol lens line', () => { + const symbols = [sym(K.Function, 9, 30)]; // function lens at line 9 + // hunk changeStart 10 → anchor line 9, same as the function lens → skipped + const hunks = [{ newStart: 10, newEnd: 12, changeStart: 10, changeEnd: 12 }]; + expect(buildAllLensDescriptors('a/b.ts', symbols, hunks)).toEqual([ + { line: 0, title: 'Forward to Builder', refText: 'a/b.ts ' }, + { line: 9, title: 'Forward to Builder', refText: 'a/b.ts:L10-L31 ' }, + ]); + }); + + it('skips a whole-file hunk (changeStart 1) that collides with the file-level lens', () => { + const hunks = [{ newStart: 1, newEnd: 17, changeStart: 1, changeEnd: 17 }]; + expect(buildAllLensDescriptors('a/b.ts', [], hunks)).toEqual([ + { line: 0, title: 'Forward to Builder', refText: 'a/b.ts ' }, + ]); + }); +}); diff --git a/packages/vscode/src/commands/view-diff.ts b/packages/vscode/src/commands/view-diff.ts index 6da215159..318d64a86 100644 --- a/packages/vscode/src/commands/view-diff.ts +++ b/packages/vscode/src/commands/view-diff.ts @@ -28,6 +28,7 @@ import { execFile } from 'node:child_process'; import { promisify } from 'node:util'; import * as path from 'node:path'; import type { ConnectionManager } from '../connection-manager.js'; +import { parseHunkRanges, parseUnifiedDiff } from '../diff-inject-ref.js'; import { setDiffInjectSession, upsertDiffInjectEntry, type DiffInjectSessionEntry } from '../diff-inject-codelens.js'; const execFileAsync = promisify(execFile); @@ -361,14 +362,28 @@ export async function viewDiff( }); // CodeLens "Forward to Builder" actions (#789): register the right-side fs - // paths + owning builder for the files just opened. The lenses themselves - // come from document symbols, resolved lazily by the provider — no git here. + // paths + owning builder, plus per-hunk ranges so changed regions get their + // own lens alongside the symbol/file lenses (symbols resolve lazily in the + // provider). One extra git call here; a patch failure is non-fatal — the + // symbol/file lenses still render. + let hunksByPath = new Map>(); + try { + const { stdout: patch } = await execFileAsync( + 'git', + ['-C', wt, 'diff', '-M', '--unified=3', baseRef], + { maxBuffer: 64 * 1024 * 1024 }, + ); + hunksByPath = parseUnifiedDiff(patch); + } catch { + // keep empty — symbol/file lenses still render + } const entries: DiffInjectSessionEntry[] = plans .filter(plan => plan.right.kind === 'file') .map(plan => ({ fsPath: path.join(wt, plan.resourcePath), builderId: builder.id, relPath: plan.resourcePath, + hunks: hunksByPath.get(plan.resourcePath) ?? [], })); setDiffInjectSession(entries); @@ -384,21 +399,34 @@ export async function viewDiff( * `vscode.diff` opened from the Builders tree (`openBuilderFileDiff`). Mirrors * what `viewDiff` does for the whole delta, but for one file, so the "Forward * to Builder" lenses appear even when the reviewer opens a file diff without - * first running View Diff. The lenses come from document symbols, so no git is - * needed here — just the path → builder mapping. + * first running View Diff. Symbol lenses resolve lazily in the provider; the + * per-hunk ranges are computed here (non-fatal on git failure). */ -export function registerFileInjectSession(args: { +export async function registerFileInjectSession(args: { worktreePath: string; + baseRef: string; builderId: string; plan: ResourcePlan; -}): void { +}): Promise { // Deleted/binary files have no right-side `file:` document to host a lens. if (args.plan.right.kind !== 'file') { return; } const relPath = args.plan.resourcePath; + let hunks: ReturnType = []; + try { + const { stdout } = await execFileAsync( + 'git', + ['-C', args.worktreePath, 'diff', '-M', '--unified=3', args.baseRef, '--', relPath], + { maxBuffer: 64 * 1024 * 1024 }, + ); + hunks = parseHunkRanges(stdout); + } catch { + // keep empty — symbol/file lenses still render + } upsertDiffInjectEntry({ fsPath: path.join(args.worktreePath, relPath), builderId: args.builderId, relPath, + hunks, }); } diff --git a/packages/vscode/src/diff-inject-codelens.ts b/packages/vscode/src/diff-inject-codelens.ts index 0519c7c5c..3c63f112a 100644 --- a/packages/vscode/src/diff-inject-codelens.ts +++ b/packages/vscode/src/diff-inject-codelens.ts @@ -27,7 +27,7 @@ */ import * as vscode from 'vscode'; -import { buildSymbolLensDescriptors, type SymbolNode } from './diff-inject-ref.js'; +import { buildAllLensDescriptors, type HunkRange, type SymbolNode } from './diff-inject-ref.js'; /** Command id the lenses invoke. Registered in `extension.ts`, NOT declared in * `contributes.commands`, so it never appears in the Command Palette. */ @@ -45,6 +45,8 @@ export interface DiffInjectSessionEntry { builderId: string; /** Repo-relative path injected into the prompt. */ relPath: string; + /** New-side hunk ranges for the per-hunk lenses (empty = file/symbol lenses only). */ + hunks: HunkRange[]; } /** Map a `vscode.DocumentSymbol` tree to the pure `SymbolNode` shape. */ @@ -100,7 +102,7 @@ class DiffInjectCodeLensProvider implements vscode.CodeLensProvider { const nodes = symbols.map(toSymbolNode); const lastLine = Math.max(document.lineCount - 1, 0); - return buildSymbolLensDescriptors(entry.relPath, nodes).map(d => { + return buildAllLensDescriptors(entry.relPath, nodes, entry.hunks).map(d => { const line = Math.min(Math.max(d.line, 0), lastLine); const range = new vscode.Range(line, 0, line, 0); return new vscode.CodeLens(range, { diff --git a/packages/vscode/src/diff-inject-ref.ts b/packages/vscode/src/diff-inject-ref.ts index b959867e1..6fece8dbc 100644 --- a/packages/vscode/src/diff-inject-ref.ts +++ b/packages/vscode/src/diff-inject-ref.ts @@ -65,6 +65,94 @@ const TOP_LEVEL_KINDS = new Set([ KIND.Module, ]); +/** + * New-side line numbers of one diff hunk (1-based, inclusive). `newStart`/ + * `newEnd` are the header span (context included); `changeStart`/`changeEnd` + * are the first/last lines *actually added* on the new side, so a lens anchors + * on the real edit rather than the up-to-3 leading context lines git emits with + * `--unified=3`. A pure-deletion hunk collapses both change fields to `newStart`. + */ +export interface HunkRange { + newStart: number; + newEnd: number; + changeStart: number; + changeEnd: number; +} + +const HUNK_HEADER = /^@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@/; + +/** + * Parse the `@@ -a,b +c,d @@` hunk headers from a single file's unified diff + * into new-side ranges, walking each hunk body to find the first/last added + * (`+`) lines. + */ +export function parseHunkRanges(patch: string): HunkRange[] { + const lines = patch.split('\n'); + const ranges: HunkRange[] = []; + for (let i = 0; i < lines.length; i++) { + const m = HUNK_HEADER.exec(lines[i]!); + if (!m) { continue; } + const newStart = Number(m[1]); + const len = m[2] === undefined ? 1 : Number(m[2]); + const newEnd = len <= 0 ? Math.max(newStart, 1) : newStart + len - 1; + + let cur = newStart; + let firstChange = -1; + let lastChange = -1; + for (let j = i + 1; j < lines.length; j++) { + const l = lines[j]!; + if (HUNK_HEADER.test(l) || l.startsWith('diff --git')) { break; } + if (l.startsWith('\\')) { continue; } + if (l.startsWith('+')) { + if (firstChange === -1) { firstChange = cur; } + lastChange = cur; + cur++; + } else if (l.startsWith('-')) { + // old-side only — does not advance the new-side counter + } else { + cur++; // context line + } + } + + const changeStart = firstChange === -1 ? Math.max(newStart, 1) : firstChange; + const changeEnd = lastChange === -1 ? changeStart : lastChange; + ranges.push({ + newStart: len <= 0 ? Math.max(newStart, 1) : newStart, + newEnd, + changeStart, + changeEnd, + }); + } + return ranges; +} + +/** + * Split a multi-file unified diff (`git diff -M --unified=N `) into a map + * from each file's **new** path to its hunk ranges. The new path is read from + * the `+++ b/` line; deleted files (`+++ /dev/null`) are omitted. + */ +export function parseUnifiedDiff(patch: string): Map { + const out = new Map(); + const sections = patch.split(/^diff --git .*$/m).slice(1); + for (const section of sections) { + const newPath = newPathFromSection(section); + if (!newPath) { continue; } + out.set(newPath, parseHunkRanges(section)); + } + return out; +} + +/** Extract the new-side path from a single file section's `+++ b/` line. */ +function newPathFromSection(section: string): string | null { + for (const line of section.split('\n')) { + if (!line.startsWith('+++ ')) { continue; } + const target = line.slice(4).trim(); + if (target === '/dev/null') { return null; } + return target.startsWith('b/') ? target.slice(2) : target; + } + return null; +} + /** The whole file: ` ` (trailing space, no Enter). */ export function buildBuilderFileRef(relPath: string): string { return `${relPath} `; @@ -123,3 +211,30 @@ export function buildSymbolLensDescriptors(relPath: string, symbols: SymbolNode[ return lenses; } + +/** + * Combine the symbol lenses (file-level + declarations, bare "Forward to + * Builder") with per-hunk lenses (labeled "Forward to Builder (lines N-M)" on + * each changed region). A hunk lens is skipped when its anchor line already has + * a symbol/file lens — declaration-line changes show the structural lens; body + * changes show the hunk lens — so no two lenses stack on one line. + */ +export function buildAllLensDescriptors( + relPath: string, + symbols: SymbolNode[], + hunks: HunkRange[], +): LensDescriptor[] { + const lenses = buildSymbolLensDescriptors(relPath, symbols); + const usedLines = new Set(lenses.map(l => l.line)); + for (const h of hunks) { + const line = Math.max(h.changeStart - 1, 0); + if (usedLines.has(line)) { continue; } // file-level (line 0) or a symbol lens already here + usedLines.add(line); + lenses.push({ + line, + title: `Forward to Builder (lines ${h.changeStart}-${h.changeEnd})`, + refText: buildBuilderRangeRef(relPath, h.changeStart, h.changeEnd), + }); + } + return lenses; +} diff --git a/packages/vscode/src/extension.ts b/packages/vscode/src/extension.ts index 791a50a63..2682905f8 100644 --- a/packages/vscode/src/extension.ts +++ b/packages/vscode/src/extension.ts @@ -851,8 +851,9 @@ export async function activate(context: vscode.ExtensionContext) { // to Builder" lenses (#789) render on the diff without a prior View // Diff run, then offer to enable diffEditor.codeLens (off by default // — VS Code hides CodeLens in diff editors). - registerFileInjectSession({ + await registerFileInjectSession({ worktreePath: arg.worktreePath, + baseRef: arg.baseRef, builderId: arg.builderId, plan: arg.plan, }); From 4f329a780c1f5eae4c4588da0fe968f8f0e285bf Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 10 Jun 2026 17:03:31 +1000 Subject: [PATCH 25/39] [PIR #789] Plan + thread: record option B (hunk + symbol lenses) --- codev/plans/789-vscode-inject-file-hunk-refere.md | 2 +- codev/state/pir-789_thread.md | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/codev/plans/789-vscode-inject-file-hunk-refere.md b/codev/plans/789-vscode-inject-file-hunk-refere.md index dc534af8d..143b6cdb8 100644 --- a/codev/plans/789-vscode-inject-file-hunk-refere.md +++ b/codev/plans/789-vscode-inject-file-hunk-refere.md @@ -75,7 +75,7 @@ Revised approach (architect-directed at the gate): - Excluded: Property, Field, EnumMember, TypeParameter, Package, value kinds, one-line/non-top-level Variable/Constant. - A symbol lens that anchors on line 0 is skipped (the file-level lens covers it). 2. **File-level lens** retained (whole file). -3. **Hunk lenses removed** — the git-diff hunk parsing (`parseHunkRanges`/`parseUnifiedDiff`) is no longer used for lenses. +3. **Hunk lenses coexist with symbol lenses** (option B, after a round of review): the symbol lenses (bare "Forward to Builder" on declarations) are layered with per-hunk lenses ("Forward to Builder (lines N-M)" on each changed region) via `buildAllLensDescriptors`. A hunk lens is skipped when its anchor collides with a symbol/file lens, so nothing stacks. (`parseHunkRanges`/`parseUnifiedDiff` retained and fed into the registry entry's `hunks`.) 4. **Right-click "Forward Selection to Builder"** — an `editor/context` command on a non-empty selection injects `:L-L ` for the exact selected range. Context menus are *not* suppressed in the multi-file View Diff editor, so this is the granular path that works in the scan view. Scoped via a `codev.activeEditorIsBuilderFile` context key (set when the active editor is a tracked builder-diff file) + the built-in `editorHasSelection`. Surfaces: symbol/file lenses → per-file diff + normal tab (CodeLens limitation). Selection context-menu → works in the multi-file View Diff editor too (to be validated at the gate). diff --git a/codev/state/pir-789_thread.md b/codev/state/pir-789_thread.md index 2e9775eb1..234110e82 100644 --- a/codev/state/pir-789_thread.md +++ b/codev/state/pir-789_thread.md @@ -57,6 +57,11 @@ Validation: 368 unit tests, check-types, lint, bundle all green. **Needs human r Deferred to future issue(s): hover-triggered forward; deeper nesting; per-kind density settings. +## Follow-ups during dev-approval +- Renamed lens label → "Forward to Builder"; command ids `codev.forwardToBuilder` / `codev.forwardSelectionToBuilder`. +- Added `Cmd/Ctrl+K B` keybinding for Forward Selection (when matched to the menu so the hint renders); menu title prefixed "Codev:". +- **Option B**: restored per-hunk lenses to coexist with symbol lenses (`buildAllLensDescriptors`, dedup by anchor line so hunk lenses don't stack on symbol/file lenses). Hunk parsing (`parseHunkRanges`/`parseUnifiedDiff`) re-added and fed into the registry entry. 373 tests green. + ## Dev-approval iteration: hunk-lens placement fix Human observed hunk lenses landing on the wrong function's signature. Root cause: `--unified=3` reports each hunk's new-side start up to 3 *context* lines above the first real change, so near a function boundary the anchor lands on the preceding `def`/`return`. Fixed: `parseHunkRanges` now walks the hunk body to find the first/last actually-added (`+`) new-side lines (`changeStart`/`changeEnd`); `buildLensDescriptors` anchors AND labels on those instead of the header span. Deviation from issue's literal "hunk header range" — intentional, more accurate. 374 tests green. Needs human re-test after F5 relaunch. From 29f040838d433ec264872fc15170324038c2a587 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 10 Jun 2026 17:20:23 +1000 Subject: [PATCH 26/39] [PIR #789] Open diffs before computing hunks (fix open-delay regression) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Option B started awaiting a git 'diff --unified' subprocess before opening the per-file diff (and the View Diff editor), delaying when the diff appeared. Open the editor first, then register the lens entry synchronously (symbol/file lenses render immediately) and fill the per-hunk ranges afterward — the provider's change event refreshes the hunk lenses in once git resolves. --- packages/vscode/src/commands/view-diff.ts | 68 +++++++++++++---------- packages/vscode/src/extension.ts | 17 +++--- 2 files changed, 48 insertions(+), 37 deletions(-) diff --git a/packages/vscode/src/commands/view-diff.ts b/packages/vscode/src/commands/view-diff.ts index 318d64a86..d173aabe8 100644 --- a/packages/vscode/src/commands/view-diff.ts +++ b/packages/vscode/src/commands/view-diff.ts @@ -362,36 +362,45 @@ export async function viewDiff( }); // CodeLens "Forward to Builder" actions (#789): register the right-side fs - // paths + owning builder, plus per-hunk ranges so changed regions get their - // own lens alongside the symbol/file lenses (symbols resolve lazily in the - // provider). One extra git call here; a patch failure is non-fatal — the - // symbol/file lenses still render. - let hunksByPath = new Map>(); - try { - const { stdout: patch } = await execFileAsync( - 'git', - ['-C', wt, 'diff', '-M', '--unified=3', baseRef], - { maxBuffer: 64 * 1024 * 1024 }, - ); - hunksByPath = parseUnifiedDiff(patch); - } catch { - // keep empty — symbol/file lenses still render - } - const entries: DiffInjectSessionEntry[] = plans - .filter(plan => plan.right.kind === 'file') - .map(plan => ({ + // paths + owning builder now (no hunks yet) so symbol/file lenses and the + // selection menu work as soon as a file is opened — without blocking the + // editor on a git call. + const filePlans = plans.filter(plan => plan.right.kind === 'file'); + setDiffInjectSession( + filePlans.map(plan => ({ fsPath: path.join(wt, plan.resourcePath), builderId: builder.id, relPath: plan.resourcePath, - hunks: hunksByPath.get(plan.resourcePath) ?? [], - })); - setDiffInjectSession(entries); + hunks: [], + })), + ); await vscode.commands.executeCommand( 'vscode.changes', `Reviewing #${builder.issueId ?? builder.id} (${defaultBranch} ↔ HEAD)`, resources, ); + + // Fill per-hunk ranges after the editor is open; the provider's change event + // refreshes the lenses. Non-fatal on git failure — symbol/file lenses remain. + try { + const { stdout: patch } = await execFileAsync( + 'git', + ['-C', wt, 'diff', '-M', '--unified=3', baseRef], + { maxBuffer: 64 * 1024 * 1024 }, + ); + const hunksByPath = parseUnifiedDiff(patch); + setDiffInjectSession( + filePlans.map(plan => ({ + fsPath: path.join(wt, plan.resourcePath), + builderId: builder.id, + relPath: plan.resourcePath, + hunks: hunksByPath.get(plan.resourcePath) ?? [], + })), + ); + } catch { + // keep the hunk-less entries already registered + } } /** @@ -411,23 +420,22 @@ export async function registerFileInjectSession(args: { // Deleted/binary files have no right-side `file:` document to host a lens. if (args.plan.right.kind !== 'file') { return; } const relPath = args.plan.resourcePath; - let hunks: ReturnType = []; + const fsPath = path.join(args.worktreePath, relPath); + // Register immediately with no hunks so the symbol/file lenses render right + // away — the git hunk computation below must not gate them. + upsertDiffInjectEntry({ fsPath, builderId: args.builderId, relPath, hunks: [] }); try { const { stdout } = await execFileAsync( 'git', ['-C', args.worktreePath, 'diff', '-M', '--unified=3', args.baseRef, '--', relPath], { maxBuffer: 64 * 1024 * 1024 }, ); - hunks = parseHunkRanges(stdout); + // Re-upsert with hunks; the provider's change event refreshes the lenses, + // adding the per-hunk ones. + upsertDiffInjectEntry({ fsPath, builderId: args.builderId, relPath, hunks: parseHunkRanges(stdout) }); } catch { - // keep empty — symbol/file lenses still render + // keep the symbol/file lenses already registered } - upsertDiffInjectEntry({ - fsPath: path.join(args.worktreePath, relPath), - builderId: args.builderId, - relPath, - hunks, - }); } interface BuilderLike { diff --git a/packages/vscode/src/extension.ts b/packages/vscode/src/extension.ts index 2682905f8..8bcf62fd7 100644 --- a/packages/vscode/src/extension.ts +++ b/packages/vscode/src/extension.ts @@ -847,20 +847,23 @@ export async function activate(context: vscode.ExtensionContext) { }), reg('codev.openBuilderFileDiff', async (arg: unknown) => { if (!(arg instanceof BuilderFileTreeItem)) { return; } - // Register the inject-codelens entry for this file so the "Forward - // to Builder" lenses (#789) render on the diff without a prior View - // Diff run, then offer to enable diffEditor.codeLens (off by default - // — VS Code hides CodeLens in diff editors). + // Open the diff FIRST so it appears instantly. Lens registration and + // the git hunk computation happen after — the entry registers + // synchronously (symbol/file lenses render right away) and the hunk + // lenses refresh in once git resolves (#789). Doing this before the + // open used to block the diff on a git subprocess. + const { left, right } = diffUrisForChange(arg.plan, { wt: arg.worktreePath, ref: arg.baseRef }); + const title = `${arg.plan.resourcePath} (#${arg.builderId})`; + await vscode.commands.executeCommand('vscode.diff', left, right, title); await registerFileInjectSession({ worktreePath: arg.worktreePath, baseRef: arg.baseRef, builderId: arg.builderId, plan: arg.plan, }); + // Offer to enable diffEditor.codeLens (off by default — VS Code hides + // CodeLens in diff editors). After the open, so it never delays it. await ensureDiffEditorCodeLens(context); - const { left, right } = diffUrisForChange(arg.plan, { wt: arg.worktreePath, ref: arg.baseRef }); - const title = `${arg.plan.resourcePath} (#${arg.builderId})`; - await vscode.commands.executeCommand('vscode.diff', left, right, title); }), regCli('codev.runWorktreeDev', (arg: vscode.TreeItem | string | undefined) => runWorktreeDev(connectionManager!, terminalManager!, extractBuilderId(arg))), From a9bbb47883cc50e1207acc46684633082abab9d8 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 10 Jun 2026 17:28:21 +1000 Subject: [PATCH 27/39] [PIR #789] One change-lens per contiguous changed run, not per git hunk git coalesces nearby edits into a single @@ hunk, so 3 separate 'return undefined' edits shared one lens. Split each hunk into runs of consecutive added lines (broken by context, not by deletions) and lens each run, so every visible change block gets its own lens. Single-line runs read '(line N)' / inject ':L'. --- .../src/__tests__/diff-inject-ref.test.ts | 70 ++++++---- packages/vscode/src/diff-inject-codelens.ts | 6 +- packages/vscode/src/diff-inject-ref.ts | 121 +++++++++--------- 3 files changed, 108 insertions(+), 89 deletions(-) diff --git a/packages/vscode/src/__tests__/diff-inject-ref.test.ts b/packages/vscode/src/__tests__/diff-inject-ref.test.ts index f99ef5d8f..ded97b8b4 100644 --- a/packages/vscode/src/__tests__/diff-inject-ref.test.ts +++ b/packages/vscode/src/__tests__/diff-inject-ref.test.ts @@ -109,58 +109,72 @@ describe('buildSymbolLensDescriptors', () => { }); }); -describe('parseHunkRanges', () => { - it('reads the header span and the first/last added new-side lines', () => { +describe('parseHunkRanges (one range per contiguous changed run)', () => { + it('splits a hunk into separate runs broken by context lines', () => { + // Three `return;` → `return undefined;` edits separated by context, in one + // git hunk — each is its own run, so each gets its own range. const patch = [ - '@@ -1,4 +1,6 @@', - ' a', // new line 1 (context) - '+b', // new line 2 (added) - ' c', - '@@ -20,3 +22,10 @@ func()', - ' x', // 22 - ' y', // 23 - '+z1', // 24 (first change) - '+z2', // 25 (last change) - ' w', + '@@ -10,12 +10,12 @@', + ' a', // new 10 (ctx) + '-return;', // old only + '+return undefined;', // new 11 ← run 1 + ' b', // new 12 (ctx) — breaks run + ' c', // new 13 + '-return;', + '+return undefined;', // new 14 ← run 2 + ' d', // new 15 (ctx) — breaks run + '-return;', + '+return undefined;', // new 16 ← run 3 ].join('\n'); expect(parseHunkRanges(patch)).toEqual([ - { newStart: 1, newEnd: 6, changeStart: 2, changeEnd: 2 }, - { newStart: 22, newEnd: 31, changeStart: 24, changeEnd: 25 }, + { start: 11, end: 11 }, + { start: 14, end: 14 }, + { start: 16, end: 16 }, ]); }); - it('does not let deleted (-) lines advance the new-side counter', () => { - const patch = ['@@ -10,4 +10,3 @@', ' ctx', '-gone', '+added', ' tail'].join('\n'); + it('groups consecutive added lines into one run; deletions do not break it', () => { + const patch = ['@@ -1,3 +1,4 @@', ' a', '+b', '-x', '+c', ' d'].join('\n'); + // new 1=a(ctx), 2=b(+), 3=c(+) [x is old-only], 4=d(ctx) → one run 2-3 + expect(parseHunkRanges(patch)).toEqual([{ start: 2, end: 3 }]); + }); + + it('yields no range for a pure-deletion hunk', () => { + expect(parseHunkRanges('@@ -5,3 +4,0 @@\n-gone1\n-gone2')).toEqual([]); + }); + + it('separates runs across multiple hunks', () => { + const patch = ['@@ -1,1 +1,2 @@', '+a', '@@ -20,1 +22,2 @@', '+z'].join('\n'); expect(parseHunkRanges(patch)).toEqual([ - { newStart: 10, newEnd: 12, changeStart: 11, changeEnd: 11 }, + { start: 1, end: 1 }, + { start: 22, end: 22 }, ]); }); }); -describe('buildAllLensDescriptors (symbol + hunk lenses)', () => { - it('adds per-hunk lenses below the symbol/file lenses', () => { +describe('buildAllLensDescriptors (symbol + change lenses)', () => { + it('adds one lens per changed run below the symbol/file lenses', () => { const symbols = [sym(K.Function, 4, 30)]; // function lens at line 4 - const hunks = [{ newStart: 8, newEnd: 12, changeStart: 10, changeEnd: 12 }]; - expect(buildAllLensDescriptors('a/b.ts', symbols, hunks)).toEqual([ + const ranges = [{ start: 10, end: 12 }, { start: 18, end: 18 }]; + expect(buildAllLensDescriptors('a/b.ts', symbols, ranges)).toEqual([ { line: 0, title: 'Forward to Builder', refText: 'a/b.ts ' }, { line: 4, title: 'Forward to Builder', refText: 'a/b.ts:L5-L31 ' }, { line: 9, title: 'Forward to Builder (lines 10-12)', refText: 'a/b.ts:L10-L12 ' }, + { line: 17, title: 'Forward to Builder (line 18)', refText: 'a/b.ts:L18 ' }, ]); }); - it('skips a hunk lens that collides with a symbol lens line', () => { + it('skips a change lens that collides with a symbol lens line', () => { const symbols = [sym(K.Function, 9, 30)]; // function lens at line 9 - // hunk changeStart 10 → anchor line 9, same as the function lens → skipped - const hunks = [{ newStart: 10, newEnd: 12, changeStart: 10, changeEnd: 12 }]; - expect(buildAllLensDescriptors('a/b.ts', symbols, hunks)).toEqual([ + const ranges = [{ start: 10, end: 12 }]; // anchor line 9 → collides → skipped + expect(buildAllLensDescriptors('a/b.ts', symbols, ranges)).toEqual([ { line: 0, title: 'Forward to Builder', refText: 'a/b.ts ' }, { line: 9, title: 'Forward to Builder', refText: 'a/b.ts:L10-L31 ' }, ]); }); - it('skips a whole-file hunk (changeStart 1) that collides with the file-level lens', () => { - const hunks = [{ newStart: 1, newEnd: 17, changeStart: 1, changeEnd: 17 }]; - expect(buildAllLensDescriptors('a/b.ts', [], hunks)).toEqual([ + it('skips a change run at line 1 that collides with the file-level lens', () => { + expect(buildAllLensDescriptors('a/b.ts', [], [{ start: 1, end: 17 }])).toEqual([ { line: 0, title: 'Forward to Builder', refText: 'a/b.ts ' }, ]); }); diff --git a/packages/vscode/src/diff-inject-codelens.ts b/packages/vscode/src/diff-inject-codelens.ts index 3c63f112a..685d58c84 100644 --- a/packages/vscode/src/diff-inject-codelens.ts +++ b/packages/vscode/src/diff-inject-codelens.ts @@ -27,7 +27,7 @@ */ import * as vscode from 'vscode'; -import { buildAllLensDescriptors, type HunkRange, type SymbolNode } from './diff-inject-ref.js'; +import { buildAllLensDescriptors, type ChangedRange, type SymbolNode } from './diff-inject-ref.js'; /** Command id the lenses invoke. Registered in `extension.ts`, NOT declared in * `contributes.commands`, so it never appears in the Command Palette. */ @@ -45,8 +45,8 @@ export interface DiffInjectSessionEntry { builderId: string; /** Repo-relative path injected into the prompt. */ relPath: string; - /** New-side hunk ranges for the per-hunk lenses (empty = file/symbol lenses only). */ - hunks: HunkRange[]; + /** Changed new-side line runs for the per-change lenses (empty = file/symbol lenses only). */ + hunks: ChangedRange[]; } /** Map a `vscode.DocumentSymbol` tree to the pure `SymbolNode` shape. */ diff --git a/packages/vscode/src/diff-inject-ref.ts b/packages/vscode/src/diff-inject-ref.ts index 6fece8dbc..bc83fc8f8 100644 --- a/packages/vscode/src/diff-inject-ref.ts +++ b/packages/vscode/src/diff-inject-ref.ts @@ -66,73 +66,72 @@ const TOP_LEVEL_KINDS = new Set([ ]); /** - * New-side line numbers of one diff hunk (1-based, inclusive). `newStart`/ - * `newEnd` are the header span (context included); `changeStart`/`changeEnd` - * are the first/last lines *actually added* on the new side, so a lens anchors - * on the real edit rather than the up-to-3 leading context lines git emits with - * `--unified=3`. A pure-deletion hunk collapses both change fields to `newStart`. + * A contiguous run of added new-side lines (1-based, inclusive) — a single + * visible change block. git groups nearby edits into one `@@` hunk, so each + * hunk is split into one `ChangedRange` per run of `+` lines (broken by + * unchanged context lines), and each run gets its own lens. `-` lines don't + * break a run (they don't occupy a new-side line); a pure-deletion hunk yields + * no range (nothing to point a new-side lens at). */ -export interface HunkRange { - newStart: number; - newEnd: number; - changeStart: number; - changeEnd: number; +export interface ChangedRange { + start: number; + end: number; } const HUNK_HEADER = /^@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@/; /** - * Parse the `@@ -a,b +c,d @@` hunk headers from a single file's unified diff - * into new-side ranges, walking each hunk body to find the first/last added - * (`+`) lines. + * Parse a unified diff into one `ChangedRange` per contiguous run of added + * (`+`) new-side lines, tracking the new-side line number across each hunk. + * Context lines break a run; deletions don't (they have no new-side line). */ -export function parseHunkRanges(patch: string): HunkRange[] { - const lines = patch.split('\n'); - const ranges: HunkRange[] = []; - for (let i = 0; i < lines.length; i++) { - const m = HUNK_HEADER.exec(lines[i]!); - if (!m) { continue; } - const newStart = Number(m[1]); - const len = m[2] === undefined ? 1 : Number(m[2]); - const newEnd = len <= 0 ? Math.max(newStart, 1) : newStart + len - 1; - - let cur = newStart; - let firstChange = -1; - let lastChange = -1; - for (let j = i + 1; j < lines.length; j++) { - const l = lines[j]!; - if (HUNK_HEADER.test(l) || l.startsWith('diff --git')) { break; } - if (l.startsWith('\\')) { continue; } - if (l.startsWith('+')) { - if (firstChange === -1) { firstChange = cur; } - lastChange = cur; - cur++; - } else if (l.startsWith('-')) { - // old-side only — does not advance the new-side counter - } else { - cur++; // context line - } +export function parseHunkRanges(patch: string): ChangedRange[] { + const ranges: ChangedRange[] = []; + let inHunk = false; + let cur = 0; // next new-side line number + let runStart = -1; + let runEnd = -1; + const flush = (): void => { + if (runStart !== -1) { + ranges.push({ start: runStart, end: runEnd }); + runStart = -1; + runEnd = -1; } + }; - const changeStart = firstChange === -1 ? Math.max(newStart, 1) : firstChange; - const changeEnd = lastChange === -1 ? changeStart : lastChange; - ranges.push({ - newStart: len <= 0 ? Math.max(newStart, 1) : newStart, - newEnd, - changeStart, - changeEnd, - }); + for (const l of patch.split('\n')) { + const m = HUNK_HEADER.exec(l); + if (m) { + flush(); + cur = Number(m[1]); + inHunk = true; + continue; + } + if (!inHunk) { continue; } + if (l.startsWith('diff --git')) { flush(); inHunk = false; continue; } + if (l.startsWith('\\')) { continue; } // "\ No newline at end of file" + if (l.startsWith('+')) { + if (runStart === -1) { runStart = cur; } + runEnd = cur; + cur++; + } else if (l.startsWith('-')) { + // old-side only — does not advance the new-side counter, does not break the run + } else { + flush(); // context line ends the current run + cur++; + } } + flush(); return ranges; } /** * Split a multi-file unified diff (`git diff -M --unified=N `) into a map - * from each file's **new** path to its hunk ranges. The new path is read from - * the `+++ b/` line; deleted files (`+++ /dev/null`) are omitted. + * from each file's **new** path to its changed ranges. The new path is read + * from the `+++ b/` line; deleted files (`+++ /dev/null`) are omitted. */ -export function parseUnifiedDiff(patch: string): Map { - const out = new Map(); +export function parseUnifiedDiff(patch: string): Map { + const out = new Map(); const sections = patch.split(/^diff --git .*$/m).slice(1); for (const section of sections) { const newPath = newPathFromSection(section); @@ -158,9 +157,15 @@ export function buildBuilderFileRef(relPath: string): string { return `${relPath} `; } -/** A line range: `:L-L ` (trailing space, no Enter). */ +/** A line range: `:L-L `, or `:L ` for a single line + * (trailing space, no Enter). */ export function buildBuilderRangeRef(relPath: string, start: number, end: number): string { - return `${relPath}:L${start}-L${end} `; + return start === end ? `${relPath}:L${start} ` : `${relPath}:L${start}-L${end} `; +} + +/** Label suffix for a line range: `(line N)` for one line, `(lines N-M)` otherwise. */ +function rangeLabel(start: number, end: number): string { + return start === end ? `(line ${start})` : `(lines ${start}-${end})`; } /** @@ -222,18 +227,18 @@ export function buildSymbolLensDescriptors(relPath: string, symbols: SymbolNode[ export function buildAllLensDescriptors( relPath: string, symbols: SymbolNode[], - hunks: HunkRange[], + ranges: ChangedRange[], ): LensDescriptor[] { const lenses = buildSymbolLensDescriptors(relPath, symbols); const usedLines = new Set(lenses.map(l => l.line)); - for (const h of hunks) { - const line = Math.max(h.changeStart - 1, 0); + for (const r of ranges) { + const line = Math.max(r.start - 1, 0); if (usedLines.has(line)) { continue; } // file-level (line 0) or a symbol lens already here usedLines.add(line); lenses.push({ line, - title: `Forward to Builder (lines ${h.changeStart}-${h.changeEnd})`, - refText: buildBuilderRangeRef(relPath, h.changeStart, h.changeEnd), + title: `Forward to Builder ${rangeLabel(r.start, r.end)}`, + refText: buildBuilderRangeRef(relPath, r.start, r.end), }); } return lenses; From 60dcb6c2eaaac951355d6db3ed32bfc31c6f238f Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 10 Jun 2026 17:33:50 +1000 Subject: [PATCH 28/39] [PIR #789] Show line range in symbol lens labels too Symbol lenses were bare 'Forward to Builder' while change lenses showed '(lines N-M)'. On a new/added file there are no change lenses (whole-file hunk dedupes against the file lens; untracked files have no git diff), so no lens showed line numbers at all. Give symbol lenses the same range label. --- .../src/__tests__/diff-inject-ref.test.ts | 20 +++++++++---------- packages/vscode/src/diff-inject-ref.ts | 6 ++++-- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/packages/vscode/src/__tests__/diff-inject-ref.test.ts b/packages/vscode/src/__tests__/diff-inject-ref.test.ts index ded97b8b4..00a7ca967 100644 --- a/packages/vscode/src/__tests__/diff-inject-ref.test.ts +++ b/packages/vscode/src/__tests__/diff-inject-ref.test.ts @@ -57,9 +57,9 @@ describe('buildSymbolLensDescriptors', () => { ]; expect(buildSymbolLensDescriptors('a/b.ts', symbols)).toEqual([ { line: 0, title: 'Forward to Builder', refText: 'a/b.ts ' }, - { line: 4, title: 'Forward to Builder', refText: 'a/b.ts:L5-L10 ' }, - { line: 12, title: 'Forward to Builder', refText: 'a/b.ts:L13-L19 ' }, - { line: 20, title: 'Forward to Builder', refText: 'a/b.ts:L21-L25 ' }, + { line: 4, title: 'Forward to Builder (lines 5-10)', refText: 'a/b.ts:L5-L10 ' }, + { line: 12, title: 'Forward to Builder (lines 13-19)', refText: 'a/b.ts:L13-L19 ' }, + { line: 20, title: 'Forward to Builder (lines 21-25)', refText: 'a/b.ts:L21-L25 ' }, ]); }); @@ -71,9 +71,9 @@ describe('buildSymbolLensDescriptors', () => { ]); expect(buildSymbolLensDescriptors('a/b.ts', [cls])).toEqual([ { line: 0, title: 'Forward to Builder', refText: 'a/b.ts ' }, - { line: 3, title: 'Forward to Builder', refText: 'a/b.ts:L4-L41 ' }, // class - { line: 5, title: 'Forward to Builder', refText: 'a/b.ts:L6-L9 ' }, // constructor - { line: 10, title: 'Forward to Builder', refText: 'a/b.ts:L11-L21 ' }, // method + { line: 3, title: 'Forward to Builder (lines 4-41)', refText: 'a/b.ts:L4-L41 ' }, // class + { line: 5, title: 'Forward to Builder (lines 6-9)', refText: 'a/b.ts:L6-L9 ' }, // constructor + { line: 10, title: 'Forward to Builder (lines 11-21)', refText: 'a/b.ts:L11-L21 ' }, // method ]); }); @@ -84,7 +84,7 @@ describe('buildSymbolLensDescriptors', () => { ]; expect(buildSymbolLensDescriptors('a/b.ts', symbols)).toEqual([ { line: 0, title: 'Forward to Builder', refText: 'a/b.ts ' }, - { line: 4, title: 'Forward to Builder', refText: 'a/b.ts:L5-L13 ' }, + { line: 4, title: 'Forward to Builder (lines 5-13)', refText: 'a/b.ts:L5-L13 ' }, ]); }); @@ -104,7 +104,7 @@ describe('buildSymbolLensDescriptors', () => { ]); expect(buildSymbolLensDescriptors('a/b.ts', [cls])).toEqual([ { line: 0, title: 'Forward to Builder', refText: 'a/b.ts ' }, - { line: 2, title: 'Forward to Builder', refText: 'a/b.ts:L3-L51 ' }, + { line: 2, title: 'Forward to Builder (lines 3-51)', refText: 'a/b.ts:L3-L51 ' }, ]); }); }); @@ -158,7 +158,7 @@ describe('buildAllLensDescriptors (symbol + change lenses)', () => { const ranges = [{ start: 10, end: 12 }, { start: 18, end: 18 }]; expect(buildAllLensDescriptors('a/b.ts', symbols, ranges)).toEqual([ { line: 0, title: 'Forward to Builder', refText: 'a/b.ts ' }, - { line: 4, title: 'Forward to Builder', refText: 'a/b.ts:L5-L31 ' }, + { line: 4, title: 'Forward to Builder (lines 5-31)', refText: 'a/b.ts:L5-L31 ' }, { line: 9, title: 'Forward to Builder (lines 10-12)', refText: 'a/b.ts:L10-L12 ' }, { line: 17, title: 'Forward to Builder (line 18)', refText: 'a/b.ts:L18 ' }, ]); @@ -169,7 +169,7 @@ describe('buildAllLensDescriptors (symbol + change lenses)', () => { const ranges = [{ start: 10, end: 12 }]; // anchor line 9 → collides → skipped expect(buildAllLensDescriptors('a/b.ts', symbols, ranges)).toEqual([ { line: 0, title: 'Forward to Builder', refText: 'a/b.ts ' }, - { line: 9, title: 'Forward to Builder', refText: 'a/b.ts:L10-L31 ' }, + { line: 9, title: 'Forward to Builder (lines 10-31)', refText: 'a/b.ts:L10-L31 ' }, ]); }); diff --git a/packages/vscode/src/diff-inject-ref.ts b/packages/vscode/src/diff-inject-ref.ts index bc83fc8f8..fa6dd8adf 100644 --- a/packages/vscode/src/diff-inject-ref.ts +++ b/packages/vscode/src/diff-inject-ref.ts @@ -189,10 +189,12 @@ export function buildSymbolLensDescriptors(relPath: string, symbols: SymbolNode[ const addLens = (s: SymbolNode): void => { const line = Math.max(s.startLine, 0); if (line === 0) { return; } // collides with the file-level lens + const start = s.startLine + 1; + const end = s.endLine + 1; lenses.push({ line, - title: 'Forward to Builder', - refText: buildBuilderRangeRef(relPath, s.startLine + 1, s.endLine + 1), + title: `Forward to Builder ${rangeLabel(start, end)}`, + refText: buildBuilderRangeRef(relPath, start, end), }); }; From aa07ba9a2453b7a15fda48cd0857cda4b6331e21 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 10 Jun 2026 17:46:32 +1000 Subject: [PATCH 29/39] chore(porch): 789 dev-approval gate-approved --- .../projects/789-vscode-inject-file-hunk-refere/status.yaml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/codev/projects/789-vscode-inject-file-hunk-refere/status.yaml b/codev/projects/789-vscode-inject-file-hunk-refere/status.yaml index 7e8b33521..bb5fe1543 100644 --- a/codev/projects/789-vscode-inject-file-hunk-refere/status.yaml +++ b/codev/projects/789-vscode-inject-file-hunk-refere/status.yaml @@ -10,12 +10,13 @@ gates: requested_at: '2026-06-09T22:26:05.749Z' approved_at: '2026-06-09T22:58:51.188Z' dev-approval: - status: pending + status: approved requested_at: '2026-06-09T23:04:34.034Z' + approved_at: '2026-06-10T07:46:32.265Z' pr: status: pending iteration: 1 build_complete: false history: [] started_at: '2026-06-09T22:20:14.795Z' -updated_at: '2026-06-09T23:04:34.035Z' +updated_at: '2026-06-10T07:46:32.266Z' From 2659486317f42bb8918aa3cba9ce93aca18ef447 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 10 Jun 2026 17:46:41 +1000 Subject: [PATCH 30/39] chore(porch): 789 review phase-transition --- codev/projects/789-vscode-inject-file-hunk-refere/status.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codev/projects/789-vscode-inject-file-hunk-refere/status.yaml b/codev/projects/789-vscode-inject-file-hunk-refere/status.yaml index bb5fe1543..e92a6734e 100644 --- a/codev/projects/789-vscode-inject-file-hunk-refere/status.yaml +++ b/codev/projects/789-vscode-inject-file-hunk-refere/status.yaml @@ -1,7 +1,7 @@ id: '789' title: vscode-inject-file-hunk-refere protocol: pir -phase: implement +phase: review plan_phases: [] current_plan_phase: null gates: @@ -19,4 +19,4 @@ iteration: 1 build_complete: false history: [] started_at: '2026-06-09T22:20:14.795Z' -updated_at: '2026-06-10T07:46:32.266Z' +updated_at: '2026-06-10T07:46:41.280Z' From 1acaabea6f857edabba5d9d294fdd947f9f44ede Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 10 Jun 2026 17:51:36 +1000 Subject: [PATCH 31/39] [PIR #789] Cache document symbols per version (fix ~100% CPU spin) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit provideCodeLenses ran executeDocumentSymbolProvider on every refresh, and the diff editor re-queries CodeLens constantly — so it hammered the language server and pegged the CPU. Resolve symbols at most once per (uri, version), caching the in-flight promise; non-edit refreshes hit the cache and re-resolve only happens after an actual edit. Also breaks any refresh loop (a cache hit returns without re-querying). --- packages/vscode/src/diff-inject-codelens.ts | 44 +++++++++++++++------ 1 file changed, 33 insertions(+), 11 deletions(-) diff --git a/packages/vscode/src/diff-inject-codelens.ts b/packages/vscode/src/diff-inject-codelens.ts index 685d58c84..9369243e9 100644 --- a/packages/vscode/src/diff-inject-codelens.ts +++ b/packages/vscode/src/diff-inject-codelens.ts @@ -64,6 +64,13 @@ class DiffInjectCodeLensProvider implements vscode.CodeLensProvider { private readonly _onDidChangeCodeLenses = new vscode.EventEmitter(); readonly onDidChangeCodeLenses = this._onDidChangeCodeLenses.event; + // Symbol resolution is the expensive part (a full language-server query). The + // diff editor re-queries CodeLens often, so cache the result per document + // *version* — keyed by uri, holding the in-flight promise so concurrent + // refreshes share one resolution. Without this, `provideCodeLenses` re-ran + // `executeDocumentSymbolProvider` on every refresh and pegged the CPU. + private readonly symbolCache = new Map }>(); + setSession(entries: DiffInjectSessionEntry[]): void { this.registry = new Map(entries.map(e => [e.fsPath, e])); this._onDidChangeCodeLenses.fire(); @@ -88,19 +95,9 @@ class DiffInjectCodeLensProvider implements vscode.CodeLensProvider { const entry = this.registry.get(document.uri.fsPath); if (!entry) { return []; } - let symbols: vscode.DocumentSymbol[] = []; - try { - symbols = - (await vscode.commands.executeCommand( - 'vscode.executeDocumentSymbolProvider', - document.uri, - )) ?? []; - } catch { - symbols = []; - } + const nodes = await this.symbolsFor(document); if (token.isCancellationRequested) { return []; } - const nodes = symbols.map(toSymbolNode); const lastLine = Math.max(document.lineCount - 1, 0); return buildAllLensDescriptors(entry.relPath, nodes, entry.hunks).map(d => { const line = Math.min(Math.max(d.line, 0), lastLine); @@ -113,6 +110,31 @@ class DiffInjectCodeLensProvider implements vscode.CodeLensProvider { }); } + /** + * Document symbols for a file, resolved at most once per (uri, version). + * Caching the in-flight promise both avoids redundant language-server work on + * every CodeLens refresh and breaks any refresh loop (a cache hit returns + * without re-querying the symbol provider, so it can't keep re-triggering). + */ + private symbolsFor(document: vscode.TextDocument): Promise { + const key = document.uri.toString(); + const cached = this.symbolCache.get(key); + if (cached && cached.version === document.version) { return cached.nodes; } + const nodes = (async (): Promise => { + try { + const syms = await vscode.commands.executeCommand( + 'vscode.executeDocumentSymbolProvider', + document.uri, + ); + return (syms ?? []).map(toSymbolNode); + } catch { + return []; + } + })(); + this.symbolCache.set(key, { version: document.version, nodes }); + return nodes; + } + dispose(): void { this._onDidChangeCodeLenses.dispose(); } From a93d6ac52b724e73cc9b0a4e8453ad375ae778e7 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 10 Jun 2026 18:13:39 +1000 Subject: [PATCH 32/39] [PIR #789] Remove symbol cache (misdiagnosed; risked pinning empty symbols) The cache was added believing per-refresh symbol resolution caused the CPU spike, but that was an unrelated extension (the Extension Test Runner) walking the worktree farm. provideCodeLenses is not a hot path (VSCode caches lenses; we invalidate only a couple times per file), so the cache bought nothing and risked pinning an empty result if the language server was warming up on first open. Resolve symbols directly per call. --- packages/vscode/src/diff-inject-codelens.ts | 44 ++++++--------------- 1 file changed, 11 insertions(+), 33 deletions(-) diff --git a/packages/vscode/src/diff-inject-codelens.ts b/packages/vscode/src/diff-inject-codelens.ts index 9369243e9..685d58c84 100644 --- a/packages/vscode/src/diff-inject-codelens.ts +++ b/packages/vscode/src/diff-inject-codelens.ts @@ -64,13 +64,6 @@ class DiffInjectCodeLensProvider implements vscode.CodeLensProvider { private readonly _onDidChangeCodeLenses = new vscode.EventEmitter(); readonly onDidChangeCodeLenses = this._onDidChangeCodeLenses.event; - // Symbol resolution is the expensive part (a full language-server query). The - // diff editor re-queries CodeLens often, so cache the result per document - // *version* — keyed by uri, holding the in-flight promise so concurrent - // refreshes share one resolution. Without this, `provideCodeLenses` re-ran - // `executeDocumentSymbolProvider` on every refresh and pegged the CPU. - private readonly symbolCache = new Map }>(); - setSession(entries: DiffInjectSessionEntry[]): void { this.registry = new Map(entries.map(e => [e.fsPath, e])); this._onDidChangeCodeLenses.fire(); @@ -95,9 +88,19 @@ class DiffInjectCodeLensProvider implements vscode.CodeLensProvider { const entry = this.registry.get(document.uri.fsPath); if (!entry) { return []; } - const nodes = await this.symbolsFor(document); + let symbols: vscode.DocumentSymbol[] = []; + try { + symbols = + (await vscode.commands.executeCommand( + 'vscode.executeDocumentSymbolProvider', + document.uri, + )) ?? []; + } catch { + symbols = []; + } if (token.isCancellationRequested) { return []; } + const nodes = symbols.map(toSymbolNode); const lastLine = Math.max(document.lineCount - 1, 0); return buildAllLensDescriptors(entry.relPath, nodes, entry.hunks).map(d => { const line = Math.min(Math.max(d.line, 0), lastLine); @@ -110,31 +113,6 @@ class DiffInjectCodeLensProvider implements vscode.CodeLensProvider { }); } - /** - * Document symbols for a file, resolved at most once per (uri, version). - * Caching the in-flight promise both avoids redundant language-server work on - * every CodeLens refresh and breaks any refresh loop (a cache hit returns - * without re-querying the symbol provider, so it can't keep re-triggering). - */ - private symbolsFor(document: vscode.TextDocument): Promise { - const key = document.uri.toString(); - const cached = this.symbolCache.get(key); - if (cached && cached.version === document.version) { return cached.nodes; } - const nodes = (async (): Promise => { - try { - const syms = await vscode.commands.executeCommand( - 'vscode.executeDocumentSymbolProvider', - document.uri, - ); - return (syms ?? []).map(toSymbolNode); - } catch { - return []; - } - })(); - this.symbolCache.set(key, { version: document.version, nodes }); - return nodes; - } - dispose(): void { this._onDidChangeCodeLenses.dispose(); } From aa96ba8f67c2583d81aa775d33804f3ab81e0ce3 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 10 Jun 2026 18:20:14 +1000 Subject: [PATCH 33/39] [PIR #789] Enable diffEditor.codeLens at Global scope (personal editor preference) It's a personal editor-behavior setting, not project config, so write it at the user level. Writing Workspace would edit the repo's shared committed .vscode/settings.json and force the choice on all collaborators. --- packages/vscode/src/ensure-diff-codelens.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/vscode/src/ensure-diff-codelens.ts b/packages/vscode/src/ensure-diff-codelens.ts index 66444f088..5d54bf35e 100644 --- a/packages/vscode/src/ensure-diff-codelens.ts +++ b/packages/vscode/src/ensure-diff-codelens.ts @@ -27,6 +27,10 @@ export async function ensureDiffEditorCodeLens( DISMISS, ); if (choice === ENABLE) { + // `diffEditor.codeLens` is a personal editor-behavior preference, so write + // it at the user (Global) level. Deliberately NOT Workspace — that would + // edit the repo's shared, committed `.vscode/settings.json` and force the + // choice on every collaborator. await cfg.update('codeLens', true, vscode.ConfigurationTarget.Global); } else if (choice === DISMISS) { await context.globalState.update(DISMISS_KEY, true); From 82d0219704ba6a37f50a15c852956f12c0b5cbfa Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 10 Jun 2026 18:24:56 +1000 Subject: [PATCH 34/39] [PIR #789] Review + retrospective; lessons-learned (diff CodeLens constraints) --- codev/resources/lessons-learned.md | 3 + .../789-vscode-inject-file-hunk-refere.md | 72 +++++++++++++++++++ codev/state/pir-789_thread.md | 6 ++ 3 files changed, 81 insertions(+) create mode 100644 codev/reviews/789-vscode-inject-file-hunk-refere.md diff --git a/codev/resources/lessons-learned.md b/codev/resources/lessons-learned.md index fd8975066..8145cea56 100644 --- a/codev/resources/lessons-learned.md +++ b/codev/resources/lessons-learned.md @@ -316,6 +316,9 @@ Generalizable wisdom extracted from review documents, ordered by impact. Updated - [From #983] Gate a remedy prompt on the condition the remedy actually fixes, not on any divergence. The Tower "Restart Tower" toast first fired on `running < max(installedCLI, extensionVersion)`, but a restart only reloads the *installed* binary — so when the extension was newer than an up-to-date CLI, it prompted a restart that reloaded the same version (a no-op) and named a version that wasn't installed. Narrowing to `running < installedCLI` made every prompt actionable and every word true. When two conditions can trigger one action, fire only on the sub-condition the action resolves; route the others to their real owners (here: "CLI behind extension" is the separate `codev --version` preflight's job). - [From #983] To detect that a long-running daemon is serving stale code after an in-place upgrade, probe the **running process's** in-memory version over its own API — the on-disk binary version (`--version`) only reports what's installed, not what's executing. The two diverge for the whole window between `npm install -g` and the next restart. Expose it from request-context state (not a fresh disk read) so the value is provably the running process's. - [From #983] When a decision needs two independently-fetched async inputs (here: installed-CLI version from a ~400ms spawn, running-Tower version from a connect-triggered probe), nothing orders them — whichever loses the race leaves the decision running on partial data (a null input silently read as "fine"). Re-run the decision when the *later* input resolves, so the comparison always runs with both values present. A one-shot "fire on event X" is a latent bug when X can beat the data it depends on. +- [From #789] CodeLens placement in diff editors is doubly constrained, and it dictated a whole feature's design. (1) VSCode's **multi-file diff editor** (`vscode.changes` — what `codev.viewDiff` opens) renders **no** CodeLenses at all; (2) the **single-file** diff editor hides them unless `diffEditor.codeLens` is enabled (off by default — microsoft/vscode#97640). So a "lens on the diff" feature can only live in the per-file `vscode.diff` / normal editor tabs, and must prompt to enable the setting (at **Global** scope — it's a personal editor preference; writing Workspace would edit the repo's shared `.vscode/settings.json` and force it on collaborators). **Context menus are NOT suppressed in the multi-file editor**, so a right-click/keybinding action is the only affordance that works in the scan view. Validate the rendering surface with a running-worktree spike *before* building on it — unit tests assert lens *shape*, never whether the host editor displays them (sibling to [From 799]). +- [From #789] For annotating a diff, drive granularity off the **code (document symbols), not git hunks**. A brand-new file is a single whole-file hunk, so hunk-driven lenses give it exactly one lens regardless of size; symbol-driven lenses (via `vscode.executeDocumentSymbolProvider`) give every function/class/interface its own affordance, so new and modified files are equally addressable. Also: emit one lens per **contiguous changed run** (split each hunk on context lines; deletions don't break a run), not per git hunk — git coalesces adjacent edits into one hunk, which would collapse several distinct changes onto one lens. +- [From #789] Don't add a "performance" cache before confirming the hot path. A per-version document-symbol cache was added to fix a ~100% CPU spike, but the spike was an unrelated newly-installed extension (the **Extension Test Runner**) walking the `.builders/` worktree farm (`rg --no-ignore --follow` over pnpm symlinks) — the extension-host CPU profile was ~99% idle. `provideCodeLenses` is not a hot path (VSCode caches lenses and only re-queries on invalidation), so the cache bought nothing and risked pinning an empty result when the language server was still warming up on first open. Profile first; cache only a proven-hot, proven-pure path. ## Documentation diff --git a/codev/reviews/789-vscode-inject-file-hunk-refere.md b/codev/reviews/789-vscode-inject-file-hunk-refere.md new file mode 100644 index 000000000..fe759179b --- /dev/null +++ b/codev/reviews/789-vscode-inject-file-hunk-refere.md @@ -0,0 +1,72 @@ +# PIR Review: Forward file/symbol/hunk/selection references into the builder PTY (CodeLens) + +Fixes #789 + +## Summary + +Adds a one-click way to forward a code reference from a builder's diff into that builder's terminal prompt — no path retyping. In a builder file diff (or a normal editor tab on a builder worktree file) a `Forward to Builder` CodeLens appears at the file top, above each declaration (function/class/interface/enum/struct/namespace/method/constructor + multi-line top-level const), and above each changed hunk; clicking injects `` or `:L-L` into the builder's prompt **without** pressing Enter, mirroring the existing architect-reference pattern. A right-click **"Codev: Forward Selection to Builder"** (and `Cmd/Ctrl+K B`) forwards any selected range, and works inside the multi-file View Diff editor too. + +The issue's original mechanism (CodeLens on hunks in the `codev.viewDiff` editor) proved unworkable and the design pivoted during the `dev-approval` gate — see Lessons Learned. The implementation that shipped is symbol-driven (granularity follows the code, so new files are as forwardable as modified ones), with per-hunk lenses layered on, plus the selection action as the path that survives in the multi-file editor. + +## Files Changed + +- `packages/vscode/src/diff-inject-ref.ts` (+247 / new) — pure helpers: symbol selection (`buildSymbolLensDescriptors`), changed-run parsing (`parseHunkRanges`/`parseUnifiedDiff`), `buildAllLensDescriptors` (symbol + hunk lenses, deduped by anchor line), ref builders. +- `packages/vscode/src/diff-inject-codelens.ts` (+156 / new) — `CodeLensProvider` over a per-diff registry; resolves document symbols; backs the `codev.activeEditorIsBuilderFile` context key. +- `packages/vscode/src/ensure-diff-codelens.ts` (+38 / new) — one-click prompt to enable `diffEditor.codeLens` (Global scope) when off. +- `packages/vscode/src/commands/view-diff.ts` (+79 / -…) — populate the registry on `viewDiff` and the per-file diff; open the editor before computing hunks. +- `packages/vscode/src/terminal-manager.ts` (+32) — `injectBuilderText`; `openBuilderByRoleOrId` returns the resolved canonical id. +- `packages/vscode/src/extension.ts` (+62) — register `codev.forwardToBuilder` (palette-hidden) and `codev.forwardSelectionToBuilder`; activate the provider. +- `packages/vscode/package.json` (+21) — selection command + `editor/context` menu + `Cmd/Ctrl+K B` keybinding (all scoped + palette-hidden). +- `packages/vscode/src/__tests__/diff-inject-ref.test.ts` (+181 / new) — unit tests for the pure helpers. + +## Commits + +Full list via `git log main..HEAD --oneline` (26 feature commits). Highlights: + +- `7ee8068a` Add diff-inject ref helpers + CodeLens provider +- `4c6af3ee` terminal-manager: injectBuilderText + resolved-id return +- `fecb9658` Option A: lenses on per-file diff + diffEditor.codeLens prompt +- `9984f8c4` Drive lenses off document symbols, not git hunks +- `75c03eb5` Right-click "Forward Selection to Builder" editor/context action +- `aea085f4` Restore per-hunk lenses alongside symbol lenses (option B) +- `a9bbb478` One change-lens per contiguous changed run, not per git hunk +- `29f04083` Open diffs before computing hunks (fix open-delay regression) +- `a93d6ac5` Remove symbol cache (misdiagnosed; risked pinning empty symbols) +- `aa96ba8f` Enable diffEditor.codeLens at Global scope + +## Test Results + +- `pnpm --filter codev-vscode check-types`: ✓ pass +- `pnpm --filter codev-vscode lint`: ✓ pass +- `pnpm --filter codev-vscode test:unit`: ✓ pass (375 tests, ~20 new for symbol selection / changed-run parsing / lens building) +- `node esbuild.js` bundle: ✓ +- Manual (human, at `dev-approval` in the Extension Dev Host): file/symbol/hunk lenses render in per-file diff and normal tabs with `diffEditor.codeLens` on; clicking injects the ref into the builder terminal without Enter; right-click "Forward Selection to Builder" + `Cmd+K B` forward a selection (incl. in the multi-file View Diff editor); new-file lenses carry line ranges; the enable-prompt writes the setting. + +## Architecture Updates + +No `arch.md` changes needed — this is a self-contained VSCode UI feature (a CodeLens provider + two commands + terminal injection) that doesn't alter module boundaries or introduce a cross-cutting pattern. The durable platform constraints it surfaced are captured in `lessons-learned.md` instead (see below). + +## Lessons Learned Updates + +Added one entry to `codev/resources/lessons-learned.md` (UI/UX): CodeLens does **not** render in VSCode's multi-file `vscode.changes` editor, and is hidden by default in single diff editors (`diffEditor.codeLens`) — the constraint that forced this feature's whole design pivot. The full design-process lesson (symbol- vs hunk-driven granularity, and surfacing the multi-file-editor limit early) is recorded in this review. + +## Things to Look At During PR Review + +- **`buildAllLensDescriptors` dedup** (`diff-inject-ref.ts`): a hunk lens is suppressed when its anchor line already has a symbol/file lens, so declaration-line changes show the structural lens and body changes show the hunk lens — no stacking. Worth a careful read. +- **`parseHunkRanges`** now emits one range per *contiguous changed run* (broken by context lines; deletions don't break a run), not per git hunk — this is what makes adjacent edits get individual lenses. +- **Surface limits**: symbol/hunk CodeLenses live in per-file diff + normal tabs (CodeLens is suppressed in the multi-file `vscode.changes` editor); the selection context-menu is the path that works in the multi-file editor. This is a VSCode constraint, not a bug. +- **No 3-way consult iteration** (PIR single-pass): if the consult flags anything, it's addressed/rebutted here, not re-reviewed. + +## How to Test Locally + +This is a VSCode **extension** change, so `afx dev` won't exercise it — load the extension: + +- **Extension Dev Host**: open `packages/vscode` (or the repo) and press **F5** ("Run Codev Extension"). +- Ensure `diffEditor.codeLens` is on (the feature prompts to enable it on first file-diff open). +- Builders tree → click a changed file → confirm `Forward to Builder` lenses (file, symbols, hunks); click one → the builder terminal focuses and the ref is typed with no Enter. +- Select lines → right-click **Codev: Forward Selection to Builder** (or `Cmd/Ctrl+K B`); confirm it works in the multi-file **View Diff** editor too. +- Verify `codev.forwardToBuilder` / `codev.forwardSelectionToBuilder` are absent from the Command Palette. + +## Related + +- #1022 — workspace `files.watcherExclude`/`search.exclude` for `.builders/` + `node_modules`, and dropping the Extension Test Runner recommendation (a CPU storm discovered while reviewing this feature; not part of this change). diff --git a/codev/state/pir-789_thread.md b/codev/state/pir-789_thread.md index 234110e82..c5210ae36 100644 --- a/codev/state/pir-789_thread.md +++ b/codev/state/pir-789_thread.md @@ -57,6 +57,12 @@ Validation: 368 unit tests, check-types, lint, bundle all green. **Needs human r Deferred to future issue(s): hover-triggered forward; deeper nesting; per-kind density settings. +## Review phase + +dev-approval approved. During final testing found a ~100% CPU spike — traced (via extension-host CPU profile = 99% idle, then live `ps` of the rg processes) to a newly-installed **Extension Test Runner** extension walking the `.builders/` worktree farm, NOT this feature. Filed **#1022** (workspace excludes + drop the recommendation). Also: removed the misdiagnosed symbol cache; fixed the diffEditor.codeLens enable-toast to write Global scope (personal pref, not Workspace); identified a stray uncommitted workspace `false` (from diagnostics) shadowing it — user reverts. + +Review written (`codev/reviews/789-...md`), lessons-learned updated (3 entries: diff-editor CodeLens constraints, symbol- vs hunk-driven granularity, profile-before-caching). Opening PR. + ## Follow-ups during dev-approval - Renamed lens label → "Forward to Builder"; command ids `codev.forwardToBuilder` / `codev.forwardSelectionToBuilder`. - Added `Cmd/Ctrl+K B` keybinding for Forward Selection (when matched to the menu so the hint renders); menu title prefixed "Codev:". From 463f8a388c5a3ea6f7f1a143fe5be98db3dde493 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 10 Jun 2026 18:25:20 +1000 Subject: [PATCH 35/39] chore(porch): 789 record PR #1023 --- .../789-vscode-inject-file-hunk-refere/status.yaml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/codev/projects/789-vscode-inject-file-hunk-refere/status.yaml b/codev/projects/789-vscode-inject-file-hunk-refere/status.yaml index e92a6734e..e8fa0f457 100644 --- a/codev/projects/789-vscode-inject-file-hunk-refere/status.yaml +++ b/codev/projects/789-vscode-inject-file-hunk-refere/status.yaml @@ -19,4 +19,9 @@ iteration: 1 build_complete: false history: [] started_at: '2026-06-09T22:20:14.795Z' -updated_at: '2026-06-10T07:46:41.280Z' +updated_at: '2026-06-10T08:25:20.463Z' +pr_history: + - phase: review + pr_number: 1023 + branch: builder/pir-789 + created_at: '2026-06-10T08:25:20.462Z' From 5d61b4c2bd1dede7078fd28871a71335a86d5090 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 10 Jun 2026 18:25:34 +1000 Subject: [PATCH 36/39] chore(porch): 789 review build-complete --- codev/projects/789-vscode-inject-file-hunk-refere/status.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codev/projects/789-vscode-inject-file-hunk-refere/status.yaml b/codev/projects/789-vscode-inject-file-hunk-refere/status.yaml index e8fa0f457..bf0a1527f 100644 --- a/codev/projects/789-vscode-inject-file-hunk-refere/status.yaml +++ b/codev/projects/789-vscode-inject-file-hunk-refere/status.yaml @@ -16,10 +16,10 @@ gates: pr: status: pending iteration: 1 -build_complete: false +build_complete: true history: [] started_at: '2026-06-09T22:20:14.795Z' -updated_at: '2026-06-10T08:25:20.463Z' +updated_at: '2026-06-10T08:25:34.010Z' pr_history: - phase: review pr_number: 1023 From 9b87b82113b706c8945e013d1a4f08f3dd28aff1 Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 10 Jun 2026 18:32:25 +1000 Subject: [PATCH 37/39] [PIR #789] Re-sync builder-file context key on registry change (consult REQUEST_CHANGES) Codex consult (HIGH): the codev.activeEditorIsBuilderFile key only synced on editor-focus change, but openBuilderFileDiff opens the diff before registering its file, so the selection menu + Cmd/Ctrl+K B were dead on the just-opened diff until focus changed. Re-sync the key on every registry change (subscribe to the provider's onDidChangeCodeLenses). Adds a regression test. --- .../789-vscode-inject-file-hunk-refere.md | 6 ++ .../__tests__/diff-inject-context-key.test.ts | 99 +++++++++++++++++++ packages/vscode/src/diff-inject-codelens.ts | 7 ++ 3 files changed, 112 insertions(+) create mode 100644 packages/vscode/src/__tests__/diff-inject-context-key.test.ts diff --git a/codev/reviews/789-vscode-inject-file-hunk-refere.md b/codev/reviews/789-vscode-inject-file-hunk-refere.md index fe759179b..02801b69a 100644 --- a/codev/reviews/789-vscode-inject-file-hunk-refere.md +++ b/codev/reviews/789-vscode-inject-file-hunk-refere.md @@ -57,6 +57,12 @@ Added one entry to `codev/resources/lessons-learned.md` (UI/UX): CodeLens does * - **Surface limits**: symbol/hunk CodeLenses live in per-file diff + normal tabs (CodeLens is suppressed in the multi-file `vscode.changes` editor); the selection context-menu is the path that works in the multi-file editor. This is a VSCode constraint, not a bug. - **No 3-way consult iteration** (PIR single-pass): if the consult flags anything, it's addressed/rebutted here, not re-reviewed. +### Consultation outcome (PR-creation, single advisory pass) + +- **Claude: APPROVE** (HIGH) — no issues. +- **Codex: REQUEST_CHANGES** (HIGH) — **real defect, fixed.** The `codev.activeEditorIsBuilderFile` context key (gating the right-click "Forward Selection to Builder" + `Cmd/Ctrl+K B`) was synced only on `onDidChangeActiveTextEditor`. Because `openBuilderFileDiff` opens the diff (making it active) *before* registering its file, the key was left `false` on the just-opened diff until focus changed, so the selection entry point was dead. **Fix** (`diff-inject-codelens.ts`): re-sync the key on every registry change by subscribing to the provider's `onDidChangeCodeLenses` (fired by `setSession`/`upsert`). **Regression test**: `src/__tests__/diff-inject-context-key.test.ts` asserts the key flips `true` after the active file is registered (and back to `false` when a new session drops it) — it fails without the re-sync. Since PIR is single-pass, this fix was **not** independently re-reviewed; flagging for human verification at the `pr` gate. +- **Gemini: skipped** — the agy/Gemini path returned "the current workspace is empty" on both attempts (it didn't ingest the diff). Best-effort per the consult design; no verdict. + ## How to Test Locally This is a VSCode **extension** change, so `afx dev` won't exercise it — load the extension: diff --git a/packages/vscode/src/__tests__/diff-inject-context-key.test.ts b/packages/vscode/src/__tests__/diff-inject-context-key.test.ts new file mode 100644 index 000000000..b46b2a259 --- /dev/null +++ b/packages/vscode/src/__tests__/diff-inject-context-key.test.ts @@ -0,0 +1,99 @@ +/** + * Regression (#789, raised by the PR consultation): the + * `codev.activeEditorIsBuilderFile` context key — which gates the right-click + * "Forward Selection to Builder" menu and the Cmd/Ctrl+K B keybinding — must + * re-sync when the diff registry changes, not only on editor-focus changes. + * + * `openBuilderFileDiff` opens the diff (making it the active editor) BEFORE + * registering its file, so the active-editor event fires while the registry is + * still empty. Without a registry-change re-sync the key stays `false` on the + * just-opened diff until focus changes, leaving the selection action dead. + */ + +import { describe, it, expect, beforeEach, vi } from 'vitest'; + +const h = vi.hoisted(() => { + class EventEmitter { + private handlers: Array<(e: T) => void> = []; + event = (fn: (e: T) => void): { dispose(): void } => { + this.handlers.push(fn); + return { dispose() {} }; + }; + fire(value: T): void { + for (const fn of this.handlers) { fn(value); } + } + dispose(): void {} + } + const state = { + activeEditor: undefined as unknown, + setContextCalls: [] as Array<{ key: string; value: unknown }>, + }; + return { EventEmitter, state }; +}); + +vi.mock('vscode', () => ({ + EventEmitter: h.EventEmitter, + Range: class {}, + CodeLens: class {}, + languages: { registerCodeLensProvider: () => ({ dispose() {} }) }, + commands: { + executeCommand: (cmd: string, ...args: unknown[]) => { + if (cmd === 'setContext') { + h.state.setContextCalls.push({ key: args[0] as string, value: args[1] }); + } + return Promise.resolve(); + }, + }, + window: { + get activeTextEditor() { return h.state.activeEditor; }, + onDidChangeActiveTextEditor: () => ({ dispose() {} }), + }, +})); + +const { + activateDiffInjectCodeLens, + setDiffInjectSession, + upsertDiffInjectEntry, + BUILDER_FILE_CONTEXT_KEY, +} = await import('../diff-inject-codelens.js'); + +function editorFor(fsPath: string): unknown { + return { document: { uri: { fsPath, toString: () => `file://${fsPath}` } } }; +} +function lastContext(): { key: string; value: unknown } | undefined { + return h.state.setContextCalls[h.state.setContextCalls.length - 1]; +} + +const ENTRY = { fsPath: '/wt/pkg/src/a.ts', builderId: 'b1', relPath: 'pkg/src/a.ts', hunks: [] }; + +describe('#789 — activeEditorIsBuilderFile re-syncs on registry change', () => { + beforeEach(() => { + h.state.activeEditor = undefined; + setDiffInjectSession([]); // clear the singleton registry + h.state.setContextCalls.length = 0; + }); + + it('sets the key true after the active file is registered (not only on focus change)', () => { + h.state.activeEditor = editorFor(ENTRY.fsPath); + + activateDiffInjectCodeLens({ subscriptions: [] } as never); + // Diff is active but not yet registered → key false. + expect(lastContext()).toEqual({ key: BUILDER_FILE_CONTEXT_KEY, value: false }); + + // openBuilderFileDiff order: open the diff, THEN register its file. + upsertDiffInjectEntry(ENTRY); + + // The registry-change re-sync must flip the key true (the bug left it false). + expect(lastContext()).toEqual({ key: BUILDER_FILE_CONTEXT_KEY, value: true }); + }); + + it('clears the key when a new session no longer tracks the active file', () => { + h.state.activeEditor = editorFor(ENTRY.fsPath); + activateDiffInjectCodeLens({ subscriptions: [] } as never); + upsertDiffInjectEntry(ENTRY); + expect(lastContext()?.value).toBe(true); + + setDiffInjectSession([]); // e.g. a fresh viewDiff run without this file + expect(lastContext()?.value).toBe(false); + }); +}); diff --git a/packages/vscode/src/diff-inject-codelens.ts b/packages/vscode/src/diff-inject-codelens.ts index 685d58c84..33877ffad 100644 --- a/packages/vscode/src/diff-inject-codelens.ts +++ b/packages/vscode/src/diff-inject-codelens.ts @@ -133,6 +133,13 @@ export function activateDiffInjectCodeLens(context: vscode.ExtensionContext): vo context.subscriptions.push( vscode.languages.registerCodeLensProvider({ scheme: 'file' }, provider), vscode.window.onDidChangeActiveTextEditor(syncContextKey), + // The registry often changes AFTER the diff editor is already active — + // `openBuilderFileDiff` opens the diff, then registers its file — so the + // active-editor event fires while the registry is still empty. Re-sync the + // key whenever the registry changes (the provider fires this on + // setSession/upsert), or the selection menu + Cmd/Ctrl+K B would stay + // disabled on the just-opened diff until focus changes (#789). + provider.onDidChangeCodeLenses(() => syncContextKey(vscode.window.activeTextEditor)), provider, ); } From a5f8847b544de13e96e240776cae26fe5f653ebb Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 10 Jun 2026 18:35:47 +1000 Subject: [PATCH 38/39] chore(porch): 789 pr gate-requested --- codev/projects/789-vscode-inject-file-hunk-refere/status.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/codev/projects/789-vscode-inject-file-hunk-refere/status.yaml b/codev/projects/789-vscode-inject-file-hunk-refere/status.yaml index bf0a1527f..b22175f3a 100644 --- a/codev/projects/789-vscode-inject-file-hunk-refere/status.yaml +++ b/codev/projects/789-vscode-inject-file-hunk-refere/status.yaml @@ -15,13 +15,15 @@ gates: approved_at: '2026-06-10T07:46:32.265Z' pr: status: pending + requested_at: '2026-06-10T08:35:47.462Z' iteration: 1 build_complete: true history: [] started_at: '2026-06-09T22:20:14.795Z' -updated_at: '2026-06-10T08:25:34.010Z' +updated_at: '2026-06-10T08:35:47.463Z' pr_history: - phase: review pr_number: 1023 branch: builder/pir-789 created_at: '2026-06-10T08:25:20.462Z' +pr_ready_for_human: true From 0b69ff521c523a78d5fda0b959f8cc8f0fc7b15c Mon Sep 17 00:00:00 2001 From: Amr Elsayed Date: Wed, 10 Jun 2026 18:43:33 +1000 Subject: [PATCH 39/39] chore(porch): 789 pr gate-approved --- .../789-vscode-inject-file-hunk-refere/status.yaml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/codev/projects/789-vscode-inject-file-hunk-refere/status.yaml b/codev/projects/789-vscode-inject-file-hunk-refere/status.yaml index b22175f3a..1a9e75ad6 100644 --- a/codev/projects/789-vscode-inject-file-hunk-refere/status.yaml +++ b/codev/projects/789-vscode-inject-file-hunk-refere/status.yaml @@ -14,16 +14,17 @@ gates: requested_at: '2026-06-09T23:04:34.034Z' approved_at: '2026-06-10T07:46:32.265Z' pr: - status: pending + status: approved requested_at: '2026-06-10T08:35:47.462Z' + approved_at: '2026-06-10T08:43:33.506Z' iteration: 1 build_complete: true history: [] started_at: '2026-06-09T22:20:14.795Z' -updated_at: '2026-06-10T08:35:47.463Z' +updated_at: '2026-06-10T08:43:33.508Z' pr_history: - phase: review pr_number: 1023 branch: builder/pir-789 created_at: '2026-06-10T08:25:20.462Z' -pr_ready_for_human: true +pr_ready_for_human: false