feat: new comment format

internal/plugin/reporter/github_pr.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"fmt"
 	"io"
+	"sort"
 	"strings"
 
 	"github.com/pkg/errors"
@@ -89,44 +90,47 @@ func (s *GithubPullRequest) createCommentBody(changedLinesWithCoverage domain.So
 
 	modules := collectModules(changedLinesWithCoverage)
 
-	bodyLines := []string{
-		"## 📊 Code Coverage — Changed Lines\n",
-		"\n",
-		"> Coverage is measured **only for the lines this PR changes**, not the whole file or repo.\n",
-		"\n",
-	}
+	covered := changedLinesWithCoverage.TotalCoveredInstructions()
+	missed := changedLinesWithCoverage.TotalMissedInstructions()
+	linesWithData := changedLinesWithCoverage.TotalLinesWithData()
+	linesWithoutData := changedLinesWithCoverage.TotalLinesWithoutData()
+
+	totalInstructions := covered + missed
+	totalLines := linesWithData + linesWithoutData
+
+	coveredPct := toPercent(safeDiv(float32(covered), float32(totalInstructions), 1))
+	missedPct := toPercent(safeDiv(float32(missed), float32(totalInstructions), 0))
+	withDataPct := toPercent(safeDiv(float32(linesWithData), float32(totalLines), 1))
+	withoutDataPct := toPercent(safeDiv(float32(linesWithoutData), float32(totalLines), 0))
+
+	var b strings.Builder
+
+	b.WriteString("## 🛡️ Patch Coverage Report\n\n")
+	b.WriteString("> Scope: **changed lines only** — the code this PR adds or edits, not whole files or the repo. ")
+	b.WriteString("It answers one thing — *did your tests run the code you just touched?*\n\n")
 
 	if len(modules) > 0 {
-		bodyLines = append(bodyLines, fmt.Sprintf("*Modules: %v*\n\n", strings.Join(modules, ", ")))
-	}
-
-	bodyLines = append(bodyLines, generateSummaryLines(changedLinesWithCoverage, func(linesWithDataCount int, linesWithoutDataCount int, covered int, missed int) []string {
-		totalLines := linesWithDataCount + linesWithoutDataCount
-		totalInstructions := covered + missed
-
-		coveredPct := toPercent(safeDiv(float32(covered), float32(totalInstructions), 1))
-		missedPct := toPercent(safeDiv(float32(missed), float32(totalInstructions), 0))
-		withDataPct := toPercent(safeDiv(float32(linesWithDataCount), float32(totalLines), 1))
-		withoutDataPct := toPercent(safeDiv(float32(linesWithoutDataCount), float32(totalLines), 0))
-
-		return []string{
-			fmt.Sprintf("### %v Covered Instructions: %.f%% (%d)\n", coverageStatusEmoji(coveredPct), coveredPct, covered),
-			"\n",
-			"| Metric | Result | What it means |\n",
-			"| :-- | :-: | :-- |\n",
-			fmt.Sprintf("| 🟢 **Covered Instructions** | **%.f%%** (%d) | Changed code your tests executed. Higher is better. |\n", coveredPct, covered),
-			fmt.Sprintf("| 🔴 **Missed Instructions** | %.f%% (%d) | Changed code your tests never ran. Lower is better. |\n", missedPct, missed),
-			fmt.Sprintf("| 📈 Lines With Coverage Data | %.f%% (%d) | Changed lines the coverage tool could track. |\n", withDataPct, linesWithDataCount),
-			fmt.Sprintf("| ⚪ Lines Without Coverage Data | %.f%% (%d) | Changed lines with no data: comments, blanks, declarations. |\n", withoutDataPct, linesWithoutDataCount),
-		}
-	})...)
+		fmt.Fprintf(&b, "*Modules:* %v\n\n", strings.Join(modules, ", "))
+	}
+
+	fmt.Fprintf(&b, "**Diff coverage:** `%.f%%` %v — `%d` of `%d` changed instructions covered\n\n",
+		coveredPct, coverageStatusEmoji(coveredPct), covered, totalInstructions)
+
+	b.WriteString("| Metric | Value | |\n")
+	b.WriteString("| :-- | --: | :-- |\n")
+	fmt.Fprintf(&b, "| 🟢 Covered instructions | `%d` (%.f%%) | changed code your tests executed |\n", covered, coveredPct)
+	fmt.Fprintf(&b, "| 🔴 Missed instructions | `%d` (%.f%%) | changed code your tests never ran |\n", missed, missedPct)
+	fmt.Fprintf(&b, "| 📈 Tracked changed lines | `%d` (%.f%%) | lines the coverage tool could measure |\n", linesWithData, withDataPct)
+	fmt.Fprintf(&b, "| ⚪ Untracked changed lines | `%d` (%.f%%) | comments, blanks, declarations |\n", linesWithoutData, withoutDataPct)
+	b.WriteString("\n")
+	b.WriteString("<sub>**Lines** = the source lines you changed. **Instructions** = the executable units the coverage tool counts inside those lines — one line can hold several (e.g. JaCoCo bytecode), so the two counts differ.</sub>\n\n")
 
