Skip to content

fix(nc-review): render findings as blocks, not a table - #1212

Merged
will-lamerton merged 1 commit into
mainfrom
fix/nc-review-readable-output
Sep 6, 2026
Merged

fix(nc-review): render findings as blocks, not a table#1212
will-lamerton merged 1 commit into
mainfrom
fix/nc-review-readable-output

Conversation

@will-lamerton

Copy link
Copy Markdown
Member

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. Sorted blocking → important → nit, so the thing that matters is at the top rather than wherever the model happened to emit it.

Before

| | Area | Where | Finding |
|---|---|---|---|
| 🟠 | `tests` | `source/vscode/discovery.spec.ts:497` | The test captures rename and never substitutes it: const realRename = rename; realRename; // referenced only to silence unused-locals — so the publish path is never made to fail, and the test asserts started === true regardless of the behaviour it claims to cover. Fix: monkey-patch the mkdir/writeFile call sites, or reuse the chmodSync(parentDir, 0o555) pattern from the neighbouring test. |

After

🟠 important · tests · source/vscode/discovery.spec.ts:497

The test captures rename and never substitutes it:

const realRename = rename;
// ...
realRename; // referenced only to silence unused-locals

So the publish path is never made to fail, and the test asserts started === true regardless of the behaviour it claims to cover. Fix: monkey-patch the mkdir/writeFile call sites, or reuse the chmodSync(parentDir, 0o555) pattern from the neighbouring test.

Not just markup

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.

Verified against a three-finding fixture covering all severities, a finding with file and line, one with file only, and one with neither (location line omitted cleanly).

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
@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

No changeset found

This PR does not add a changeset, so it will not appear in the changelog or trigger a release.

If the change is user-facing, add one:

pnpm changeset

Pick a bump (patch / minor / major) and write the changelog entry in our usual voice ("Added X... Thanks to @you. Closes #123."), then commit the generated .changeset/*.md file.

If this PR is docs-only or a chore that needs no release note, you can ignore this - or run pnpm changeset --empty to record that intentionally.

@github-actions github-actions Bot added the area:ci GitHub Actions and CI label Sep 6, 2026
@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

nc-review: nothing to raise

Workflow and rubric change to render nc-review findings as block-quoted markdown blocks instead of a table, sorted blocking → important → nit. The jq sorts via a numeric rank helper on .severity (correct jq syntax) and emits each finding as a bold heading (icon + severity word + area + optional file:line) followed by the raw detail markdown, preserving newlines and fenced code blocks. The rubric's added paragraph and JSON-with-embedded-fenced-code example accurately describe what the workflow now accepts. No tests needed (CI workflow + doc), no changeset needed (not user-facing), and no duplicates among the open PR list (none touch nc-review files).


🔴 blocking · 🟠 a reviewer would ask for a change · ⚪ optional

Automated code review — correctness, security, design, tests, plus duplicates and scope. A human still decides; this is not a substitute for review and is not exhaustive. The required status checks separately cover lint, formatting, types, unused dependencies, the test suite and the build. This bot never merges. Maintainers can rerun with /re-review.

@github-actions github-actions Bot added the agent:clean nc-review had nothing to raise label Sep 6, 2026
@will-lamerton
will-lamerton merged commit c8a77aa into main Sep 6, 2026
15 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent:clean nc-review had nothing to raise area:ci GitHub Actions and CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant