fix(prompts): add PR already-reviewed guard and reduce redundant issue comments

[ellen-goc]

Problem

Three issues in askcc/definitions.py prompts generate redundant comments on PRs and linked issues.

Issue 1 — REVIEWPR_AGENT_PROMPT lacks an already-reviewed guard

pr-review is triggered by review_requested notifications. If a new notification fires for the same PR (e.g. after a force-push that re-requests review), there is no instruction to check whether Ellen's most recent substantial review already postdates the last commit. Ellen re-reviews the PR in full and posts a new review even when nothing new was pushed.

CLAUDE.md documents this check but it is not in the prompt:

PR already-reviewed check — before submitting or triggering any PR review, confirm the most recent substantial ellen-goc review (body > 50 chars) predates the PR's most recent commit.

Suggested addition to REVIEWPR_AGENT_PROMPT (pre-review section):

Pre-review dedup (skip if already reviewed since last push):
- gh pr view <number> -R <owner/repo> --json reviews,commits \
  --jq '{last_commit: (.commits | sort_by(.committedDate) | last | .committedDate), last_review: ([.reviews[] | select(.author.login == "ellen-goc" and (.body|length > 50)) | .submittedAt] | sort | last // "")}'
- If last_review is non-empty AND last_review >= last_commit → skip entirely (no review, no comment).

Issue 2 — REVIEWPR unconditionally posts a linked-issue comment for every review

REVIEWPR_AGENT_PROMPT line: "Also post a brief summary on the linked issue: gh issue comment"

Every review cycle creates both a PR review AND a linked-issue comment. When Ellen reviews multiple times (once per commit cycle), the linked issue fills with duplicate review summaries. The PR review itself is the record — the issue comment is noise.

Suggested fix: Remove or condition the instruction. If kept, restrict to the first review only (i.e., skip if Ellen already has an issue comment on the linked issue).

Issue 3 — DEVELOP_AGENT_PROMPT always posts an issue comment on completion

DEVELOP_AGENT_PROMPT "On completion" section: "Comment on the issue summarizing what was implemented."

This runs on every develop call, including follow-up commits to an existing PR. If Ellen pushes three times to fix review feedback, the linked issue gets three "what was implemented" comments.

Suggested fix:

- If this session opened a new PR: comment on the issue summarizing what was implemented.
- If a PR already existed before this session: skip the issue comment (the PR description update is sufficient).

Acceptance Criteria

• REVIEWPR_AGENT_PROMPT contains a Pre-review dedup block placed inside the pre-review section (before Pre-merge guard) that runs the suggested gh pr view ... --json reviews,commits query and instructs the agent to skip entirely (no review, no linked-issue comment) when last_review is non-empty and last_review >= last_commit.
• REVIEWPR_AGENT_PROMPT no longer instructs the agent to "Also post a brief summary on the linked issue" — that line is removed from the "Posting the review" section. The PR review itself is the only deliverable from pr-review.
• DEVELOP_AGENT_PROMPT "On completion" section replaces the unconditional "Comment on the issue summarizing what was implemented." with a two-branch instruction: comment on the issue only when the session opened a new PR; skip the issue comment when a PR already existed (the PR description update is sufficient).
• New tests in tests/test_askcc.py (TestReviewprPromptDedupGuard, TestReviewprPromptNoLinkedIssueComment, TestDevelopPromptConditionalIssueComment) assert the new prompt content via literal-substring matches, mirroring the existing TestReviewprPromptMergeGuard / TestDevelopPromptTestPlanUpdate style. Old assertions that contradict the new wording (none currently) are updated together with the prompt.
• uv run pytest, uv run ruff check, and uv run pyright all pass locally before opening the PR.

Dependencies

None identified. All edits are confined to askcc/definitions.py and tests/test_askcc.py. No schema, settings, or runner changes are required (the prompts are bootstrapped to user templates by existing code without per-prompt knowledge).

Implementation Plan

Current state

• askcc/definitions.py:506-563 — REVIEWPR_AGENT_PROMPT definition.
  • Pre-review: section at askcc/definitions.py:520-523 (3 bullets: read prompt, gh pr checkout, run tests).
  • Pre-merge guard (CHANGES_REQUESTED): at askcc/definitions.py:525-529 (template for the new dedup block — same gh pr view <number> -R <owner/repo> --json … shape).
  • Posting the review: block at askcc/definitions.py:546-549; the noisy line is askcc/definitions.py:549 — - Also post a brief summary on the linked issue: gh issue comment.
• askcc/definitions.py:232-346 — DEVELOP_AGENT_PROMPT definition.
  • On completion: section at askcc/definitions.py:327-333; the unconditional comment is askcc/definitions.py:333 — - Comment on the issue summarizing what was implemented.
• Tests in tests/test_askcc.py:
  • TestReviewprPromptMergeGuard at tests/test_askcc.py:377-393 — model for our new TestReviewprPromptDedupGuard.
  • TestDevelopPromptTestPlanUpdate at tests/test_askcc.py:287-312 — model for our new TestDevelopPromptConditionalIssueComment.
