Skip to content

Commit 0e67981

Browse files
phodalphodal
committed
refactor(prompt): extract and simplify lint summary formatting #463
Refactored formatSummary to use helper methods for file and issue grouping, improving readability and maintainability. Removed groupIssuesByContext and inlined context grouping logic. Updated tests to use ModifiedCodeRange directly.
1 parent 5b47539 commit 0e67981

File tree

2 files changed

+118
-144
lines changed

2 files changed

+118
-144
lines changed

‎mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/CodeReviewAgentPromptRenderer.kt‎

Lines changed: 93 additions & 132 deletions
Original file line numberDiff line numberDiff line change
@@ -281,178 +281,139 @@ $result
281281
fun formatSummary(
282282
lintResults: List<LintFileResult>,
283283
modifiedCodeRanges: Map<String, List<ModifiedCodeRange>>
284-
): String = if (lintResults.isNotEmpty()) {
285-
buildString {
286-
val totalErrors = lintResults.sumOf { it.errorCount }
287-
val totalWarnings = lintResults.sumOf { it.warningCount }
284+
): String {
285+
if (lintResults.isEmpty()) {
286+
return "No lint issues found."
287+
}
288288

289+
val totalErrors = lintResults.sumOf { it.errorCount }
290+
val totalWarnings = lintResults.sumOf { it.warningCount }
291+
val filesWithErrors = lintResults.filter { it.errorCount > 0 }.sortedByDescending { it.errorCount }
292+
val filesWithWarningsOnly = lintResults.filter { it.errorCount == 0 && it.warningCount > 0 }
293+
294+
return buildString {
289295
appendLine("**Summary**: $totalErrors error(s), $totalWarnings warning(s) across ${lintResults.size} file(s)")
290296
appendLine()
291297

292-
// Show files with errors, grouped by function context
293-
val filesWithErrors = lintResults.filter { it.errorCount > 0 }.sortedByDescending { it.errorCount }
298+
// Files with errors (CRITICAL)
294299
if (filesWithErrors.isNotEmpty()) {
295300
appendLine("## 🔴 CRITICAL - Files with Errors (MUST FIX)")
296301
appendLine()
297-
298302
filesWithErrors.forEach { file ->
299-
appendLine("### File: ${file.filePath}")
300-
appendLine("**Total**: ${file.errorCount} error(s), ${file.warningCount} warning(s)")
301-
appendLine()
302-
303-
// Group errors by function context
304-
val errors = file.issues.filter { it.severity == LintSeverity.ERROR }
305-
if (errors.isNotEmpty()) {
306-
val errorsByContext = groupIssuesByContext(file.filePath, errors, modifiedCodeRanges)
307-
308-
appendLine("**Errors (grouped by function/class):**")
309-
errorsByContext.forEach { (contextKey, issues) ->
310-
val context = contextKey.context
311-
val header = if (context != null) {
312-
"**In `${context.elementName}` (${context.elementType.lowercase()}, lines ${context.startLine}-${context.endLine})**:"
313-
} else {
314-
"**No specific function context**:"
315-
}
316-
appendLine("- $header")
317-
318-
issues.forEach { issue ->
319-
appendLine(" - Line ${issue.line}: ${issue.message}")
320-
if (issue.rule != null && issue.rule.isNotBlank()) {
321-
appendLine(" - Rule: `${issue.rule}`")
322-
}
323-
if (issue.suggestion != null && issue.suggestion.isNotBlank()) {
324-
appendLine(" - Suggestion: ${issue.suggestion}")
325-
}
326-
}
327-
appendLine()
328-
}
329-
}
330-
331-
// Show warnings for the same file (if any), also grouped
332-
val warnings = file.issues.filter { issue -> issue.severity == LintSeverity.WARNING }
333-
if (warnings.isNotEmpty()) {
334-
val warningsByContext = groupIssuesByContext(file.filePath, warnings, modifiedCodeRanges)
335-
336-
appendLine("**Warnings** (${warnings.size} total, grouped by function/class):")
337-
warningsByContext.entries.take(3).forEach { entry ->
338-
val context = entry.key.context
339-
val issues = entry.value
340-
val header = if (context != null) {
341-
"In `${context.elementName}` (${context.elementType.lowercase()})"
342-
} else {
343-
"No specific function context"
344-
}
345-
val lines = issues.map { issue -> issue.line }.sorted().joinToString(", ")
346-
appendLine("- $header - ${issues.size} warning(s) at lines: $lines")
347-
}
348-
if (warningsByContext.size > 3) {
349-
appendLine("... and ${warningsByContext.size - 3} more contexts with warnings")
350-
}
351-
appendLine()
352-
}
303+
formatFileIssues(file, modifiedCodeRanges, this)
353304
}
354305
appendLine("---")
355306
appendLine()
356307
}
357308

358-
// Show warning-only files (grouped by function context)
359-
val filesWithWarningsOnly = lintResults.filter { it.errorCount == 0 && it.warningCount > 0 }
309+
// Files with warnings only (Lower priority)
360310
if (filesWithWarningsOnly.isNotEmpty()) {
361311
appendLine("## ⚠️ WARNINGS ONLY - Lower Priority")
362312
appendLine()
363313
appendLine("**${filesWithWarningsOnly.size} file(s) with warnings only:**")
364314
appendLine()
365-
366-
val totalWarningsCount = filesWithWarningsOnly.sumOf { it.warningCount }
367-
appendLine("Total warnings: $totalWarningsCount across ${filesWithWarningsOnly.size} file(s)")
368-
appendLine()
369-
370-
// Show all files with warnings grouped by function context
371315
filesWithWarningsOnly.forEach { file ->
372-
appendLine("### File: ${file.filePath}")
373-
appendLine("**Total**: ${file.warningCount} warning(s)")
374-
appendLine()
375-
376-
// Group warnings by context (function/class)
377-
val warnings = file.issues.filter { issue -> issue.severity == LintSeverity.WARNING }
378-
val warningsByContext = groupIssuesByContext(file.filePath, warnings, modifiedCodeRanges)
379-
380-
warningsByContext.entries.take(5).forEach { entry ->
381-
val context = entry.key.context
382-
val issues = entry.value
383-
if (context != null) {
384-
val lines = issues.map { issue -> issue.line }.sorted().joinToString(", ")
385-
appendLine("- **In `${context.elementName}` (${context.elementType.lowercase()})**: ${issues.size} warning(s) at lines $lines")
386-
// Show first warning message as example
387-
val firstIssue = issues.first()
388-
appendLine(" - Example: ${firstIssue.message}")
389-
if (firstIssue.rule != null && firstIssue.rule!!.isNotBlank()) {
390-
appendLine(" - Rule: `${firstIssue.rule}`")
391-
}
392-
} else {
393-
// No context - group by rule instead
394-
val byRule = issues.groupBy { issue -> issue.rule ?: "no-rule" }
395-
byRule.forEach { (rule, ruleIssues) ->
396-
val lines = ruleIssues.map { issue -> issue.line }.sorted().joinToString(", ")
397-
appendLine("- **Rule `$rule`**: ${ruleIssues.size} warning(s) at lines $lines")
398-
appendLine(" - ${ruleIssues.first().message}")
399-
}
400-
}
401-
}
402-
403-
if (warningsByContext.size > 5) {
404-
appendLine("... and ${warningsByContext.size - 5} more function/class contexts with warnings")
405-
}
406-
appendLine()
316+
formatFileIssues(file, modifiedCodeRanges, this, showWarningsOnly = true)
407317
}
408318
}
409319
}
410-
} else {
411-
"No lint issues found."
412320
}
413321

414-
private data class ContextKey(val context: ModifiedCodeRange?)
322+
private fun formatFileIssues(
323+
file: LintFileResult,
324+
modifiedCodeRanges: Map<String, List<ModifiedCodeRange>>,
325+
output: StringBuilder,
326+
showWarningsOnly: Boolean = false
327+
) {
328+
output.appendLine("### File: ${file.filePath}")
329+
output.appendLine("**Total**: ${file.errorCount} error(s), ${file.warningCount} warning(s)")
330+
output.appendLine()
331+
332+
val errors = file.issues.filter { it.severity == LintSeverity.ERROR }
333+
val warnings = file.issues.filter { it.severity == LintSeverity.WARNING }
334+
335+
// Format errors
336+
if (errors.isNotEmpty() && !showWarningsOnly) {
337+
formatIssuesByContext(file.filePath, errors, modifiedCodeRanges, "Errors", output)
338+
}
415339

416-
/**
417-
* Group lint issues by their function/class context
418-
* Issues in the same function will be grouped together
419-
*
420-
* @param filePath The file path
421-
* @param issues List of lint issues to group
422-
* @param modifiedCodeRanges Map of file path to modified code ranges
423-
* @return Map of context to list of issues in that context
424-
*/
425-
private fun groupIssuesByContext(
340+
// Format warnings
341+
if (warnings.isNotEmpty()) {
342+
val label = if (showWarningsOnly) "Warnings" else "Warnings (${warnings.size} total, grouped by function/class)"
343+
formatIssuesByContext(file.filePath, warnings, modifiedCodeRanges, label, output, maxContexts = if (showWarningsOnly) 5 else 3, showDetails = !showWarningsOnly)
344+
}
345+
346+
output.appendLine()
347+
}
348+
349+
private fun formatIssuesByContext(
426350
filePath: String,
427351
issues: List<LintIssue>,
428-
modifiedCodeRanges: Map<String, List<ModifiedCodeRange>>
429-
): Map<ContextKey, List<LintIssue>> {
430-
return issues.groupBy { issue ->
431-
val context = findFunctionContext(filePath, issue.line, modifiedCodeRanges)
432-
ContextKey(context)
352+
modifiedCodeRanges: Map<String, List<ModifiedCodeRange>>,
353+
label: String,
354+
output: StringBuilder,
355+
maxContexts: Int = Int.MAX_VALUE,
356+
showDetails: Boolean = true
357+
) {
358+
val issuesByContext = issues.groupBy { issue ->
359+
findFunctionContext(filePath, issue.line, modifiedCodeRanges)
360+
}
361+
362+
output.appendLine("**$label:**")
363+
364+
issuesByContext.entries.take(maxContexts).forEach { (context, contextIssues) ->
365+
val header = if (context != null) {
366+
if (showDetails) {
367+
"**In `${context.elementName}` (${context.elementType.lowercase()}, lines ${context.startLine}-${context.endLine})**:"
368+
} else {
369+
"In `${context.elementName}` (${context.elementType.lowercase()})"
370+
}
371+
} else {
372+
if (showDetails) {
373+
"**No specific function context**:"
374+
} else {
375+
"No specific function context"
376+
}
377+
}
378+
output.appendLine("- $header")
379+
380+
if (showDetails) {
381+
contextIssues.forEach { issue ->
382+
output.appendLine(" - Line ${issue.line}: ${issue.message}")
383+
if (issue.rule != null && issue.rule.isNotBlank()) {
384+
output.appendLine(" - Rule: `${issue.rule}`")
385+
}
386+
if (issue.suggestion != null && issue.suggestion.isNotBlank()) {
387+
output.appendLine(" - Suggestion: ${issue.suggestion}")
388+
}
389+
}
390+
} else {
391+
// For warnings in error files, just show line numbers
392+
val lines = contextIssues.map { it.line }.sorted().joinToString(", ")
393+
output.appendLine(" - ${contextIssues.size} warning(s) at lines: $lines")
394+
}
395+
output.appendLine()
396+
}
397+
398+
if (issuesByContext.size > maxContexts) {
399+
output.appendLine("... and ${issuesByContext.size - maxContexts} more contexts")
400+
output.appendLine()
433401
}
434402
}
435403

436404
/**
437405
* Find the function/class context for a given line number
438406
* Returns the most specific (smallest) context that contains the line
439-
*
440-
* @param filePath The file path
441-
* @param lineNumber The line number to find context for
442-
* @param modifiedCodeRanges Map of file path to modified code ranges
443-
* @return The code range containing this line, or null if not found
444407
*/
445408
private fun findFunctionContext(
446409
filePath: String,
447410
lineNumber: Int,
448411
modifiedCodeRanges: Map<String, List<ModifiedCodeRange>>
449412
): ModifiedCodeRange? {
450413
val ranges = modifiedCodeRanges[filePath] ?: return null
451-
val matchingContexts = ranges.filter { range ->
452-
lineNumber in range.startLine..range.endLine
453-
}
454-
455-
return matchingContexts.minByOrNull { it.endLine - it.startLine }
414+
return ranges
415+
.filter { lineNumber in it.startLine..it.endLine }
416+
.minByOrNull { it.endLine - it.startLine }
456417
}
457418
}
458419

‎mpp-core/src/commonTest/kotlin/cc/unitmesh/agent/CodeReviewAgentPromptRendererTest.kt‎

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package cc.unitmesh.agent
22

3+
import cc.unitmesh.agent.codereview.ModifiedCodeRange
34
import cc.unitmesh.agent.linter.LintFileResult
45
import cc.unitmesh.agent.linter.LintIssue
56
import cc.unitmesh.agent.linter.LintSeverity
@@ -103,17 +104,21 @@ class CodeReviewAgentPromptRendererTest {
103104

104105
val modifiedCodeRanges = mapOf(
105106
"mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/CodeReviewAgent.kt" to listOf(
106-
CodeReviewAgentPromptRenderer.CodeContext(
107+
ModifiedCodeRange(
108+
filePath = "mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/CodeReviewAgent.kt",
107109
elementName = "CodeReviewAgent",
108110
elementType = "CLASS",
109111
startLine = 79,
110-
endLine = 444
112+
endLine = 444,
113+
modifiedLines = emptyList()
111114
),
112-
CodeReviewAgentPromptRenderer.CodeContext(
115+
ModifiedCodeRange(
116+
filePath = "mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/CodeReviewAgent.kt",
113117
elementName = "generateFixes",
114118
elementType = "FUNCTION",
115119
startLine = 197,
116-
endLine = 317
120+
endLine = 317,
121+
modifiedLines = emptyList()
117122
)
118123
)
119124
)
@@ -183,11 +188,13 @@ class CodeReviewAgentPromptRendererTest {
183188

184189
val modifiedCodeRanges = mapOf(
185190
"src/main/kotlin/Example.kt" to listOf(
186-
CodeReviewAgentPromptRenderer.CodeContext(
191+
ModifiedCodeRange(
192+
filePath = "src/main/kotlin/Example.kt",
187193
elementName = "processData",
188194
elementType = "METHOD",
189195
startLine = 10,
190-
endLine = 50
196+
endLine = 50,
197+
modifiedLines = emptyList()
191198
)
192199
)
193200
)
@@ -265,11 +272,13 @@ class CodeReviewAgentPromptRendererTest {
265272

266273
val modifiedCodeRanges = mapOf(
267274
"src/main/kotlin/Example.kt" to listOf(
268-
CodeReviewAgentPromptRenderer.CodeContext(
275+
ModifiedCodeRange(
276+
filePath = "src/main/kotlin/Example.kt",
269277
elementName = "calculate",
270278
elementType = "FUNCTION",
271279
startLine = 10,
272-
endLine = 30
280+
endLine = 30,
281+
modifiedLines = emptyList()
273282
)
274283
)
275284
)
@@ -335,11 +344,13 @@ class CodeReviewAgentPromptRendererTest {
335344

336345
val modifiedCodeRanges = mapOf(
337346
"src/main/kotlin/Example.kt" to listOf(
338-
CodeReviewAgentPromptRenderer.CodeContext(
347+
ModifiedCodeRange(
348+
filePath = "src/main/kotlin/Example.kt",
339349
elementName = "MyClass",
340350
elementType = "CLASS",
341351
startLine = 20,
342-
endLine = 50
352+
endLine = 50,
353+
modifiedLines = emptyList()
343354
)
344355
)
345356
)
@@ -397,11 +408,13 @@ class CodeReviewAgentPromptRendererTest {
397408

398409
val modifiedCodeRanges = mapOf(
399410
"mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/CodeReviewAgent.kt" to listOf(
400-
CodeReviewAgentPromptRenderer.CodeContext(
411+
ModifiedCodeRange(
412+
filePath = "mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/CodeReviewAgent.kt",
401413
elementName = "CodeReviewAgent",
402414
elementType = "CLASS",
403415
startLine = 79,
404-
endLine = 444
416+
endLine = 444,
417+
modifiedLines = emptyList()
405418
)
406419
)
407420
)

0 commit comments

Comments
 (0)