Add read_dir AI tool for reading directory contents #2414
Conversation
|
|
Co-authored-by: sawka <2722291+sawka@users.noreply.github.com>
Co-authored-by: sawka <2722291+sawka@users.noreply.github.com>
Copilot
AI
changed the title
|
@copilot can you undo the changes to the go.mod and go.sum files in the tsunami directories. those shouldn't be changed by this PR (out of scope) |
Co-authored-by: sawka <2722291+sawka@users.noreply.github.com>
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the WalkthroughAdds a new read_dir tool to aiusechat. Implements read_dir logic to parse inputs (path required, optional max_entries with default), expand ~, validate directory, read entries, sort (dirs first, then files; alphabetical within groups), truncate per max_entries, and build structured results plus a human-readable listing. Returns metadata including path, absolute_path, entry_count, total_entries, entries, listing, truncated flag, and parent_dir when applicable. Exposes GetReadDirToolDefinition and a ReadDirDefaultMaxEntries constant. Integrates the tool into the tools list in tools.go when widgetAccess is true, immediately after the read_text_file tool. Includes comprehensive unit tests for success, errors, truncation, and definition metadata. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pkg/aiusechat/tools_readdir_test.go (1)
106-109: Avoid slicing the error string to check its prefix.Taking
err.Error()[:len(expectedErrSubstr)]will panic if the message ever shortens. A simplestrings.Contains(orstrings.HasPrefix) keeps the assertion stable without depending on length.- expectedErrSubstr := "path is not a directory" - if err.Error()[:len(expectedErrSubstr)] != expectedErrSubstr { - t.Errorf("Expected error containing %q, got %q", expectedErrSubstr, err.Error()) - } + if !strings.Contains(err.Error(), "path is not a directory") { + t.Errorf("Expected error containing %q, got %q", "path is not a directory", err.Error()) + }Add
stringsto the import list above.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
pkg/aiusechat/tools.go(1 hunks)pkg/aiusechat/tools_readdir.go(1 hunks)pkg/aiusechat/tools_readdir_test.go(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Analyze (go)
- GitHub Check: merge-gatekeeper
- GitHub Check: Build for TestDriver.ai
pkg/aiusechat/tools_readdir.go
Outdated
| maxEntries := *params.MaxEntries | ||
| var truncated bool | ||
| if len(entries) > maxEntries { | ||
| entries = entries[:maxEntries] | ||
| truncated = true | ||
| } | ||
|
|
||
| // Sort entries: directories first, then files, alphabetically within each group | ||
| sort.Slice(entries, func(i, j int) bool { | ||
| if entries[i].IsDir() != entries[j].IsDir() { | ||
| return entries[i].IsDir() | ||
| } | ||
| return entries[i].Name() < entries[j].Name() | ||
| }) | ||
|
|
||
| var entryList []map[string]any | ||
| for _, entry := range entries { | ||
| info, err := entry.Info() | ||
| if err != nil { | ||
| // Skip entries we can't stat | ||
| continue | ||
| } | ||
|
|
||
| entryData := map[string]any{ | ||
| "name": entry.Name(), | ||
| "is_dir": entry.IsDir(), | ||
| "mode": info.Mode().String(), | ||
| "modified": utilfn.FormatRelativeTime(info.ModTime()), | ||
| } | ||
|
|
||
| if !entry.IsDir() { | ||
| entryData["size"] = info.Size() | ||
| } | ||
|
|
||
| entryList = append(entryList, entryData) | ||
| } | ||
|
|
||
| // Create a formatted directory listing | ||
| var listing strings.Builder | ||
| for _, entry := range entryList { | ||
| name := entry["name"].(string) | ||
| isDir := entry["is_dir"].(bool) | ||
| mode := entry["mode"].(string) | ||
| modified := entry["modified"].(string) | ||
|
|
||
| if isDir { | ||
| listing.WriteString(fmt.Sprintf("[DIR] %-40s %s %s\n", name, mode, modified)) | ||
| } else { | ||
| size := entry["size"].(int64) | ||
| listing.WriteString(fmt.Sprintf("[FILE] %-40s %10d %s %s\n", name, size, mode, modified)) | ||
| } | ||
| } | ||
|
|
||
| result := map[string]any{ | ||
| "path": params.Path, | ||
| "absolute_path": expandedPath, | ||
| "entry_count": len(entryList), | ||
| "total_entries": len(entries), | ||
| "entries": entryList, | ||
| "listing": strings.TrimSuffix(listing.String(), "\n"), | ||
| } | ||
|
|
||
| if truncated { | ||
| result["truncated"] = true | ||
| result["truncated_message"] = fmt.Sprintf("Directory listing truncated to %d entries (out of %d+ total). Increase max_entries to see more.", maxEntries, maxEntries) | ||
| } |
There was a problem hiding this comment.
Sort before truncating and preserve the real totals.
We slice entries before sorting them, so when a directory alphabetically falls beyond the first max_entries results, it never makes the listing—even though the feature promises “directories first”. In addition, total_entries (and the truncation message) now report the truncated count, so callers can’t tell how much was omitted. Please sort the full set first, keep the original total, and only then apply truncation so the metadata stays accurate.
- entries, err := os.ReadDir(expandedPath)
+ entries, err := os.ReadDir(expandedPath)
if err != nil {
return nil, fmt.Errorf("failed to read directory: %w", err)
}
- maxEntries := *params.MaxEntries
- var truncated bool
- if len(entries) > maxEntries {
- entries = entries[:maxEntries]
- truncated = true
- }
-
- // Sort entries: directories first, then files, alphabetically within each group
- sort.Slice(entries, func(i, j int) bool {
+ totalEntries := len(entries)
+ maxEntries := *params.MaxEntries
+
+ // Sort entries: directories first, then files, alphabetically within each group
+ sort.Slice(entries, func(i, j int) bool {
if entries[i].IsDir() != entries[j].IsDir() {
return entries[i].IsDir()
}
return entries[i].Name() < entries[j].Name()
})
+
+ var truncated bool
+ if totalEntries > maxEntries {
+ entries = entries[:maxEntries]
+ truncated = true
+ }
@@
result := map[string]any{
"path": params.Path,
"absolute_path": expandedPath,
"entry_count": len(entryList),
- "total_entries": len(entries),
+ "total_entries": totalEntries,
"entries": entryList,
"listing": strings.TrimSuffix(listing.String(), "\n"),
}
if truncated {
result["truncated"] = true
- result["truncated_message"] = fmt.Sprintf("Directory listing truncated to %d entries (out of %d+ total). Increase max_entries to see more.", maxEntries, maxEntries)
+ result["truncated_message"] = fmt.Sprintf("Directory listing truncated to %d entries out of %d total. Increase max_entries to see more.", len(entryList), totalEntries)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| maxEntries := *params.MaxEntries | |
| var truncated bool | |
| if len(entries) > maxEntries { | |
| entries = entries[:maxEntries] | |
| truncated = true | |
| } | |
| // Sort entries: directories first, then files, alphabetically within each group | |
| sort.Slice(entries, func(i, j int) bool { | |
| if entries[i].IsDir() != entries[j].IsDir() { | |
| return entries[i].IsDir() | |
| } | |
| return entries[i].Name() < entries[j].Name() | |
| }) | |
| var entryList []map[string]any | |
| for _, entry := range entries { | |
| info, err := entry.Info() | |
| if err != nil { | |
| // Skip entries we can't stat | |
| continue | |
| } | |
| entryData := map[string]any{ | |
| "name": entry.Name(), | |
| "is_dir": entry.IsDir(), | |
| "mode": info.Mode().String(), | |
| "modified": utilfn.FormatRelativeTime(info.ModTime()), | |
| } | |
| if !entry.IsDir() { | |
| entryData["size"] = info.Size() | |
| } | |
| entryList = append(entryList, entryData) | |
| } | |
| // Create a formatted directory listing | |
| var listing strings.Builder | |
| for _, entry := range entryList { | |
| name := entry["name"].(string) | |
| isDir := entry["is_dir"].(bool) | |
| mode := entry["mode"].(string) | |
| modified := entry["modified"].(string) | |
| if isDir { | |
| listing.WriteString(fmt.Sprintf("[DIR] %-40s %s %s\n", name, mode, modified)) | |
| } else { | |
| size := entry["size"].(int64) | |
| listing.WriteString(fmt.Sprintf("[FILE] %-40s %10d %s %s\n", name, size, mode, modified)) | |
| } | |
| } | |
| result := map[string]any{ | |
| "path": params.Path, | |
| "absolute_path": expandedPath, | |
| "entry_count": len(entryList), | |
| "total_entries": len(entries), | |
| "entries": entryList, | |
| "listing": strings.TrimSuffix(listing.String(), "\n"), | |
| } | |
| if truncated { | |
| result["truncated"] = true | |
| result["truncated_message"] = fmt.Sprintf("Directory listing truncated to %d entries (out of %d+ total). Increase max_entries to see more.", maxEntries, maxEntries) | |
| } | |
| entries, err := os.ReadDir(expandedPath) | |
| if err != nil { | |
| return nil, fmt.Errorf("failed to read directory: %w", err) | |
| } | |
| totalEntries := len(entries) | |
| maxEntries := *params.MaxEntries | |
| // Sort entries: directories first, then files, alphabetically within each group | |
| sort.Slice(entries, func(i, j int) bool { | |
| if entries[i].IsDir() != entries[j].IsDir() { | |
| return entries[i].IsDir() | |
| } | |
| return entries[i].Name() < entries[j].Name() | |
| }) | |
| var truncated bool | |
| if totalEntries > maxEntries { | |
| entries = entries[:maxEntries] | |
| truncated = true | |
| } | |
| var entryList []map[string]any | |
| for _, entry := range entries { | |
| info, err := entry.Info() | |
| if err != nil { | |
| // Skip entries we can't stat | |
| continue | |
| } | |
| entryData := map[string]any{ | |
| "name": entry.Name(), | |
| "is_dir": entry.IsDir(), | |
| "mode": info.Mode().String(), | |
| "modified": utilfn.FormatRelativeTime(info.ModTime()), | |
| } | |
| if !entry.IsDir() { | |
| entryData["size"] = info.Size() | |
| } | |
| entryList = append(entryList, entryData) | |
| } | |
| // Create a formatted directory listing | |
| var listing strings.Builder | |
| for _, entry := range entryList { | |
| name := entry["name"].(string) | |
| isDir := entry["is_dir"].(bool) | |
| mode := entry["mode"].(string) | |
| modified := entry["modified"].(string) | |
| if isDir { | |
| listing.WriteString(fmt.Sprintf("[DIR] %-40s %s %s\n", name, mode, modified)) | |
| } else { | |
| size := entry["size"].(int64) | |
| listing.WriteString(fmt.Sprintf("[FILE] %-40s %10d %s %s\n", name, size, mode, modified)) | |
| } | |
| } | |
| result := map[string]any{ | |
| "path": params.Path, | |
| "absolute_path": expandedPath, | |
| "entry_count": len(entryList), | |
| "total_entries": totalEntries, | |
| "entries": entryList, | |
| "listing": strings.TrimSuffix(listing.String(), "\n"), | |
| } | |
| if truncated { | |
| result["truncated"] = true | |
| result["truncated_message"] = fmt.Sprintf( | |
| "Directory listing truncated to %d entries out of %d total. Increase max_entries to see more.", | |
| len(entryList), totalEntries, | |
| ) | |
| } |
🤖 Prompt for AI Agents
In pkg/aiusechat/tools_readdir.go around lines 77 to 142: the code currently
truncates entries before sorting and uses the truncated length as total_entries
and in the truncated message; instead, preserve the full entries slice for
sorting, record originalTotal := len(entries) before any truncation, sort the
full entries (directories first, then names), then apply truncation into a
separate slice when originalTotal > maxEntries and set truncated = true; set
"total_entries" to originalTotal (not len of truncated slice) and format the
truncated_message to show maxEntries and originalTotal so callers know how many
were omitted.
|
Sort before truncating and preserve the real totals. We slice entries before sorting them, so when a directory alphabetically falls beyond the first max_entries results, it never makes the listing—even though the feature promises “directories first”. In addition, total_entries (and the truncation message) now report the truncated count, so callers can’t tell how much was omitted. Please sort the full set first, keep the original total, and only then apply truncation so the metadata stays accurate. |
Co-authored-by: sawka <2722291+sawka@users.noreply.github.com>
Fixed in commit b723292. Now sorts the full entry list first (ensuring directories come first), preserves the original total count, then truncates. The |
|
closing/reopening to try to re-trigger CLA |
|
closing/reopening to try to re-trigger CLA |
3 participants
Summary
Successfully implemented a new
read_dirAI tool that reads and lists directory contents, following the same pattern as the existingread_text_filetool.Key Features:
Files Changed:
pkg/aiusechat/tools_readdir.go- Main implementation (189 lines)pkg/aiusechat/tools_readdir_test.go- Comprehensive tests (211 lines)pkg/aiusechat/tools.go- Tool registration (1 line)Testing:
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.