• Verified absent (will be added):
  • grep -n "ellen-goc" askcc/definitions.py → no match
  • grep -n "already.review" askcc/ → no match
  • grep -n "last_commit\|last_review\|sort_by.*committedDate" → no match

Step-by-step tasks

1. Add Pre-review dedup block in REVIEWPR_AGENT_PROMPT (Issue 1). Edit askcc/definitions.py between the existing Pre-review: block (ends at line 523) and Pre-merge guard (CHANGES_REQUESTED): (line 525). Insert:

   Pre-review dedup (skip if already reviewed since last push):
   - `gh pr view <number> -R <owner/repo> --json reviews,commits --jq '{last_commit: (.commits | sort_by(.committedDate) | last | .committedDate), last_review: ([.reviews[] | select(.author.login == "ellen-goc" and (.body|length > 50)) | .submittedAt] | sort | last // "")}'`
   - If `last_review` is non-empty AND `last_review >= last_commit` → skip entirely (no review, no linked-issue comment). Stop immediately.
   

   Place this before Pre-merge guard so the dedup short-circuit runs before checkout/tests are wasted.

2. Remove the unconditional linked-issue comment in REVIEWPR_AGENT_PROMPT (Issue 2). In askcc/definitions.py, delete askcc/definitions.py:549:

   - Also post a brief summary on the linked issue: `gh issue comment`.
   

   The remaining two Posting the review: bullets (--approve / --request-changes) stay; the PR review is the canonical record.

3. Make the DEVELOP completion comment conditional (Issue 3). In askcc/definitions.py:333, replace the single bullet with two bullets:

   - If this session opened a new PR: comment on the issue summarizing what was implemented.
   - If a PR already existed before this session (follow-up commits): skip the issue comment — the PR description update is sufficient.
   

   Keep the surrounding On completion: bullets (/simplify, commit/push, PR open/update, test plan update) untouched.

4. Add prompt-content tests in tests/test_askcc.py. After TestReviewprPromptMergeGuard (tests/test_askcc.py:377-393):

   • TestReviewprPromptDedupGuard:
     • test_includes_pre_review_dedup_heading — "Pre-review dedup" in REVIEWPR_AGENT_PROMPT.
     • test_includes_dedup_check_command — assert each fragment: 'sort_by(.committedDate)', '.author.login == "ellen-goc"', '(.body|length > 50)', '--json reviews,commits'.
     • test_includes_skip_instruction — "skip entirely" in REVIEWPR_AGENT_PROMPT.
     • test_dedup_guard_placed_before_pre_merge_guard — REVIEWPR_AGENT_PROMPT.index("Pre-review dedup") < REVIEWPR_AGENT_PROMPT.index("Pre-merge guard").
   • TestReviewprPromptNoLinkedIssueComment:
     • test_does_not_post_linked_issue_comment — "Also post a brief summary on the linked issue" not in REVIEWPR_AGENT_PROMPT.
   • TestDevelopPromptConditionalIssueComment:
     • test_does_not_unconditionally_comment_on_issue — the literal "- Comment on the issue summarizing what was implemented." no longer appears as a standalone bullet (the wording lives only inside the conditional branch).
     • test_includes_new_pr_branch — "If this session opened a new PR" in DEVELOP_AGENT_PROMPT.
     • test_includes_existing_pr_skip_branch — both "PR already existed" in DEVELOP_AGENT_PROMPT and "PR description update is sufficient" in DEVELOP_AGENT_PROMPT.

5. Verify locally. Run all three:

   • uv run pytest tests/test_askcc.py -v — all green, new test classes pass.
   • uv run ruff check — clean.
   • uv run pyright — 0 errors.

6. Open PR linked to this issue. Title: fix(prompts): add PR already-reviewed guard and reduce redundant issue comments (matches issue title). Body must include Closes #100, a ## Verification section pasting the three command results, a ## Key Flows mermaid diagram covering the new dedup short-circuit decision in pr-review, and a ## Test plan checklist whose items are auto-checked by the verification gate.

Risks / open questions

• Issue 2 — full removal vs. first-review-only conditioning. The acceptance criteria language permits either option. This plan removes the line outright because (a) the new dedup guard from Issue 1 already short-circuits any re-post on the same commit, (b) the PR review itself is the canonical record per the issue rationale, and (c) conditioning on "first review only" would require another gh issue view ... --json comments --jq query, increasing prompt complexity for marginal user value. If the maintainer prefers a one-time linked-issue ping per PR, the alternative is to add a guard like gh issue view <linked-issue> --json comments --jq '[.comments[] | select(.author.login == "ellen-goc")] | length' and only post when the count is 0. Recommendation: remove. Easy to reverse later if someone files a follow-up issue.

• Hardcoded ellen-goc reviewer login. The dedup query uses the literal "ellen-goc" author, matching the CLAUDE.md text quoted in the issue. If askcc later runs under a different bot identity, this string becomes brittle. Out of scope for this issue — flag it as a future cleanup if the reviewer identity is parameterized.

• No runtime behavior tests. Like the existing TestReviewprPromptMergeGuard / TestConfigBoundaryConstraints classes, the new tests assert prompt-content substrings only; the runtime effect (Ellen actually skipping a duplicate review) is verified by observing weyucou/sops PRs on the next review cycle after merge. This matches the precedent set by issues review action: should not merge when PR has CHANGES_REQUESTED #86 / develop action: update test plan checklist in PR description after PR creation #88 / develop and fix-ci prompts: never modify linter (ruff, pyright, etc.) configuration without explicit request #89.

[monkut]

Plan posted

Updated the issue body with ## Acceptance Criteria, ## Dependencies, and ## Implementation Plan sections, and assigned to @monkut.

Sections added/updated:

• Acceptance Criteria → 5 verifiable - [ ] items covering the three prompt edits, the new test classes, and the pytest/ruff/pyright gate.
• Dependencies → "None identified." (all edits live in askcc/definitions.py + tests/test_askcc.py).
• Implementation Plan → current-state map (with file:line cites for both prompts and the analogous test classes), 6-step task list, and risks/open questions.

Approach summary:

1. Insert a Pre-review dedup block in REVIEWPR_AGENT_PROMPT before Pre-merge guard (askcc/definitions.py:520-525) using the gh pr view ... --json reviews,commits --jq '{last_commit, last_review}' query from the issue. Skip review + comment when last_review >= last_commit.
2. Delete the unconditional - Also post a brief summary on the linked issue: \gh issue comment`.line ataskcc/definitions.py:549`.
3. Replace the unconditional issue comment at askcc/definitions.py:333 with two conditional bullets: comment only when the session opened a new PR; skip on follow-up commits.
4. Add three test classes (TestReviewprPromptDedupGuard, TestReviewprPromptNoLinkedIssueComment, TestDevelopPromptConditionalIssueComment) in tests/test_askcc.py after TestReviewprPromptMergeGuard (tests/test_askcc.py:377), using the same literal-substring assertion style as review action: should not merge when PR has CHANGES_REQUESTED #86 / develop action: update test plan checklist in PR description after PR creation #88 / develop and fix-ci prompts: never modify linter (ruff, pyright, etc.) configuration without explicit request #89.
5. Verify with uv run pytest, uv run ruff check, uv run pyright.
6. Open a PR with Closes #100, verification block, and a ## Key Flows mermaid for the dedup short-circuit.

Open question (no decision block — has a clear default): Issue 2's AC permits either full removal or first-review-only conditioning of the linked-issue comment. Plan removes the line outright because the new dedup guard already short-circuits re-posts and the PR review is the canonical record. Maintainer can override toward "first review only" if a one-time linked-issue ping per PR is desired (would require an extra gh issue view --json comments query in the prompt).

Risks flagged: hardcoded ellen-goc reviewer login (matches CLAUDE.md text — out of scope for this issue); prompt-content tests only (runtime effect verified by observing weyucou/sops next review cycle, mirroring how #86/#88/#89 are tested).

:tokens-used: model: N/A, input: 51, output: 18586

[monkut]

Implemented in #101

Opened PR #101 with the three prompt edits + new test classes.

Changes (askcc/definitions.py):

1. REVIEWPR_AGENT_PROMPT — added Pre-review dedup (skip if already reviewed since last push): block placed before Pre-merge guard (per AC). Uses the gh pr view <number> -R <owner/repo> --json reviews,commits --jq '{last_commit, last_review}' query from the issue body and instructs the agent to skip entirely (no review, no linked-issue comment) when last_review is non-empty and >= last_commit.
2. REVIEWPR_AGENT_PROMPT — removed the Also post a brief summary on the linked issue: gh issue comment. line from the Posting the review: section. The two --approve / --request-changes bullets remain; the PR review is the only deliverable.
3. DEVELOP_AGENT_PROMPT — replaced the unconditional Comment on the issue summarizing what was implemented. in On completion: with two conditional bullets: comment only when the session opened a new PR; skip on follow-up commits (the PR description update is sufficient).

Tests (tests/test_askcc.py):

• TestReviewprPromptDedupGuard — heading, query fragments, skip instruction, ordering.
• TestReviewprPromptNoLinkedIssueComment — the removed line is absent.
• TestDevelopPromptConditionalIssueComment — unconditional bullet gone, both conditional branches present.

Style mirrors TestReviewprPromptMergeGuard / TestDevelopPromptTestPlanUpdate (literal-substring asserts).

Verification gate (all green locally):

• uv run pytest — 233 passed (+8 new tests)
• uv run ruff check — passed
• uv run pyright — 0 errors

Plan deviation: none. Took the "remove" branch on Issue 2's open question, as the plan recommended (the new dedup guard from Issue 1 already short-circuits same-commit re-posts; conditioning on first-review-only would require an extra gh issue view --json comments query for marginal value).

:tokens-used: model: N/A, input: 53, output: 18177