Skip to content

Strengthen closed-PR search in analyze phase Step 3.4#2

Merged
LuciferDono merged 2 commits into
LuciferDono:masterfrom
ShivamShrivastava18:fix/closed-pr-prior-attempt-search
May 18, 2026
Merged

Strengthen closed-PR search in analyze phase Step 3.4#2
LuciferDono merged 2 commits into
LuciferDono:masterfrom
ShivamShrivastava18:fix/closed-pr-prior-attempt-search

Conversation

@ShivamShrivastava18

@ShivamShrivastava18 ShivamShrivastava18 commented May 16, 2026

Copy link
Copy Markdown
Contributor

Summary

Strengthen the "Check for related work" step in phase-analyze.md (Step 3.4) to surface function-specific prior attempts in addition to the existing issue-number search. The current single search misses prior closed PRs that targeted the same function but did not link the tracker issue — a hole that bites contributors on tracker issues with many sub-tasks.

The bug, with a concrete repro

I used the plugin to contribute a docstring example to scipy.signal.vectorstrength against scipy/scipy#7168 (a tracker with 152 sub-tasks across many functions). The discover and analyze phases verified the issue wasn't taken — both true — and gave it a green light.

After my PR was open, I discovered scipy/scipy#24592 (closed): a prior bulk DOC PR covering the same function (among 20 others) that had been closed three months earlier by a scipy maintainer with the comment "We don't accept pull requests from AI agents." Issue #7168 was still open, and gh search prs --repo scipy/scipy "#7168" --state=closed returned that PR — but among many other sub-PRs, none of which mentioned my specific function. The signal was buried.

A targeted search by the function name (gh search prs --repo scipy/scipy --state=closed vectorstrength) would have surfaced both that bulk PR and another single-function attempt (#25023) immediately, with their closing reasons — letting me pivot before investing analyze/work/test/submit effort.

The fix

phase-analyze.md Step 3.4 now instructs two searches:

  • a) By issue number (the existing behavior — unchanged).
  • b) By specific target — search by function/symbol name and file basename for both open and closed PRs, then read closing comments on every closed hit. Maintainer rejections of the exact approach are escalated to BLOCKER-level risks in the contribution brief.

A short rationale explains why both searches matter for tracker-style issues.

Scope

  • One file changed: skills/contribute/references/phase-analyze.md
  • 30 lines added, 1 deleted
  • No SKILL.md changes (per CONTRIBUTING.md: "Keep SKILL.md changes separate")
  • No new files, no cross-reference changes, no version bump
  • Reproduces the contributing guide's structure: ATX headings, fenced code blocks with bash language tag, no trailing whitespace

Testing

Pure-markdown change, so per CONTRIBUTING.md "test by running any phase and verifying coherence":

  • Read the modified Step 3.4 end-to-end; the flow reads coherently as "two searches → read closing comments → escalate to BLOCKER → rationale".
  • Reran the validate.yml gate checks locally (frontmatter, trailing whitespace, conflict markers, cross-references, file-starts-with-heading) — all pass.

AI Generation Disclosure

I am the human contributor; the plugin's own contribution workflow was used (/contribute analyze/contribute work/contribute test) with Claude Code as the assistant. The bug we are fixing is one I personally hit while using the plugin to contribute elsewhere; I reviewed every line of the diff before opening this PR and can explain the change. AI-assisted text in this PR: the wording of Step 3.4 itself and this PR description.

Summary by CodeRabbit

  • Documentation
    • Enhanced contribution guide: expanded "Check for related work" to a two-part PR search (by issue number and by specific targets), added guidance to inspect closing comments on closed PRs, and clarified which outcomes should be treated as BLOCKER-level risks. Added rationale for both searches to reduce noise and surface prior attempts.

Review Change Stack

Add target-specific search alongside the existing issue-number search,
so prior attempts on the same function/file are surfaced even when the
PR did not link the tracker issue. Important for tracker-style issues
with many sub-tasks where issue-number search returns too much noise.
@coderabbitai

coderabbitai Bot commented May 16, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: de09ecc8-86fb-4139-adb9-f5b7f798672a

📥 Commits

Reviewing files that changed from the base of the PR and between 4aacb2e and b644bf8.

📒 Files selected for processing (1)
  • skills/contribute/references/phase-analyze.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • skills/contribute/references/phase-analyze.md

📝 Walkthrough

Walkthrough

This PR expands the "Check for related work" guidance in the Analyze phase: replace the single-search approach with two separate searches (issue-number and target-based), add reviewing closing comments on closed PRs, and flag certain closed-PR outcomes as BLOCKER-level risks.

Changes

Contributor Guide: Related Work Search

Layer / File(s) Summary
Two-part related work search workflow
skills/contribute/references/phase-analyze.md
The "Check for related work" section is rewritten to require two searches: search PRs by bare issue number (open & closed) and search PRs by targets (function/symbol names or file basenames, open & closed). Instructs inspection of closing comments on closed PRs and identifies BLOCKER-level outcomes (maintainer rejection, obsolete/out-of-scope closures, already-resolved sub-tasks). An explanation clarifies why both searches reduce noise and reveal unlinked prior attempts.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hop through PRs both wide and deep,
By issue first, then targets I keep.
Closed comments whisper what went wrong,
BLOCKER flags keep my fixes strong.
A little double-check saves the day—hip, hop, hooray!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and specifically describes the main change: strengthening the closed-PR search process in a particular step of the analyze phase.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@skills/contribute/references/phase-analyze.md`:
- Around line 93-102: The examples currently show gh search prs with
--state=closed which contradicts the preceding text that instructs to search
both open and closed PRs; update the sample commands (the lines containing gh
search prs and the --state=closed flag and the placeholders "<TARGET_SYMBOL>"
and "<TARGET_FILE_BASENAME>") so they actually cover both open and closed PRs —
either remove the --state filter to search all states or provide two examples
(one with --state=open and one with --state=closed) and keep the placeholders
intact.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 64e681a0-26e0-44b3-a9da-baad6478dec4

📥 Commits

Reviewing files that changed from the base of the PR and between 6186f01 and 4aacb2e.

📒 Files selected for processing (1)
  • skills/contribute/references/phase-analyze.md

Comment thread skills/contribute/references/phase-analyze.md
Address CodeRabbit feedback on PR LuciferDono#2: prose said 'both open and closed PRs'
but the example commands only showed --state=closed. Adds the matching
--state=open lines for both the symbol and the file-basename searches.
@ShivamShrivastava18

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented May 16, 2026

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@LuciferDono

Copy link
Copy Markdown
Owner

Nice catch on this. The scipy #7168 case is exactly what the issue-number-only search would miss. Two-search structure handles it, and CodeRabbit's open/closed nit is already in the second commit.

Squashing on merge.

@LuciferDono LuciferDono merged commit 6058741 into LuciferDono:master May 18, 2026
2 checks passed
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.

2 participants