vscode: Forward file/symbol/hunk/selection references into the builder PTY (#789)#1023
Merged
Conversation
…rom openBuilderByRoleOrId
…ompt 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.
…ext 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.
…ine 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.
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.
…bols replace it)
…ext 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.
…nu title with 'Codev:'
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.
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.
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<n>'.
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.
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).
…mbols) 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.
…r 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.
…lt 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 BuilderCodeLens 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<path>or<path>:L<start>-L<end>into the builder's prompt without pressing Enter, mirroring the existing architect-reference pattern. A right-click "Codev: Forward Selection to Builder" (andCmd/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.viewDiffeditor) proved unworkable and the design pivoted during thedev-approvalgate — 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) —CodeLensProviderover a per-diff registry; resolves document symbols; backs thecodev.activeEditorIsBuilderFilecontext key.packages/vscode/src/ensure-diff-codelens.ts(+38 / new) — one-click prompt to enablediffEditor.codeLens(Global scope) when off.packages/vscode/src/commands/view-diff.ts(+79 / -…) — populate the registry onviewDiffand the per-file diff; open the editor before computing hunks.packages/vscode/src/terminal-manager.ts(+32) —injectBuilderText;openBuilderByRoleOrIdreturns the resolved canonical id.packages/vscode/src/extension.ts(+62) — registercodev.forwardToBuilder(palette-hidden) andcodev.forwardSelectionToBuilder; activate the provider.packages/vscode/package.json(+21) — selection command +editor/contextmenu +Cmd/Ctrl+K Bkeybinding (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:7ee8068aAdd diff-inject ref helpers + CodeLens provider4c6af3eeterminal-manager: injectBuilderText + resolved-id returnfecb9658Option A: lenses on per-file diff + diffEditor.codeLens prompt9984f8c4Drive lenses off document symbols, not git hunks75c03eb5Right-click "Forward Selection to Builder" editor/context actionaea085f4Restore per-hunk lenses alongside symbol lenses (option B)a9bbb478One change-lens per contiguous changed run, not per git hunk29f04083Open diffs before computing hunks (fix open-delay regression)a93d6ac5Remove symbol cache (misdiagnosed; risked pinning empty symbols)aa96ba8fEnable diffEditor.codeLens at Global scopeTest Results
pnpm --filter codev-vscode check-types: ✓ passpnpm --filter codev-vscode lint: ✓ passpnpm --filter codev-vscode test:unit: ✓ pass (375 tests, ~20 new for symbol selection / changed-run parsing / lens building)node esbuild.jsbundle: ✓dev-approvalin the Extension Dev Host): file/symbol/hunk lenses render in per-file diff and normal tabs withdiffEditor.codeLenson; clicking injects the ref into the builder terminal without Enter; right-click "Forward Selection to Builder" +Cmd+K Bforward 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.mdchanges 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 inlessons-learned.mdinstead (see below).Lessons Learned Updates
Added one entry to
codev/resources/lessons-learned.md(UI/UX): CodeLens does not render in VSCode's multi-filevscode.changeseditor, 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
buildAllLensDescriptorsdedup (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.parseHunkRangesnow 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.vscode.changeseditor); the selection context-menu is the path that works in the multi-file editor. This is a VSCode constraint, not a bug.How to Test Locally
This is a VSCode extension change, so
afx devwon't exercise it — load the extension:packages/vscode(or the repo) and press F5 ("Run Codev Extension").diffEditor.codeLensis on (the feature prompts to enable it on first file-diff open).Forward to Builderlenses (file, symbols, hunks); click one → the builder terminal focuses and the ref is typed with no Enter.Cmd/Ctrl+K B); confirm it works in the multi-file View Diff editor too.codev.forwardToBuilder/codev.forwardSelectionToBuilderare absent from the Command Palette.Related
files.watcherExclude/search.excludefor.builders/+node_modules, and dropping the Extension Test Runner recommendation (a CPU storm discovered while reviewing this feature; not part of this change).