-	body := strings.Join(bodyLines, "")
-	body += missedInstructionsSection(changedLinesWithCoverage)
-	body += "\n<sub>🤖 Generated by <a href=\"https://github.com/target/pull-request-code-coverage\">pull-request-code-coverage</a> — coverage for changed lines only.</sub>\n"
+	b.WriteString(fileCoverageSection(changedLinesWithCoverage))
+	b.WriteString(missedInstructionsSection(changedLinesWithCoverage))
+	b.WriteString("\n<sub>🤖 Generated by <a href=\"https://github.com/target/pull-request-code-coverage\">pull-request-code-coverage</a> — coverage for changed lines only.</sub>\n")
 
 	data := map[string]string{
-		"body": body,
+		"body": b.String(),
 	}
 
 	dataBytes, marshalErr := s.jsonClient.Marshal(data)
@@ -138,6 +142,107 @@ func (s *GithubPullRequest) createCommentBody(changedLinesWithCoverage domain.So
 	return bytes.NewBuffer(dataBytes), nil
 }
 
+// fileCoverage holds the aggregated changed-line coverage for a single file.
+type fileCoverage struct {
+	path             string
+	covered          int
+	missed           int
+	linesWithData    int
+	linesWithoutData int
+}
+
+// fileCoverageSection renders a per-file breakdown of changed-line coverage,
+// worst-covered files first so the riskiest changes surface at the top.
+func fileCoverageSection(changedLinesWithCoverage domain.SourceLineCoverageReport) string {
+	files := collectFileCoverage(changedLinesWithCoverage)
+
+	if len(files) == 0 {
+		return ""
+	}
+
+	// Files whose only changed lines carry no coverage data (config, docs,
+	// generated, test-only) would just be a wall of "n/a" rows, so keep them
+	// out of the table and summarise the count underneath instead.
+	unmeasured := 0
+
+	var b strings.Builder
+
+	b.WriteString("### Coverage by file\n\n")
+	b.WriteString("| File | Diff coverage | Covered / Missed |\n")
+	b.WriteString("| :-- | :-: | :-: |\n")
+
+	for _, f := range files {
+		instructions := f.covered + f.missed
+
+		if instructions == 0 {
+			unmeasured++
+			continue
+		}
+
+		pct := toPercent(safeDiv(float32(f.covered), float32(instructions), 1))
+		fmt.Fprintf(&b, "| `%v` | %v %.f%% | %d / %d |\n",
+			f.path, coverageStatusEmoji(pct), pct, f.covered, f.missed)
+	}
+
+	b.WriteString("\n")
+
+	if unmeasured > 0 {
+		fmt.Fprintf(&b, "<sub>%d changed file(s) with no measurable lines (config, docs, generated, or test-only) omitted.</sub>\n\n", unmeasured)
+	}
+
+	return b.String()
+}
+
+// collectFileCoverage aggregates changed-line coverage per file, sorted with the
+// lowest-covered files first (files with no measurable instructions last) so a
+// reviewer sees the riskiest files at the top of the table.
+func collectFileCoverage(changedLinesWithCoverage domain.SourceLineCoverageReport) []fileCoverage {
+	order := []string{}
+	byPath := map[string]*fileCoverage{}
+
+	for _, line := range changedLinesWithCoverage {
+		path := filePath(line.SourceLine)
+
+		fc, ok := byPath[path]
+		if !ok {
+			fc = &fileCoverage{path: path}
+			byPath[path] = fc
+			order = append(order, path)
+		}
+
+		fc.covered += line.CoveredInstructionCount
+		fc.missed += line.MissedInstructionCount
+
+		if line.HasData() {
+			fc.linesWithData++
+		} else {
+			fc.linesWithoutData++
+		}
+	}
+
+	result := make([]fileCoverage, 0, len(order))
+	for _, path := range order {
+		result = append(result, *byPath[path])
+	}
+
+	sort.SliceStable(result, func(i, j int) bool {
+		return fileCoveragePct(result[i]) < fileCoveragePct(result[j])
+	})
+
+	return result
+}
+
+// fileCoveragePct returns a file's changed-line coverage as a sortable value;
+// files with no measurable instructions sort last (returned above 100%).
+func fileCoveragePct(f fileCoverage) float32 {
+	instructions := f.covered + f.missed
+	if instructions == 0 {
+		return 101
+	}
+
+	return toPercent(safeDiv(float32(f.covered), float32(instructions), 1))
+}
+
 // missedInstructionsSection renders a collapsible block listing each changed
 // line that was not executed by tests. Returns "" when nothing was missed.
 func missedInstructionsSection(changedLinesWithCoverage domain.SourceLineCoverageReport) string {
@@ -156,7 +261,7 @@ func missedInstructionsSection(changedLinesWithCoverage domain.SourceLineCoverag
 		return ""
 	}
 
-	return fmt.Sprintf("\n<details><summary>🔍 Missed instructions (%d)</summary>\n\n", missedLineCount) +
+	return fmt.Sprintf("\n<details><summary>🔍 Uncovered lines (%d)</summary>\n\n", missedLineCount) +
 		"```\n" + missedInstructions + "```" + "\n</details>\n"
 }
 

internal/plugin/reporter/simple.go

@@ -5,10 +5,13 @@ import (
 
 	"io"
 	"log"
+	"strings"
 
 	"github.com/target/pull-request-code-coverage/internal/plugin/domain"
 )
 
+const consoleRule = "──────────────────────────────────────────────────────────────"
+
 type Simple struct {
 	Out          io.Writer
 	WritingFuncf func(io.Writer, string, ...interface{}) (int, error)
@@ -24,49 +27,119 @@ func NewSimple(out io.Writer) *Simple {
 }
 
 func (s *Simple) Write(changedLinesWithCoverage domain.SourceLineCoverageReport) error {
-	s.printf("Missed Instructions:\n")
-	for _, r := range changedLinesWithCoverage {
-		if r.MissedInstructionCount > 0 {
-			s.printf("--- %v\n", lineDescription(r.SourceLine))
-			s.printf("%v\n", r.LineValue)
-		}
-	}
+	covered := changedLinesWithCoverage.TotalCoveredInstructions()
+	missed := changedLinesWithCoverage.TotalMissedInstructions()
+	linesWithData := changedLinesWithCoverage.TotalLinesWithData()
+	linesWithoutData := changedLinesWithCoverage.TotalLinesWithoutData()
 
-	summaryLines := generateSummaryLines(changedLinesWithCoverage, func(linesWithDataCount int, linesWithoutDataCount int, covered int, missed int) []string {
-		totalLines := linesWithDataCount + linesWithoutDataCount
-		totalInstructions := covered + missed
+	totalInstructions := covered + missed
+	totalLines := linesWithData + linesWithoutData
 
-		result := make([]string, 5)
+	coveredPct := toPercent(safeDiv(float32(covered), float32(totalInstructions), 1))
+	missedPct := toPercent(safeDiv(float32(missed), float32(totalInstructions), 0))
+	withDataPct := toPercent(safeDiv(float32(linesWithData), float32(totalLines), 1))
+	withoutDataPct := toPercent(safeDiv(float32(linesWithoutData), float32(totalLines), 0))
 
-		result[0] = "Code Coverage Summary:\n"
-		result[1] = fmt.Sprintf("Lines Without Coverage Data -> %.f%% (%d)\n", toPercent(safeDiv(float32(linesWithoutDataCount), float32(totalLines), 0)), linesWithoutDataCount)
-		result[2] = fmt.Sprintf("Lines With Coverage Data    -> %.f%% (%d)\n", toPercent(safeDiv(float32(linesWithDataCount), float32(totalLines), 1)), linesWithDataCount)
-		result[3] = fmt.Sprintf("Covered Instructions        -> %.f%% (%d)\n", toPercent(safeDiv(float32(covered), float32(totalInstructions), 1)), covered)
-		result[4] = fmt.Sprintf("Missed Instructions         -> %.f%% (%d)\n", toPercent(safeDiv(float32(missed), float32(totalInstructions), 0)), missed)
+	// Build the whole report first and emit it in one write so it stays a
+	// contiguous block in the CI console instead of interleaving with logs.
+	var b strings.Builder
 
-		return result
-	})
+	b.WriteString(consoleRule + "\n")
+	b.WriteString(" 📊 Patch Coverage Report  —  changed lines only\n")
+	b.WriteString(consoleRule + "\n")
 
-	s.print("\n")
-	for _, line := range summaryLines {
-		s.print(line)
+	if modules := collectModules(changedLinesWithCoverage); len(modules) > 0 {
+		fmt.Fprintf(&b, " Modules: %s\n", strings.Join(modules, ", "))
 	}
 
+	fmt.Fprintf(&b, "\n Diff coverage: %.f%% %s  —  %d of %d changed instructions covered\n\n",
+		coveredPct, coverageStatusEmoji(coveredPct), covered, totalInstructions)
+
+	b.WriteString(" Summary\n")
+	fmt.Fprintf(&b, "   %-26s%3.f%%  (%d)\n", "Covered instructions", coveredPct, covered)
+	fmt.Fprintf(&b, "   %-26s%3.f%%  (%d)\n", "Missed instructions", missedPct, missed)
+	fmt.Fprintf(&b, "   %-26s%3.f%%  (%d)\n", "Tracked changed lines", withDataPct, linesWithData)
+	fmt.Fprintf(&b, "   %-26s%3.f%%  (%d)\n", "Untracked changed lines", withoutDataPct, linesWithoutData)
+
+	b.WriteString("\n Note: \"lines\" are the source lines you changed; \"instructions\" are the\n")
+	b.WriteString(" executable units the coverage tool counts inside them (one line can hold\n")
+	b.WriteString(" several, e.g. JaCoCo bytecode), so the two counts differ.\n\n")
+
+	b.WriteString(fileCoverageConsoleSection(changedLinesWithCoverage))
+	b.WriteString(uncoveredLinesConsoleSection(changedLinesWithCoverage))
+
+	b.WriteString(consoleRule + "\n")
+
+	s.print(b.String())
+
 	return nil
 }
 
 func (s *Simple) GetName() string {
 	return "simple stdout reporter"
 }
 
-func generateSummaryLines(changedLinesWithCoverage domain.SourceLineCoverageReport, formatter func(linesWithDataCount int, linesWithoutDataCount int, covered int, missed int) []string) []string {
-	linesWithDataCount := changedLinesWithCoverage.TotalLinesWithData()
-	linesWithoutDataCount := changedLinesWithCoverage.TotalLinesWithoutData()
+// fileCoverageConsoleSection renders the per-file breakdown for the console,
+// lowest-covered first, omitting files with no measurable lines.
+func fileCoverageConsoleSection(changedLinesWithCoverage domain.SourceLineCoverageReport) string {
+	files := collectFileCoverage(changedLinesWithCoverage)
 
-	covered := changedLinesWithCoverage.TotalCoveredInstructions()
-	missed := changedLinesWithCoverage.TotalMissedInstructions()
+	var b strings.Builder
+	b.WriteString(" Coverage by file  (lowest coverage first)\n")
+
+	measured := 0
+	unmeasured := 0
+
+	for _, f := range files {
+		instructions := f.covered + f.missed
+
+		if instructions == 0 {
+			unmeasured++
+			continue
+		}
+
+		measured++
+		pct := toPercent(safeDiv(float32(f.covered), float32(instructions), 1))
+		fmt.Fprintf(&b, "   %3.f%%   %3d cov / %3d miss   %s\n", pct, f.covered, f.missed, f.path)
+	}
+
+	if measured == 0 {
+		b.WriteString("   (no files with measurable lines)\n")
+	}
+
+	if unmeasured > 0 {
+		fmt.Fprintf(&b, "   (%d file(s) with no measurable lines omitted)\n", unmeasured)
+	}
+
+	b.WriteString("\n")
+
+	return b.String()
+}
+
+// uncoveredLinesConsoleSection lists each changed line that tests never ran.
+func uncoveredLinesConsoleSection(changedLinesWithCoverage domain.SourceLineCoverageReport) string {
+	var rows []string
+
+	for _, r := range changedLinesWithCoverage {
+		if r.MissedInstructionCount > 0 {
+			rows = append(rows, fmt.Sprintf("   - %s\n         %s\n", lineDescription(r.SourceLine), r.LineValue))
+		}
+	}
+
+	var b strings.Builder
+	fmt.Fprintf(&b, " Uncovered lines (%d)\n", len(rows))
+
+	if len(rows) == 0 {
+		b.WriteString("   none 🎉\n")
+	}
+
+	for _, row := range rows {
+		b.WriteString(row)
+	}
+
+	b.WriteString("\n")
 
-	return formatter(linesWithDataCount, linesWithoutDataCount, covered, missed)
+	return b.String()
 }
 
 func toPercent(decimal float32) float32 {

internal/plugin/reporter/utils.go

@@ -8,6 +8,12 @@ import (
 )
 
 func lineDescription(l domain.SourceLine) string {
+	return fmt.Sprintf("%v:%v", filePath(l), l.LineNumber)
+}
+
+// filePath joins the non-empty path segments of a source line into a single
+// file path (without the line number), used to group coverage by file.
+func filePath(l domain.SourceLine) string {
 	rawFileNameParts := []string{
 		l.Module, l.SrcDir, l.Pkg, l.FileName,
 	}
@@ -19,5 +25,5 @@ func lineDescription(l domain.SourceLine) string {
 		}
 	}
 
-	return fmt.Sprintf("%v:%v", strings.Join(fileNameParts, "/"), l.LineNumber)
+	return strings.Join(fileNameParts, "/")
 }