diff --git a/askcc/definitions.py b/askcc/definitions.py index d61db8e..a6d89eb 100644 --- a/askcc/definitions.py +++ b/askcc/definitions.py @@ -321,6 +321,10 @@ C -- any fail --> E[stay in develop] `` ` ``` +- Mermaid label safety: quote labels containing `/`, `\\`, `(`, `)`, `|`, `:`, or starting \ +with punctuation — e.g. `B["/simplify, commit, push"]`, not `B[/simplify, commit, push]`. \ +Unquoted leading `/` or `\\` is parsed as parallelogram/trapezoid shape syntax and breaks \ +rendering with a "Lexical error / Unrecognized text" message. - Keep diagrams concise — one or two covering the most important flows. \ Skip this section for trivial changes (config-only, docs-only, single-line fix). @@ -328,9 +332,11 @@ - Run /simplify or /refactor to improve the code. - Commit and push the feature branch. - If no PR exists, open one linked to the issue. -- If a PR exists (follow-up changes), review and update its description (see "PR description update") before commenting. +- If a PR exists (follow-up changes), review and update its description (see "PR description update"). - Update the test plan checklist (see "Test plan update"). -- Comment on the issue summarizing what was implemented. +- 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. {DEVELOP_PR_DESCRIPTION_UPDATE} @@ -522,6 +528,14 @@ - Check out the PR branch: `gh pr checkout `. - Run the test suite to confirm all tests pass. +Pre-review dedup (skip if already reviewed since last push): +- `gh pr view -R --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. + Pre-merge guard (CHANGES_REQUESTED): - Before merging, check for unresolved CHANGES_REQUESTED reviews: \ `gh pr view -R --json reviews --jq '[.reviews[] | select(.state == "CHANGES_REQUESTED")]'`. @@ -546,7 +560,6 @@ Posting the review: - All pass: `gh pr review -R --approve --body ""` - Any fail: `gh pr review -R --request-changes --body ""` -- Also post a brief summary on the linked issue: `gh issue comment`. Update test plan in PR description: - Read PR body: `gh pr view -R --json body -q .body`. diff --git a/pyproject.toml b/pyproject.toml index 47bdc41..02e29ac 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "askcc" -version = "0.2.10" +version = "0.2.11" description = "A one-shot cc cli executor" authors = [{ name = "mknt", email = "shane.cousins@gmail.com" }] readme = "README.md" diff --git a/tests/test_askcc.py b/tests/test_askcc.py index 5ed7758..274d5e8 100644 --- a/tests/test_askcc.py +++ b/tests/test_askcc.py @@ -393,6 +393,63 @@ def test_includes_pre_merge_guard_heading(self): assert "Pre-merge guard" in REVIEWPR_AGENT_PROMPT +class TestReviewprPromptDedupGuard: + """The pr-review prompt must skip duplicate reviews when no new commits since last review (issue #100).""" + + def test_includes_pre_review_dedup_heading(self): + assert "Pre-review dedup" in REVIEWPR_AGENT_PROMPT + + def test_includes_dedup_check_command(self): + assert "sort_by(.committedDate)" in REVIEWPR_AGENT_PROMPT + assert '.author.login == "ellen-goc"' in REVIEWPR_AGENT_PROMPT + assert "(.body|length > 50)" in REVIEWPR_AGENT_PROMPT + assert "--json reviews,commits" in REVIEWPR_AGENT_PROMPT + + def test_includes_skip_instruction(self): + assert "skip entirely" in REVIEWPR_AGENT_PROMPT + + def test_dedup_guard_placed_before_pre_merge_guard(self): + assert REVIEWPR_AGENT_PROMPT.index("Pre-review dedup") < REVIEWPR_AGENT_PROMPT.index("Pre-merge guard") + + +class TestReviewprPromptNoLinkedIssueComment: + """The pr-review prompt must not unconditionally post a linked-issue comment (issue #100).""" + + def test_does_not_post_linked_issue_comment(self): + assert "Also post a brief summary on the linked issue" not in REVIEWPR_AGENT_PROMPT + + +class TestDevelopPromptConditionalIssueComment: + """The develop prompt must only comment on the issue when opening a new PR (issue #100).""" + + def test_does_not_unconditionally_comment_on_issue(self): + assert "- Comment on the issue summarizing what was implemented." not in DEVELOP_AGENT_PROMPT + + def test_includes_new_pr_branch(self): + assert "If this session opened a new PR" in DEVELOP_AGENT_PROMPT + + def test_includes_existing_pr_skip_branch(self): + assert "PR already existed" in DEVELOP_AGENT_PROMPT + assert "PR description update is sufficient" in DEVELOP_AGENT_PROMPT + + +class TestDevelopPromptMermaidLabelSafety: + r"""The develop prompt must instruct quoting mermaid labels with shape-reserved chars. + + Without quoting, labels starting with `/` or `\` are parsed as parallelogram/trapezoid + shape syntax (e.g. `B[/simplify]`) and break rendering with a lexical error. + """ + + def test_includes_label_safety_guidance(self): + assert "Mermaid label safety" in DEVELOP_AGENT_PROMPT + + def test_includes_quoted_example(self): + assert 'B["/simplify, commit, push"]' in DEVELOP_AGENT_PROMPT + + def test_warns_against_parallelogram_collision(self): + assert "parallelogram/trapezoid shape syntax" in DEVELOP_AGENT_PROMPT + + class TestConfigBoundaryConstraints: """Both develop and fix-ci prompts must forbid relaxing linter/type-checker config (issue #89).""" diff --git a/uv.lock b/uv.lock index 70fa014..b9cea8a 100644 --- a/uv.lock +++ b/uv.lock @@ -4,7 +4,7 @@ requires-python = "==3.14.*" [[package]] name = "askcc" -version = "0.2.10" +version = "0.2.11" source = { editable = "." } [package.dev-dependencies]