diff --git a/AGENTS.md b/AGENTS.md index d2249f246..59af5f634 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -277,6 +277,8 @@ During normal plan review, an Archive sidebar tab provides the same browsing via | `/api/pr-switch` | POST | Switch to a different PR in-place (body: `{ url }`) | | `/api/tour/:jobId` | GET | Fetch Code Tour result (greeting, stops, checklist) for a completed tour job | | `/api/tour/:jobId/checklist` | PUT | Persist checklist item state for a Code Tour | +| `/api/code-nav/resolve` | POST | Search for symbol definitions and references via ripgrep (body: `{ symbol, filePath, line, charStart, side, language? }`) | +| `/api/code-nav/file` | GET | Read file from working tree for code-nav preview (`?path=`) | ### Annotate Server (`packages/server/annotate.ts`) diff --git a/apps/pi-extension/server/serverReview.ts b/apps/pi-extension/server/serverReview.ts index 8386cbb98..98fa2deaa 100644 --- a/apps/pi-extension/server/serverReview.ts +++ b/apps/pi-extension/server/serverReview.ts @@ -1,4 +1,4 @@ -import { execSync } from "node:child_process"; +import { execSync, spawn } from "node:child_process"; import { readFileSync, existsSync } from "node:fs"; import { createServer } from "node:http"; import os from "node:os"; @@ -84,6 +84,13 @@ import { transformClaudeFindings, } from "../generated/claude-review.js"; import { createTourSession, TOUR_EMPTY_OUTPUT_ERROR } from "../generated/tour-review.js"; +import { + type CodeNavRequest, + type CodeNavRuntime, + resolveCodeNav, + validateCodeNavRequest, + extractChangedFiles, +} from "../generated/code-nav.js"; import { canStageFiles, detectRemoteDefaultCompareTarget, @@ -96,6 +103,37 @@ import { unstageFile, } from "./vcs.js"; +const piCodeNavRuntime: CodeNavRuntime = { + runCommand(command, args, options) { + return new Promise((resolve) => { + const proc = spawn(command, args, { + cwd: options?.cwd, + stdio: ["ignore", "pipe", "pipe"], + }); + let timer: ReturnType | undefined; + if (options?.timeoutMs) { + timer = setTimeout(() => proc.kill(), options.timeoutMs); + } + const stdoutChunks: Buffer[] = []; + const stderrChunks: Buffer[] = []; + proc.stdout!.on("data", (chunk: Buffer) => stdoutChunks.push(chunk)); + proc.stderr!.on("data", (chunk: Buffer) => stderrChunks.push(chunk)); + proc.on("close", (code: number | null) => { + if (timer) clearTimeout(timer); + resolve({ + stdout: Buffer.concat(stdoutChunks).toString("utf-8"), + stderr: Buffer.concat(stderrChunks).toString("utf-8"), + exitCode: code ?? 1, + }); + }); + proc.on("error", () => { + if (timer) clearTimeout(timer); + resolve({ stdout: "", stderr: "command not found", exitCode: 1 }); + }); + }); + }, +}; + /** Detect if running inside WSL (Windows Subsystem for Linux) */ function detectWSL(): boolean { if (process.platform !== "linux") return false; @@ -965,6 +1003,48 @@ export async function startReviewServer(options: { } json(res, { error: "No file access available" }, 400); + } else if (url.pathname === "/api/code-nav/resolve" && req.method === "POST") { + const hasCodeNavAccess = !!options.gitContext || !!options.agentCwd || !!options.worktreePool; + if (!hasCodeNavAccess) { + json(res, { error: "Code navigation requires local access" }, 400); + return; + } + try { + const body = (await parseBody(req)) as unknown as CodeNavRequest; + const error = validateCodeNavRequest(body); + if (error) { + json(res, { error }, 400); + return; + } + const navCwd = resolveAgentCwd(); + const changedFiles = extractChangedFiles(currentPatch); + const result = await resolveCodeNav(piCodeNavRuntime, body, navCwd, changedFiles); + json(res, result); + } catch (err) { + json(res, { error: err instanceof Error ? err.message : "Code navigation failed" }, 500); + } + } else if (url.pathname === "/api/code-nav/file" && req.method === "GET") { + const hasCodeNavAccess = !!options.gitContext || !!options.agentCwd || !!options.worktreePool; + if (!hasCodeNavAccess) { + json(res, { error: "Code navigation requires local access" }, 400); + return; + } + const filePath = url.searchParams.get("path"); + if (!filePath) { + json(res, { error: "Missing path" }, 400); + return; + } + try { validateFilePath(filePath); } catch { + json(res, { error: "Invalid path" }, 400); + return; + } + try { + const navCwd = resolveAgentCwd(); + const content = readFileSync(`${navCwd}/${filePath}`, "utf-8"); + json(res, { content }); + } catch { + json(res, { error: "File not found" }, 404); + } } else if (url.pathname === "/api/config" && req.method === "POST") { try { const body = (await parseBody(req)) as { displayName?: string; diffOptions?: Record; conventionalComments?: boolean }; diff --git a/apps/pi-extension/vendor.sh b/apps/pi-extension/vendor.sh index 205a185e4..17ac9c681 100755 --- a/apps/pi-extension/vendor.sh +++ b/apps/pi-extension/vendor.sh @@ -6,7 +6,7 @@ cd "$(dirname "$0")" mkdir -p generated generated/ai/providers -for f in feedback-templates prompts review-core jj-core vcs-core review-args storage draft project pr-types pr-provider pr-stack pr-github pr-gitlab checklist integrations-common repo reference-common favicon code-file resolve-file config external-annotation agent-jobs worktree worktree-pool html-to-markdown url-to-markdown tour annotate-args at-reference pfm-reminder improvement-hooks; do +for f in feedback-templates prompts review-core jj-core vcs-core review-args storage draft project pr-types pr-provider pr-stack pr-github pr-gitlab checklist integrations-common repo reference-common favicon code-file resolve-file config external-annotation agent-jobs worktree worktree-pool html-to-markdown url-to-markdown tour annotate-args at-reference pfm-reminder improvement-hooks code-nav; do src="../../packages/shared/$f.ts" printf '// @generated — DO NOT EDIT. Source: packages/shared/%s.ts\n' "$f" | cat - "$src" > "generated/$f.ts" done diff --git a/docs/issue-694-code-navigation-recap.md b/docs/issue-694-code-navigation-recap.md new file mode 100644 index 000000000..91b491ed8 --- /dev/null +++ b/docs/issue-694-code-navigation-recap.md @@ -0,0 +1,254 @@ +# Issue 694 Code Navigation Recap + +Issue: https://github.com/backnotprop/plannotator/issues/694 + +## What The User Is Asking For + +The feature request asks for IDE-like semantic code navigation inside the Plannotator code review UI. The examples in the issue are: + +- Ctrl/Cmd-click an identifier to find references. +- Show references in a sidebar. +- Peek definition. +- Navigate to definitions, references, and implementations without leaving the review context. + +The user-facing value is not "AST parsing" by itself. The value is that while reviewing a diff, a reviewer can quickly answer: where is this symbol defined, where else is it used, and what related code should I inspect before annotating? + +## Current Plannotator Context + +Plannotator already has the right UI surface to capture the interaction: + +- The code review UI renders diffs through `@pierre/diffs`. +- Pierre exposes token-level events with line number, character range, token text, token DOM element, and diff side. +- Plannotator already wires Pierre token clicks into the annotation toolbar. +- The review server already serves old/new file contents for changed files through `/api/file-content`. +- Dockview already gives us a natural place to add a "References" or "Peek Definition" panel. + +Important constraint: Pierre does not provide semantic code intelligence. It can tell us "the user clicked this token at this location." It cannot tell us where the symbol is defined or referenced. That needs a separate backend resolver. + +## PR Diff Constraints + +PR mode matters because code navigation depends on which version of the repository we are asking about. + +Layer PR diffs are platform diffs. In that mode, Plannotator has the patch and can fetch file contents from GitHub/GitLab by SHA, but it may not have a complete local repository to search. + +Full-stack PR mode and local review mode can use a local checkout/worktree. Those modes are much better for repo-wide code navigation because the backend can run local search tools against actual files. + +The practical rule should be: + +- Platform-only PR mode: support changed-file/current-diff navigation and clear degradation. +- Local checkout/worktree mode: support repo-wide references and likely definitions. + +## What We Explored + +### Full LSP + +Bundling and running language servers inside Plannotator is not a good MVP path. It is heavyweight, language-specific, expensive to bundle, harder to sandbox, and can be slow or brittle across arbitrary user repos. + +LSP can remain an optional future accuracy tier if a project already has the needed tooling installed. + +### SCIP / LSIF + +SCIP is the right shape for precise code intelligence if an index already exists. It can represent definitions, references, and richer symbol relationships. But generating SCIP indexes means invoking language-specific indexers, which brings back the same cost problem as LSP. + +Good future path: consume SCIP when present. Do not generate it by default in the MVP. + +### Tree-sitter / Stack Graphs + +Tree-sitter can parse source and identify syntax cheaply. Stack Graphs can model scope and name resolution for supported languages. This is more principled than regex search, but it still requires language grammars, queries, and integration work. + +Good future path: use this to improve definitions and ranking. Not needed for the first useful version. + +### Universal Ctags + +Universal Ctags can produce a symbol index for definitions across many languages. It is lightweight compared with LSP, but it is not guaranteed to be installed. On this machine, only the older Xcode `ctags` is present, not Universal Ctags. + +Good future path: detect Universal Ctags if available and use it as a definition indexer. + +### Ripgrep + +Ripgrep is the best MVP foundation: + +- It is commonly installed in developer environments. +- It is very fast. +- It needs no index. +- It respects ignore files by default. +- It can return JSON output. +- It is easy to cap, timeout, and cancel. + +On this repo, exact whole-word JSON searches over roughly 800 tracked files completed around 20-25ms. That is fast enough to use lazily on click. + +## Recommended MVP + +Build a search-based code navigation backend first. + +Flow: + +1. Pierre emits a token interaction: file path, side, line, char range, token text. +2. Frontend sends that to the server. +3. Server runs a bounded exact-symbol `rg` search for references. +4. Server runs a second bounded search for likely definitions using simple language-aware regex patterns. +5. Server ranks results. +6. Server returns snippets, result kind, confidence, elapsed time, and whether results were capped. +7. Frontend can later render this in a polished IDE-like sidebar or peek panel. + +Example endpoint: + +```ts +POST /api/code-nav/resolve +{ + symbol: "startReviewServer", + filePath: "packages/server/review.ts", + line: 134, + charStart: 22, + side: "new", + language: "typescript" +} +``` + +Example response: + +```ts +{ + backend: "search", + complete: true, + definitions: [ + { + kind: "definition", + confidence: "likely", + filePath: "packages/server/review.ts", + line: 134, + column: 22, + snippet: "export async function startReviewServer(" + } + ], + references: [ + { + kind: "reference", + filePath: "apps/hook/server/index.ts", + line: 516, + column: 23, + snippet: "const server = await startReviewServer({" + } + ], + stats: { + elapsedMs: 24, + capped: false + } +} +``` + +## Definition Heuristics + +For the MVP, definitions should be "likely definitions," not falsely marketed as perfect semantic answers. + +For TypeScript/JavaScript, patterns can cover: + +- `function symbol` +- `async function symbol` +- `export function symbol` +- `export async function symbol` +- `const symbol =` +- `let symbol =` +- `var symbol =` +- `class symbol` +- `interface symbol` +- `type symbol` +- `enum symbol` +- `symbol(` inside object/class method contexts + +Other language patterns can be added incrementally: + +- Python: `def symbol`, `class symbol` +- Go: `func symbol`, `func (...) symbol`, `type symbol` +- Rust: `fn symbol`, `struct symbol`, `enum symbol`, `trait symbol`, `impl` + +The backend should label these as `likely_definition` unless a stronger backend produced the result. + +## Ranking + +Ranking matters more than perfect completeness in the MVP. + +Recommended ranking: + +1. Exact match in the current file. +2. Exact match in changed files. +3. Likely definition in the same directory. +4. Likely definition in imported/exported files. +5. Same language/extension. +6. Test files lower unless clicked symbol came from a test. +7. Docs and generated files lower. +8. Everything else. + +The server should return capped results rather than trying to be exhaustive. + +## Performance Rules + +The backend should be lazy, bounded, and cancelable. + +- No startup indexing. +- Do no work until the user asks for code navigation. +- Use current diff/current file matches immediately in memory. +- Run `rg` with exact whole-word matching. +- Cap result count. +- Cap files searched. +- Apply a short timeout. +- Cancel stale searches when the user clicks another symbol. +- Cache recent symbol queries per repo state. +- Respect `.gitignore` and skip `node_modules`, `dist`, build outputs, binary files, and vendored directories. +- Return partial results if a search is capped or times out. + +This keeps the feature cheap for normal use and prevents pathological repos from freezing the review server. + +## Backend Capability Tiers + +The capability model should be explicit: + +1. `search`: always available if `rg` is present. Provides exact references and likely definitions. +2. `ctags`: optional if Universal Ctags is installed. Improves definitions. +3. `tree-sitter`: optional later. Improves symbol classification and local scoping. +4. `scip`: optional if an index is already present. Provides precise code intelligence. +5. `lsp`: optional future integration only, never required for baseline behavior. + +The UI can show this honestly: + +- "References" for exact search matches. +- "Likely definition" for regex/ctags results. +- "Precise definition" only when a precise backend produced it. + +## What This Enables On The Frontend + +Once this backend exists, the frontend can become more IDE-like without depending on heavyweight infrastructure: + +- Ctrl/Cmd-click a token in the Pierre diff. +- Hover with modifier key to show that the token is navigable. +- Show references in a sidebar panel. +- Show a peek definition panel. +- Jump to a changed-file result in the existing diff view. +- Open unchanged-file results in a read-only source preview panel. +- Highlight all visible references in the current diff. +- Add annotations directly from search/navigation results. + +The frontend can be polished later. The backend only needs to return stable, fast, ranked results. + +## Non-Goals For The MVP + +- Do not bundle language servers. +- Do not build a full repo index on server startup. +- Do not promise perfect semantic correctness. +- Do not require Universal Ctags, Tree-sitter, SCIP, or language-specific tools. +- Do not make platform-only PR mode pretend it has full repo navigation when no local checkout exists. + +## Final Recommendation + +Implement the first version as: + +```text +Pierre token click + -> /api/code-nav/resolve + -> bounded rg references + -> regex-ranked likely definitions + -> snippets + confidence labels + -> sidebar/peek-ready response +``` + +This gives users most of the value they are asking for while keeping Plannotator lightweight. It also creates a clean upgrade path: richer backends can be added later without changing the frontend contract. diff --git a/packages/review-editor/App.tsx b/packages/review-editor/App.tsx index 7b6aca616..6c09036e1 100644 --- a/packages/review-editor/App.tsx +++ b/packages/review-editor/App.tsx @@ -28,6 +28,8 @@ import { useCodeAnnotationDraft } from '@plannotator/ui/hooks/useCodeAnnotationD import { useGitAdd } from './hooks/useGitAdd'; import { generateId } from './utils/generateId'; import { useAIChat } from './hooks/useAIChat'; +import { toast, Toaster } from 'sonner'; +import { useCodeNav, type CodeNavRequest } from './hooks/useCodeNav'; import { extractLinesFromPatch } from './utils/patchParser'; import { isTypingTarget, useReviewSearch, type ReviewSearchMatch } from './hooks/useReviewSearch'; import { useEditorAnnotations } from '@plannotator/ui/hooks/useEditorAnnotations'; @@ -67,6 +69,7 @@ import { REVIEW_PR_COMMENTS_PANEL_ID, REVIEW_PR_CHECKS_PANEL_ID, REVIEW_ALL_FILES_PANEL_ID, + REVIEW_CODE_NAV_PANEL_ID, } from './dock/reviewPanelTypes'; import type { DiffFile } from './types'; import type { DiffOption, WorktreeInfo, GitContext } from '@plannotator/shared/types'; @@ -422,6 +425,36 @@ const ReviewApp: React.FC = () => { reasoningEffort: aiConfig.reasoningEffort, }); + const codeNav = useCodeNav(); + + const handleCodeNavRequest = useCallback((request: CodeNavRequest) => { + if (!gitContext && !agentCwd) { + toast('Code navigation requires a local checkout', { + description: 'Re-run with --local for PR reviews', + duration: 4000, + }); + return; + } + codeNav.resolve(request); + if (!dockApi) return; + const existing = dockApi.getPanel(REVIEW_CODE_NAV_PANEL_ID); + if (existing) { + existing.api.setTitle(`References: ${request.symbol}`); + existing.api.setActive(); + } else { + const refPanel = isAllFilesActive + ? REVIEW_ALL_FILES_PANEL_ID + : REVIEW_DIFF_PANEL_ID; + dockApi.addPanel({ + id: REVIEW_CODE_NAV_PANEL_ID, + component: REVIEW_PANEL_TYPES.CODE_NAV, + title: `References: ${request.symbol}`, + position: { direction: 'below', referencePanel: refPanel }, + initialHeight: 250, + }); + } + }, [codeNav.resolve, dockApi, isAllFilesActive, gitContext, agentCwd]); + // Check AI capabilities on mount useEffect(() => { fetch('/api/ai/capabilities') @@ -1374,6 +1407,10 @@ const ReviewApp: React.FC = () => { onAllFilesVisibleFileChange: setAllFilesVisibleFile, isAllFilesActive, openTourPanel: handleOpenTour, + onCodeNavRequest: handleCodeNavRequest, + codeNavResult: codeNav.result, + codeNavIsLoading: codeNav.isLoading, + codeNavActiveSymbol: codeNav.activeSymbol, }), [ files, activeFileIndex, diffStyle, diffOverflow, diffIndicators, diffLineDiffType, diffShowLineNumbers, diffShowBackground, @@ -1390,6 +1427,7 @@ const ReviewApp: React.FC = () => { aiHistoryForSelection, agentJobs.jobs, prMetadata, prContext, isPRContextLoading, prContextError, fetchPRContext, platformUser, openDiffFile, handleOpenTour, isAllFilesActive, handleAddAnnotationForFile, + handleCodeNavRequest, codeNav.result, codeNav.isLoading, codeNav.activeSymbol, ]); // Separate context for high-frequency job logs — prevents re-rendering all panels on every SSE event @@ -2445,6 +2483,16 @@ const ReviewApp: React.FC = () => { )} + diff --git a/packages/review-editor/components/AllFilesDiffView.tsx b/packages/review-editor/components/AllFilesDiffView.tsx index 3effd0011..a255531d8 100644 --- a/packages/review-editor/components/AllFilesDiffView.tsx +++ b/packages/review-editor/components/AllFilesDiffView.tsx @@ -2,6 +2,8 @@ import React, { useMemo, useCallback, useRef, useState, useEffect } from 'react' import { type DiffLineAnnotation } from '@pierre/diffs/react'; import { getSingularPatch } from '@pierre/diffs'; import { CodeAnnotation, CodeAnnotationType, SelectedLineRange, DiffAnnotationMetadata, TokenAnnotationMeta, ConventionalLabel, ConventionalDecoration } from '@plannotator/ui/types'; +import type { DiffTokenEventBaseProps } from '@pierre/diffs'; +import { buildCodeNavRequest } from '../utils/buildCodeNavRequest'; import { usePierreTheme } from '../hooks/usePierreTheme'; import { LazyFileDiff } from './LazyFileDiff'; import { ToolbarHost, type ToolbarHostHandle } from './ToolbarHost'; @@ -51,6 +53,7 @@ interface AllFilesDiffViewProps { isAILoading?: boolean; onViewAIResponse?: (questionId?: string) => void; aiHistoryForSelection?: AIChatEntry[]; + onCodeNavRequest?: (request: import('@plannotator/shared/code-nav').CodeNavRequest) => void; } export const AllFilesDiffView: React.FC = ({ @@ -89,6 +92,7 @@ export const AllFilesDiffView: React.FC = ({ isAILoading = false, onViewAIResponse, aiHistoryForSelection = [], + onCodeNavRequest, }) => { const pierreTheme = usePierreTheme({ fontFamily, fontSize }); const [activeFilePath, setActiveFilePath] = useState(null); @@ -478,6 +482,21 @@ export const AllFilesDiffView: React.FC = ({ } onLineSelection(range); }, + ...(onCodeNavRequest && { + onTokenClick: (props: DiffTokenEventBaseProps, event: MouseEvent) => { + if (event.metaKey || event.ctrlKey) { + onCodeNavRequest(buildCodeNavRequest(props, file.path)); + } + }, + onTokenEnter: (props: DiffTokenEventBaseProps, event: PointerEvent) => { + if (event.metaKey || event.ctrlKey) { + props.tokenElement.classList.add('pn-token-nav'); + } + }, + onTokenLeave: (props: DiffTokenEventBaseProps) => { + props.tokenElement.classList.remove('pn-token-nav'); + }, + }), }} annotations={fileAnnotations} selectedLines={activeFilePath === file.path ? (pendingSelection || undefined) : undefined} diff --git a/packages/review-editor/components/DiffViewer.tsx b/packages/review-editor/components/DiffViewer.tsx index fd99a2b4c..a4181b0fc 100644 --- a/packages/review-editor/components/DiffViewer.tsx +++ b/packages/review-editor/components/DiffViewer.tsx @@ -7,6 +7,7 @@ import { usePierreTheme } from '../hooks/usePierreTheme'; import { CommentPopover } from '@plannotator/ui/components/CommentPopover'; import { storage } from '@plannotator/ui/utils/storage'; import { detectLanguage } from '../utils/detectLanguage'; +import { buildCodeNavRequest } from '../utils/buildCodeNavRequest'; import { ToolbarHost, type ToolbarHostHandle } from './ToolbarHost'; import { OverlayScrollArea } from '@plannotator/ui/components/OverlayScrollArea'; import { useOverlayViewport } from '@plannotator/ui/hooks/useOverlayViewport'; @@ -161,6 +162,8 @@ interface DiffViewerProps { onClickAIMarker?: (questionId: string) => void; /** AI messages overlapping the current pending selection */ aiHistoryMessages?: AIChatEntry[]; + // Code navigation + onCodeNavRequest?: (request: import('@plannotator/shared/code-nav').CodeNavRequest) => void; } export const DiffViewer: React.FC = ({ @@ -206,6 +209,7 @@ export const DiffViewer: React.FC = ({ aiMessages = [], onClickAIMarker, aiHistoryMessages = [], + onCodeNavRequest, }) => { const pierreTheme = usePierreTheme({ fontFamily, fontSize }); // containerRef must point at the actual scrolling element (the @@ -533,15 +537,23 @@ export const DiffViewer: React.FC = ({ // Token interaction handlers (code area clicks) const handleTokenClick = useCallback((props: DiffTokenEventBaseProps, event: MouseEvent) => { + if ((event.metaKey || event.ctrlKey) && onCodeNavRequest) { + onCodeNavRequest(buildCodeNavRequest(props, filePath)); + return; + } toolbarHostRef.current?.handleTokenClick(props, event); - }, []); + }, [filePath, onCodeNavRequest]); - const handleTokenEnter = useCallback((props: DiffTokenEventBaseProps) => { + const handleTokenEnter = useCallback((props: DiffTokenEventBaseProps, event: PointerEvent) => { props.tokenElement.classList.add('pn-token-hover'); - }, []); + if ((event.metaKey || event.ctrlKey) && onCodeNavRequest) { + props.tokenElement.classList.add('pn-token-nav'); + } + }, [onCodeNavRequest]); const handleTokenLeave = useCallback((props: DiffTokenEventBaseProps) => { props.tokenElement.classList.remove('pn-token-hover'); + props.tokenElement.classList.remove('pn-token-nav'); }, []); const splitGridStyle = useMemo(() => { diff --git a/packages/review-editor/dock/ReviewStateContext.tsx b/packages/review-editor/dock/ReviewStateContext.tsx index 2d96a4547..f9cf02a1b 100644 --- a/packages/review-editor/dock/ReviewStateContext.tsx +++ b/packages/review-editor/dock/ReviewStateContext.tsx @@ -99,6 +99,12 @@ export interface ReviewState { // Tour openTourPanel: (jobId: string) => void; + + // Code navigation + onCodeNavRequest?: (request: import('@plannotator/shared/code-nav').CodeNavRequest) => void; + codeNavResult: import('@plannotator/shared/code-nav').CodeNavResponse | null; + codeNavIsLoading: boolean; + codeNavActiveSymbol: string | null; } const ReviewStateContext = createContext(null); diff --git a/packages/review-editor/dock/panels/ReviewAllFilesDiffPanel.tsx b/packages/review-editor/dock/panels/ReviewAllFilesDiffPanel.tsx index 6e118d403..3ccc88959 100644 --- a/packages/review-editor/dock/panels/ReviewAllFilesDiffPanel.tsx +++ b/packages/review-editor/dock/panels/ReviewAllFilesDiffPanel.tsx @@ -43,6 +43,7 @@ export const ReviewAllFilesDiffPanel: React.FC = () => { isAILoading={state.isAILoading} onViewAIResponse={state.onViewAIResponse} aiHistoryForSelection={state.aiHistoryForSelection} + onCodeNavRequest={state.onCodeNavRequest} /> ); }; diff --git a/packages/review-editor/dock/panels/ReviewCodeNavPanel.tsx b/packages/review-editor/dock/panels/ReviewCodeNavPanel.tsx new file mode 100644 index 000000000..e6fe5d3c4 --- /dev/null +++ b/packages/review-editor/dock/panels/ReviewCodeNavPanel.tsx @@ -0,0 +1,266 @@ +import React, { useEffect, useMemo, useState, useCallback, useRef } from 'react'; +import type { IDockviewPanelProps } from 'dockview-react'; +import { useReviewState } from '../ReviewStateContext'; +import { useCodeNavPreview, type PreviewData } from '../../hooks/useCodeNavPreview'; +import { HighlightedCode } from '../../components/HighlightedCode'; +import { detectLanguage } from '../../utils/detectLanguage'; +import type { CodeNavLocation } from '@plannotator/shared/code-nav'; + +function basename(filePath: string): string { + const i = filePath.lastIndexOf('/'); + return i === -1 ? filePath : filePath.slice(i + 1); +} + +function dirname(filePath: string): string { + const i = filePath.lastIndexOf('/'); + return i === -1 ? '' : filePath.slice(0, i); +} + +interface FileGroup { + filePath: string; + locations: CodeNavLocation[]; +} + +function groupByFile(locations: CodeNavLocation[]): FileGroup[] { + const map = new Map(); + for (const loc of locations) { + const existing = map.get(loc.filePath); + if (existing) existing.push(loc); + else map.set(loc.filePath, [loc]); + } + return Array.from(map.entries()).map(([filePath, locations]) => ({ + filePath, + locations, + })); +} + +const CodePreview: React.FC<{ + preview: PreviewData | null; + isLoading: boolean; +}> = ({ preview, isLoading }) => { + const targetRef = useRef(null); + + useEffect(() => { + if (targetRef.current) { + targetRef.current.scrollIntoView({ block: 'center' }); + } + }, [preview?.targetLine, preview?.filePath]); + if (isLoading) { + return ( +
+
+
+ ); + } + + if (!preview) { + return ( +
+ Select a reference to preview +
+ ); + } + + const language = detectLanguage(preview.filePath); + + return ( +
+ + + {preview.lines.map((line, i) => { + const lineNum = preview.startLine + i; + const isTarget = lineNum === preview.targetLine; + const targetStyle = isTarget ? { backgroundColor: 'var(--muted)' } : undefined; + return ( + + + + + ); + })} + +
+ {lineNum} + + +
+
+ ); +}; + +const ReferenceList: React.FC<{ + groups: FileGroup[]; + selectedLocation: CodeNavLocation | null; + onSelect: (loc: CodeNavLocation) => void; +}> = ({ groups, selectedLocation, onSelect }) => { + const [collapsed, setCollapsed] = useState>(new Set()); + + const toggleCollapse = (filePath: string) => { + setCollapsed((prev) => { + const next = new Set(prev); + if (next.has(filePath)) next.delete(filePath); + else next.add(filePath); + return next; + }); + }; + + return ( +
+ {groups.map((group) => { + const isCollapsed = collapsed.has(group.filePath); + return ( +
+ + {!isCollapsed && ( +
+ {group.locations.map((loc, i) => { + const isSelected = + selectedLocation?.filePath === loc.filePath && + selectedLocation?.line === loc.line; + return ( + + ); + })} +
+ )} +
+ ); + })} +
+ ); +}; + +export const ReviewCodeNavPanel: React.FC = (props) => { + const state = useReviewState(); + const preview = useCodeNavPreview(); + const containerRef = useRef(null); + const { codeNavResult, codeNavIsLoading, codeNavActiveSymbol } = state; + + const allLocations = useMemo(() => { + if (!codeNavResult) return []; + return [...codeNavResult.definitions, ...codeNavResult.references]; + }, [codeNavResult]); + + const groups = useMemo(() => groupByFile(allLocations), [allLocations]); + + const [selectedLocation, setSelectedLocation] = useState(null); + + useEffect(() => { + if (allLocations.length > 0) { + const first = allLocations[0]; + setSelectedLocation(first); + preview.selectLocation(first.filePath, first.line); + } else { + setSelectedLocation(null); + preview.clear(); + } + }, [allLocations]); + + const handleSelect = useCallback( + (loc: CodeNavLocation) => { + setSelectedLocation(loc); + preview.selectLocation(loc.filePath, loc.line); + }, + [preview.selectLocation], + ); + + useEffect(() => { + const el = containerRef.current; + if (!el) return; + const handler = (e: KeyboardEvent) => { + if (e.key === 'Escape') { + e.stopPropagation(); + props.api.close(); + } + }; + el.addEventListener('keydown', handler); + return () => el.removeEventListener('keydown', handler); + }, [props.api]); + + if (codeNavIsLoading) { + return ( +
+
+ Searching for {codeNavActiveSymbol} +
+ ); + } + + if (codeNavResult?.backend === 'unavailable') { + return ( +
+ Install ripgrep for code navigation +
+ ); + } + + if (!codeNavResult || allLocations.length === 0) { + return ( +
+ No results for {codeNavActiveSymbol} +
+ ); + } + + return ( +
+
+
+ +
+
+ +
+
+
+ ); +}; diff --git a/packages/review-editor/dock/panels/ReviewDiffPanel.tsx b/packages/review-editor/dock/panels/ReviewDiffPanel.tsx index 5e08df4fb..8498fcc68 100644 --- a/packages/review-editor/dock/panels/ReviewDiffPanel.tsx +++ b/packages/review-editor/dock/panels/ReviewDiffPanel.tsx @@ -113,6 +113,7 @@ export const ReviewDiffPanel: React.FC = (props) => { aiMessages={aiMessagesForFile} onClickAIMarker={state.onClickAIMarker} aiHistoryMessages={isFocusedFile ? state.aiHistoryForSelection : []} + onCodeNavRequest={state.onCodeNavRequest} />
); diff --git a/packages/review-editor/dock/reviewPanelComponents.ts b/packages/review-editor/dock/reviewPanelComponents.ts index 883e1671e..8e5799eb7 100644 --- a/packages/review-editor/dock/reviewPanelComponents.ts +++ b/packages/review-editor/dock/reviewPanelComponents.ts @@ -5,6 +5,7 @@ import { ReviewPRSummaryPanel } from './panels/ReviewPRSummaryPanel'; import { ReviewPRCommentsPanel } from './panels/ReviewPRCommentsPanel'; import { ReviewPRChecksPanel } from './panels/ReviewPRChecksPanel'; import { ReviewAllFilesDiffPanel } from './panels/ReviewAllFilesDiffPanel'; +import { ReviewCodeNavPanel } from './panels/ReviewCodeNavPanel'; /** * Component registry for dockview — maps panel type strings to React components. @@ -17,4 +18,5 @@ export const reviewPanelComponents = { [REVIEW_PANEL_TYPES.PR_COMMENTS]: ReviewPRCommentsPanel, [REVIEW_PANEL_TYPES.PR_CHECKS]: ReviewPRChecksPanel, [REVIEW_PANEL_TYPES.ALL_FILES]: ReviewAllFilesDiffPanel, + [REVIEW_PANEL_TYPES.CODE_NAV]: ReviewCodeNavPanel, } as const; diff --git a/packages/review-editor/dock/reviewPanelTypes.ts b/packages/review-editor/dock/reviewPanelTypes.ts index 73aebeef6..67ff72d96 100644 --- a/packages/review-editor/dock/reviewPanelTypes.ts +++ b/packages/review-editor/dock/reviewPanelTypes.ts @@ -12,6 +12,7 @@ export const REVIEW_PANEL_TYPES = { PR_COMMENTS: 'review-pr-comments', PR_CHECKS: 'review-pr-checks', ALL_FILES: 'review-all-files', + CODE_NAV: 'review-code-nav', } as const; export const REVIEW_DIFF_PANEL_ID = 'review-diff'; @@ -27,6 +28,7 @@ export const REVIEW_PR_SUMMARY_PANEL_ID = 'review-pr-summary'; export const REVIEW_PR_COMMENTS_PANEL_ID = 'review-pr-comments'; export const REVIEW_PR_CHECKS_PANEL_ID = 'review-pr-checks'; export const REVIEW_ALL_FILES_PANEL_ID = 'review-all-files'; +export const REVIEW_CODE_NAV_PANEL_ID = 'review-code-nav'; export function isReviewDiffPanelId(panelId: string): boolean { return panelId === REVIEW_DIFF_PANEL_ID; diff --git a/packages/review-editor/hooks/useCodeNav.ts b/packages/review-editor/hooks/useCodeNav.ts new file mode 100644 index 000000000..e0930fdb8 --- /dev/null +++ b/packages/review-editor/hooks/useCodeNav.ts @@ -0,0 +1,49 @@ +import { useState, useCallback, useRef } from 'react'; +import type { CodeNavRequest, CodeNavResponse } from '@plannotator/shared/code-nav'; + +export type { CodeNavRequest, CodeNavResponse }; + +export function useCodeNav() { + const [result, setResult] = useState(null); + const [isLoading, setIsLoading] = useState(false); + const [activeSymbol, setActiveSymbol] = useState(null); + const abortRef = useRef(null); + + const resolve = useCallback(async (request: CodeNavRequest) => { + abortRef.current?.abort(); + const controller = new AbortController(); + abortRef.current = controller; + + setActiveSymbol(request.symbol); + setIsLoading(true); + setResult(null); + + try { + const res = await fetch('/api/code-nav/resolve', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(request), + signal: controller.signal, + }); + if (!res.ok) throw new Error('Failed'); + const data: CodeNavResponse = await res.json(); + setResult(data); + } catch (err) { + if (err instanceof DOMException && err.name === 'AbortError') return; + setResult(null); + } finally { + if (abortRef.current === controller) { + setIsLoading(false); + } + } + }, []); + + const clear = useCallback(() => { + abortRef.current?.abort(); + setResult(null); + setActiveSymbol(null); + setIsLoading(false); + }, []); + + return { result, isLoading, activeSymbol, resolve, clear }; +} diff --git a/packages/review-editor/hooks/useCodeNavPreview.ts b/packages/review-editor/hooks/useCodeNavPreview.ts new file mode 100644 index 000000000..50e77fd12 --- /dev/null +++ b/packages/review-editor/hooks/useCodeNavPreview.ts @@ -0,0 +1,82 @@ +import { useState, useCallback, useRef } from 'react'; + +const MAX_CACHE_ENTRIES = 10; + +export interface PreviewData { + lines: string[]; + startLine: number; + targetLine: number; + filePath: string; +} + +export function useCodeNavPreview() { + const [previewData, setPreviewData] = useState(null); + const [isLoading, setIsLoading] = useState(false); + const cacheRef = useRef(new Map()); + const abortRef = useRef(null); + + const selectLocation = useCallback( + async (filePath: string, line: number) => { + abortRef.current?.abort(); + const controller = new AbortController(); + abortRef.current = controller; + + const cache = cacheRef.current; + const cached = cache.get(filePath); + + if (cached) { + setIsLoading(false); + const allLines = cached.split('\n'); + setPreviewData({ + lines: allLines, + startLine: 1, + targetLine: line, + filePath, + }); + return; + } + + setIsLoading(true); + try { + const res = await fetch( + `/api/code-nav/file?path=${encodeURIComponent(filePath)}`, + { signal: controller.signal }, + ); + if (!res.ok) throw new Error('Failed'); + const data: { content: string } = await res.json(); + + if (cache.size >= MAX_CACHE_ENTRIES) { + const firstKey = cache.keys().next().value; + if (firstKey) cache.delete(firstKey); + } + cache.set(filePath, data.content); + + if (controller.signal.aborted) return; + + const allLines = data.content.split('\n'); + setPreviewData({ + lines: allLines, + startLine: 1, + targetLine: line, + filePath, + }); + } catch (err) { + if (err instanceof DOMException && err.name === 'AbortError') return; + setPreviewData(null); + } finally { + if (abortRef.current === controller) { + setIsLoading(false); + } + } + }, + [], + ); + + const clear = useCallback(() => { + abortRef.current?.abort(); + setPreviewData(null); + setIsLoading(false); + }, []); + + return { previewData, isLoading, selectLocation, clear }; +} diff --git a/packages/review-editor/hooks/usePierreTheme.ts b/packages/review-editor/hooks/usePierreTheme.ts index 95c59d80f..655febc00 100644 --- a/packages/review-editor/hooks/usePierreTheme.ts +++ b/packages/review-editor/hooks/usePierreTheme.ts @@ -225,6 +225,11 @@ export function usePierreTheme(options?: { fontFamily?: string; fontSize?: strin text-underline-offset: 2px; cursor: pointer; } + .pn-token-nav { + text-decoration-thickness: 2px; + cursor: pointer; + opacity: 0.85; + } /* Separator bars — slimmer, semi-transparent, integrated with theme */ :host { diff --git a/packages/review-editor/index.css b/packages/review-editor/index.css index a6c37ac83..d11bee39a 100644 --- a/packages/review-editor/index.css +++ b/packages/review-editor/index.css @@ -1283,3 +1283,9 @@ diffs-container { .pr-switch-overlay { animation: overlay-fade-in 0.15s ease-out both; } + +/* Code navigation peek panel — strip hljs hardcoded background */ +.code-nav-peek code, +.code-nav-peek .hljs { + background: transparent !important; +} diff --git a/packages/review-editor/utils/buildCodeNavRequest.ts b/packages/review-editor/utils/buildCodeNavRequest.ts new file mode 100644 index 000000000..0ad6dd7f1 --- /dev/null +++ b/packages/review-editor/utils/buildCodeNavRequest.ts @@ -0,0 +1,17 @@ +import type { CodeNavRequest } from '@plannotator/shared/code-nav'; +import type { DiffTokenEventBaseProps } from '@pierre/diffs'; +import { detectLanguage } from './detectLanguage'; + +export function buildCodeNavRequest( + props: DiffTokenEventBaseProps, + filePath: string, +): CodeNavRequest { + return { + symbol: props.tokenText, + filePath, + line: props.lineNumber, + charStart: props.lineCharStart, + side: props.side === 'additions' ? 'new' : 'old', + language: detectLanguage(filePath), + }; +} diff --git a/packages/server/code-nav.ts b/packages/server/code-nav.ts new file mode 100644 index 000000000..26e4f0cb4 --- /dev/null +++ b/packages/server/code-nav.ts @@ -0,0 +1,73 @@ +/** + * Code navigation — Bun runtime adapter and request handler. + */ + +import { + type CodeNavRequest, + type CodeNavRuntime, + type CodeNavResponse, + resolveCodeNav, + validateCodeNavRequest, + extractChangedFiles, +} from "@plannotator/shared/code-nav"; + +export type { CodeNavRequest, CodeNavResponse }; + +const bunCodeNavRuntime: CodeNavRuntime = { + async runCommand(command, args, options) { + let proc; + try { + proc = Bun.spawn([command, ...args], { + cwd: options?.cwd, + stdout: "pipe", + stderr: "pipe", + }); + } catch { + return { stdout: "", stderr: "command not found", exitCode: 1 }; + } + + let timer: ReturnType | undefined; + if (options?.timeoutMs) { + timer = setTimeout(() => proc.kill(), options.timeoutMs); + } + + const [stdout, stderr, exitCode] = await Promise.all([ + new Response(proc.stdout).text(), + new Response(proc.stderr).text(), + proc.exited, + ]); + + if (timer) clearTimeout(timer); + return { stdout, stderr, exitCode }; + }, +}; + +export async function handleCodeNavResolve( + req: Request, + cwd: string, + changedFiles: string[], +): Promise { + try { + const body = (await req.json()) as CodeNavRequest; + const error = validateCodeNavRequest(body); + if (error) { + return Response.json({ error }, { status: 400 }); + } + + const result = await resolveCodeNav( + bunCodeNavRuntime, + body, + cwd, + changedFiles, + ); + + return Response.json(result); + } catch (err) { + return Response.json( + { error: err instanceof Error ? err.message : "Code navigation failed" }, + { status: 500 }, + ); + } +} + +export { extractChangedFiles }; diff --git a/packages/server/review.ts b/packages/server/review.ts index 4e4c12488..c31646398 100644 --- a/packages/server/review.ts +++ b/packages/server/review.ts @@ -48,6 +48,7 @@ import { loadConfig, saveConfig, detectGitUser, getServerConfig } from "./config import { type PRMetadata, type PRReviewFileComment, type PRStackTree, type PRListItem, fetchPR, fetchPRFileContent, fetchPRContext, submitPRReview, fetchPRViewedFiles, markPRFilesViewed, fetchPRStack, fetchPRList, getPRUser, parsePRUrl, prRefFromMetadata, isSameProject, getDisplayRepo, getMRLabel, getMRNumberLabel } from "./pr"; import { createAIEndpoints, ProviderRegistry, SessionManager, createProvider, type AIEndpoints, type PiSDKConfig } from "@plannotator/ai"; import { isWSL } from "./browser"; +import { handleCodeNavResolve, extractChangedFiles } from "./code-nav"; // Re-export utilities export { isRemoteSession, getServerPort } from "./remote"; @@ -902,6 +903,42 @@ export async function startReviewServer( return Response.json({ error: "No file access available" }, { status: 400 }); } + // API: Code navigation (search-based symbol resolution) + if (url.pathname === "/api/code-nav/resolve" && req.method === "POST") { + const hasCodeNavAccess = !!gitContext || !!options.agentCwd || !!options.worktreePool; + if (!hasCodeNavAccess) { + return Response.json( + { error: "Code navigation requires local access" }, + { status: 400 }, + ); + } + const navCwd = resolveAgentCwd(); + const changedFiles = extractChangedFiles(currentPatch); + return handleCodeNavResolve(req, navCwd, changedFiles); + } + + // API: Code navigation file preview (read file from working tree) + if (url.pathname === "/api/code-nav/file" && req.method === "GET") { + const hasCodeNavAccess = !!gitContext || !!options.agentCwd || !!options.worktreePool; + if (!hasCodeNavAccess) { + return Response.json({ error: "Code navigation requires local access" }, { status: 400 }); + } + const filePath = url.searchParams.get("path"); + if (!filePath) { + return Response.json({ error: "Missing path" }, { status: 400 }); + } + try { validateFilePath(filePath); } catch { + return Response.json({ error: "Invalid path" }, { status: 400 }); + } + try { + const navCwd = resolveAgentCwd(); + const content = await Bun.file(`${navCwd}/${filePath}`).text(); + return Response.json({ content }); + } catch { + return Response.json({ error: "File not found" }, { status: 404 }); + } + } + // API: Stage / unstage a file (disabled when VCS doesn't support it) if (url.pathname === "/api/git-add" && req.method === "POST") { const stageCwd = resolveVcsCwd(currentDiffType, gitContext?.cwd); diff --git a/packages/shared/code-nav.test.ts b/packages/shared/code-nav.test.ts new file mode 100644 index 000000000..75aa96c55 --- /dev/null +++ b/packages/shared/code-nav.test.ts @@ -0,0 +1,515 @@ +import { describe, expect, test } from "bun:test"; +import { + buildRgArgs, + classifyMatch, + rankLocations, + parseRgJsonOutput, + validateCodeNavRequest, + extractChangedFiles, + type CodeNavLocation, +} from "./code-nav"; + +// --------------------------------------------------------------------------- +// classifyMatch +// --------------------------------------------------------------------------- + +describe("classifyMatch", () => { + describe("TypeScript/JavaScript", () => { + const lang = "typescript"; + + test("function declaration", () => { + expect(classifyMatch("function startServer(", "startServer", lang)).toBe("definition"); + }); + + test("async function declaration", () => { + expect(classifyMatch("export async function startServer(", "startServer", lang)).toBe("definition"); + }); + + test("export function", () => { + expect(classifyMatch("export function handleRequest(", "handleRequest", lang)).toBe("definition"); + }); + + test("const assignment", () => { + expect(classifyMatch("const startServer = async () => {", "startServer", lang)).toBe("definition"); + }); + + test("let assignment", () => { + expect(classifyMatch("let counter = 0;", "counter", lang)).toBe("definition"); + }); + + test("class declaration", () => { + expect(classifyMatch("export class ReviewServer {", "ReviewServer", lang)).toBe("definition"); + }); + + test("interface declaration", () => { + expect(classifyMatch("export interface CodeNavRequest {", "CodeNavRequest", lang)).toBe("definition"); + }); + + test("type declaration", () => { + expect(classifyMatch("type DiffType = 'unified' | 'split';", "DiffType", lang)).toBe("definition"); + }); + + test("enum declaration", () => { + expect(classifyMatch("enum Status {", "Status", lang)).toBe("definition"); + }); + + test("method in class/object", () => { + expect(classifyMatch(" async handleRequest(", "handleRequest", lang)).toBe("definition"); + }); + + test("plain reference (function call)", () => { + expect(classifyMatch(" const result = startServer(config);", "startServer", lang)).toBe("reference"); + }); + + test("bare indented call is not a definition", () => { + expect(classifyMatch(" startServer(config);", "startServer", lang)).toBe("reference"); + }); + + test("indented call in if/return is not a definition", () => { + expect(classifyMatch(" return startServer(config);", "startServer", lang)).toBe("reference"); + }); + + test("plain reference (import)", () => { + expect(classifyMatch('import { startServer } from "./server";', "startServer", lang)).toBe("reference"); + }); + + test("const with type annotation", () => { + expect(classifyMatch("const runtime: CodeNavRuntime = {", "runtime", lang)).toBe("definition"); + }); + }); + + describe("Python", () => { + const lang = "python"; + + test("def function", () => { + expect(classifyMatch("def handle_request(self, req):", "handle_request", lang)).toBe("definition"); + }); + + test("class declaration", () => { + expect(classifyMatch("class ReviewServer:", "ReviewServer", lang)).toBe("definition"); + }); + + test("top-level assignment", () => { + expect(classifyMatch("DEFAULT_PORT = 8080", "DEFAULT_PORT", lang)).toBe("definition"); + }); + + test("plain reference", () => { + expect(classifyMatch(" server = ReviewServer()", "ReviewServer", lang)).toBe("reference"); + }); + }); + + describe("Go", () => { + const lang = "go"; + + test("func declaration", () => { + expect(classifyMatch("func StartServer(config Config) error {", "StartServer", lang)).toBe("definition"); + }); + + test("method declaration", () => { + expect(classifyMatch("func (s *Server) StartServer() error {", "StartServer", lang)).toBe("definition"); + }); + + test("type declaration", () => { + expect(classifyMatch("type Config struct {", "Config", lang)).toBe("definition"); + }); + + test("plain reference", () => { + expect(classifyMatch(" err := StartServer(cfg)", "StartServer", lang)).toBe("reference"); + }); + }); + + describe("Rust", () => { + const lang = "rust"; + + test("fn declaration", () => { + expect(classifyMatch("fn start_server() -> Result<()> {", "start_server", lang)).toBe("definition"); + }); + + test("pub fn declaration", () => { + expect(classifyMatch("pub fn start_server(config: Config) {", "start_server", lang)).toBe("definition"); + }); + + test("struct declaration", () => { + expect(classifyMatch("pub struct Config {", "Config", lang)).toBe("definition"); + }); + + test("enum declaration", () => { + expect(classifyMatch("pub enum Status {", "Status", lang)).toBe("definition"); + }); + + test("trait declaration", () => { + expect(classifyMatch("pub trait Handler {", "Handler", lang)).toBe("definition"); + }); + + test("plain reference", () => { + expect(classifyMatch(" let server = start_server(config);", "start_server", lang)).toBe("reference"); + }); + }); + + describe("generic fallback", () => { + test("function keyword (unknown language)", () => { + expect(classifyMatch("function startServer(", "startServer")).toBe("definition"); + }); + + test("class keyword (unknown language)", () => { + expect(classifyMatch("class MyClass {", "MyClass")).toBe("definition"); + }); + + test("const keyword (unknown language)", () => { + expect(classifyMatch("const PORT = 8080;", "PORT")).toBe("definition"); + }); + + test("no definition pattern matches", () => { + expect(classifyMatch(" startServer(config);", "startServer")).toBe("reference"); + }); + }); + + describe("edge cases", () => { + test("regex metacharacter in symbol ($)", () => { + expect(classifyMatch("const $el = document.querySelector('div');", "$el", "typescript")).toBe("definition"); + }); + + test("regex metacharacter in symbol (.)", () => { + expect(classifyMatch(" obj.method();", "obj.method")).toBe("reference"); + }); + }); +}); + +// --------------------------------------------------------------------------- +// rankLocations +// --------------------------------------------------------------------------- + +describe("rankLocations", () => { + function loc(overrides: Partial): CodeNavLocation { + return { + kind: "reference", + confidence: "possible", + filePath: "src/other.ts", + line: 1, + column: 0, + snippet: "some code", + ...overrides, + }; + } + + test("same file ranks first", () => { + const locations = [ + loc({ filePath: "src/other.ts", line: 10 }), + loc({ filePath: "src/main.ts", line: 5 }), + ]; + const result = rankLocations(locations, { + sourceFilePath: "src/main.ts", + changedFiles: [], + isTestFile: false, + }); + expect(result.references[0].filePath).toBe("src/main.ts"); + }); + + test("changed files rank above non-changed", () => { + const locations = [ + loc({ filePath: "lib/utils.ts" }), + loc({ filePath: "src/changed.ts" }), + ]; + const result = rankLocations(locations, { + sourceFilePath: "src/main.ts", + changedFiles: ["src/changed.ts"], + isTestFile: false, + }); + expect(result.references[0].filePath).toBe("src/changed.ts"); + }); + + test("definitions rank above references in same tier", () => { + const locations = [ + loc({ filePath: "src/a.ts", kind: "reference" }), + loc({ filePath: "src/a.ts", kind: "definition", confidence: "likely" }), + ]; + const result = rankLocations(locations, { + sourceFilePath: "src/main.ts", + changedFiles: [], + isTestFile: false, + }); + expect(result.definitions).toHaveLength(1); + expect(result.references).toHaveLength(1); + }); + + test("test files demoted when source is not a test", () => { + const locations = [ + loc({ filePath: "src/__tests__/main.test.ts", kind: "reference" }), + loc({ filePath: "src/utils.ts", kind: "reference" }), + ]; + const result = rankLocations(locations, { + sourceFilePath: "src/main.ts", + changedFiles: [], + isTestFile: false, + }); + expect(result.references[0].filePath).toBe("src/utils.ts"); + }); + + test("test files NOT demoted when source is a test", () => { + const locations = [ + loc({ filePath: "src/__tests__/main.test.ts", kind: "reference" }), + loc({ filePath: "src/utils.ts", kind: "reference" }), + ]; + const result = rankLocations(locations, { + sourceFilePath: "src/__tests__/other.test.ts", + changedFiles: [], + isTestFile: true, + }); + expect(result.references[0].filePath).toBe("src/__tests__/main.test.ts"); + }); + + test("caps results", () => { + const locations = Array.from({ length: 100 }, (_, i) => + loc({ filePath: `src/file${i}.ts`, line: i }), + ); + const result = rankLocations(locations, { + sourceFilePath: "src/main.ts", + changedFiles: [], + isTestFile: false, + }, 10); + expect(result.references).toHaveLength(10); + expect(result.capped).toBe(true); + }); + + test("not capped when under limit", () => { + const locations = [loc({}), loc({})]; + const result = rankLocations(locations, { + sourceFilePath: "src/main.ts", + changedFiles: [], + isTestFile: false, + }); + expect(result.capped).toBe(false); + }); +}); + +// --------------------------------------------------------------------------- +// buildRgArgs +// --------------------------------------------------------------------------- + +describe("buildRgArgs", () => { + test("includes --json flag", () => { + const args = buildRgArgs("mySymbol"); + expect(args).toContain("--json"); + }); + + test("includes --word-regexp", () => { + const args = buildRgArgs("mySymbol"); + expect(args).toContain("--word-regexp"); + }); + + test("includes glob exclusions for node_modules", () => { + const args = buildRgArgs("mySymbol"); + const nodeModulesIdx = args.indexOf("!node_modules"); + expect(nodeModulesIdx).toBeGreaterThan(-1); + }); + + test("escapes regex metacharacters", () => { + const args = buildRgArgs("$scope"); + const patternIdx = args.indexOf("--") + 1; + expect(args[patternIdx]).toContain("\\$scope"); + }); + + test("includes max-count", () => { + const args = buildRgArgs("x"); + const idx = args.indexOf("--max-count"); + expect(idx).toBeGreaterThan(-1); + expect(args[idx + 1]).toBe("50"); + }); + + test("searches from current directory", () => { + const args = buildRgArgs("x"); + expect(args[args.length - 1]).toBe("."); + }); + + test("adds --type filter for known language", () => { + const args = buildRgArgs("x", "typescript"); + const typeIdx = args.indexOf("--type"); + expect(typeIdx).toBeGreaterThan(-1); + expect(args[typeIdx + 1]).toBe("ts"); + }); + + test("no --type filter for unknown language", () => { + const args = buildRgArgs("x", "brainfuck"); + expect(args).not.toContain("--type"); + }); + + test("no --type filter when language is undefined", () => { + const args = buildRgArgs("x"); + expect(args).not.toContain("--type"); + }); +}); + +// --------------------------------------------------------------------------- +// parseRgJsonOutput +// --------------------------------------------------------------------------- + +describe("parseRgJsonOutput", () => { + test("parses match lines", () => { + const lines = [ + JSON.stringify({ + type: "match", + data: { + path: { text: "src/server.ts" }, + lines: { text: "export function startServer() {\n" }, + line_number: 42, + submatches: [{ start: 16, end: 27, match: { text: "startServer" } }], + }, + }), + JSON.stringify({ type: "summary", data: {} }), + ].join("\n"); + + const result = parseRgJsonOutput(lines, "startServer", "typescript"); + expect(result).toHaveLength(1); + expect(result[0].filePath).toBe("src/server.ts"); + expect(result[0].line).toBe(42); + expect(result[0].column).toBe(16); + expect(result[0].kind).toBe("definition"); + expect(result[0].confidence).toBe("likely"); + }); + + test("classifies references correctly", () => { + const lines = JSON.stringify({ + type: "match", + data: { + path: { text: "src/index.ts" }, + lines: { text: " const s = startServer();\n" }, + line_number: 10, + submatches: [{ start: 14, end: 25, match: { text: "startServer" } }], + }, + }); + + const result = parseRgJsonOutput(lines, "startServer", "typescript"); + expect(result[0].kind).toBe("reference"); + expect(result[0].confidence).toBe("possible"); + }); + + test("skips non-JSON lines", () => { + const result = parseRgJsonOutput("not json\n\n", "x"); + expect(result).toHaveLength(0); + }); + + test("strips leading ./ from file paths", () => { + const line = JSON.stringify({ + type: "match", + data: { + path: { text: "./src/server.ts" }, + lines: { text: " startServer();\n" }, + line_number: 10, + submatches: [{ start: 2, end: 13, match: { text: "startServer" } }], + }, + }); + const result = parseRgJsonOutput(line, "startServer"); + expect(result[0].filePath).toBe("src/server.ts"); + }); + + test("skips non-match type lines", () => { + const line = JSON.stringify({ type: "begin", data: { path: { text: "a.ts" } } }); + const result = parseRgJsonOutput(line, "x"); + expect(result).toHaveLength(0); + }); + + test("truncates long snippets", () => { + const longLine = "x".repeat(300); + const line = JSON.stringify({ + type: "match", + data: { + path: { text: "a.ts" }, + lines: { text: longLine }, + line_number: 1, + submatches: [{ start: 0, end: 1 }], + }, + }); + const result = parseRgJsonOutput(line, "x"); + expect(result[0].snippet.length).toBeLessThanOrEqual(201); + }); +}); + +// --------------------------------------------------------------------------- +// validateCodeNavRequest +// --------------------------------------------------------------------------- + +describe("validateCodeNavRequest", () => { + const valid = { + symbol: "startServer", + filePath: "src/server.ts", + line: 42, + charStart: 10, + side: "new" as const, + language: "typescript", + }; + + test("accepts valid request", () => { + expect(validateCodeNavRequest(valid)).toBeNull(); + }); + + test("rejects null body", () => { + expect(validateCodeNavRequest(null)).toBe("Invalid request body"); + }); + + test("rejects empty symbol", () => { + expect(validateCodeNavRequest({ ...valid, symbol: "" })).toBe("Missing or empty symbol"); + }); + + test("rejects missing filePath", () => { + expect(validateCodeNavRequest({ ...valid, filePath: "" })).toBe("Missing filePath"); + }); + + test("rejects directory traversal", () => { + expect(validateCodeNavRequest({ ...valid, filePath: "../etc/passwd" })).toBe("Invalid filePath"); + }); + + test("rejects absolute path", () => { + expect(validateCodeNavRequest({ ...valid, filePath: "/etc/passwd" })).toBe("Invalid filePath"); + }); + + test("rejects invalid side", () => { + expect(validateCodeNavRequest({ ...valid, side: "both" })).toBe("side must be 'old' or 'new'"); + }); +}); + +// --------------------------------------------------------------------------- +// extractChangedFiles +// --------------------------------------------------------------------------- + +describe("extractChangedFiles", () => { + test("extracts paths from unified diff", () => { + const patch = `diff --git a/src/server.ts b/src/server.ts +--- a/src/server.ts ++++ b/src/server.ts +@@ -1,3 +1,3 @@ +diff --git a/src/utils.ts b/src/utils.ts +--- a/src/utils.ts ++++ b/src/utils.ts`; + + const result = extractChangedFiles(patch); + expect(result).toEqual(["src/server.ts", "src/utils.ts"]); + }); + + test("extracts both old and new paths for renames", () => { + const patch = `diff --git a/src/oldName.ts b/src/newName.ts +similarity index 95% +rename from src/oldName.ts +rename to src/newName.ts`; + + const result = extractChangedFiles(patch); + expect(result).toContain("src/oldName.ts"); + expect(result).toContain("src/newName.ts"); + }); + + test("deduplicates when paths are the same", () => { + const patch = `diff --git a/src/server.ts b/src/server.ts +--- a/src/server.ts ++++ b/src/server.ts`; + + const result = extractChangedFiles(patch); + expect(result).toEqual(["src/server.ts"]); + }); + + test("returns empty for null patch", () => { + expect(extractChangedFiles(null)).toEqual([]); + }); + + test("returns empty for empty string", () => { + expect(extractChangedFiles("")).toEqual([]); + }); +}); diff --git a/packages/shared/code-nav.ts b/packages/shared/code-nav.ts new file mode 100644 index 000000000..4e05262f0 --- /dev/null +++ b/packages/shared/code-nav.ts @@ -0,0 +1,436 @@ +/** + * Search-based code navigation — shared types and pure logic. + * + * Runtime-agnostic: both Bun and Node servers provide their own + * CodeNavRuntime implementation to run subprocess commands. + */ + +function validateFilePath(filePath: string): void { + if (filePath.includes("..") || filePath.startsWith("/")) { + throw new Error("Invalid file path"); + } +} + +// --------------------------------------------------------------------------- +// Types +// --------------------------------------------------------------------------- + +export interface CodeNavRequest { + symbol: string; + filePath: string; + line: number; + charStart: number; + side: "old" | "new"; + language?: string; +} + +export interface CodeNavLocation { + kind: "definition" | "reference"; + confidence: "likely" | "possible"; + filePath: string; + line: number; + column: number; + snippet: string; +} + +export interface CodeNavResponse { + backend: "search" | "unavailable"; + complete: boolean; + definitions: CodeNavLocation[]; + references: CodeNavLocation[]; + stats: { elapsedMs: number; capped: boolean }; + searchScope: "head"; +} + +export interface CodeNavRuntime { + runCommand: ( + command: string, + args: string[], + options?: { cwd?: string; timeoutMs?: number }, + ) => Promise<{ stdout: string; stderr: string; exitCode: number }>; +} + +// --------------------------------------------------------------------------- +// Constants +// --------------------------------------------------------------------------- + +const CODE_NAV_IGNORED_GLOBS = [ + "node_modules", + ".git", + "dist", + "build", + ".next", + "__pycache__", + ".turbo", + ".cache", + "target", + "vendor", + "coverage", + ".venv", + ".pytest_cache", +]; + +const RG_TYPE_MAP: Record = { + typescript: "ts", + javascript: "js", + python: "py", + go: "go", + rust: "rust", + java: "java", + ruby: "ruby", + cpp: "cpp", + c: "c", +}; + +// --------------------------------------------------------------------------- +// Definition patterns +// --------------------------------------------------------------------------- + +interface DefinitionPatternSet { + languages: string[]; + patterns: string[]; +} + +const DEFINITION_PATTERNS: DefinitionPatternSet[] = [ + { + languages: ["typescript", "javascript"], + patterns: [ + String.raw`(?:export\s+)?(?:async\s+)?function\s+SYMBOL\b`, + String.raw`(?:export\s+)?(?:const|let|var)\s+SYMBOL\s*[=:]`, + String.raw`(?:export\s+)?class\s+SYMBOL\b`, + String.raw`(?:export\s+)?(?:interface|type)\s+SYMBOL\b`, + String.raw`(?:export\s+)?enum\s+SYMBOL\b`, + String.raw`^\s+(?:(?:async|static|readonly|get|set|private|protected|public)\s+)+SYMBOL\s*[(<:]`, + ], + }, + { + languages: ["python"], + patterns: [ + String.raw`(?:^|\s)def\s+SYMBOL\s*\(`, + String.raw`(?:^|\s)class\s+SYMBOL\b`, + String.raw`^SYMBOL\s*=`, + ], + }, + { + languages: ["go"], + patterns: [ + String.raw`func\s+(?:\([^)]+\)\s+)?SYMBOL\s*\(`, + String.raw`type\s+SYMBOL\s`, + String.raw`var\s+SYMBOL\s`, + ], + }, + { + languages: ["rust"], + patterns: [ + String.raw`(?:pub(?:\([^)]*\))?\s+)?fn\s+SYMBOL\b`, + String.raw`(?:pub(?:\([^)]*\))?\s+)?struct\s+SYMBOL\b`, + String.raw`(?:pub(?:\([^)]*\))?\s+)?enum\s+SYMBOL\b`, + String.raw`(?:pub(?:\([^)]*\))?\s+)?trait\s+SYMBOL\b`, + String.raw`(?:pub(?:\([^)]*\))?\s+)?type\s+SYMBOL\b`, + String.raw`(?:pub(?:\([^)]*\))?\s+)?mod\s+SYMBOL\b`, + ], + }, +]; + +const GENERIC_DEFINITION_PATTERNS: string[] = [ + String.raw`(?:function|def|func|fn|class|struct|enum|trait|interface|type)\s+SYMBOL\b`, + String.raw`(?:const|let|var|val)\s+SYMBOL\s*[=:]`, +]; + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +function escapeRegex(str: string): string { + return str.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); +} + +function sameDirectory(a: string, b: string): boolean { + const dirA = a.lastIndexOf("/"); + const dirB = b.lastIndexOf("/"); + if (dirA === -1 && dirB === -1) return true; + return a.slice(0, dirA) === b.slice(0, dirB); +} + +function isTestFile(filePath: string): boolean { + return /(?:test|spec|__tests__|_test\.|\.test\.|\.spec\.)/i.test(filePath); +} + +// --------------------------------------------------------------------------- +// rg argument construction +// --------------------------------------------------------------------------- + +export function buildRgArgs(symbol: string, language?: string): string[] { + const args: string[] = [ + "--json", + "--line-number", + "--column", + "--max-count", + "50", + "--max-filesize", + "1M", + "--no-messages", + ]; + + for (const dir of CODE_NAV_IGNORED_GLOBS) { + args.push("--glob", `!${dir}`); + } + + if (language) { + const rgType = RG_TYPE_MAP[language]; + if (rgType) args.push("--type", rgType); + } + + args.push("--word-regexp", "--", escapeRegex(symbol), "."); + + return args; +} + +// --------------------------------------------------------------------------- +// rg JSON output parsing +// --------------------------------------------------------------------------- + +interface RgMatchData { + path: { text: string }; + lines: { text: string }; + line_number: number; + submatches: Array<{ start: number; end: number }>; +} + +const PARSE_CAP = 500; + +export function parseRgJsonOutput( + stdout: string, + symbol: string, + language?: string, +): CodeNavLocation[] { + const locations: CodeNavLocation[] = []; + const lines = stdout.split("\n"); + + for (const line of lines) { + if (locations.length >= PARSE_CAP) break; + if (!line.trim()) continue; + + let parsed: { type: string; data: RgMatchData }; + try { + parsed = JSON.parse(line); + } catch { + continue; + } + + if (parsed.type !== "match") continue; + + const d = parsed.data; + const snippet = d.lines.text.trimEnd(); + const column = d.submatches?.[0]?.start ?? 0; + const kind = classifyMatch(snippet, symbol, language); + const filePath = d.path.text.startsWith("./") + ? d.path.text.slice(2) + : d.path.text; + + locations.push({ + kind, + confidence: kind === "definition" ? "likely" : "possible", + filePath, + line: d.line_number, + column, + snippet: snippet.length > 200 ? snippet.slice(0, 200) + "…" : snippet, + }); + } + + return locations; +} + +// --------------------------------------------------------------------------- +// Match classification +// --------------------------------------------------------------------------- + +export function classifyMatch( + snippet: string, + symbol: string, + language?: string, +): "definition" | "reference" { + const escaped = escapeRegex(symbol); + + if (language) { + const langPatterns = DEFINITION_PATTERNS.find((p) => + p.languages.includes(language), + ); + if (langPatterns) { + for (const pattern of langPatterns.patterns) { + const re = new RegExp(pattern.replace("SYMBOL", escaped)); + if (re.test(snippet)) return "definition"; + } + } + } + + for (const pattern of GENERIC_DEFINITION_PATTERNS) { + const re = new RegExp(pattern.replace("SYMBOL", escaped)); + if (re.test(snippet)) return "definition"; + } + + return "reference"; +} + +// --------------------------------------------------------------------------- +// Ranking +// --------------------------------------------------------------------------- + +export function rankLocations( + locations: CodeNavLocation[], + context: { + sourceFilePath: string; + changedFiles: string[]; + isTestFile: boolean; + }, + cap = 50, +): { definitions: CodeNavLocation[]; references: CodeNavLocation[]; capped: boolean } { + const capped = locations.length > cap; + const changedSet = new Set(context.changedFiles); + + function score(loc: CodeNavLocation): number { + let s = 0; + + if (loc.filePath === context.sourceFilePath) s += 1000; + else if (changedSet.has(loc.filePath)) s += 500; + else if (sameDirectory(loc.filePath, context.sourceFilePath)) s += 200; + + if (isTestFile(loc.filePath) && !context.isTestFile) s -= 300; + + if (loc.kind === "definition") s += 100; + if (loc.confidence === "likely") s += 50; + + return s; + } + + const sorted = [...locations].sort((a, b) => score(b) - score(a)); + const truncated = sorted.slice(0, cap); + + return { + definitions: truncated.filter((l) => l.kind === "definition"), + references: truncated.filter((l) => l.kind === "reference"), + capped, + }; +} + +// --------------------------------------------------------------------------- +// Changed files extraction from unified diff patch +// --------------------------------------------------------------------------- + +export function extractChangedFiles(patch: string | null): string[] { + if (!patch) return []; + const set = new Set(); + const re = /^diff --git a\/(.+?) b\/(.+)$/gm; + let m: RegExpExecArray | null; + while ((m = re.exec(patch)) !== null) { + set.add(m[1]); + set.add(m[2]); + } + return [...set]; +} + +// --------------------------------------------------------------------------- +// Validation +// --------------------------------------------------------------------------- + +export function validateCodeNavRequest( + body: unknown, +): string | null { + if (!body || typeof body !== "object") return "Invalid request body"; + const b = body as Record; + + if (typeof b.symbol !== "string" || !b.symbol.trim()) { + return "Missing or empty symbol"; + } + if (typeof b.filePath !== "string" || !b.filePath.trim()) { + return "Missing filePath"; + } + try { + validateFilePath(b.filePath as string); + } catch { + return "Invalid filePath"; + } + if (b.side !== "old" && b.side !== "new") { + return "side must be 'old' or 'new'"; + } + + return null; +} + +// --------------------------------------------------------------------------- +// Main entry point +// --------------------------------------------------------------------------- + +let rgAvailable: boolean | null = null; + +export async function resolveCodeNav( + runtime: CodeNavRuntime, + request: CodeNavRequest, + cwd: string, + changedFiles: string[], +): Promise { + const start = Date.now(); + + if (rgAvailable === null) { + const check = await runtime.runCommand("rg", ["--version"], { + cwd, + timeoutMs: 2000, + }); + rgAvailable = check.exitCode === 0; + } + + if (!rgAvailable) { + return { + backend: "unavailable", + complete: true, + definitions: [], + references: [], + searchScope: "head", + stats: { elapsedMs: Date.now() - start, capped: false }, + }; + } + + const args = buildRgArgs(request.symbol, request.language); + + const result = await runtime.runCommand("rg", args, { + cwd, + timeoutMs: 5000, + }); + + // Exit code 1 = no matches (normal), exit code 2 = error + if (result.exitCode === 2) { + return { + backend: "search", + complete: true, + definitions: [], + references: [], + searchScope: "head", + stats: { elapsedMs: Date.now() - start, capped: false }, + }; + } + + const locations = parseRgJsonOutput( + result.stdout, + request.symbol, + request.language, + ); + + const ranked = rankLocations(locations, { + sourceFilePath: request.filePath, + changedFiles, + isTestFile: isTestFile(request.filePath), + }); + + return { + backend: "search", + complete: true, + definitions: ranked.definitions, + references: ranked.references, + searchScope: "head", + stats: { elapsedMs: Date.now() - start, capped: ranked.capped }, + }; +} + +export function resetRgCache(): void { + rgAvailable = null; +} diff --git a/packages/shared/package.json b/packages/shared/package.json index e408fcf11..3c83c7856 100644 --- a/packages/shared/package.json +++ b/packages/shared/package.json @@ -38,7 +38,8 @@ "./url-to-markdown": "./url-to-markdown.ts", "./tour": "./tour.ts", "./annotate-args": "./annotate-args.ts", - "./at-reference": "./at-reference.ts" + "./at-reference": "./at-reference.ts", + "./code-nav": "./code-nav.ts" }, "dependencies": { "@joplin/turndown-plugin-gfm": "^1.0.64",