Add composer path tagging with workspace entry search#62
Merged
juliusmarminge merged 14 commits intomainfrom Feb 18, 2026
Merged
Add composer path tagging with workspace entry search#62juliusmarminge merged 14 commits intomainfrom
juliusmarminge merged 14 commits intomainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds workspace file/directory search with server-side indexing (git-aware + fs fallback), caching and search API; WebSocket method; contracts/schemas; frontend composer trigger logic, UI and client API; VSCode icon sync/resolution and related tests. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant ChatView
participant ComposerLogic
participant WSClient
participant Server
User->>ChatView: Type in composer textarea
ChatView->>ComposerLogic: detectComposerTrigger(text, cursor)
ComposerLogic-->>ChatView: ComposerTrigger { kind, query, rangeStart, rangeEnd }
alt Path trigger
ChatView->>WSClient: projects.searchEntries({cwd, query, limit})
WSClient->>Server: WS: projects.searchEntries
Server->>Server: get/build index (git or fs crawl)
Server->>Server: filter & rank entries
Server-->>WSClient: ProjectSearchEntriesResult { entries, truncated }
WSClient-->>ChatView: entries, truncated
ChatView->>ChatView: render composer menu (entries)
end
User->>ChatView: Select item / press Enter
ChatView->>ComposerLogic: replaceTextRange(text, rangeStart, rangeEnd, replacement)
ComposerLogic-->>ChatView: { text: updatedText, cursor: newCursor }
ChatView->>ChatView: applyPromptReplacement, reset composer state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
Contributor
Add composer path tagging and implement
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
apps/web/src/components/ChatView.tsx (2)
116-120: Consider reusingbasenameOffromworkspaceEntries.tsor extracting to shared utility.This function duplicates the logic in
apps/server/src/workspaceEntries.ts:basenameOf. While the server function isn't directly importable in the web app, consider extracting path utilities to a shared package (e.g.,@t3tools/contracts) to avoid drift.For now, this is fine as an isolated helper.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/ChatView.tsx` around lines 116 - 120, This duplicates path-basename logic in basenameOfPath; either import/reuse the existing basenameOf from workspaceEntries.ts (by moving the shared logic into a common package like `@t3tools/contracts`) or extract basenameOfPath into that shared utility and switch both ChatView.tsx and server workspaceEntries.ts to use the single shared function. Update references to the function name basenameOfPath (or rename to basenameOf) in ChatView.tsx and ensure workspaceEntries.ts imports the new shared util so the logic is centralized and not duplicated.
1351-1372: Cursor tracking on multiple events may cause redundant state updates.The cursor is tracked on
onChange,onKeyUp,onClick, andonSelect. While this ensures accurate cursor position, it may trigger multiple state updates for a single user action.This is likely fine for UX responsiveness, but if performance issues arise, consider debouncing or consolidating these updates.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/ChatView.tsx` around lines 1351 - 1372, The cursor position is being updated redundantly across multiple handlers (onChange calling onPromptChange with selectionStart, plus setComposerCursor in onKeyUp, onClick, and onSelect), causing duplicate state updates; update the component to centralize cursor updates by removing selectionStart handling from onChange and only calling setComposerCursor from onSelect and onKeyUp (or wrap setComposerCursor with a lightweight debounce/requestAnimationFrame to coalesce rapid updates), keeping onPromptChange focused solely on text updates—look for the handlers onPromptChange and setComposerCursor in the diff to apply this change.apps/server/src/workspaceEntries.ts (2)
150-164: Concurrent requests for the samecwdmay trigger redundant scans.When multiple requests arrive simultaneously for an uncached
cwd, each will callbuildWorkspaceIndexbefore any result is cached. Under load this could cause redundant filesystem traversals.Consider using a pending-promise pattern to coalesce concurrent requests:
♻️ Optional: Coalesce concurrent index builds
+const pendingIndexBuilds = new Map<string, Promise<WorkspaceIndex>>(); + async function getWorkspaceIndex(cwd: string): Promise<WorkspaceIndex> { const cached = workspaceIndexCache.get(cwd); if (cached && Date.now() - cached.scannedAt < WORKSPACE_CACHE_TTL_MS) { return cached; } + const pending = pendingIndexBuilds.get(cwd); + if (pending) { + return pending; + } + - const next = await buildWorkspaceIndex(cwd); + const buildPromise = buildWorkspaceIndex(cwd); + pendingIndexBuilds.set(cwd, buildPromise); + let next: WorkspaceIndex; + try { + next = await buildPromise; + } finally { + pendingIndexBuilds.delete(cwd); + } workspaceIndexCache.set(cwd, next); // ... rest unchangedBased on coding guidelines: "Maintain predictable behavior under load and during failures (session restarts, reconnects, partial streams)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/server/src/workspaceEntries.ts` around lines 150 - 164, Concurrent calls to getWorkspaceIndex for the same cwd can trigger duplicate buildWorkspaceIndex runs; implement a pending-promise coalescing pattern: create a module-scoped map (e.g., pendingIndexBuilds) keyed by cwd, have getWorkspaceIndex check pendingIndexBuilds before calling buildWorkspaceIndex, store the Promise there while the build is in-flight, await that single Promise for all callers, on success insert the resolved WorkspaceIndex into workspaceIndexCache and delete the pending entry, and on failure delete the pending entry and rethrow so subsequent calls can retry; reference getWorkspaceIndex, workspaceIndexCache, buildWorkspaceIndex, and ensure pending promises are cleaned up on both resolve and reject.
73-148: Consider adding hidden file filtering.The scan ignores specific directory names (
.git,node_modules, etc.) but doesn't filter other hidden files/directories starting with.(e.g.,.env,.vscode). Depending on the intended UX, you may want to either:
- Exclude all dot-prefixed entries for a cleaner list.
- Keep them visible (current behavior) if users need to reference config files.
Current behavior is reasonable if referencing dotfiles is a valid use case.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/server/src/workspaceEntries.ts` around lines 73 - 148, The scan currently includes dot-prefixed files/dirs; update buildWorkspaceIndex to skip hidden entries by adding a check in the dirents loop (inside function buildWorkspaceIndex) to continue when dirent.name startsWith('.') (after the existing "."/".." check) or implement a small whitelist for allowed dotfiles (e.g., ALLOWED_DOTFILES) and only include dot entries if they are explicitly allowed; update IGNORED_DIRECTORY_NAMES handling or add a new HIDE_DOTFILES constant to control this behavior so the scanner excludes hidden entries unless opted-in.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/server/src/workspaceEntries.ts`:
- Around line 150-164: Concurrent calls to getWorkspaceIndex for the same cwd
can trigger duplicate buildWorkspaceIndex runs; implement a pending-promise
coalescing pattern: create a module-scoped map (e.g., pendingIndexBuilds) keyed
by cwd, have getWorkspaceIndex check pendingIndexBuilds before calling
buildWorkspaceIndex, store the Promise there while the build is in-flight, await
that single Promise for all callers, on success insert the resolved
WorkspaceIndex into workspaceIndexCache and delete the pending entry, and on
failure delete the pending entry and rethrow so subsequent calls can retry;
reference getWorkspaceIndex, workspaceIndexCache, buildWorkspaceIndex, and
ensure pending promises are cleaned up on both resolve and reject.
- Around line 73-148: The scan currently includes dot-prefixed files/dirs;
update buildWorkspaceIndex to skip hidden entries by adding a check in the
dirents loop (inside function buildWorkspaceIndex) to continue when dirent.name
startsWith('.') (after the existing "."/".." check) or implement a small
whitelist for allowed dotfiles (e.g., ALLOWED_DOTFILES) and only include dot
entries if they are explicitly allowed; update IGNORED_DIRECTORY_NAMES handling
or add a new HIDE_DOTFILES constant to control this behavior so the scanner
excludes hidden entries unless opted-in.
In `@apps/web/src/components/ChatView.tsx`:
- Around line 116-120: This duplicates path-basename logic in basenameOfPath;
either import/reuse the existing basenameOf from workspaceEntries.ts (by moving
the shared logic into a common package like `@t3tools/contracts`) or extract
basenameOfPath into that shared utility and switch both ChatView.tsx and server
workspaceEntries.ts to use the single shared function. Update references to the
function name basenameOfPath (or rename to basenameOf) in ChatView.tsx and
ensure workspaceEntries.ts imports the new shared util so the logic is
centralized and not duplicated.
- Around line 1351-1372: The cursor position is being updated redundantly across
multiple handlers (onChange calling onPromptChange with selectionStart, plus
setComposerCursor in onKeyUp, onClick, and onSelect), causing duplicate state
updates; update the component to centralize cursor updates by removing
selectionStart handling from onChange and only calling setComposerCursor from
onSelect and onKeyUp (or wrap setComposerCursor with a lightweight
debounce/requestAnimationFrame to coalesce rapid updates), keeping
onPromptChange focused solely on text updates—look for the handlers
onPromptChange and setComposerCursor in the diff to apply this change.
This comment has been minimized.
This comment has been minimized.
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
Greptile SummaryAdds workspace file/directory search with Key changes:
Issues found:
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant ChatView
participant ReactQuery
participant WebSocket
participant wsServer
participant workspaceEntries
User->>ChatView: types "@src/comp"
ChatView->>ChatView: detectComposerTrigger(text, cursor)
ChatView->>ChatView: debounce query (120ms)
ChatView->>ReactQuery: projectSearchEntriesQuery
ReactQuery->>WebSocket: WS_METHODS.projectsSearchEntries
WebSocket->>wsServer: { cwd, query, limit }
wsServer->>workspaceEntries: searchWorkspaceEntries(params)
alt cache hit (< 15s old)
workspaceEntries->>workspaceEntries: return cached index
else cache miss or expired
alt in-flight build exists
workspaceEntries->>workspaceEntries: await pending promise
else new build needed
workspaceEntries->>workspaceEntries: buildWorkspaceIndex(cwd)
Note over workspaceEntries: tries git ls-files,<br/>falls back to fs walk
workspaceEntries->>workspaceEntries: filterGitIgnoredPaths (chunked)
workspaceEntries->>workspaceEntries: cache result (15s TTL)
end
end
workspaceEntries->>workspaceEntries: filter by query
workspaceEntries->>workspaceEntries: rank by score
workspaceEntries->>workspaceEntries: slice to limit
workspaceEntries-->>wsServer: { entries, truncated }
wsServer-->>WebSocket: response
WebSocket-->>ReactQuery: data
ReactQuery-->>ChatView: workspaceEntries
ChatView->>ChatView: render ComposerCommandMenu
User->>ChatView: press Tab/Enter
ChatView->>ChatView: replaceTextRange("@src/components/Button.tsx ")
ChatView->>ChatView: update prompt state
Last reviewed commit: 2136fee |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Additional Comments (1)
When the command palette is open but has no matching items (e.g., Consider handling Enter when the menu is open but empty to prevent accidental sends: |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/server/src/workspaceEntries.ts (1)
175-194: Consider limiting parallelreaddircalls.
Promise.allover large directory batches can create hundreds/thousands of concurrent file handles in big workspaces. A small concurrency cap (e.g., p-limit) would make load more predictable.As per coding guidelines "Maintain predictable behavior under load and during failures (session restarts, reconnects, partial streams)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/server/src/workspaceEntries.ts` around lines 175 - 194, The current use of Promise.all over currentDirectories in the scan loop (pendingDirectories → currentDirectories → directoryEntries) can spawn unbounded concurrent fs.readdir calls; limit concurrency by replacing Promise.all with a bounded task runner (e.g., p-limit or a simple semaphore) that caps concurrent calls to fs.readdir (use an adjustable constant like MAX_CONCURRENCY), and run the mapping through that limiter so directoryEntries is built with controlled parallelism while preserving the existing error handling and returned shape.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/server/src/workspaceEntries.ts`:
- Around line 250-264: The getWorkspaceIndex implementation currently triggers
duplicate scans when multiple callers race after cache expiry; change it to
memoize the in‑flight Promise per cwd so concurrent callers share one
buildWorkspaceIndex call: when cache miss, immediately place a placeholder
Promise for buildWorkspaceIndex(cwd) into workspaceIndexCache (instead of
waiting for await), have callers await that Promise, and when it resolves
replace the cached Promise with the resolved WorkspaceIndex (including
scannedAt); also ensure that if the Promise rejects you remove the cache entry
so future calls can retry. Update references around getWorkspaceIndex,
workspaceIndexCache, buildWorkspaceIndex, WORKSPACE_CACHE_TTL_MS and the
existing workspaceIndexCache.set/delete logic to implement this behavior.
In `@apps/web/src/components/ChatView.tsx`:
- Around line 1293-1299: The empty-state string for model selection is
incorrect: in the JSX that renders when composerMenuItems.length === 0 (using
workspaceEntriesQuery and composerTrigger), update the conditional that
currently returns "No matching command." when composerTrigger?.kind !== "path"
so it returns "No matching model." specifically when composerTrigger?.kind ===
"model" (keep existing "No matching command." for other non-path kinds),
ensuring the ternary/conditional in the ChatView component that checks
composerTrigger?.kind is adjusted accordingly.
---
Nitpick comments:
In `@apps/server/src/workspaceEntries.ts`:
- Around line 175-194: The current use of Promise.all over currentDirectories in
the scan loop (pendingDirectories → currentDirectories → directoryEntries) can
spawn unbounded concurrent fs.readdir calls; limit concurrency by replacing
Promise.all with a bounded task runner (e.g., p-limit or a simple semaphore)
that caps concurrent calls to fs.readdir (use an adjustable constant like
MAX_CONCURRENCY), and run the mapping through that limiter so directoryEntries
is built with controlled parallelism while preserving the existing error
handling and returned shape.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
🧹 Nitpick comments (3)
apps/web/src/vscode-icons.ts (1)
4-5: Version hardcoded in two places - potential drift risk.
VSCODE_ICONS_VERSIONisv12.17.0here, while the sync script defaults to12.17.0. Consider importing the version from the generated JSON manifest metadata to ensure consistency.♻️ Use version from manifest
import vscodeIconsManifest from "./vscode-icons-manifest.json"; import languageAssociationsData from "./vscode-icons-language-associations.json"; -const VSCODE_ICONS_VERSION = "v12.17.0"; +const VSCODE_ICONS_VERSION = `v${languageAssociationsData.version}`; const VSCODE_ICONS_BASE_URL = `https://cdn.jsdelivr.net/gh/vscode-icons/vscode-icons@${VSCODE_ICONS_VERSION}/icons`;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/vscode-icons.ts` around lines 4 - 5, VSCODE_ICONS_VERSION is hardcoded separately from the sync script; change the code to read the version from the generated manifest JSON instead of hardcoding it so they can't drift: replace uses of VSCODE_ICONS_VERSION (and construction of VSCODE_ICONS_BASE_URL) with a runtime import/read of the generated icons manifest (e.g., read manifest.version or manifest.metadata.version) and derive the base URL from that value; update any module-level constants VSCODE_ICONS_VERSION and VSCODE_ICONS_BASE_URL to be computed from the manifest value (e.g., manifest.version) so the version is single-sourced.apps/web/src/components/ChatView.tsx (1)
113-117:basenameOfPathis duplicated.This helper function is identical to the one in
apps/web/src/vscode-icons.ts. Consider extracting it to a shared utility module to avoid duplication.♻️ Extract to shared utility
Create in
apps/web/src/lib/path-utils.ts:export function basenameOfPath(pathValue: string): string { const slashIndex = pathValue.lastIndexOf("/"); if (slashIndex === -1) return pathValue; return pathValue.slice(slashIndex + 1); }Then import in both files.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/ChatView.tsx` around lines 113 - 117, The duplicate helper basenameOfPath should be extracted into a single shared utility module: create and export a basenameOfPath(pathValue: string): string function in a new shared file and replace the local implementations in ChatView.tsx and the other file by importing this exported function; ensure the function signature and behavior match the existing implementation and update imports in the places that used the duplicated code (e.g., references to basenameOfPath in ChatView and vscode-icons) so both modules use the single shared export.scripts/sync-vscode-icons.mjs (1)
49-59: The script assumesunzipis available on the system.This script uses the
unzipcommand-line tool which may not be available on all systems (e.g., Windows without WSL). Consider documenting this dependency or adding a check.💡 Optional: Add availability check
+async function checkUnzipAvailable() { + try { + await execFileAsync("unzip", ["-v"]); + } catch { + throw new Error("'unzip' command not found. Please install it to run this script."); + } +} + async function extractManifestFromVsix(vsixPath) {Then call
await checkUnzipAvailable();at the start ofmain().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/sync-vscode-icons.mjs` around lines 49 - 59, The script assumes the external "unzip" binary is present when extractManifestFromVsix calls execFileAsync("unzip", ...); add a short availability check function (e.g., checkUnzipAvailable) that verifies "unzip" exists (using a lightweight spawn/exec of "which unzip" or "unzip -v") and throws a clear error or logs instructions if missing, then call await checkUnzipAvailable(); at the start of main() before extractManifestFromVsix is used; alternatively add a comment near extractManifestFromVsix and main() documenting "unzip" as a required dependency for users on platforms like Windows.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/server/src/workspaceEntries.ts`:
- Around line 312-326: getWorkspaceIndex currently allows multiple concurrent
cache misses to call buildWorkspaceIndex concurrently; create a per-cwd
in-flight Promise map (e.g., inFlightIndexPromises) and memoize the Promise for
a given cwd when starting buildWorkspaceIndex so concurrent callers await the
same Promise; on success set workspaceIndexCache with the returned
WorkspaceIndex and remove the entry from inFlightIndexPromises, and on failure
remove the in-flight entry and rethrow so callers see the error (do not populate
workspaceIndexCache on failure). Ensure you still check the existing
workspaceIndexCache TTL at the top of getWorkspaceIndex and reference
getWorkspaceIndex, buildWorkspaceIndex, workspaceIndexCache, and the new
inFlightIndexPromises map in your changes.
In `@apps/web/src/components/ChatView.tsx`:
- Around line 1344-1351: The empty-state text incorrectly shows "No matching
command." when composerTrigger?.kind is "slash-model"; update the conditional in
ChatView.tsx (the JSX rendering that checks composerMenuItems.length and
composerTrigger?.kind) to return "No matching model." when composerTrigger?.kind
=== "slash-model" (preserving existing cases for "path" and the default command
case) so the message accurately reflects model searches.
---
Nitpick comments:
In `@apps/web/src/components/ChatView.tsx`:
- Around line 113-117: The duplicate helper basenameOfPath should be extracted
into a single shared utility module: create and export a
basenameOfPath(pathValue: string): string function in a new shared file and
replace the local implementations in ChatView.tsx and the other file by
importing this exported function; ensure the function signature and behavior
match the existing implementation and update imports in the places that used the
duplicated code (e.g., references to basenameOfPath in ChatView and
vscode-icons) so both modules use the single shared export.
In `@apps/web/src/vscode-icons.ts`:
- Around line 4-5: VSCODE_ICONS_VERSION is hardcoded separately from the sync
script; change the code to read the version from the generated manifest JSON
instead of hardcoding it so they can't drift: replace uses of
VSCODE_ICONS_VERSION (and construction of VSCODE_ICONS_BASE_URL) with a runtime
import/read of the generated icons manifest (e.g., read manifest.version or
manifest.metadata.version) and derive the base URL from that value; update any
module-level constants VSCODE_ICONS_VERSION and VSCODE_ICONS_BASE_URL to be
computed from the manifest value (e.g., manifest.version) so the version is
single-sourced.
In `@scripts/sync-vscode-icons.mjs`:
- Around line 49-59: The script assumes the external "unzip" binary is present
when extractManifestFromVsix calls execFileAsync("unzip", ...); add a short
availability check function (e.g., checkUnzipAvailable) that verifies "unzip"
exists (using a lightweight spawn/exec of "which unzip" or "unzip -v") and
throws a clear error or logs instructions if missing, then call await
checkUnzipAvailable(); at the start of main() before extractManifestFromVsix is
used; alternatively add a comment near extractManifestFromVsix and main()
documenting "unzip" as a required dependency for users on platforms like
Windows.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/web/src/components/ChatView.tsx (1)
1259-1274: Scheduling side effects inside setState callback is an anti-pattern.The
requestAnimationFramecall within thesetPromptupdater function is problematic because React may invoke the updater multiple times (e.g., in StrictMode), leading to redundant RAF schedules. Although the duplicate focus/cursor operations are idempotent here, consider refactoring to schedule the RAF after the state update completes.♻️ Suggested refactor
const applyPromptReplacement = useCallback( (rangeStart: number, rangeEnd: number, replacement: string) => { + const next = replaceTextRange(prompt, rangeStart, rangeEnd, replacement); + setPrompt(next.text); + window.requestAnimationFrame(() => { + const textarea = textareaRef.current; + if (!textarea) return; + textarea.focus(); + textarea.setSelectionRange(next.cursor, next.cursor); + setComposerCursor(next.cursor); + }); - setPrompt((existing) => { - const next = replaceTextRange(existing, rangeStart, rangeEnd, replacement); - window.requestAnimationFrame(() => { - const textarea = textareaRef.current; - if (!textarea) return; - textarea.focus(); - textarea.setSelectionRange(next.cursor, next.cursor); - setComposerCursor(next.cursor); - }); - return next.text; - }); }, - [], + [prompt], );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/ChatView.tsx` around lines 1259 - 1274, The updater in applyPromptReplacement schedules a requestAnimationFrame inside setPrompt's functional updater; move the side-effect (focus + setSelectionRange + setComposerCursor) out of the setPrompt callback: compute the replacement result with replaceTextRange before/alongside calling setPrompt (use the current prompt state or a promptRef to get the existing value), call setPrompt(next.text) (no RAF or DOM ops inside), then call window.requestAnimationFrame(() => { const textarea = textareaRef.current; if (!textarea) return; textarea.focus(); textarea.setSelectionRange(next.cursor, next.cursor); setComposerCursor(next.cursor); }); This keeps applyPromptReplacement, setPrompt, textareaRef, and setComposerCursor intact but ensures DOM side-effects run after state is scheduled, avoiding RAF inside the updater.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/web/src/components/ChatView.tsx`:
- Around line 306-313: The empty-state message uses props.triggerKind but
doesn't handle the "slash-model" case, causing a misleading "No matching
command." text; update the JSX in ChatView (where props.items, props.isLoading,
and props.triggerKind are used) to add a branch for props.triggerKind ===
"slash-model" (e.g., props.triggerKind === "path" ? "No matching files or
folders." : props.triggerKind === "slash-model" ? "No matching model." : "No
matching command.") so the message accurately reflects model selection triggers.
---
Nitpick comments:
In `@apps/web/src/components/ChatView.tsx`:
- Around line 1259-1274: The updater in applyPromptReplacement schedules a
requestAnimationFrame inside setPrompt's functional updater; move the
side-effect (focus + setSelectionRange + setComposerCursor) out of the setPrompt
callback: compute the replacement result with replaceTextRange before/alongside
calling setPrompt (use the current prompt state or a promptRef to get the
existing value), call setPrompt(next.text) (no RAF or DOM ops inside), then call
window.requestAnimationFrame(() => { const textarea = textareaRef.current; if
(!textarea) return; textarea.focus(); textarea.setSelectionRange(next.cursor,
next.cursor); setComposerCursor(next.cursor); }); This keeps
applyPromptReplacement, setPrompt, textareaRef, and setComposerCursor intact but
ensures DOM side-effects run after state is scheduled, avoiding RAF inside the
updater.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/web/src/components/ChatView.tsx (1)
126-130: Consider extractingbasenameOfPathto a shared utility.This function is duplicated in
vscode-icons.ts. For consistency and DRY, consider moving it to a shared utilities module.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/ChatView.tsx` around lines 126 - 130, The function basenameOfPath is duplicated (here in ChatView.tsx and in vscode-icons.ts); extract it into a shared utility module (e.g., utils/path.ts or similar) and replace the local implementations by importing basenameOfPath from that module in both ChatView.tsx and vscode-icons.ts; ensure the exported function signature (basenameOfPath(pathValue: string): string) and behavior remain identical and update imports where the local definitions were removed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/web/src/components/ChatView.tsx`:
- Around line 307-314: The empty-state message in ChatView.tsx uses
props.triggerKind to decide text but wrongly shows "No matching command." for
model selection; update the conditional that renders the fallback string (within
the JSX that checks props.items.length === 0) so that when props.triggerKind ===
"model" it returns "No matching model." instead of "No matching command.",
preserving the existing branches for "path" and loading state.
---
Nitpick comments:
In `@apps/web/src/components/ChatView.tsx`:
- Around line 126-130: The function basenameOfPath is duplicated (here in
ChatView.tsx and in vscode-icons.ts); extract it into a shared utility module
(e.g., utils/path.ts or similar) and replace the local implementations by
importing basenameOfPath from that module in both ChatView.tsx and
vscode-icons.ts; ensure the exported function signature
(basenameOfPath(pathValue: string): string) and behavior remain identical and
update imports where the local definitions were removed.
This comment has been minimized.
This comment has been minimized.
| setPrompt(""); | ||
| setComposerImages([]); | ||
| setComposerCursor(0); | ||
| setComposerMenuIndex(0); |
Contributor
There was a problem hiding this comment.
Path tag helpers never wired into send flow
High Severity
buildPromptInput and buildUserVisiblePrompt are implemented, exported, and tested in composer-logic.ts, but never actually called in the onSend flow in ChatView.tsx. The raw trimmed prompt (containing literal @path tokens) is sent directly to both the message store and the model provider. This means the PR's stated goal of "sending structured 'Referenced paths' context to the model" while "showing @path tags" to the user doesn't actually happen — the model just receives raw @src/file.tsx text with no structured path context.
Additional Locations (1)
This comment has been minimized.
This comment has been minimized.
- add `projects.searchEntries` API, contracts, and server-side indexed workspace search - add composer trigger logic for `@` path suggestions and `/model` command completion - include tagged paths in sent prompt/user-visible text and cover behavior with server/web tests
- Build workspace index from `git ls-files` when in a Git repo, excluding ignored paths - Skip configured ignored top-level directories during filesystem fallback indexing - Update chat composer to insert `@path` directly in prompt and remove separate tagged-path chips - Keep command menu navigation visible by scrolling the active/next item into view
- render theme-aware VS Code icons for path suggestions in chat composer - exclude `.convex` and gitignored paths more reliably in workspace indexing - add tests plus a sync script and icon manifest/association data
- Extract composer menu into memoized components for cleaner ChatView rendering - Debounce path trigger queries and keep previous results while fetching - Precompute searchable model fields to reduce repeated lowercase work
- Replace slash-command menu glyph in ChatView with `BotIcon` - Map `yml`/`yaml` to base YAML language id to avoid overly specific icons - Add test coverage for GitHub workflow YAML icon selection
- add `@tanstack/react-pacer` and switch composer path debounce to `useDebouncedValue` - extract reusable `projectSearchEntriesQueryOptions` for workspace entry lookup - add `serverKeybindingsQueryOptions` and update consumers in chat/sidebar - keep branch toolbar mutation error handling at call sites
- Reuse in-flight workspace index builds per cwd and cap `readdir` scan concurrency at 32 - Add server tests for build deduplication and directory read concurrency limits - Track composer text in a ref so prompt replacements use current text after send/reset
- Introduce a new test file to validate chunking behavior of git check-ignore to prevent oversized input strings. - Adjust workspaceEntries.ts to implement chunking logic for git check-ignore, improving performance with large ignored path lists. - Set a maximum byte limit for stdin in git check-ignore to enhance stability and prevent errors during processing.
juliusmarminge
force-pushed
the
codething/58603bc6
branch
from
e440110 to
be9b9e3
Compare
| const source = rawSource | ||
| .replace(/^import[^;]+;\s*/gm, "") | ||
| .replace(/export const languages(?:\s*:\s*[^=]+)?\s*=/, "const languages =") | ||
| .replace(/\}\s*satisfies\s*Record<[^;]+>;/, "};"); |
Contributor
There was a problem hiding this comment.
🟢 Low
scripts/sync-vscode-icons.mjs:70 The regex doesn't strip as const assertions, which would cause a SyntaxError in vm.runInContext. Consider adding a replacement like .replace(/\}\s*as\s+const\s*;/, '};') alongside the satisfies handling.
Suggested change
| .replace(/\}\s*satisfies\s*Record<[^;]+>;/, "};"); | |
| .replace(/\}\s*satisfies\s*Record<[^;]+;/, "};") | |
| .replace(/\}\s*as\s+const\s*;/, "};"); |
🚀 Want me to fix this? Reply ex: "fix it for me".
🤖 Prompt for AI
In file scripts/sync-vscode-icons.mjs around line 70:
The regex doesn't strip `as const` assertions, which would cause a `SyntaxError` in `vm.runInContext`. Consider adding a replacement like `.replace(/\}\s*as\s+const\s*;/, '};')` alongside the `satisfies` handling.
Evidence trail:
Viewed `scripts/sync-vscode-icons.mjs` lines ~60–74 at commit `be9b9e3` showing the `.replace(...satisfies...)` cleanup and absence of any `as const` replacement.
Co-authored-by: codex <codex@users.noreply.github.com>
| }); | ||
| }, | ||
| [], | ||
| ); |
Contributor
There was a problem hiding this comment.
Deferred cursor state causes stale trigger flash after selection
Medium Severity
setComposerCursor(next.cursor) is called inside requestAnimationFrame, but setPrompt(next.text) is called synchronously. This causes a render cycle where composerTrigger is computed with the new prompt text but the old stale cursor position. detectComposerTrigger will find an @-token in the middle of the replaced text, briefly reopening the command menu with incorrect results before the rAF fires and updates the cursor. Moving setComposerCursor outside the rAF (alongside setPrompt) would fix the stale intermediate state.
Contributor
|
Bugbot Autofix prepared fixes for 1 of the 1 bugs found in the latest run.
Or push these changes by commenting: Preview (583765b883)diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx
--- a/apps/web/src/components/ChatView.tsx
+++ b/apps/web/src/components/ChatView.tsx
@@ -1246,12 +1246,12 @@
const next = replaceTextRange(promptRef.current, rangeStart, rangeEnd, replacement);
promptRef.current = next.text;
setPrompt(next.text);
+ setComposerCursor(next.cursor);
window.requestAnimationFrame(() => {
const textarea = textareaRef.current;
if (!textarea) return;
textarea.focus();
textarea.setSelectionRange(next.cursor, next.cursor);
- setComposerCursor(next.cursor);
});
},
[], |
| }, | ||
| enabled: (input.enabled ?? true) && Boolean(input.api && input.cwd), | ||
| staleTime: input.staleTime ?? DEFAULT_SEARCH_ENTRIES_STALE_TIME, | ||
| placeholderData: (previous) => previous ?? EMPTY_SEARCH_ENTRIES_RESULT, |
Contributor
There was a problem hiding this comment.
🟢 Low
lib/projectReactQuery.ts:40 placeholderData persists stale entries when cwd becomes null and the query is disabled. Consider returning undefined (or the empty result) when input.cwd is falsy to avoid showing files from a previous project.
Suggested change
| placeholderData: (previous) => previous ?? EMPTY_SEARCH_ENTRIES_RESULT, | |
| placeholderData: (previous) => (input.cwd ? previous : undefined) ?? EMPTY_SEARCH_ENTRIES_RESULT, |
🚀 Want me to fix this? Reply ex: "fix it for me".
🤖 Prompt for AI
In file apps/web/src/lib/projectReactQuery.ts around line 40:
`placeholderData` persists stale entries when `cwd` becomes `null` and the query is disabled. Consider returning `undefined` (or the empty result) when `input.cwd` is falsy to avoid showing files from a previous project.
Evidence trail:
Viewed `apps/web/src/lib/projectReactQuery.ts:26-41` at commit `b73b2c9c` (shows `enabled` condition and `placeholderData` returning previous data).
- Switch composer menu arrow-key navigation to `Command`'s internal highlight handling - Track highlighted item by id instead of manual index/keyboard state - Remove custom hover/highlight plumbing and hidden-scroll sync logic
Member
Author
|
@greptileai review |
| { wait: COMPOSER_PATH_QUERY_DEBOUNCE_MS }, | ||
| (debouncerState) => ({ isPending: debouncerState.isPending }), | ||
| ); | ||
| const effectivePathQuery = pathTriggerQuery.length > 0 ? debouncedPathQuery : ""; |
There was a problem hiding this comment.
empty query should skip debouncing to show results immediately
Suggested change
| const effectivePathQuery = pathTriggerQuery.length > 0 ? debouncedPathQuery : ""; | |
| const effectivePathQuery = pathTriggerQuery.length > 0 ? debouncedPathQuery : pathTriggerQuery; |
Additional Comments (1)
|
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.
Summary
projects.searchEntriescontracts and WebSocket API plumbing so clients can query workspace files/directories by prefix text@pathtags and/modelcommand completion, including cursor-aware token replacement helpersChatViewto show command palette suggestions, select path/model items from keyboard/mouse, render removable path chips, and include tagged paths in sent prompts@pathtags while sending structured "Referenced paths" context to the modelTesting
apps/server/src/workspaceEntries.test.ts: verifies workspace scan output, ignored directories, and query filtering/ranking behaviorapps/server/src/wsServer.test.ts: verifiesWS_METHODS.projectsSearchEntriesis handled and returns expected file/directory entriesapps/web/src/composer-logic.test.ts: verifies trigger detection (@,/model), text replacement, prompt input building, and visible tag formattingpackages/contracts/src/project.test.ts: verifies search input defaults/validation and search result schema parsingNote
Medium Risk
Touches filesystem scanning and git subprocess invocation with caching/concurrency controls, which can affect performance and correctness across diverse repos; UI changes are moderate but rely on new RPC behavior.
Overview
Adds a new
projects.searchEntriesRPC exposed over WebSocket and consumed by the web app to query workspace files/directories by text, returning ranked results with truncation.Implements server-side workspace indexing (
workspaceEntries.ts) with short-lived caching, git-aware discovery (git ls-files) plus.gitignorefiltering (including chunkedgit check-ignorestdin), a filesystem-walk fallback with boundedreaddirconcurrency, and hard caps on indexed entries.Updates the chat composer to show a command-palette-style menu for
@path tagging and/modelcompletion (debounced queries, keyboard navigation, cursor-aware text replacement, VS Code-style icons), and refactors branch toolbar mutations to attach error handlers permutatecall. Adds comprehensive unit/integration tests for indexing behavior, git-ignore chunking, and the new websocket method.Written by Cursor Bugbot for commit f0bf427. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
New Features
Tests