-
Notifications
You must be signed in to change notification settings - Fork 523
Support exclude paths in manual context management #774
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
@@ -123,6 +124,7 @@ export type ContextPathResult = GlobPath & { | |||
export type ContextPathResults = { | |||
contextPaths: ContextPathResult[]; | |||
smartContextAutoIncludes: ContextPathResult[]; | |||
excludePaths: ContextPathResult[]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cubic analysis
3 issues found across 16 files • Review in cubic
React with 👍 or 👎 to teach cubic. You can also tag @cubic-dev-ai
to give feedback, ask questions, or re-run the review.
globPath, | ||
}), | ||
), | ||
excludePaths: currentExcludePaths.map( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same mapping logic for converting ContextPathResult → GlobPath is duplicated in three different places; consider extracting a small helper (e.g., toGlobPathArray) to keep the implementation DRY and easier to maintain.
Prompt for AI agents
Address the following comment on src/hooks/useContextPaths.ts at line 71:
<comment>The same mapping logic for converting ContextPathResult → GlobPath is duplicated in three different places; consider extracting a small helper (e.g., toGlobPathArray) to keep the implementation DRY and easier to maintain.</comment>
<file context>
@@ -46,28 +60,63 @@ export function useContextPaths() {
const updateContextPaths = async (paths: GlobPath[]) => {
const currentAutoIncludes =
contextPathsData?.smartContextAutoIncludes || [];
+ const currentExcludePaths = contextPathsData?.excludePaths || [];
return updateContextPathsMutation.mutateAsync({
contextPaths: paths,
- smartContextAutoIncludes: currentAutoIncludes.map(({ globPath }) => ({
- globPath,
- })),
</file context>
const { formattedOutput, files } = await extractCodebase({ | ||
appPath, | ||
chatContext: { | ||
contextPaths: [excludePath], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
excludePath
is supplied as an include path, so excluded files are incorrectly treated as included ones, giving wrong results (Based on your team's feedback about catching logical errors that stem from incorrect property usage).
Prompt for AI agents
Address the following comment on src/ipc/handlers/context_paths_handlers.ts at line 84:
<comment>`excludePath` is supplied as an include path, so excluded files are incorrectly treated as included ones, giving wrong results (Based on your team's feedback about catching logical errors that stem from incorrect property usage).</comment>
<file context>
@@ -76,6 +76,23 @@ export function registerContextPathsHandlers() {
tokens: totalTokens,
});
}
+
+ for (const excludePath of excludePaths || []) {
+ const { formattedOutput, files } = await extractCodebase({
+ appPath,
+ chatContext: {
+ contextPaths: [excludePath],
</file context>
contextPaths: [excludePath], | |
+ excludePaths: [excludePath], |
appPath, | ||
globPath: p.globPath, | ||
}); | ||
const matches = await glob(pattern, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Glob search for excludePaths does not ignore node_modules, causing unnecessary filesystem traversal and performance overhead
Prompt for AI agents
Address the following comment on src/utils/codebase.ts at line 503:
<comment>Glob search for excludePaths does not ignore node_modules, causing unnecessary filesystem traversal and performance overhead</comment>
<file context>
@@ -492,12 +493,35 @@ export async function extractCodebase({
}
}
+ // Add files from excludePaths
+ if (excludePaths && excludePaths.length > 0) {
+ for (const p of excludePaths) {
+ const pattern = createFullGlobPath({
+ appPath,
+ globPath: p.globPath,
</file context>
const matches = await glob(pattern, { | |
const matches = await glob(pattern, { nodir: true, absolute: true, ignore: "**/node_modules/**" }); |
No description provided.