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

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.