feat(agent): group semantically related files for multi-file review

[lizhengfeng101]

Summary

• Introduce semantic file grouping: related files (e.g., .go + _test.go, header + implementation) are reviewed together in a single LLM conversation for better cross-file analysis.
• Add grouping.go with grouping logic and enforceMaxFilesPerGroup (max 10 files/group) + token budget enforcement.
• Revamp plan/review prompts: replace JSON plan format with a structured MUST/SHOULD directive using [quick]/[deep] cost tags; remove MAY category to reduce wasted investigation rounds.
• Update main_task_system.md to encourage "comment as you go" instead of batching comments until all items are investigated.
• Preserve comments on failure: when all groups fail but CommentCollector already has comments, return them instead of discarding. Rounds-exhausted groups with existing comments are no longer marked as hard errors.
• Adapt code_comment tool to accept a path field for multi-file group attribution.

Motivation

Per-file review misses cross-file inconsistencies (e.g., interface change without updating callers). Grouping related files in one conversation enables the model to catch these issues. The prompt and failure-handling changes address recall regression observed during evaluation (model over-investigating without producing comments, and produced comments being discarded on partial failures).

Test plan

• make check passes (format, vet, license headers)
• make test passes (unit tests including new grouping tests)
• Run evaluation benchmark to confirm recall recovery

[github-actions]

🔍 OpenCodeReview found 4 issue(s) in this PR.

• ✅ Successfully posted inline: 4 comment(s)

[github-actions]

[bug · critical]
CommentWorkerPool key mismatch: The AwaitKey call here uses groupKey (derived from fileGroupKey(g.Diffs)), but the LLM loop's executeToolCall (in internal/llmloop/loop.go:434) still submits work via pool.SubmitFor(newPath, ...) where newPath is the individual file path. Since groupKey for multi-file groups is a comma-separated sorted string of paths, AwaitKey(groupKey) will wait on a key that was never submitted, potentially blocking indefinitely or returning immediately (depending on the AwaitKey implementation for unknown keys). Either the submission key must also be groupKey, or a different awaiting strategy is needed.

[github-actions]

[maintainability · low]
The IsDeleted case in the switch statement is unreachable dead code. In agent.go:521-527, deleted files are filtered out before calling groupDiffs, so d.IsDeleted will never be true for any diff passed to this function. This dead code is misleading and adds unnecessary complexity. Either remove the case or add a comment explaining it's defensive programming for future-proofing.

[github-actions]

[maintainability · medium]
The fence-stripping logic in parseGroupingResponse duplicates functionality already available in stripMarkdownFences (util.go:34). The existing utility is more robust and handles language-tagged fences like ```json. Additionally, this manual implementation doesn't handle cases where the LLM adds explanatory text before/after the JSON block, which will cause parsing failures and trigger the per-file fallback unnecessarily. Consider using stripMarkdownFences or implementing JSON extraction logic similar to `parseFilterResponse` to improve robustness.

Suggestion:

Suggested change

	content = strings.TrimSpace(content)
	// Strip markdown code fences if present
	if strings.HasPrefix(content, "```") {
	lines := strings.Split(content, "\n")
	if len(lines) >= 2 {
	lines = lines[1:]
	}
	if len(lines) > 0 && strings.HasPrefix(strings.TrimSpace(lines[len(lines)-1]), "```") {
	lines = lines[:len(lines)-1]
	}
	content = strings.Join(lines, "\n")
	}
	content = stripMarkdownFences(content)

[github-actions]

[performance · medium]
When a group's total tokens exceed tokenLimit, every file in the group is split into its own single-file group, even if some files are small and could be safely combined. This can result in many unnecessary LLM calls. Consider a smarter splitting strategy that attempts to pack files into groups that fit within the budget, rather than immediately falling back to per-file dispatch for all files in the group.