diff --git a/README.md b/README.md index 9209625..8893bb7 100644 --- a/README.md +++ b/README.md @@ -268,10 +268,35 @@ gh-llm pr review-submit \ --pr 77938 --repo PaddlePaddle/Paddle ``` +Pick the strongest explicit review outcome the evidence supports: + +- `APPROVE`: ready to merge from your side +- `REQUEST_CHANGES`: blocking issues remain +- `COMMENT`: non-blocking notes or intermediate status only + `pr comment-edit`, `issue comment-edit`, `thread-reply`, `review-comment`, `review-suggest`, and `review-submit` all support `--body-file -` to read multi-line text from standard input. `review-suggest` also supports `--suggestion-file -` for the suggestion block itself. > Note: `review-suggest --body-file - --suggestion-file -` is intentionally rejected because standard input can only be consumed once. Use separate files when both the reason text and suggestion block need external input. +## Multiline body safety + +GitHub stores body text exactly as sent. If you pass literal escape sequences such as `\n\n` inside `--body`, those backslashes may be stored literally and show up in the final review/comment. + +Use `--body` for short one-line text. Use `--body-file` for quotes, multiple paragraphs, bullet lists, and code fences: + +```bash +cat <<'EOF' > /tmp/review.md +> Reviewer point + +Fixed in `python/demo.py:42`. +Validation: `pytest test/demo_test.py -q` +EOF + +gh-llm pr thread-reply PRRT_xxx --body-file /tmp/review.md --pr 77938 --repo PaddlePaddle/Paddle +gh-llm pr review-submit --event COMMENT --body-file /tmp/review.md --pr 77938 --repo PaddlePaddle/Paddle +gh pr comment 77938 --repo PaddlePaddle/Paddle --body-file /tmp/review.md +``` + Submit behavior: - If you already have a pending review on this PR, `review-submit` submits that pending review. diff --git a/skills/github-conversation/SKILL.md b/skills/github-conversation/SKILL.md index 9e39fe0..8c80524 100644 --- a/skills/github-conversation/SKILL.md +++ b/skills/github-conversation/SKILL.md @@ -22,6 +22,40 @@ metadata: 2. Use `gh` for simple write actions (comment, labels, assignees, reviewers, close/reopen, merge). 3. If context is incomplete, do not reply yet; expand first. +## Message body fidelity + +GitHub stores the body text exactly as sent. + +1. Do not write multi-paragraph bodies as literal escape sequences such as `\n` or `\n\n`. +2. If you send `--body 'line1\n\nline2'`, GitHub may store the backslashes literally, and the rendered review/comment will show `\n\n`. +3. Use `--body` only for short single-paragraph text. +4. For quotes, bullets, code fences, or multiple paragraphs, prefer `--body-file` with a file or `-` on standard input. +5. For review suggestions that span multiple lines, prefer `--suggestion-file`. + +Safe patterns: + +```bash +cat <<'EOF' > /tmp/reply.md +> Reviewer point + +Fixed in `python/demo.py:42`. +Validation: `pytest test/demo_test.py -q` +EOF + +gh-llm pr thread-reply PRRT_xxx --body-file /tmp/reply.md --pr --repo +gh-llm pr review-submit --event COMMENT --body-file /tmp/reply.md --pr --repo +gh pr comment --repo --body-file /tmp/reply.md +``` + +```bash +cat <<'EOF' | gh-llm pr review-submit --event COMMENT --body-file - --pr --repo +Round-up: + +- fixed overload selection +- added regression coverage +EOF +``` + ## Install gh-llm Prerequisites: @@ -145,6 +179,7 @@ For PRs, check: A single reply should answer the target point only. Do not mix unrelated updates. +For multi-paragraph replies, use `--body-file` instead of embedding `\n\n` inside `--body`. ### 2) Be verifiable @@ -155,6 +190,16 @@ When making technical claims, include at least one concrete reference: 3. check/log link 4. reproduction command +### 2.5) Distinguish observed facts from intended actions + +Only treat a GitHub write action as completed after the command returns a success status or object id. + +1. Do not say "I already left an inline comment" unless the command output confirms it. +2. For `review-comment`, wait for `status: commented` and record the returned `thread` / `comment` id. +3. For `review-suggest`, wait for `status: suggested` and record the returned `thread` / `comment` id. +4. For `thread-reply`, wait for `status: replied` and record the returned `thread` / `reply_comment_id`. +5. If a write command was only drafted, described, or planned, say so explicitly instead of implying it already happened. + ### 3) Quote only when needed Use `>` when: @@ -176,6 +221,11 @@ Use plain status language: If partially fixed or unchanged, include reason and next step. +### 5) Preserve quoting and paragraph breaks + +When you need `>` quotes, bullets, numbered lists, or fenced code blocks, write the body through `--body-file`. +Do not hand-escape markdown structure inside a shell string unless the content is genuinely one line. + ## Review workflow ### Review a PR as reviewer @@ -243,6 +293,16 @@ gh-llm pr review-submit --event REQUEST_CHANGES --body '' --pr --r gh-llm pr review-submit --event APPROVE --body '' --pr --repo ``` +### Review conclusion + +The review outcome should be explicit whenever the current state is already clear. + +1. Use `APPROVE` when the change is ready to merge from your side. +2. Use `REQUEST_CHANGES` when blocking issues remain. +3. Use `COMMENT` mainly for non-blocking notes, partial context gathering, or intermediate status updates before the final conclusion is clear. +4. Do not leave the review state implicit if your evidence already supports approval or blocking. +5. In the review body, state the conclusion in plain language as well, especially when using `REQUEST_CHANGES`. + Use the final review summary to group the round: 1. what is blocking @@ -268,6 +328,22 @@ gh-llm pr thread-expand --pr --repo 5. If a reviewer's suggestion is substantially adopted, add proper co-author credit in the follow-up commit. 6. After a batch of fixes, post one concise round-up so the reviewer can re-check efficiently. +### Inline feedback choice + +Prefer the smallest tool that matches the intent: + +1. `thread-reply`: respond inside an existing review thread. +2. `review-comment`: raise a new inline point without an exact replacement. +3. `review-suggest`: propose a concrete patch the author can apply directly. + +Use `review-suggest` by default when all of the following are true: + +1. the change is local to one hunk +2. the replacement text is known exactly +3. the explanation fits in one short rationale paragraph + +Do not force `review-suggest` when the fix spans multiple files, requires design discussion, or depends on behavior you have not verified. + ## Issue workflow ### Opening an issue diff --git a/src/gh_llm/github_api.py b/src/gh_llm/github_api.py index 0a48fa4..1f0780f 100644 --- a/src/gh_llm/github_api.py +++ b/src/gh_llm/github_api.py @@ -2909,7 +2909,12 @@ def _render_review_thread_block( resolve_cmd = display_command_with(f"pr thread-resolve {thread_id} --pr {ref.number} --repo {ref.owner}/{ref.name}") lines.append(f" ◌ thread_id: {thread_id}") lines.append(" ⌨ reply_body: ''") + lines.append(" ⌨ reply_body_file: ''") lines.append(f" ⏎ Reply via {display_command()}: `{reply_cmd}`") + lines.append( + " ⏎ Multi-line reply via " + + f"{display_command()}: `{display_command_with(f'pr thread-reply {thread_id} --body-file --pr {ref.number} --repo {ref.owner}/{ref.name}')}`" + ) if is_resolved: lines.append(f" ⏎ Unresolve via {display_command()}: `{unresolve_cmd}`") else: @@ -3014,9 +3019,14 @@ def _render_review_comment_block( edit_cmd = display_command_with( f"pr comment-edit {comment_id} --body '' --pr {ref.number} --repo {ref.owner}/{ref.name}" ) + edit_file_cmd = display_command_with( + f"pr comment-edit {comment_id} --body-file --pr {ref.number} --repo {ref.owner}/{ref.name}" + ) lines.append(f" ◌ comment_id: {comment_id}") lines.append(" ⌨ comment_body: ''") + lines.append(" ⌨ comment_body_file: ''") lines.append(f" ⏎ Edit comment via {display_command()}: `{edit_cmd}`") + lines.append(f" ⏎ Multi-line edit via {display_command()}: `{edit_file_cmd}`") if not body and not diff_hunk: lines.append(" (empty review comment)") diff --git a/src/gh_llm/render.py b/src/gh_llm/render.py index 6f79756..c26af81 100644 --- a/src/gh_llm/render.py +++ b/src/gh_llm/render.py @@ -152,6 +152,8 @@ def render_pr_actions(context: TimelineContext, *, include_diff: bool = True, in [ "⌨ comment_body: ''", f"⏎ Comment via gh: `gh pr comment {context.number} --repo {repo} --body ''`", + "⌨ comment_body_file: ''", + f"⏎ Multi-line comment via gh: `gh pr comment {context.number} --repo {repo} --body-file `", *close_or_reopen_lines, "⌨ labels_csv: ','", f"⏎ Add labels via gh: `gh pr edit {context.number} --repo {repo} --add-label ','`", @@ -177,6 +179,8 @@ def render_issue_actions(context: TimelineContext) -> list[str]: "## Actions", "⌨ comment_body: ''", f"⏎ Comment via gh: `gh issue comment {context.number} --repo {repo} --body ''`", + "⌨ comment_body_file: ''", + f"⏎ Multi-line comment via gh: `gh issue comment {context.number} --repo {repo} --body-file `", *close_or_reopen_lines, "⌨ labels_csv: ','", f"⏎ Add labels via gh: `gh issue edit {context.number} --repo {repo} --add-label ','`", @@ -409,9 +413,14 @@ def _render_item(index: int, event: TimelineEvent, context: TimelineContext, com edit_cmd = display_command_with( f"{command_group} comment-edit {event.editable_comment_id} --body '' --{selector_name} {context.number} --repo {context.owner}/{context.name}" ) + edit_file_cmd = display_command_with( + f"{command_group} comment-edit {event.editable_comment_id} --body-file --{selector_name} {context.number} --repo {context.owner}/{context.name}" + ) lines.append(f" ◌ comment_id: {event.editable_comment_id}") lines.append(" ⌨ comment_body: ''") + lines.append(" ⌨ comment_body_file: ''") lines.append(f" ⏎ Edit comment via {display_command()}: `{edit_cmd}`") + lines.append(f" ⏎ Multi-line edit via {display_command()}: `{edit_file_cmd}`") else: lines.extend(_indent_block(display_summary)) if event.resolved_hidden_count > 0: diff --git a/tests/test_cli.py b/tests/test_cli.py index 796a476..39fdecc 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -559,6 +559,7 @@ def test_view_and_expand_use_real_cursor_pagination( assert "[IN_PROGRESS/NONE] unit-tests (check-run)" in out assert "passed checks hidden." in out assert "gh pr comment 77928 --repo PaddlePaddle/Paddle --body ''" in out + assert "gh pr comment 77928 --repo PaddlePaddle/Paddle --body-file " in out assert "gh pr close 77928 --repo PaddlePaddle/Paddle" in out assert "gh pr edit 77928 --repo PaddlePaddle/Paddle --add-label ','" in out assert "gh pr edit 77928 --repo PaddlePaddle/Paddle --remove-label ','" in out @@ -569,6 +570,7 @@ def test_view_and_expand_use_real_cursor_pagination( "Edit comment via gh-llm: `gh-llm pr comment-edit c3 --body '' --pr 77928 --repo PaddlePaddle/Paddle`" in out ) + assert "Multi-line edit via gh-llm: `gh-llm pr comment-edit c3 --body-file --pr 77928 --repo PaddlePaddle/Paddle`" in out pre_expand_calls = len(responder.calls) code = cli.run(["pr", "timeline-expand", "2", "--pr", "77928", "--repo", "PaddlePaddle/Paddle", "--page-size", "2"]) @@ -613,6 +615,9 @@ def test_view_and_expand_use_real_cursor_pagination( "Edit comment via gh-llm: `gh-llm pr comment-edit PRRC_self_1 --body '' --pr 77928 --repo PaddlePaddle/Paddle`" in out ) + assert "reply_body_file: ''" in out + assert "Multi-line reply via gh-llm: `gh-llm pr thread-reply PRRT_mock_1 --body-file --pr 77928 --repo PaddlePaddle/Paddle`" in out + assert "Multi-line edit via gh-llm: `gh-llm pr comment-edit PRRC_self_1 --body-file --pr 77928 --repo PaddlePaddle/Paddle`" in out assert "Unresolve via gh-llm:" in out code = cli.run( @@ -905,6 +910,7 @@ def test_issue_view_and_expand_use_real_cursor_pagination( assert "run `gh-llm issue comment-expand ic1 --issue 77924 --repo PaddlePaddle/Paddle` for full comment" in out assert "## Actions" in out assert "gh issue comment 77924 --repo PaddlePaddle/Paddle --body ''" in out + assert "gh issue comment 77924 --repo PaddlePaddle/Paddle --body-file " in out assert "gh issue close 77924 --repo PaddlePaddle/Paddle" in out assert "gh issue edit 77924 --repo PaddlePaddle/Paddle --add-label ','" in out assert "gh issue edit 77924 --repo PaddlePaddle/Paddle --remove-label ','" in out @@ -913,6 +919,7 @@ def test_issue_view_and_expand_use_real_cursor_pagination( "Edit comment via gh-llm: `gh-llm issue comment-edit ic2 --body '' --issue 77924 --repo PaddlePaddle/Paddle`" in out ) + assert "Multi-line edit via gh-llm: `gh-llm issue comment-edit ic2 --body-file --issue 77924 --repo PaddlePaddle/Paddle`" in out assert "cross-reference by @alice (Alice)" in out assert "gh-llm pr view 77900 --repo PaddlePaddle/Paddle" in out assert "issue/closed by @ShigureNyako" in out