Skip to content

fix(review): make review summaries and inline comments clearer - #28

Merged
niranjan94 merged 1 commit into
release/v2from
fix/review-output-clarity
Jun 30, 2026
Merged

niranjan94 merged 1 commit into
release/v2from
fix/review-output-clarity

Conversation

@niranjan94

Copy link
Copy Markdown
Owner

Why

Code review output was hard to read. Clean approvals dumped a per-lens summary bullet for all four reviewers even when there was nothing to act on, and change-requests repeated per-lens prose alongside jargon section headers (Lens failures, Findings dropped (line not in PR diff hunks)). The signal that mattered (the inline comments) was buried in noise.

What changed

  • Clean approval is now a single line plus a small Checked: footnote. The four per-lens summaries are gone.
    ✅ Shopfloor review passed. No issues found.
    
  • Changes requested leads with an issue count, points at the inline comments, and shows a category-breakdown table instead of per-lens prose:
    🔧 Shopfloor review: 3 issues to address (iteration 1/3)
    
    See the 2 inline comments below for specifics.
    
    | Category | Count |
    | Bug | 2 |
    | Security | 1 |
    
    Operational problems are surfaced in plain language only when they occur:
    • "Findings dropped (line not in PR diff hunks)" → "N issues fall outside the changed lines (so they couldn't be attached inline)", with detail listed since these have no inline anchor.
    • "Lens failures" / "Lenses blocked" → merged into "Some checks couldn't complete and will be retried".
    • New "Reviewer notes" section only when a lens flagged a concern that didn't clear the confidence bar, so a no-comment change-request isn't left unexplained.
  • Inline comment headers are now readable: 🐛 Bug · high confidence (85/100), replacing [bug / confidence 85].
  • Shared display vocabulary (labels, emojis, confidence wording, marker) extracted into src/stages/review/format.ts.

Notes

  • The <!-- shopfloor-review --> marker stays the first body line so check-review-skip's startsWith match keeps working.
  • Em dashes avoided in the committed copy per repo convention; the · middle-dots are intentional.

Testing

  • pnpm test — 307 passed
  • pnpm typecheck — clean
  • pnpm build — bundle rebuilt and committed

Clean approvals dumped a per-lens summary bullet for all four reviewers,
and change-requests repeated per-lens prose alongside jargon section
headers ("Lens failures", "Findings dropped (line not in PR diff
hunks)"), confusing readers with noise that wasn't actionable.

- Clean approval is now a single line plus a small "Checked:" footnote;
  the per-lens summaries are gone.
- Change-requests lead with an issue count, point to the inline
  comments, and show a category breakdown table instead of per-lens
  prose. Operational problems (failed/blocked checks, off-diff findings)
  appear in plain language only when they occur.
- Inline comment headers are now readable: "Bug, high confidence
  (85/100)" with a category emoji, replacing "[bug / confidence 85]".
- Extract the shared display vocabulary (labels, emojis, confidence
  wording, marker) into src/stages/review/format.ts.

The <!-- shopfloor-review --> marker stays the first body line so
check-review-skip's startsWith match keeps working.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Shopfloor review passed. No issues found.

Checked: compliance · bugs · security · code smells

@niranjan94
niranjan94 merged commit 7d47cf8 into release/v2 Jun 30, 2026
6 checks passed
@niranjan94
niranjan94 deleted the fix/review-output-clarity branch June 30, 2026 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant