Skip to content

✨ feat: unify file-based inputs for comment edits and suggestions#35

Merged
SigureMo merged 4 commits intoShigureLab:mainfrom
ShigureNyako:feat/review-input-consistency-followup
Mar 22, 2026
Merged

✨ feat: unify file-based inputs for comment edits and suggestions#35
SigureMo merged 4 commits intoShigureLab:mainfrom
ShigureNyako:feat/review-input-consistency-followup

Conversation

@ShigureNyako
Copy link
Member

Motivation

PR #34 closed issue #20 by adding --body-file support to the main PR review write commands. This follow-up covers the remaining write-input gaps from issue #28 so comment editing and inline suggestions can use the same file/stdin-friendly workflow.

Changes

  • add mutually exclusive --body / --body-file support to:
    • gh-llm pr comment-edit
    • gh-llm issue comment-edit
  • add mutually exclusive --suggestion / --suggestion-file support to gh-llm pr review-suggest
  • move shared file/stdin text loading into gh_llm.commands.options so PR and issue commands use the same resolver logic
  • update README.md with comment-edit --body-file and review-suggest --suggestion-file examples
  • update skills/github-conversation/SKILL.md so the agent workflow docs mention the new file/stdin patterns
  • add CLI tests for file/stdin edits, suggestion-file input, and mutual-exclusion validation

Validation

  • uv run ruff format --check src/gh_llm/commands/options.py src/gh_llm/commands/pr.py src/gh_llm/commands/issue.py tests/test_cli.py
  • uv run ruff check .
  • uv run pyright src/gh_llm tests
  • uv run pytest -q
  • prettier --check README.md skills/github-conversation/SKILL.md

Related Issues

@ShigureNyako
Copy link
Member Author

Self-reviewed the diff and PR description.

@SigureMo Please review this PR when you have time. Thanks!

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0e8d984a81

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR completes the remaining “write input” CLI consistency work by adding file/stdin-friendly inputs to comment editing and inline suggestion content, and centralizing the shared resolver logic so PR and issue commands behave the same.

Changes:

  • Add mutually exclusive --body / --body-file to gh-llm pr comment-edit and gh-llm issue comment-edit.
  • Add mutually exclusive --suggestion / --suggestion-file to gh-llm pr review-suggest.
  • Centralize file/stdin text loading into src/gh_llm/commands/options.py, plus documentation and CLI test updates.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/gh_llm/commands/options.py Introduces shared stdin/file + inline text resolver utilities used by multiple commands.
src/gh_llm/commands/pr.py Wires comment-edit to --body-file and adds --suggestion-file support for review-suggest.
src/gh_llm/commands/issue.py Wires issue comment-edit to accept --body-file using the shared resolver.
tests/test_cli.py Adds CLI tests for file/stdin inputs and mutual-exclusion validation for the new flags.
README.md Documents the new --body-file and --suggestion-file workflows.
skills/github-conversation/SKILL.md Updates agent workflow docs with the new file/stdin patterns.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Member Author

@ShigureNyako ShigureNyako left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

结论:REQUEST_CHANGES(当前登录账号就是该 PR 作者,GitHub 不允许我对自己的 PR 直接提交 REQUEST_CHANGES 事件,所以这里只能以 COMMENT review 形式提交,但审查结论明确是需要修改)。

阻塞点只有 1 个:gh-llm pr review-suggest 新增 --suggestion-file - 后,当前实现允许它与 --body-file - 同时使用,但处理函数会顺序两次读取 stdin(src/gh_llm/commands/pr.py 里先读 suggestion,再读 body;共享读取逻辑在 src/gh_llm/commands/options.py)。真实 stdin 是一次性的,第一次 read() 会把流耗尽,第二次只能拿到空串。

我本地用 mock 跑了一次,最终提交出去的 body 变成了:


```suggestion
reason from stdin
---
replacement_from_stdin()

也就是说明文本被吞进了 suggestion block,CLI 行为和这次补齐的一致性目标、README 里宣称的 stdin 工作流都不一致。这里至少需要:

1. 禁止 `--body-file -` 与 `--suggestion-file -` 同时使用,或改成只读取一次 stdin 后再明确分配;
2. 补一条覆盖这条边界的回归测试。

除此之外,`comment-edit` / `issue comment-edit` 的输入统一化方向、README / skill 文档补充以及现有测试覆盖整体思路我认可。

Copy link
Member Author

@ShigureNyako ShigureNyako left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

补充提交行内意见:对应阻塞点已标在 src/gh_llm/commands/pr.py 的相关 diff 位置。

ShigureNyako added a commit to ShigureNyako/gh-llm that referenced this pull request Mar 22, 2026
@ShigureNyako ShigureNyako force-pushed the feat/review-input-consistency-followup branch from dfb9d26 to 16205b2 Compare March 22, 2026 05:37
@ShigureNyako ShigureNyako force-pushed the feat/review-input-consistency-followup branch from 16205b2 to 1d3303a Compare March 22, 2026 05:40
@SigureMo SigureMo merged commit 628fe52 into ShigureLab:main Mar 22, 2026
2 checks passed
Copy link
Member Author

@ShigureNyako ShigureNyako left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

结论:APPROVE(基于最新提交 08b1d5d1c2510c6dbfda4865f45935c4036b184a 的技术审查结论)。

我重新独立复核了这轮更新,之前阻塞的双 stdin 问题已经按正确方向修复:

  • src/gh_llm/commands/pr.py 新增 _validate_review_suggest_stdin_sources(),会显式拒绝 --body-file ---suggestion-file - 同时使用;
  • cmd_pr_review_suggest() 在真正发起 GraphQL 请求前先做这层校验,避免再次出现 stdin 被顺序读空的问题;
  • tests/test_cli.py 已补上 test_pr_review_suggest_rejects_dual_stdin_inputs 回归测试,我本地也复现确认该组合现在直接报错且不会发送 GraphQL 请求;
  • src/gh_llm/commands/options.py 也同步改成 if file_path is not None,并补了显式空路径输入测试,避免把空字符串误当成“未提供”;
  • README.md 已补充双 stdin 组合会被拒绝的说明,相关 comment-edit / suggestion-file 文档仍保留正确。

我这边观察到当前 checks 状态也已经完成并通过:

  • lint and fmt - Python 3.14 on x64:SUCCESS
  • unittest - Python 3.14 on x64:SUCCESS

补充说明:当前登录账号就是该 PR 作者,GitHub 不允许我对自己的 PR 直接提交 APPROVE 事件,所以这里仍以 COMMENT review 形式提交;但就最新代码状态而言,我的审查结论是 APPROVE。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

在 #20 之外,继续补齐 comment-edit / suggestion-file / stdin 的写入输入一致性

3 participants