Skip to content

fix(pr-management): keep a finding's suggestion in the review - #1987

Merged
potiuk merged 1 commit into
apache:mainfrom
potiuk:fix/code-review-keep-suggestion
Nov 4, 2026
Merged

potiuk merged 1 commit into
apache:mainfrom
potiuk:fix/code-review-keep-suggestion

Conversation

@potiuk

@potiuk potiuk commented Nov 4, 2026

Copy link
Copy Markdown
Member

Summary

  • code-review render built a finding's text from its explanation or its suggestion, never both. A finding carrying both lost the recommended fix from the review body and from its inline comment.
  • _text now joins the two, and still falls back to the rule when a finding has neither.
  • The test covers both an inline comment and a finding folded into Smaller observations.

Found during a review on 2026-11-04, when the fix text was missing from a rendered draft. An earlier round needed a follow-up comment on several PRs for the same reason.

Type of change

  • Skill change (.claude/skills/<name>/) — eval fixtures updated below
  • Tool / bridge contract (tools/<system>/*.md)
  • Python package (tools/*/ with pyproject.toml)
  • Groovy reference impl
  • Cross-cutting (RFC, AGENTS.md, sandbox, privacy-LLM)
  • Documentation (docs/, README.md, CONTRIBUTING.md)
  • Project template (projects/_template/)
  • CI / dev loop (prek, workflows, validators)
  • Other:

Test plan

  • prek run --all-files --skip workspace-pytest passes
  • For Python packages touched: prek run workspace-pytest --from-ref <upstream main> --to-ref HEAD passes
  • Other: the new test failed before the fix

Gen-AI disclosure

Drafted with Claude Code (Claude Opus 5), and reviewed by the maintainer opening this PR.

🤖 Generated with Claude Code

code-review render posted a finding's explanation or its suggestion,
so a finding carrying both lost the fix it recommended. The review
body and inline comments now carry both.

Generated-by: Claude Opus 5
@potiuk
potiuk enabled auto-merge (squash) November 4, 2026 13:16
@github-actions github-actions Bot added family:tools tools/* substrate:analytics Tool substrate: read-only metrics / dashboards / renderers labels Nov 4, 2026
@potiuk
potiuk merged commit 9ae39cf into apache:main Nov 4, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

family:tools tools/* substrate:analytics Tool substrate: read-only metrics / dashboards / renderers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant