Skip to content

Commit a72f197

Browse files
will-lamertonclaude
andcommitted
fix(nc-review): render findings as blocks, not a table
Findings run to a paragraph or more. A table cell forces all of that onto one line with newlines collapsed, squeezed into a narrow column — run 7's single finding was a 200-word cell and effectively unreadable. Each finding is now its own block: a bold heading carrying severity, area and location, then the detail at full width. Findings are sorted blocking first, then important, then nit, so the thing that matters is at the top rather than wherever the model happened to emit it. This also removes a constraint rather than just moving markup around. The table required gsub("\n"; " ") on every detail, so the agent could never quote code. It can now, and the rubric says so with an example — for a finding like "this captured reference is never used", quoting the two offending lines makes the point faster than any description of them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZY52ePXLjwG9TaQgq2cHT
1 parent 3a77f12 commit a72f197

2 files changed

Lines changed: 29 additions & 8 deletions

File tree

.github/nc-review/rubric.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,4 +273,14 @@ before or after, no markdown fences. Schema:
273273
`warranted`, `duplicate`, `scope`, `changeset`, `contributing`.
274274
- `file` / `line` — where the finding is. Omit both if it is not tied to a
275275
specific location. Never guess a line number; omit it instead.
276+
- `detail` — Markdown, rendered as its own block under a heading. Line breaks
277+
and fenced code blocks are preserved, so quote the offending lines when that
278+
makes the point faster than describing them:
279+
280+
```
281+
"detail": "The captured reference is never used:\n\n```ts\nconst realRename = rename;\nrealRename; // silences unused-locals\n```\n\nSo the failure path is never exercised."
282+
```
283+
284+
Lead with what is wrong, then why it matters, then what would fix it. Two or
285+
three sentences is usually right; go longer only when quoting code earns it.
276286
- `duplicate_of` — PR number as an integer, or `null`. Only when confident.

.github/workflows/nc-review.yml

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -483,14 +483,25 @@ jobs:
483483
echo
484484
fi
485485
if [ "$(jq '.findings | length' "$V")" -gt 0 ]; then
486-
echo "| | Area | Where | Finding |"
487-
echo "|---|---|---|---|"
488-
jq -r '.findings[] |
489-
"| \(if .severity == "blocking" then "🔴" elif .severity == "important" then "🟠" else "⚪" end)"
490-
+ " | `\(.area)`"
491-
+ " | \(if .file then "`\(.file)\(if .line then ":\(.line)" else "" end)`" else "—" end)"
492-
+ " | \(.detail | gsub("\n"; " ")) |"' "$V"
493-
echo
486+
# One block per finding rather than a table. Findings run to a
487+
# paragraph or more, and a table cell forces all of that onto one
488+
# line with newlines collapsed — unreadable at any width, and it
489+
# squeezes the text into a narrow column. Blocks also let the
490+
# detail keep its own line breaks and code formatting.
491+
jq -r '
492+
def icon: if . == "blocking" then "🔴"
493+
elif . == "important" then "🟠"
494+
else "⚪" end;
495+
def rank: if . == "blocking" then 0
496+
elif . == "important" then 1
497+
else 2 end;
498+
.findings
499+
| sort_by(.severity | rank)
500+
| .[]
501+
| "**\(.severity | icon) \(.severity) · `\(.area)`"
502+
+ (if .file then " · `\(.file)\(if .line then ":\(.line)" else "" end)`" else "" end)
503+
+ "**\n\n\(.detail)\n"
504+
' "$V"
494505
fi
495506
echo "---"
496507
echo

0 commit comments

Comments
 (0)