-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix(glob): allow retrieving matches beyond the default page #3658
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
Changes from all commits
974e76a
3fcdf4a
2ad35c6
21c93d1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@moonshot-ai/kimi-code": patch | ||
| --- | ||
|
|
||
| Allow file searches to retrieve matches beyond the first 100 results. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,7 @@ import { ISessionSkillCatalog } from '#/features/skill/session/skillCatalog'; | |
| import { ISessionWorkspaceContext } from '#/session/workspaceContext/workspaceContext'; | ||
| import { ITelemetryService } from '#/app/telemetry/telemetry'; | ||
| import { | ||
| DEFAULT_TOOL_RESULT_MAX_RETAINED_CHARS, | ||
| ToolAccesses, | ||
| type ExecutableToolResult, | ||
| type ToolExecution, | ||
|
|
@@ -37,7 +38,7 @@ import { | |
| type GlobInput, | ||
| GlobInputSchema, | ||
| IGlobTool, | ||
| MAX_MATCHES, | ||
| DEFAULT_HEAD_LIMIT, | ||
| WINDOWS_PATH_HINT, | ||
| } from './glob'; | ||
|
|
||
|
|
@@ -238,50 +239,90 @@ export class GlobTool implements IGlobTool { | |
| } | ||
| } | ||
|
|
||
| const truncated = kept.length > MAX_MATCHES; | ||
| const limited = truncated ? kept.slice(0, MAX_MATCHES) : kept; | ||
|
|
||
| if (limited.length === 0 && !timedOut) { | ||
| if (filteredSensitive > 0) { | ||
| return { | ||
| output: `No non-sensitive matches found (${String(filteredSensitive)} sensitive file(s) filtered).`, | ||
| }; | ||
| } | ||
| return { output: 'No matches found' }; | ||
| } | ||
| const offset = args.offset ?? 0; | ||
| const headLimit = args.head_limit ?? DEFAULT_HEAD_LIMIT; | ||
| const limited = headLimit === 0 ? kept.slice(offset) : kept.slice(offset, offset + headLimit); | ||
| const partial = bufferTruncated || timedOut || traversalWarning !== undefined; | ||
|
|
||
| const pathClass = env.pathClass; | ||
| const shouldRelativize = isWithinDirectory(searchRoot, workspace.workspaceDir, pathClass); | ||
| const displayLines = limited.map((p) => | ||
| const candidates = limited.map((p) => | ||
| shouldRelativize ? relativizeIfUnder(p, searchRoot, pathClass) : p, | ||
| ); | ||
|
|
||
| const lines: string[] = []; | ||
| const warnings: string[] = []; | ||
| if (timedOut) { | ||
| lines.push( | ||
| warnings.push( | ||
| `Glob timed out after ${String(DEFAULT_TIMEOUT_MS / 1000)}s; partial results returned.`, | ||
| ); | ||
| } | ||
| if (bufferTruncated) { | ||
| lines.push( | ||
| warnings.push( | ||
| `[stdout truncated at ${String(MAX_OUTPUT_BYTES)} bytes; results may be incomplete — use a more specific pattern]`, | ||
| ); | ||
| } | ||
| if (traversalWarning !== undefined) { | ||
| lines.push(traversalWarning); | ||
| warnings.push(traversalWarning); | ||
| } | ||
| if (truncated) { | ||
| lines.push(`[Truncated at ${String(MAX_MATCHES)} matches — use a more specific pattern]`); | ||
| lines.push(`Only the first ${String(MAX_MATCHES)} matches are returned.`); | ||
| } | ||
| lines.push(...displayLines); | ||
| if (filteredSensitive > 0) { | ||
| lines.push(`Filtered ${String(filteredSensitive)} sensitive file(s).`); | ||
| const pageNotices = (count: number, characterLimited: boolean) => { | ||
| const lines = [...warnings]; | ||
| const footer: string[] = []; | ||
| const truncated = characterLimited || offset + count < kept.length; | ||
| if (count === 0) { | ||
| if (kept.length > 0) { | ||
| const resultSet = partial ? 'collected partial result set' : 'current result set'; | ||
| lines.push( | ||
| `No more matches at offset=${String(offset)} in the ${resultSet} (${String(kept.length)} matches).`, | ||
| ); | ||
| } else if (partial) { | ||
| lines.push('No matches collected; search incomplete.'); | ||
| } else if (filteredSensitive > 0) { | ||
| lines.push( | ||
| `No non-sensitive matches found (${String(filteredSensitive)} sensitive file(s) filtered).`, | ||
| ); | ||
| } else { | ||
| lines.push('No matches found'); | ||
| } | ||
| } else if (truncated || offset > 0 || partial) { | ||
| const total = partial | ||
| ? `${String(kept.length)} collected matches (partial result set)` | ||
| : String(kept.length); | ||
| lines.push(`Showing matches ${String(offset + 1)}–${String(offset + count)} of ${total}.`); | ||
| } | ||
| if (characterLimited) lines.push('Character limit reached; only complete paths are returned.'); | ||
| if (truncated) { | ||
| lines.push( | ||
| `Continue with the same search arguments and offset=${String(offset + count)}.`, | ||
| ); | ||
| if (!characterLimited) lines.push('To remove the match-count limit, omit offset and use head_limit=0.'); | ||
| } | ||
| if (filteredSensitive > 0 && (kept.length > 0 || partial)) { | ||
| footer.push(`Filtered ${String(filteredSensitive)} sensitive file(s).`); | ||
| } | ||
| if (!truncated && !partial && offset === 0 && headLimit > 0 && count === headLimit) { | ||
| footer.push(`Found ${String(count)} matches`); | ||
| } | ||
| return { lines, footer }; | ||
| }; | ||
| const noticeChars = Math.max(...[false, true].map((characterLimited) => { | ||
| const { lines, footer } = pageNotices(candidates.length, characterLimited); | ||
| return [...lines, ...footer].join('\n').length + 2; | ||
| })); | ||
| let remaining = DEFAULT_TOOL_RESULT_MAX_RETAINED_CHARS - noticeChars; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a broad search returns at least one match plus close to the 10 MiB stderr cap of ASCII traversal errors, Useful? React with 👍 / 👎. |
||
| const displayLines: string[] = []; | ||
| for (const path of candidates) { | ||
| if (path.length + 1 > remaining) break; | ||
| displayLines.push(path); | ||
| remaining -= path.length + 1; | ||
| } | ||
| if (!truncated && limited.length === MAX_MATCHES) { | ||
| lines.push(`Found ${String(limited.length)} matches`); | ||
| if (candidates.length > 0 && displayLines.length === 0) { | ||
| return { | ||
| isError: true, | ||
| output: 'Glob cannot fit a complete path and its diagnostics within the output limit. Narrow the search path or pattern.', | ||
| }; | ||
| } | ||
| return { output: lines.join('\n') }; | ||
| const notices = pageNotices(displayLines.length, displayLines.length < candidates.length); | ||
| return { output: [...notices.lines, ...displayLines, ...notices.footer].join('\n') }; | ||
| } | ||
| } | ||
|
|
||
|
|
||
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.
When
head_limit=0produces roughly 10.0–10.5 million ASCII characters, this returns the entire collected list, but the downstream truncation pipeline saves only the first 10,000,000 characters (DEFAULT_TOOL_RESULT_MAX_RETAINED_CHARS), whilerunRgOncecan collect up to10 * 1024 * 1024bytes. Consequently, the spill file advertised for retrieving all collected matches loses its tail, potentially in the middle of a path, andReadcannot recover those matches; either keep this result within the retention cap or provide a complete tool-owned spill.Useful? React with 👍 / 👎.