feat(jira): add candidates-from-pr command#451
Open
qinqon wants to merge 3 commits into
Open
Conversation
openshift-ci
Bot
added
the
needs-ok-to-test
|
Hi @qinqon. Thanks for your PR. I'm waiting for a openshift-eng member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a new Jira command/skill ChangesJira candidates-from-pr feature + manifest updates
sequenceDiagram
participant User as User
participant CLI as "/jira:candidates-from-pr"
participant Fetch as "fetch_pr.py"
participant GH as "GitHub (gh)"
participant ExtractKeys as "extract_jira_keys.py"
participant Derive as "derive_filters.py"
participant Signals as "extract_signals.py"
participant BuildJQL as "build_jql.py"
participant Jira as "Jira (search)"
participant Score as "score_candidates.py"
participant Render as "render_report.py"
User->>CLI: invoke /jira:candidates-from-pr <pr> [flags]
CLI->>Fetch: run fetch_pr (PR JSON + diff)
Fetch->>GH: gh pr view / gh pr diff
Fetch-->>CLI: PR JSON + diff (capped/truncated)
CLI->>ExtractKeys: extract explicit Jira keys (stdin PR JSON)
CLI->>Derive: derive components/target_release (stdin PR JSON)
CLI->>Signals: extract signals (stdin PR JSON + diff)
Derive->>BuildJQL: produce filters -> build JQL string
BuildJQL->>Jira: run JQL search -> candidates
Jira-->>CLI: candidate list
CLI->>Score: score_candidates (signals + candidates + filters)
Score-->>CLI: ranked candidates
CLI->>Render: render_report (pr, filters, explicit, candidates)
Render-->>User: text|json report
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (8 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@plugins/jira/commands/candidates-from-pr.md`:
- Around line 10-12: The markdown file has multiple fenced code blocks missing
language tags; update each fence to include an appropriate language (e.g., mark
the CLI example "/jira:candidates-from-pr <pr-url-or-number> [options]" as text,
the JQL query block as jql, and any shell snippets as bash or JSON examples as
json) so markdownlint MD040 warnings are cleared; locate the blocks containing
the command string and the JQL query (and the other fenced examples noted in the
review) and prepend the fence with the correct language identifier.
In `@plugins/jira/skills/candidates-from-pr/SKILL.md`:
- Around line 110-115: The Markdown has markdownlint warnings MD040/MD058: add
language tags to fenced code blocks (e.g., the mcp__atlassian__jira_get_issue
code blocks) and ensure blank lines before and after any Markdown tables to
satisfy MD058 and MD040; update all occurrences referenced (blocks around lines
showing mcp__atlassian__jira_get_issue and the ranges 124-131, 142-144, 151-157,
160-166, 223-246) by adding an appropriate language identifier (like ```bash or
```yaml) to each fenced code block and inserting blank lines above and below
each table so the document passes linting.
- Around line 141-146: The release-selection rule in SKILL.md is contradictory:
replace the "Choose the smallest version where `released == false`" instruction
with a single consistent rule to "Choose the highest unreleased numeric version"
so both bullets match; update the text around
mcp__atlassian__jira_get_project_versions(project_key=<KEY>) and the
`target_release_source` guidance to reflect this single rule (highest unreleased
numeric version) and remove the conflicting "smallest" wording.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 838048e9-b87b-485b-b2c3-25df6f4c4e7e
📒 Files selected for processing (6)
.claude-plugin/marketplace.jsonPLUGINS.mddocs/data.jsonplugins/jira/.claude-plugin/plugin.jsonplugins/jira/commands/candidates-from-pr.mdplugins/jira/skills/candidates-from-pr/SKILL.md
qinqon
force-pushed
the
feat/jira-candidates-from-pr
branch
2 times, most recently
from
c345da7 to
9e3c5a9
Compare
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
plugins/jira/skills/candidates-from-pr/SKILL.md (1)
89-94:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUnresolved markdownlint MD040 warnings — fenced blocks still lack language tags.
Lines 89, 109, and 123 all have fenced code blocks without a language identifier. This was flagged in a prior review and remains unfixed.
♻️ Proposed fix for the three unlabelled blocks
-``` +```text mcp__atlassian__jira_get_issue( issue_key=<KEY>, fields="summary,status,issuetype,components,fixVersions,customfield_10855,assignee" )Apply the same `text` (or `python`) label to the blocks at lines 109 and 123. </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@plugins/jira/skills/candidates-from-pr/SKILL.mdaround lines 89 - 94, The
fenced code blocks containing the mcp__atlassian__jira_get_issue(...) example
(and the two other similar blocks later) lack language tags and trigger
markdownlint MD040; update each fenced block around the
mcp__atlassian__jira_get_issue snippet and the two other occurrences to use a
language label (e.g.,text orpython) so the fences are explicitly tagged
and the MD040 warnings are resolved, ensuring all three blocks use the same
label as suggested.</details> </blockquote></details> </blockquote></details> <details> <summary>🧹 Nitpick comments (4)</summary><blockquote> <details> <summary>plugins/jira/skills/candidates-from-pr/scripts/fetch_pr.py (1)</summary><blockquote> `27-27`: _💤 Low value_ **Document Python version requirement or use `from __future__ import annotations`.** The code uses PEP 604 union syntax (`str | None`), which is a runtime `SyntaxError` on Python 3.9. SKILL.md states "Python 3" without specifying a minimum version. The same pattern appears in other scripts (`score_candidates.py`, `gather.py`). Either update the prerequisite section to document `>= 3.10` or add `from __future__ import annotations` at the top of the file for broader compatibility. <details> <summary>Two options</summary> Option 1 — document the constraint: ```diff +# Requires Python >= 3.10 (PEP 604 union type syntax). def resolve_pr(arg: str, repo: str | None) -> tuple[str, str, int]: ``` Option 2 — support Python 3.9+ without a stdlib change: ```diff +from __future__ import annotations + import argparse ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/jira/skills/candidates-from-pr/scripts/fetch_pr.py` at line 27, The function signature resolve_pr(arg: str, repo: str | None) uses PEP 604 union syntax which raises SyntaxError on Python <3.10; either document the minimum Python version as >=3.10 in the project prerequisites (SKILL.md) or make the code compatible with 3.9+ by adding from __future__ import annotations at the top of this file (and the other affected scripts score_candidates.py and gather.py) so the union type syntax is deferred to annotations and remains valid at runtime. ``` </details> </blockquote></details> <details> <summary>plugins/jira/skills/candidates-from-pr/scripts/score_candidates.py (3)</summary><blockquote> `122-125`: _💤 Low value_ **Redundant condition in `elif` guard.** Inside the `elif` branch the intersection is already known to be empty (it's the logical complement of the `if` at line 114), so `not (cand_components & derived_components)` is always `True` and adds noise. <details> <summary>♻️ Proposed simplification</summary> ```diff - elif component_filter_used and not (cand_components & derived_components): + elif component_filter_used: # JQL should have prevented this; guard anyway. score = 0 return score, matched ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/jira/skills/candidates-from-pr/scripts/score_candidates.py` around lines 122 - 125, The elif check redundantly tests not (cand_components & derived_components) because that case is already the complement of the earlier if; simplify the branch by removing the redundant set-intersection test and only check component_filter_used (i.e., change "elif component_filter_used and not (cand_components & derived_components):" to "elif component_filter_used:"), then keep the body that sets score = 0 and returns score, matched so behavior is unchanged but the condition is clearer; refer to the variables cand_components, derived_components, component_filter_used, score, and matched to locate the change. ``` </details> --- `48-49`: _💤 Low value_ **`lower()` helper is dead code.** The function is defined but never called; `find_lower` does not use it, and no other site in the file does either. Can be removed. <details> <summary>🧹 Proposed removal</summary> ```diff -def lower(value: str | None) -> str: - return (value or "").lower() - - def load(path: str) -> object: ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/jira/skills/candidates-from-pr/scripts/score_candidates.py` around lines 48 - 49, The helper function lower is unused dead code; remove the function definition def lower(value: str | None) -> str: return (value or "").lower() from the file and ensure no other code (e.g., find_lower or callers) rely on it—search for symbols lower and find_lower to confirm only find_lower is used and that removing lower is safe, then delete the unused lower function and run tests/lint to verify no references remain. ``` </details> --- `65-68`: _⚡ Quick win_ **`find_lower` should lowercase needles for defensive coding, but the bug doesn't manifest in practice.** The haystack is correctly pre-lowercased, but the proposed fix is still valid as a safeguard. However, `title_values` and `path_values` are already lowercased by `extract_signals.py` before being written to `signals.json`: - `title_keywords()` lowercases every token (line 110: `lo = t.lower()`) - `path_tags()` lowercases every path component (line 126: `part.lower()`) So the actual scoring impact is zero—mixed-case keywords will not "silently zero out" contributions because they never reach `find_lower()` in mixed case. The function works correctly in its current context. That said, `find_lower()`'s design suggests intent for case-insensitive matching. Adding `.lower()` to needles remains a defensible refactor to prevent future breakage if upstream processing changes, but this is a code quality improvement rather than a bug fix. <details> <summary>Suggested fix (defensive coding)</summary> ```diff def find_lower(needles: list[str], text_lower: str) -> list[str]: if not needles or not text_lower: return [] - return [n for n in needles if n and n in text_lower] + return [n for n in needles if n and n.lower() in text_lower] ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/jira/skills/candidates-from-pr/scripts/score_candidates.py` around lines 65 - 68, The function find_lower currently checks membership against a pre-lowercased haystack (text_lower) but does not defensively lowercase the needles; update find_lower to normalize each needle by calling .lower() (while still skipping falsy/None needles) before testing membership against text_lower so matching remains case-insensitive even if upstream stops lowercasing (reference: function name find_lower and its parameters needles and text_lower). ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.Duplicate comments:
In@plugins/jira/skills/candidates-from-pr/SKILL.md:
- Around line 89-94: The fenced code blocks containing the
mcp__atlassian__jira_get_issue(...) example (and the two other similar blocks
later) lack language tags and trigger markdownlint MD040; update each fenced
block around the mcp__atlassian__jira_get_issue snippet and the two other
occurrences to use a language label (e.g.,text orpython) so the fences
are explicitly tagged and the MD040 warnings are resolved, ensuring all three
blocks use the same label as suggested.
Nitpick comments:
In@plugins/jira/skills/candidates-from-pr/scripts/fetch_pr.py:
- Line 27: The function signature resolve_pr(arg: str, repo: str | None) uses
PEP 604 union syntax which raises SyntaxError on Python <3.10; either document
the minimum Python version as >=3.10 in the project prerequisites (SKILL.md) or
make the code compatible with 3.9+ by adding from future import annotations
at the top of this file (and the other affected scripts score_candidates.py and
gather.py) so the union type syntax is deferred to annotations and remains valid
at runtime.In
@plugins/jira/skills/candidates-from-pr/scripts/score_candidates.py:
- Around line 122-125: The elif check redundantly tests not (cand_components &
derived_components) because that case is already the complement of the earlier
if; simplify the branch by removing the redundant set-intersection test and only
check component_filter_used (i.e., change "elif component_filter_used and not
(cand_components & derived_components):" to "elif component_filter_used:"), then
keep the body that sets score = 0 and returns score, matched so behavior is
unchanged but the condition is clearer; refer to the variables cand_components,
derived_components, component_filter_used, score, and matched to locate the
change.- Around line 48-49: The helper function lower is unused dead code; remove the
function definition def lower(value: str | None) -> str: return (value or
"").lower() from the file and ensure no other code (e.g., find_lower or callers)
rely on it—search for symbols lower and find_lower to confirm only find_lower is
used and that removing lower is safe, then delete the unused lower function and
run tests/lint to verify no references remain.- Around line 65-68: The function find_lower currently checks membership against
a pre-lowercased haystack (text_lower) but does not defensively lowercase the
needles; update find_lower to normalize each needle by calling .lower() (while
still skipping falsy/None needles) before testing membership against text_lower
so matching remains case-insensitive even if upstream stops lowercasing
(reference: function name find_lower and its parameters needles and text_lower).</details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: Path: .coderabbit.yaml **Review profile**: CHILL **Plan**: Enterprise **Run ID**: `e6227704-319d-467b-9a2c-e89fad1af8b0` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 3fefac76ea91dec59cc101568fd9763caf3d5579 and 9e3c5a9a56df8b8ab3ce1d62c5397cf17160d64f. </details> <details> <summary>📒 Files selected for processing (13)</summary> * `.claude-plugin/marketplace.json` * `PLUGINS.md` * `docs/data.json` * `plugins/jira/.claude-plugin/plugin.json` * `plugins/jira/commands/candidates-from-pr.md` * `plugins/jira/skills/candidates-from-pr/SKILL.md` * `plugins/jira/skills/candidates-from-pr/scripts/build_jql.py` * `plugins/jira/skills/candidates-from-pr/scripts/derive_filters.py` * `plugins/jira/skills/candidates-from-pr/scripts/extract_jira_keys.py` * `plugins/jira/skills/candidates-from-pr/scripts/extract_signals.py` * `plugins/jira/skills/candidates-from-pr/scripts/fetch_pr.py` * `plugins/jira/skills/candidates-from-pr/scripts/render_report.py` * `plugins/jira/skills/candidates-from-pr/scripts/score_candidates.py` </details> <details> <summary>✅ Files skipped from review due to trivial changes (2)</summary> * plugins/jira/skills/candidates-from-pr/scripts/render_report.py * plugins/jira/skills/candidates-from-pr/scripts/extract_signals.py </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (1)</summary> * PLUGINS.md </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
qinqon
force-pushed
the
feat/jira-candidates-from-pr
branch
from
9e3c5a9 to
50057d1
Compare
Author
|
Rebased commit 50057d1 on main and pushed (conflicts resolved). |
qinqon
force-pushed
the
feat/jira-candidates-from-pr
branch
from
50057d1 to
1bc068a
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
plugins/jira/skills/candidates-from-pr/SKILL.md (2)
96-96:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMD040: fenced code blocks still lack language tags (previously flagged, not fully resolved).
Lines 96, 116, and 130 open with a bare
```fence. The past review flagging these was marked addressed in commit 50057d1, but the warnings persist in the current state.✏️ Proposed fix
-``` +```text mcp__atlassian__jira_get_issue(-``` +```text mcp__atlassian__jira_get_project_versions(project_key=<KEY>)-``` +```text mcp__atlassian__jira_search(Also applies to: 116-116, 130-130
🤖 Prompt for 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. In `@plugins/jira/skills/candidates-from-pr/SKILL.md` at line 96, Several fenced code blocks in SKILL.md open with bare ``` fences (lines around the mcp__atlassian__jira_get_issue, mcp__atlassian__jira_get_project_versions, and mcp__atlassian__jira_search examples) and need explicit language tags; update each opening fence to include a language (e.g., ```text) so the blocks become ```text mcp__atlassian__jira_get_issue(...), ```text mcp__atlassian__jira_get_project_versions(project_key=<KEY>), and ```text mcp__atlassian__jira_search(...), ensuring all three occurrences (around the listed function names) are changed to remove the MD040 warning.
120-120:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win"smallest" version rule still contradicts the command doc (previously flagged, not resolved).
Line 120 instructs the AI to pick the "smallest version where
released == false", butcandidates-from-pr.mdline 83 says "pick the latest unreleased version." The past review raised this exact contradiction and was marked addressed in commit 50057d1, yet the "smallest" wording persists here.For
main/masterbranches the intent is to find the current development release (highest unreleased version), so "smallest" produces the wrong target release.✏️ Proposed fix
-Pick the **smallest** version where `released == false` and rewrite `filters.json` with it (set `target_release_source = "project_versions"`). +Pick the **highest** unreleased numeric version where `released == false` and rewrite `filters.json` with it (set `target_release_source = "project_versions"`).🤖 Prompt for 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. In `@plugins/jira/skills/candidates-from-pr/SKILL.md` at line 120, Replace the contradictory phrase 'Pick the "smallest" version where `released == false`' in SKILL.md with wording that matches the other doc: "pick the latest unreleased version (highest/unreleased) and rewrite `filters.json` with it (set `target_release_source = \"project_versions\"`)"; ensure the language and any associated logic or examples explicitly state the highest unreleased version for main/master branches (current development release) so it aligns with the instruction in the other candidates-from-pr documentation.plugins/jira/commands/candidates-from-pr.md (1)
10-12:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMD040: fenced code blocks still lack language tags (previously flagged, not addressed).
Lines 10, 87, 112, 117, 122, 128, and 134 all open with bare
```fences. This was raised in a previous review with no "addressed" resolution.✏️ Proposed fix (representative examples)
-``` +```text /jira:candidates-from-pr <pr-url-or-number> [options]```diff -``` +```jql project = <KEY> AND statusCategory != Done```diff -``` +```text /jira:candidates-from-pr https://github.com/openshift/ovn-kubernetes/pull/4567</details> Apply `text` or an appropriate language identifier to each remaining bare fence block. Also applies to: 87-93, 112-114, 117-119, 122-125, 128-131, 134-136 <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@plugins/jira/commands/candidates-from-pr.mdaround lines 10 - 12, The
markdown file contains multiple fenced code blocks with barefences (notably the example command `/jira:candidates-from-pr <pr-url-or-number> [options]` and JQL examples) that must include language identifiers; update each bare fence in plugins/jira/commands/candidates-from-pr.md (including the examples around the `/jira:candidates-from-pr` snippet and the JQL blocks) by adding an appropriate language tag such as `text` for CLI examples and `jql` for query blocks so everybecomestext orjql as appropriate.</details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>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@plugins/jira/commands/candidates-from-pr.md:
- Around line 151-169: The candidates schema's "assignee" is currently a string
("Jane Doe") but SKILL.md documents it as an object with display_name and email;
update the candidates example/schema so "assignee" is an object (e.g., {
"display_name": "Jane Doe", "email": "..." }) everywhere in the "candidates"
array and any related examples (including matched_signals/explicit_references
examples) to match the SKILL.md projection and preserve the richer assignee
structure.In
@plugins/jira/skills/candidates-from-pr/scripts/fetch_pr.py:
- Around line 72-77: The assignment to diff_unavailable_reason in the rc != 0
block can raise IndexError when diff_err is all-whitespace because
("".strip().splitlines()[-1]) is empty; change the logic that computes
diff_unavailable_reason (in the rc != 0 branch where diff and
diff_unavailable_reason are set) to first pick diff_err or "unknown error",
strip it, splitlines, then if the resulting list is empty fall back to "unknown
error" (or use the original fallback) so you never index [-1] on an empty list.In
@plugins/jira/skills/candidates-from-pr/SKILL.md:
- Around line 42-45: Add
jqto the prerequisites list because step that runs
COMPS=$(jq -r '.components | join(",")' filters.json) shells out to it; update
the prerequisites block (the lines listingghCLI, Jira MCP server, and Python
- to include
jq(e.g., "jq (JSON processor)") so readers know it must be
installed before running the skill.
Duplicate comments:
In@plugins/jira/commands/candidates-from-pr.md:
- Around line 10-12: The markdown file contains multiple fenced code blocks with
barefences (notably the example command `/jira:candidates-from-pr <pr-url-or-number> [options]` and JQL examples) that must include language identifiers; update each bare fence in plugins/jira/commands/candidates-from-pr.md (including the examples around the `/jira:candidates-from-pr` snippet and the JQL blocks) by adding an appropriate language tag such as `text` for CLI examples and `jql` for query blocks so everybecomestext orjql as appropriate.In
@plugins/jira/skills/candidates-from-pr/SKILL.md:
- Line 96: Several fenced code blocks in SKILL.md open with bare
fences (lines around the mcp__atlassian__jira_get_issue, mcp__atlassian__jira_get_project_versions, and mcp__atlassian__jira_search examples) and need explicit language tags; update each opening fence to include a language (e.g.,text) so the blocks becometext mcp__atlassian__jira_get_issue(...),text
mcp__atlassian__jira_get_project_versions(project_key=), and ```text
mcp__atlassian__jira_search(...), ensuring all three occurrences (around the
listed function names) are changed to remove the MD040 warning.- Line 120: Replace the contradictory phrase 'Pick the "smallest" version where
released == false' in SKILL.md with wording that matches the other doc: "pick
the latest unreleased version (highest/unreleased) and rewritefilters.json
with it (settarget_release_source = \"project_versions\")"; ensure the
language and any associated logic or examples explicitly state the highest
unreleased version for main/master branches (current development release) so it
aligns with the instruction in the other candidates-from-pr documentation.</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: Path: .coderabbit.yaml **Review profile**: CHILL **Plan**: Enterprise **Run ID**: `ef6b8f63-c4bf-4c48-86cc-ecbaae82a80e` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 9e3c5a9a56df8b8ab3ce1d62c5397cf17160d64f and 1bc068a5c141b9a8f7b16546d28962edfc25e293. </details> <details> <summary>📒 Files selected for processing (13)</summary> * `.claude-plugin/marketplace.json` * `PLUGINS.md` * `docs/data.json` * `plugins/jira/.claude-plugin/plugin.json` * `plugins/jira/commands/candidates-from-pr.md` * `plugins/jira/skills/candidates-from-pr/SKILL.md` * `plugins/jira/skills/candidates-from-pr/scripts/build_jql.py` * `plugins/jira/skills/candidates-from-pr/scripts/derive_filters.py` * `plugins/jira/skills/candidates-from-pr/scripts/extract_jira_keys.py` * `plugins/jira/skills/candidates-from-pr/scripts/extract_signals.py` * `plugins/jira/skills/candidates-from-pr/scripts/fetch_pr.py` * `plugins/jira/skills/candidates-from-pr/scripts/render_report.py` * `plugins/jira/skills/candidates-from-pr/scripts/score_candidates.py` </details> <details> <summary>✅ Files skipped from review due to trivial changes (1)</summary> * plugins/jira/.claude-plugin/plugin.json </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (8)</summary> * .claude-plugin/marketplace.json * plugins/jira/skills/candidates-from-pr/scripts/extract_signals.py * docs/data.json * plugins/jira/skills/candidates-from-pr/scripts/build_jql.py * plugins/jira/skills/candidates-from-pr/scripts/render_report.py * PLUGINS.md * plugins/jira/skills/candidates-from-pr/scripts/derive_filters.py * plugins/jira/skills/candidates-from-pr/scripts/score_candidates.py </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
Contributor
|
Tip For best results, initiate chat on the files or code changes.
The skipped If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
qinqon
force-pushed
the
feat/jira-candidates-from-pr
branch
2 times, most recently
from
c5665e0 to
34af796
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
plugins/jira/skills/candidates-from-pr/scripts/build_jql.py (1)
34-39: 💤 Low valueConsider validating non-empty target_release.
If
target_releaseis an empty string"", the current code will generate a version clause with empty quotes:("Target Version" = "" OR fixVersion = ""). While this won't cause errors, it likely won't match any issues and may not be the intended behavior.🔍 Optional defensive check
target_release = filters.get("target_release") -if target_release: +if target_release and target_release.strip(): parts.append(🤖 Prompt for 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. In `@plugins/jira/skills/candidates-from-pr/scripts/build_jql.py` around lines 34 - 39, The code appends a JQL fragment when target_release exists but doesn't guard against empty strings; update the logic in build_jql.py where target_release is read from filters and used with quote(target_release) so you only append to parts when target_release is a non-empty, non-whitespace string (e.g. check if target_release and target_release.strip()), keeping the existing clause format that uses "Target Version" and fixVersion and still using the quote(...) helper.
🤖 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.
Nitpick comments:
In `@plugins/jira/skills/candidates-from-pr/scripts/build_jql.py`:
- Around line 34-39: The code appends a JQL fragment when target_release exists
but doesn't guard against empty strings; update the logic in build_jql.py where
target_release is read from filters and used with quote(target_release) so you
only append to parts when target_release is a non-empty, non-whitespace string
(e.g. check if target_release and target_release.strip()), keeping the existing
clause format that uses "Target Version" and fixVersion and still using the
quote(...) helper.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 99c0175d-4f00-4116-8d5a-554b27c9f53c
📒 Files selected for processing (13)
.claude-plugin/marketplace.jsonPLUGINS.mddocs/data.jsonplugins/jira/.claude-plugin/plugin.jsonplugins/jira/commands/candidates-from-pr.mdplugins/jira/skills/candidates-from-pr/SKILL.mdplugins/jira/skills/candidates-from-pr/scripts/build_jql.pyplugins/jira/skills/candidates-from-pr/scripts/derive_filters.pyplugins/jira/skills/candidates-from-pr/scripts/extract_jira_keys.pyplugins/jira/skills/candidates-from-pr/scripts/extract_signals.pyplugins/jira/skills/candidates-from-pr/scripts/fetch_pr.pyplugins/jira/skills/candidates-from-pr/scripts/render_report.pyplugins/jira/skills/candidates-from-pr/scripts/score_candidates.py
✅ Files skipped from review due to trivial changes (3)
- plugins/jira/.claude-plugin/plugin.json
- PLUGINS.md
- plugins/jira/skills/candidates-from-pr/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (6)
- docs/data.json
- .claude-plugin/marketplace.json
- plugins/jira/skills/candidates-from-pr/scripts/derive_filters.py
- plugins/jira/skills/candidates-from-pr/scripts/extract_signals.py
- plugins/jira/skills/candidates-from-pr/scripts/score_candidates.py
- plugins/jira/skills/candidates-from-pr/scripts/render_report.py
qinqon
force-pushed
the
feat/jira-candidates-from-pr
branch
from
34af796 to
b67149e
Compare
Author
|
Addressed remaining CodeRabbit nitpicks in c6080e0: score_candidates.py:
build_jql.py:
All scripts:
|
qinqon
force-pushed
the
feat/jira-candidates-from-pr
branch
3 times, most recently
from
b41c4c8 to
66444c8
Compare
openshift-ci
Bot
added
the
needs-rebase
Author
CI Failure Analysis — commit 66444c8Total failures: 1 (1 related) 🔴 Related:
|
qinqon
force-pushed
the
feat/jira-candidates-from-pr
branch
from
66444c8 to
373f2c1
Compare
openshift-ci
Bot
removed
the
needs-rebase
openshift-ci
Bot
added
the
needs-rebase
Author
|
Rebased commit 373f2c1 on main and pushed (conflicts resolved). |
qinqon
force-pushed
the
feat/jira-candidates-from-pr
branch
from
373f2c1 to
8a2666c
Compare
openshift-ci
Bot
removed
the
needs-rebase
Author
CI Failure Analysis — commit 373f2c1Total failures: 1 (1 related) 🔴 Related:
|
Author
|
Rebased commit 8a2666c on main and pushed (conflicts resolved). |
qinqon
force-pushed
the
feat/jira-candidates-from-pr
branch
from
8a2666c to
4aa2aaa
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: qinqon The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
qinqon
force-pushed
the
feat/jira-candidates-from-pr
branch
from
4aa2aaa to
1949e3d
Compare
Adds /jira:candidates-from-pr, a read-only triage helper that takes a GitHub PR and surfaces a ranked table of open Jira issues the PR may fix, scoped by component and target release. This is the inverse direction of /jira:extract-prs (Jira -> PRs). The command extracts explicitly referenced Jira keys from the PR title/body/commits separately, derives component and target release from the PR's repo and base branch (with overrides), runs a JQL search for open issues, and locally scores each candidate by symbol, error string, keyword, and component overlap with the PR diff. Output is a triage-ready table that includes assignee as a required column (with an 'unassigned' fallback when the field is null), so the report is actionable without additional Jira lookups. JSON output is also supported for downstream tooling. The command never mutates Jira state; it is intended to assist maintainer triage when a PR was opened without a clear Jira link. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Enrique Llorente Pastora <ellorent@redhat.com>
- fetch_pr.py: guard against IndexError when gh stderr is whitespace-only - SKILL.md: add jq + Python 3.10+ to prerequisites - SKILL.md: 'highest' (not 'smallest') unreleased version for main/master, matching the command doc's wording - SKILL.md, candidates-from-pr.md: tag bare fenced blocks (MD040) - candidates-from-pr.md: align candidate `assignee` schema with SKILL.md (object with display_name/email, as returned by Jira) - bump jira plugin to 0.4.8 Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Enrique Llorente Pastora <ellorent@redhat.com>
- Remove dead lower() helper from score_candidates.py - Simplify redundant elif guard in component check - Add defensive .lower() in find_lower for case-insensitive matching - Guard against empty target_release in build_jql.py - Add from __future__ import annotations to all scripts for Python 3.9+ Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Enrique Llorente Pastora <ellorent@redhat.com>
qinqon
force-pushed
the
feat/jira-candidates-from-pr
branch
from
1949e3d to
e1b9981
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
Adds
/jira:candidates-from-pr, a read-only triage helper that takes a GitHub PR and surfaces a ranked list of open Jira issues the PR may fix, scoped by component and target release. It is the inverse direction of/jira:extract-prs(which goes Jira → PRs).The command:
gh pr view/gh pr diff.--include-explicitis passed).--componentoverride) and target release from the PR's base branch (with--target-releaseoverride). For OCPBUGS the user-facingTarget Version(customfield_10855) is preferred overfixVersions— seeplugins/jira/reference/mcp-tools.md:295.statusCategory != Done, and runsmcp__atlassian__jira_search.--output jsonfor downstream tooling.The command is read-only — it never posts comments, links the PR, or modifies Jira state. It is intended to assist maintainer triage when a PR was opened without a clear Jira reference (or with an incomplete one), e.g. on
openshift/ovn-kubernetes.Files
plugins/jira/commands/candidates-from-pr.md— command spec (/jira:candidates-from-pr <pr-url-or-number>plus flags).plugins/jira/skills/candidates-from-pr/SKILL.md— full implementation guide: phases, scoring rubric, JSON schema, error handling, performance notes.plugins/jira/.claude-plugin/plugin.json— version bump 0.4.3 → 0.4.4..claude-plugin/marketplace.json,PLUGINS.md,docs/data.json— regenerated viamake update.Overlap review (per
AGENTS.md)Ran the equivalent of
/utils:review-ai-helpers-overlap --idea "given a GitHub PR, find candidate open Jiras with matching target release that the PR may fix, by analyzing the diff and component"against 100 open PRs and main-branch commands.No HIGH or MODERATE overlap found. Closest neighbors (all LOW):
jira:extract-prs(main)jira:reconcile-github(main)gh2jira, not PR-diff matching.utils:process-renovate-pr(main)bug-triage:scrubgithub:issue-triagejira:post-summary-commentValidation
make update: ✓ marketplace and docs regenerated.make lint: ✓ 0 errors, 0 warnings.Which issue(s) this PR fixes:
Fixes
Special notes for your reviewer:
--componentalways overrides.fixVersions: the JQL queries both, but reporting prefersTarget Version(customfield_10855) because for OCPBUGS that's the user-facing field whilefixVersionsis owned by the release team.--include-explicitto bypass.plugins/jira/skills/candidates-from-pr/SKILL.md) and runs entirely on in-memory PR signals vs. the Jira fields returned by the search — no per-candidate API calls./jira:extract-prs. Naming follows existing patterns (extract-prs,clone-from-github,reconcile-github).Checklist:
Assisted-By: Claude noreply@anthropic.com
Summary by CodeRabbit
New Features
Documentation
Chores