rodin/review-bot
2
1
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 6 Wiki Activity

feat: log loaded pattern files for debugging #65

Merged
aweiker merged 1 commits from issue-64 into main 2026-05-11 06:53:00 +00:00
Conversation 4 Commits 1 Files Changed 1
+14
1 changed files with 14 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
cmd/review-bot/main.go
+14
View File
@@ -544,6 +544,9 @@ func fetchPatterns(ctx context.Context, client *gitea.Client, patternsRepo, patt
}
owner, repo := parts[0], parts[1]
var repoLoadedFiles []string
var repoSkippedFiles []string
for _, path := range paths {
path = strings.TrimSpace(path)
if path == "" {
@@ -559,11 +562,22 @@ func fetchPatterns(ctx context.Context, client *gitea.Client, patternsRepo, patt
for filePath, content := range files {
// Only include markdown and text files as patterns
if !isPatternFile(filePath) {
repoSkippedFiles = append(repoSkippedFiles, filePath)
continue
}
repoLoadedFiles = append(repoLoadedFiles, filePath)
sb.WriteString(fmt.Sprintf("### %s/%s\n\n%s\n\n", repoRef, filePath, content))
}
}
if len(repoLoadedFiles) > 0 {
slog.Info("loaded pattern files", "repo", repoRef, "count", len(repoLoadedFiles), "files", repoLoadedFiles)
} else {
sonnet-review-bot commented 2026-05-11 06:38:00 +00:00
Review
Copy Link
Copy Source

[NIT] The files map from client.GetAllFilesInPath is iterated with range, which has non-deterministic order in Go. This means repoLoadedFiles and repoSkippedFiles slices will have a random order each run, making log output non-reproducible. Consider sorting the slices before logging for consistent output (though this is low priority since it's purely diagnostic).

**[NIT]** The `files` map from `client.GetAllFilesInPath` is iterated with `range`, which has non-deterministic order in Go. This means `repoLoadedFiles` and `repoSkippedFiles` slices will have a random order each run, making log output non-reproducible. Consider sorting the slices before logging for consistent output (though this is low priority since it's purely diagnostic).
slog.Warn("no pattern files loaded", "repo", repoRef, "paths", paths)
}
if len(repoSkippedFiles) > 0 {
slog.Debug("skipped non-pattern files", "repo", repoRef, "count", len(repoSkippedFiles), "files", repoSkippedFiles)
}
}
return sb.String()
}
sonnet-review-bot commented 2026-05-11 06:38:00 +00:00
Review
Copy Link
Copy Source

[NIT] The paths variable logged in the warn case ("paths", paths) is the full slice of all configured paths, not the per-repo paths that were actually attempted. This is accurate but may be slightly misleading if paths are repo-specific. The current behavior is fine given the shared paths slice, just worth noting.

**[NIT]** The `paths` variable logged in the warn case (`"paths", paths`) is the full slice of all configured paths, not the per-repo paths that were actually attempted. This is accurate but may be slightly misleading if paths are repo-specific. The current behavior is fine given the shared `paths` slice, just worth noting.