Skip to content

Commit f7419f5

Browse files
will-lamertonclaude
andcommitted
fix(nc-review): make the verdict file the deliverable, retry once
Run 6 produced a thorough, accurate analysis and threw it away. The agent narrated nineteen numbered findings to stdout — including a correct walk through umask interaction with mode 0600 — then finished without ever calling write_file. It treated the chat as the output. The instruction to write the file was buried mid-prompt behind the reading and analysis instructions. It is now last, stated as the whole deliverable, and says plainly that chat output is discarded and never reaches a human. The rubric's Output section says the same, because that is the one instruction whose failure costs an entire run. Also adds a single retry, scoped to exactly this failure: if verdict.json is missing or empty after the first attempt, run once more with a terse write-the-file prompt. A genuine agent error still falls through to the safe comment rather than being retried blindly. Roughly three minutes. Separately, a failed run no longer leaves a stale label. Applying none on failure was right, but #1184 has carried agent:clean through two failed re-reviews, which reads as "an agent looked at this and was happy" when nothing of the kind happened. The failure path now clears all three agent:* labels. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZY52ePXLjwG9TaQgq2cHT
1 parent 0f16eac commit f7419f5

2 files changed

Lines changed: 53 additions & 2 deletions

File tree

.github/nc-review/rubric.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,12 @@ clean tells a maintainer to skim past findings you spent the run producing.
240240

241241
## Output
242242

243+
**The file is the entire deliverable.** Anything you write in chat is discarded
244+
and never reaches a human — only the JSON file is read by the workflow that
245+
posts the review. A brilliant analysis narrated in chat and not written to the
246+
file is a failed run. Keep your reasoning brief; spend the effort on the file,
247+
and write it before you stop.
248+
243249
Write **only** a JSON object to the file path given in the prompt. No prose
244250
before or after, no markdown fences. Schema:
245251

.github/workflows/nc-review.yml

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -363,16 +363,52 @@ jobs:
363363
# tools refuse anything outside the project directory. An earlier
364364
# version pointed at /tmp and the agent correctly refused to invent a
365365
# verdict it could not substantiate.
366-
PROMPT='Read .github/nc-review/rubric.md for your instructions, CONTRIBUTING.md for the project rules, CLAUDE.md for the architecture, and .nc-review/context.md for the pull request under review. This is a real code review: use read_file and search_file_contents to read the source around every changed area before judging it, and do not assert a bug in code you have not read. Then write your verdict as a single JSON object to .nc-review/verdict.json, following the schema in the rubric exactly. All paths are relative to the current project directory. Write nothing to stdout except a brief note that you have finished.'
366+
# The instruction to write the file is LAST and stated as the whole
367+
# deliverable. An earlier version buried it mid-prompt and the model
368+
# narrated a long, genuinely good analysis to stdout, then stopped
369+
# without ever calling write_file — it treated the chat as the output.
370+
PROMPT='You are reviewing a pull request. Read .github/nc-review/rubric.md for your instructions, CONTRIBUTING.md for the project rules, CLAUDE.md for the architecture, and .nc-review/context.md for the pull request under review. All paths are relative to the current project directory.
371+
372+
This is a real code review: use read_file and search_file_contents to read the source around every changed area before judging it, and do not assert a bug in code you have not read.
373+
374+
YOUR ONLY DELIVERABLE IS THE FILE .nc-review/verdict.json — write it with write_file, following the schema in the rubric exactly.
375+
376+
Anything you write in chat is discarded and never reaches a human. Only the JSON file is read. Do not narrate your analysis; put your conclusions in the JSON. Keep your reasoning brief and spend your effort on the file.
377+
378+
You are not finished until write_file has succeeded on .nc-review/verdict.json. If you have analysed the pull request but not yet written that file, write it now.'
367379
368380
set +e
369381
nanocoder run "$PROMPT" \
370382
--mode yolo \
371383
--model minimax-m3 \
372384
--trust-directory
373-
echo "agent_status=$?" >> "$GITHUB_OUTPUT"
385+
STATUS=$?
374386
set -e
375387
388+
# One retry, and only for the specific failure of finishing without
389+
# writing the file. The prompt above should prevent it, but "the
390+
# analysis was good and went nowhere" is an expensive way to fail, and
391+
# a second attempt is ~3 minutes. Not a general retry: a genuine agent
392+
# error still falls through to the safe comment.
393+
if [ ! -s .nc-review/verdict.json ]; then
394+
echo "::warning::no verdict after first attempt — retrying once"
395+
RETRY='Read .github/nc-review/rubric.md and .nc-review/context.md, then write your review verdict as a single JSON object to .nc-review/verdict.json using write_file, following the schema in the rubric exactly.
396+
397+
Write the file. Do not reply in chat — chat output is discarded and only the file is read. The file is the entire task.'
398+
set +e
399+
nanocoder run "$RETRY" \
400+
--mode yolo \
401+
--model minimax-m3 \
402+
--trust-directory
403+
STATUS=$?
404+
set -e
405+
[ -s .nc-review/verdict.json ] \
406+
&& echo "retry produced a verdict" \
407+
|| echo "::warning::retry also produced no verdict"
408+
fi
409+
410+
echo "agent_status=$STATUS" >> "$GITHUB_OUTPUT"
411+
376412
- name: Post review
377413
if: always()
378414
env:
@@ -391,6 +427,15 @@ jobs:
391427
echo "::warning::no parseable verdict (agent exit ${AGENT_STATUS:-unknown})"
392428
gh pr comment "$PR" --repo "$REPO" --body \
393429
"**nc-review** could not produce a verdict this run (agent exit \`${AGENT_STATUS:-unknown}\`). This is a problem with the review agent, not with your pull request. A maintainer can retry with \`/re-review\`."
430+
# Clear any agent:* label from a previous run. Applying none was the
431+
# right call, but leaving a stale one is not: a PR has carried
432+
# agent:clean through two failed re-reviews, which reads as "an
433+
# agent looked at this and was happy" when nothing of the kind
434+
# happened.
435+
gh pr edit "$PR" --repo "$REPO" \
436+
--remove-label "agent:clean" \
437+
--remove-label "agent:comments" \
438+
--remove-label "agent:needs-work" 2>/dev/null || true
394439
exit 0
395440
fi
396441

0 commit comments

Comments
 (0)