docs(pr-fixup): Step 3 加入 PR 顶层 comment 和 description 检查 - #244
Conversation
之前 pr-fixup 只查 reviewThreads（inline review 评论），漏了 review bot 留在 PR 主时间线或 PR description 里的整体评论，导致 nit 在下一轮 review 又会被重新提一次。现在 Step 3 分 3a/3b/3c 三个数据源拉取，并用最后一次 push 时间过滤掉历史评论。
|
Claude finished @lishuceo's task in 3m 48s —— View job Claude Code Review
|
|
Claude finished @lishuceo's task in 2m 9s —— View job Claude Code Review — Re-review after sync
|
| # | Issue | Confidence | Inline |
|---|---|---|---|
| 🟡 1 | #ISSUE_COMMENT_ID placeholder can't cross-reference a comment via #NUMBER syntax (line 233) |
90 | link |
| 🟡 2 | 3c (PR description summary) has no dedup → same nit gets re-processed every loop iteration (line 194) | 85 | link |
| 🟡 3 | Step 5 exit condition doesn't handle 3b/3c reply-only paths (no thread to resolve) → undefined state when round is all-bogus 3b/3c (line 249, not in diff so listed here) | 80 | — |
| 🟡 4 | 3c keyword set (🟡, 🔴, nit, confidence) over-matches normal PR descriptions (line 192) |
78 | link |
For #3 (Step 5), the fix is to change the exit condition from "only false positives were resolved" to "all review feedback handled (inline resolved + issue comment/description replied) and no code fixes and CI green". Fix this →
Verdict
· branch feat/pr-fixup-issue-comments ·
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
- LAST_PUSH 改用 gh api 拿 head commit 的 committer date，避免本地 rebase/amend 后时间不一致 - 顶层 comment 回复链接改用完整 URL，GitHub 不会把 #COMMENT_ID 解析成 comment 跳转 - 3c PR description summary 检测改成强/弱信号双层规则，避免把普通 PR 说明里偶尔出现的 🟡 误判为 review
|
Claude finished @lishuceo's task in 3m 33s —— View job Claude Code Review — Re-review after
|
| # | Issue | Conf | Verdict |
|---|---|---|---|
| 🟢 1 | #ISSUE_COMMENT_ID placeholder → full URL with #issuecomment- anchor |
90 | Fixed (thread) |
| 🟡 2 | 3c (PR description summary) has no dedup mechanism | 85 | Still outstanding (thread) — detection rules tightened, but a 3c entry will still re-match on every loop iteration since PR body doesn't change after Claude addresses it |
| 🟡 3 | Step 5 exit condition doesn't cover 3b/3c reply-only rounds (no thread to resolve) | 80 | Still outstanding — see below; can't anchor inline since line 298 isn't in the diff |
| 🟢 4 | 3c keywords over-match → strong/weak signal split | 78 | Fixed (thread) |
🟡 Issue #3 detail (carried forward, not inline-anchorable)
Step 4 explicitly states resolve only applies to inline review threads (line 251, 274): "仅 inline review thread 适用，issue comment 和 description summary 无 thread 可 resolve". But Step 5's only "no code fix" exit branch requires "误报被 resolve":
Failure case: a round where all review feedback comes from 3b/3c and all classified as "Not an issue" → Claude posts gh pr comment replies, no thread is resolved, no code is changed. Neither Step 5 branch matches → undefined behavior.
Suggested rewording (uses "handled" instead of "resolved", uniformly covers 3a/3b/3c):
如果本轮所有 review 反馈都已处理（inline 已 reply+resolve、3b/3c 已 reply）且无代码修复且 CI 全部通过 → 结束循环
Notes
- I couldn't auto-resolve the two fixed threads (飞书 Claude 集成 #1, test: 集成 Vitest 测试框架 #4) — the GitHub Actions bot integration lacks
resolveReviewThreadpermission. You'll need to mark them resolved manually in the GitHub UI. - Other observations:
LAST_PUSHnow uses GitHub's committer date viagh api(a good fix for the rebase/amend drift I'd noted earlier).
Verdict
· branch feat/pr-fixup-issue-comments ·
review bot 在 4e1909d 后再次指出： - 3c (PR description summary) 没有去重机制，PR body 静态不变，会让 同一条目在每轮循环里都被重新匹配 - Step 5 的"只有误报被 resolve"退出分支假设了 thread 存在，3b/3c 的反驳路径只 reply 不 resolve，会让循环卡在未定义状态 修复： - 3c 加 "本轮内存记录" 去重要求，处理过的条目下一轮跳过 - Step 5 退出条件改为按"已处理"判断（inline reply+resolve、 3b/3c reply 视为已处理），不再要求 resolve 动作
|
Claude finished @lishuceo's task in 2m 32s —— View job Claude Code Review — Re-review after
|
| # | Issue | Conf | Status |
|---|---|---|---|
| 🟢 1 | #ISSUE_COMMENT_ID placeholder → full URL with #issuecomment- anchor |
90 | Fixed in 4e1909d (line 252) |
| 🟢 2 | 3c (PR description summary) dedup mechanism | 85 | Fixed in 4f0c5d5 (lines 210-215) |
| 🟢 3 | Step 5 exit condition covers 3b/3c reply-only rounds | 80 | Fixed in 4f0c5d5 (lines 304-308) |
| 🟢 4 | 3c keyword detection split into strong/weak signals | 78 | Fixed in 4e1909d (lines 203-206) |
Issue #2 verification
Lines 210-215 now explicitly mandate dedup for 3c entries, with two strategies offered and the in-memory approach recommended:
去重（重要）：…为避免同一
/pr-fixup调用内同一条 3c 条目被反复处理，必须做以下两件事之一：
- 本轮内存记录：…维护一个集合…处理过的 3c 条目下一轮直接跳过
- 镜像到顶层 comment：…让后续轮次靠 3b 的
LAST_PUSH过滤自动跳过推荐第一种（更便宜，不污染 PR 时间线）。
Because the skill is run by an LLM agent that retains conversation history across loop iterations, in-memory tracking is realistic and matches the original suggestion in the prior review.
Issue #3 verification
Lines 304-308 rewrite Step 5's "no code fix" exit branch:
如果本轮所有 review 反馈都已处理（inline 已 reply+resolve、3b/3c 已 reply）且无代码修复且 CI 全部通过：
- 输出 "✅ 第 N 轮：处理 Y 个 review 反馈（含 Z 个反驳），所有 CI checks 通过"
- 结束循环
注意：3b/3c 没有 thread 可以 resolve，"已处理"的标准是已经发出
gh pr comment回复。不要因为"没有 resolve 动作"就误判为未处理而陷入死循环。
This closes the undefined-state hole when an entire round is 3b/3c reply-only. Combined with the existing Step 3 short-circuit (no CI fails + no 3a/3b/3c feedback → exit), the loop now has complete coverage for every termination scenario:
- Mixed feedback with code fixes → "有代码修复" branch → push → loop
- All-reply-only round (no code fix) → "都已处理" branch → exit ✅ (new)
- Nothing pending → Step 3 short-circuit → exit
Other observations (non-blocking)
- Line 153's
gh pr view ... -q '{author, body, sha}'is just an exploratory JSON dump; the subsequentHEAD_SHA=$(...)/LAST_PUSH=$(...)lines do the actual variable capture. Agents should handle this fine. LAST_PUSHnow correctly uses GitHub'scommit.committer.dateviagh api, avoiding local rebase/amend drift — a good improvement from the originalgit log %cIapproach noted in an earlier review.
Verdict
✅ Approved — direction is right (real pain point: review bot summary in PR description), and all 4 prior issues are now resolved. Ready to merge.
· branch feat/pr-fixup-issue-comments ·
|
已在
3 个 inline thread 已 reply + resolve。 |
1 participant
Summary
reviewThreads，漏了 review bot 留在 PR 主时间线或 PR description summary 里的反馈gh pr comment回复Background
实际遇到的场景：review bot 在 PR description 里贴了"🟡 遗留 nit"标记，pr-fixup 没看到，下一轮 review 又把同样的问题提了一次。
Test plan
gh pr view --json comments和 PR bodygh pr comment回复能正确关联到原评论