Add /jira:generate-feature-updates command#375
Open
celebdor wants to merge 1 commit intoopenshift-eng:mainfrom
Open
Add /jira:generate-feature-updates command#375celebdor wants to merge 1 commit intoopenshift-eng:mainfrom
celebdor wants to merge 1 commit intoopenshift-eng:mainfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: celebdor 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 |
openshift-merge-robot
added
the
needs-rebase
Contributor
|
PR needs rebase. 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
WalkthroughAdds a new Jira command Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant CLI as /jira:generate-feature-updates
participant Gather as gather_status_data.py
participant Triage as triage_issues.py
participant Engine as Status Analysis Engine
participant Formatter as format_feature_markdown()
participant Validator as validate_feature_updates.py
participant Output as Output (stdout/HTML)
User->>CLI: Invoke with project-key and filters
CLI->>Gather: Phase 1 — collect issues, PRs, labels, authors
Gather-->>CLI: Emit .work/.../manifest.json and issues/*.json
CLI->>Triage: Phase 2 — classify significant vs skipped
Triage-->>CLI: Return significant issue list
CLI->>Engine: Batch process significant issues
Engine->>Formatter: Format each issue as feature_markdown
Formatter-->>Engine: Formatted markdown sections
Engine-->>CLI: Assembled feature updates
CLI->>Validator: Run handle and HTML validations
Validator-->>CLI: Validation results
CLI-->>Output: Render to stdout or serve HTML
Output-->>User: Final feature updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 7 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (7 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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: 7
🧹 Nitpick comments (1)
plugins/jira/commands/generate-feature-updates.md (1)
145-147: Use “Markdown” (proper noun) consistently in prose.Line 146, Line 235, and Line 410 use lowercase “markdown”; capitalizing improves documentation consistency.
Suggested wording tweak
-- Feature markdown formatting rules +- Feature Markdown formatting rules ... -All entries form one continuous markdown unordered list (no blank lines between items). +All entries form one continuous Markdown unordered list (no blank lines between items). ... - - Phase 2 (Processing): LLM analyzes all issues in batch with feature_markdown format + - Phase 2 (Processing): LLM analyzes all issues in batch with feature_markdown format in Markdown outputAlso applies to: 235-235, 410-410
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/jira/commands/generate-feature-updates.md` around lines 145 - 147, The doc uses lowercase "markdown" in several places; update those occurrences to the proper noun "Markdown" for consistency. Specifically, change the heading text "Feature markdown formatting rules" and any inline uses of "markdown" in the prose (the other instances flagged in the review) to "Markdown", preserving surrounding punctuation and casing for acronyms or sentence starts. Search the file for the exact token "markdown" and replace with "Markdown" except where intentional lowercase is required, then run a quick spell/grammar check to confirm no other instances remain.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/jira/skills/status-analysis/scripts/gather_status_data.py`:
- Around line 717-720: The code currently sets author_name by falling back to
login/email (author_obj.get("name") or author_obj.get("login", "Unknown")),
which can leak emails or handles; change each occurrence (e.g., where
reviews_in_range is built and the other spots around the review processing) to
only use the real display name when present (author_obj.get("name") if truthy)
and otherwise set author_name to None or an empty string (do not fall back to
login/commit email or provide a default like "Unknown"); update the fields where
author_name is assigned (references: author_obj, author_name, reviews_in_range)
to follow this rule across all mentioned blocks (lines near 717, 732-738,
749-750).
In `@plugins/jira/skills/status-analysis/scripts/summarize_issue.py`:
- Around line 141-147: The current merged_recent filter only enforces a lower
bound (date_start) and misses an upper bound, so PRs merged after the reporting
window can be included; update the filter used in summarize_issue (the
merged_recent list comprehension over prs, referencing pr["state"],
pr["dates"].get("merged_at"), and date_start) to also check against the window
end (e.g., date_end or the manifest's end date) by ensuring merged_at exists and
date_start <= merged_at <= date_end (or the manifest date range) so only PRs
merged within the full date range are included.
- Around line 65-75: The color-change detection is using legacy keys
item.get("fromString") / item.get("toString") but the per-issue JSON normalizes
changelog items to use "from" and "to"; update the loop that iterates changelog
(the block referencing field, field_id, and the color_changed logic) to read
from values using item.get("from") and item.get("to") (with safe empty-string
defaults) instead of fromString/toString, and make the same replacement in the
other occurrence around the code referenced at lines 113-117 so that
--only-significant color-change detection works correctly.
In `@plugins/jira/skills/status-analysis/scripts/triage_issues.py`:
- Around line 55-65: The changelog parsing loop is using keys that don't exist
after normalization, so color flips never detect; update the loop to read the
normalized keys (use item.get("from", "") and item.get("to", "") instead of
item.get("fromString", "")/item.get("toString", "") and stop relying on fieldId
if your normalization drops it), then feed those values into parse_color and
keep the existing oc/nc != checks to set color_changed and assign
old_color/new_color (refer to the loop over changelog entries and the
parse_color call to locate the fix).
In `@plugins/jira/skills/status-analysis/scripts/validate_feature_updates.py`:
- Around line 117-124: The current mojibake check in validate_feature_updates.py
flags any 'â', which false-positives valid UTF-8 text; update the has_mojibake
detection used where has_mojibake is computed so it no longer treats a lone 'â'
as an error—remove the generic "â" test and instead detect concrete mojibake
indicators such as the Unicode replacement character ('\ufffd') and known
garbled sequences like "ð" or "ðŸ" (or other specific "Ã" + high-byte
sequences), keeping the rest of the logic (errors.append message) unchanged so
only real encoding corruption is reported.
- Around line 52-66: The code is iterating the wrong key for PR activity—it's
using data.get("external_links", {}).get("github_prs", []) which yields no PRs
so authors stays empty; change the PR source to read the top-level "prs" list
(data.get("prs", [])) in the block that populates the authors dict (the loop
over prs and inner loop over "reviews_in_range", "review_comments_in_range",
"commits_in_range") so the markdown handle validation actually processes PR
activity for each entry and populates authors appropriately.
In `@plugins/jira/skills/status-analysis/SKILL.md`:
- Around line 366-373: The fenced example block in the SKILL.md for
`feature_markdown` (generate-feature-updates) lacks a language tag, causing
markdownlint MD040; update the triple-backtick fence that opens the example (the
block showing the ISSUE-KEY list) to specify the language `markdown` so the
block starts with ```markdown and leave the block content unchanged.
---
Nitpick comments:
In `@plugins/jira/commands/generate-feature-updates.md`:
- Around line 145-147: The doc uses lowercase "markdown" in several places;
update those occurrences to the proper noun "Markdown" for consistency.
Specifically, change the heading text "Feature markdown formatting rules" and
any inline uses of "markdown" in the prose (the other instances flagged in the
review) to "Markdown", preserving surrounding punctuation and casing for
acronyms or sentence starts. Search the file for the exact token "markdown" and
replace with "Markdown" except where intentional lowercase is required, then run
a quick spell/grammar check to confirm no other instances remain.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 67a63a12-b4f0-4338-b929-f39670cc900f
📒 Files selected for processing (13)
.claude-plugin/marketplace.jsonPLUGINS.mddocs/data.jsonplugins/jira/.claude-plugin/plugin.jsonplugins/jira/commands/generate-feature-updates.mdplugins/jira/commands/status-rollup.mdplugins/jira/commands/update-weekly-status.mdplugins/jira/skills/status-analysis/SKILL.mdplugins/jira/skills/status-analysis/formatting.mdplugins/jira/skills/status-analysis/scripts/gather_status_data.pyplugins/jira/skills/status-analysis/scripts/summarize_issue.pyplugins/jira/skills/status-analysis/scripts/triage_issues.pyplugins/jira/skills/status-analysis/scripts/validate_feature_updates.py
Comment on lines
+717
to
+720
| author_obj = review.get("author") or {} | ||
| reviews_in_range.append({ | ||
| "author": (review.get("author") or {}).get("login", "Unknown"), | ||
| "author": author_obj.get("login", "Unknown"), | ||
| "author_name": author_obj.get("name") or author_obj.get("login", "Unknown"), |
Contributor
There was a problem hiding this comment.
Keep author_name reserved for real display names.
The new feature-update flow treats author_name as human-facing attribution. Falling back to GitHub logins or commit emails here means downstream output can either leak email addresses or violate the “first names only / no handles” rule.
Suggested change
reviews_in_range.append({
"author": author_obj.get("login", "Unknown"),
- "author_name": author_obj.get("name") or author_obj.get("login", "Unknown"),
+ "author_name": author_obj.get("name"),
"state": review.get("state"),
"body": (review.get("body") or "")[:500],
"submitted_at": review.get("submittedAt"),
})
@@
commits_in_range.append({
"sha": (commit.get("oid") or "")[:7],
"message": commit.get("messageHeadline", ""),
"date": commit.get("committedDate"),
"author": commit_author.get("email", "Unknown"),
- "author_name": commit_author.get("name") or commit_author.get("email", "Unknown"),
+ "author_name": commit_author.get("name"),
})
@@
review_comments_in_range.append({
"author": comment_author.get("login", "Unknown"),
- "author_name": comment_author.get("name") or comment_author.get("login", "Unknown"),
+ "author_name": comment_author.get("name"),
"path": comment.get("path"),
"line": comment.get("line"),
"body": (comment.get("body") or "")[:300],Also applies to: 732-738, 749-750
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/jira/skills/status-analysis/scripts/gather_status_data.py` around
lines 717 - 720, The code currently sets author_name by falling back to
login/email (author_obj.get("name") or author_obj.get("login", "Unknown")),
which can leak emails or handles; change each occurrence (e.g., where
reviews_in_range is built and the other spots around the review processing) to
only use the real display name when present (author_obj.get("name") if truthy)
and otherwise set author_name to None or an empty string (do not fall back to
login/commit email or provide a default like "Unknown"); update the fields where
author_name is assigned (references: author_obj, author_name, reviews_in_range)
to follow this rule across all mentioned blocks (lines near 717, 732-738,
749-750).
Comment on lines
+65
to
+75
| for entry in changelog: | ||
| for item in entry.get("items", []): | ||
| field = item.get("field", "") | ||
| field_id = str(item.get("fieldId", "")) | ||
| if "Status Summary" in field or "customfield_12320841" in field_id: | ||
| for color in ("Red", "Yellow", "Green"): | ||
| from_has = color in (item.get("fromString") or "") | ||
| to_has = color in (item.get("toString") or "") | ||
| if from_has != to_has: | ||
| color_changed = True | ||
|
|
Contributor
There was a problem hiding this comment.
Use the normalized changelog fields here.
The per-issue JSON already stores each changelog item as field / from / to. Reading fromString / toString here breaks --only-significant color-change detection and renders every changelog diff as ? -> ?.
Suggested change
for entry in changelog:
for item in entry.get("items", []):
- field = item.get("field", "")
- field_id = str(item.get("fieldId", ""))
- if "Status Summary" in field or "customfield_12320841" in field_id:
+ field = (item.get("field") or "").lower()
+ if "status summary" in field:
for color in ("Red", "Yellow", "Green"):
- from_has = color in (item.get("fromString") or "")
- to_has = color in (item.get("toString") or "")
+ from_has = color in (item.get("from") or "")
+ to_has = color in (item.get("to") or "")
if from_has != to_has:
color_changed = True
@@
items_str = ", ".join(
- f"{i.get('field', '?')}: {i.get('fromString', '?')} -> {i.get('toString', '?')}"
+ f"{i.get('field', '?')}: {i.get('from', '?')} -> {i.get('to', '?')}"
for i in c.get("items", [])
)Also applies to: 113-117
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/jira/skills/status-analysis/scripts/summarize_issue.py` around lines
65 - 75, The color-change detection is using legacy keys item.get("fromString")
/ item.get("toString") but the per-issue JSON normalizes changelog items to use
"from" and "to"; update the loop that iterates changelog (the block referencing
field, field_id, and the color_changed logic) to read from values using
item.get("from") and item.get("to") (with safe empty-string defaults) instead of
fromString/toString, and make the same replacement in the other occurrence
around the code referenced at lines 113-117 so that --only-significant
color-change detection works correctly.
Comment on lines
+141
to
+147
| # Merged this week: filter by date range start if available | ||
| if date_start: | ||
| merged_recent = [ | ||
| pr for pr in prs | ||
| if pr["state"] == "MERGED" | ||
| and (pr["dates"].get("merged_at") or "") >= date_start | ||
| ] |
Contributor
There was a problem hiding this comment.
Merged this week needs an upper bound.
This only checks merged_at >= start_date. Because PR state is fetched live, a PR merged after the requested reporting window will still be reported as “merged this week” when summarizing an older snapshot. Compare merged_at against the full manifest date range.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/jira/skills/status-analysis/scripts/summarize_issue.py` around lines
141 - 147, The current merged_recent filter only enforces a lower bound
(date_start) and misses an upper bound, so PRs merged after the reporting window
can be included; update the filter used in summarize_issue (the merged_recent
list comprehension over prs, referencing pr["state"],
pr["dates"].get("merged_at"), and date_start) to also check against the window
end (e.g., date_end or the manifest's end date) by ensuring merged_at exists and
date_start <= merged_at <= date_end (or the manifest date range) so only PRs
merged within the full date range are included.
Comment on lines
+55
to
+65
| for entry in changelog: | ||
| for item in entry.get("items", []): | ||
| field = item.get("field", "") | ||
| field_id = str(item.get("fieldId", "")) | ||
| if "Status Summary" in field or "customfield_12320841" in field_id: | ||
| oc = parse_color(item.get("fromString", "")) | ||
| nc = parse_color(item.get("toString", "")) | ||
| if oc != nc and oc is not None and nc is not None: | ||
| color_changed = True | ||
| old_color = oc | ||
| new_color = nc |
Contributor
There was a problem hiding this comment.
Color flips are never detected with the current changelog keys.
gather_status_data.py normalizes changelog items to field, from, and to. This block looks for fieldId, fromString, and toString, so color_changed stays false and issues whose only noteworthy update is an R/Y/G change get skipped.
Suggested change
for entry in changelog:
for item in entry.get("items", []):
- field = item.get("field", "")
- field_id = str(item.get("fieldId", ""))
- if "Status Summary" in field or "customfield_12320841" in field_id:
- oc = parse_color(item.get("fromString", ""))
- nc = parse_color(item.get("toString", ""))
- if oc != nc and oc is not None and nc is not None:
+ field = (item.get("field") or "").lower()
+ if "status summary" in field:
+ oc = parse_color(item.get("from", ""))
+ nc = parse_color(item.get("to", ""))
+ if oc != nc and (oc is not None or nc is not None):
color_changed = True
old_color = oc
new_color = nc📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| for entry in changelog: | |
| for item in entry.get("items", []): | |
| field = item.get("field", "") | |
| field_id = str(item.get("fieldId", "")) | |
| if "Status Summary" in field or "customfield_12320841" in field_id: | |
| oc = parse_color(item.get("fromString", "")) | |
| nc = parse_color(item.get("toString", "")) | |
| if oc != nc and oc is not None and nc is not None: | |
| color_changed = True | |
| old_color = oc | |
| new_color = nc | |
| for entry in changelog: | |
| for item in entry.get("items", []): | |
| field = (item.get("field") or "").lower() | |
| if "status summary" in field: | |
| oc = parse_color(item.get("from", "")) | |
| nc = parse_color(item.get("to", "")) | |
| if oc != nc and (oc is not None or nc is not None): | |
| color_changed = True | |
| old_color = oc | |
| new_color = nc |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/jira/skills/status-analysis/scripts/triage_issues.py` around lines 55
- 65, The changelog parsing loop is using keys that don't exist after
normalization, so color flips never detect; update the loop to read the
normalized keys (use item.get("from", "") and item.get("to", "") instead of
item.get("fromString", "")/item.get("toString", "") and stop relying on fieldId
if your normalization drops it), then feed those values into parse_color and
keep the existing oc/nc != checks to set color_changed and assign
old_color/new_color (refer to the loop over changelog entries and the
parse_color call to locate the fix).
Comment on lines
+52
to
+66
| prs = data.get("external_links", {}).get("github_prs", []) | ||
| for pr in prs: | ||
| for source_key in ("reviews_in_range", "review_comments_in_range", "commits_in_range"): | ||
| for entry in pr.get(source_key, []): | ||
| login = entry.get("author", "") | ||
| name = entry.get("author_name", "") | ||
|
|
||
| # Skip emails (commits use email as author) and unknowns | ||
| if not login or "@" in login or login == "Unknown": | ||
| continue | ||
|
|
||
| # Prefer the longest/most complete name seen | ||
| if login not in authors or (name and len(name) > len(authors[login])): | ||
| authors[login] = name if name else login | ||
|
|
Contributor
There was a problem hiding this comment.
Read PR activity from prs.
gather_status_data.py writes per-issue PR data under the top-level prs key. Reading external_links.github_prs here leaves authors empty, so the markdown handle validation silently becomes a no-op.
Suggested change
- prs = data.get("external_links", {}).get("github_prs", [])
+ prs = data.get("prs") or data.get("external_links", {}).get("github_prs", [])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| prs = data.get("external_links", {}).get("github_prs", []) | |
| for pr in prs: | |
| for source_key in ("reviews_in_range", "review_comments_in_range", "commits_in_range"): | |
| for entry in pr.get(source_key, []): | |
| login = entry.get("author", "") | |
| name = entry.get("author_name", "") | |
| # Skip emails (commits use email as author) and unknowns | |
| if not login or "@" in login or login == "Unknown": | |
| continue | |
| # Prefer the longest/most complete name seen | |
| if login not in authors or (name and len(name) > len(authors[login])): | |
| authors[login] = name if name else login | |
| prs = data.get("prs") or data.get("external_links", {}).get("github_prs", []) | |
| for pr in prs: | |
| for source_key in ("reviews_in_range", "review_comments_in_range", "commits_in_range"): | |
| for entry in pr.get(source_key, []): | |
| login = entry.get("author", "") | |
| name = entry.get("author_name", "") | |
| # Skip emails (commits use email as author) and unknowns | |
| if not login or "@" in login or login == "Unknown": | |
| continue | |
| # Prefer the longest/most complete name seen | |
| if login not in authors or (name and len(name) > len(authors[login])): | |
| authors[login] = name if name else login |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/jira/skills/status-analysis/scripts/validate_feature_updates.py`
around lines 52 - 66, The code is iterating the wrong key for PR activity—it's
using data.get("external_links", {}).get("github_prs", []) which yields no PRs
so authors stays empty; change the PR source to read the top-level "prs" list
(data.get("prs", [])) in the block that populates the authors dict (the loop
over prs and inner loop over "reviews_in_range", "review_comments_in_range",
"commits_in_range") so the markdown handle validation actually processes PR
activity for each entry and populates authors appropriately.
Comment on lines
+117
to
+124
| # Check that emoji circles survived encoding (not mojibake) | ||
| has_circles = any(c in text for c in "\U0001f7e2\U0001f7e1\U0001f534") # 🟢🟡🔴 | ||
| has_mojibake = "ð" in text or "â" in text or "ðŸ" in text | ||
| if has_mojibake: | ||
| errors.append( | ||
| "Mojibake detected: emoji circles are garbled " | ||
| "(likely written without encoding='utf-8' or missing charset meta)" | ||
| ) |
Contributor
There was a problem hiding this comment.
Don't treat every â as mojibake.
A valid UTF-8 document can legitimately contain â in names or prose, so this will false-fail correctly encoded HTML. Check for specific mojibake sequences or replacement characters instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/jira/skills/status-analysis/scripts/validate_feature_updates.py`
around lines 117 - 124, The current mojibake check in
validate_feature_updates.py flags any 'â', which false-positives valid UTF-8
text; update the has_mojibake detection used where has_mojibake is computed so
it no longer treats a lone 'â' as an error—remove the generic "â" test and
instead detect concrete mojibake indicators such as the Unicode replacement
character ('\ufffd') and known garbled sequences like "ð" or "ðŸ" (or other
specific "Ã" + high-byte sequences), keeping the rest of the logic
(errors.append message) unchanged so only real encoding corruption is reported.
Comment on lines
+366
to
+373
| **For `feature_markdown` (generate-feature-updates)**: | ||
|
|
||
| ``` | ||
| - [ISSUE-KEY](https://issues.redhat.com/browse/ISSUE-KEY): Issue summary | ||
| - 1-3 sentences of executive prose. No metrics, no R/Y/G. | ||
| - [ISSUE-KEY-2](https://issues.redhat.com/browse/ISSUE-KEY-2): Issue summary | ||
| - Prose focusing on significant progress, deliveries, blockers, or risks. | ||
| ``` |
Contributor
There was a problem hiding this comment.
Specify a language for the fenced example block.
Line 368 uses a fenced code block without a language, which triggers markdownlint MD040. Use markdown for this example block.
Suggested fix
-```
+```markdown
- [ISSUE-KEY](https://issues.redhat.com/browse/ISSUE-KEY): Issue summary
- 1-3 sentences of executive prose. No metrics, no R/Y/G.
- [ISSUE-KEY-2](https://issues.redhat.com/browse/ISSUE-KEY-2): Issue summary
- Prose focusing on significant progress, deliveries, blockers, or risks.</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
**For `feature_markdown` (generate-feature-updates)**:
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 368-368: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/jira/skills/status-analysis/SKILL.md` around lines 366 - 373, The
fenced example block in the SKILL.md for `feature_markdown`
(generate-feature-updates) lacks a language tag, causing markdownlint MD040;
update the triple-backtick fence that opens the example (the block showing the
ISSUE-KEY list) to specify the language `markdown` so the block starts with
```markdown and leave the block content unchanged.
New command that generates concise executive-level feature summaries for weekly status documents. Analyzes recent Jira activity across issue hierarchies and produces prose-style updates suitable for the "Key Strategic Feature Updates" section of weekly reports. Key capabilities: - Efficient async batch data gathering via Python script - Label and component-based filtering (e.g. --label control-plane-work) - GitHub PR integration with author name resolution - Automatic triage: skips issues with no significant activity - --serve mode: converts output to HTML on localhost for easy copy/paste into Google Docs Supporting scripts added: - triage_issues.py: classify issues by activity level and R/Y/G status - summarize_issue.py: human-readable summary of pre-gathered issue data - validate_feature_updates.py: validates markdown for GitHub handle usage and HTML for proper UTF-8 charset/encoding (prevents emoji rendering issues in browsers) Also updates the shared status-analysis skill with feature_markdown output format and formatting rules. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
celebdor
force-pushed
the
generate-feature-updates
branch
from
a04bd6c to
1b740b8
Compare
openshift-ci
Bot
removed
the
needs-rebase
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
plugins/jira/skills/status-analysis/scripts/validate_feature_updates.py (1)
109-111: Simplify DOCTYPE check.The second condition is redundant since
text.upper()already handles case-insensitivity.Suggested fix
# Must have DOCTYPE - if "<!DOCTYPE" not in text.upper() and "<!doctype" not in text: + if "<!DOCTYPE" not in text.upper(): errors.append("Missing <!DOCTYPE html> declaration")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/jira/skills/status-analysis/scripts/validate_feature_updates.py` around lines 109 - 111, The DOCTYPE check contains a redundant case-insensitive branch; replace the two-part condition with a single case-insensitive check by calling text.upper() once and testing for "<!DOCTYPE" (i.e., change the if that currently reads if "<!DOCTYPE" not in text.upper() and "<!doctype" not in text: to a single check using text.upper()), leaving the errors.append("Missing <!DOCTYPE html> declaration") behavior unchanged; target the block using the local variable text and the errors list in validate_feature_updates.py.plugins/jira/skills/status-analysis/scripts/summarize_issue.py (1)
240-240: Remove extraneousfprefix from strings without placeholders.Ruff F541 flags f-strings that have no placeholders.
Suggested fix
- print(f"Examples:", file=sys.stderr) + print("Examples:", file=sys.stderr)- print(f"Confirm with the user whether to include them in the report.\n") + print("Confirm with the user whether to include them in the report.\n")Also applies to: 306-306
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/jira/skills/status-analysis/scripts/summarize_issue.py` at line 240, Remove the unnecessary f-string prefixes on print calls that contain no placeholders in summarize_issue.py; specifically edit the print statements (e.g., the print("Examples:", file=sys.stderr) call at the location currently using print(f"Examples:", file=sys.stderr) and the similar print at line ~306) so they use plain string literals instead of f-strings; look for print(...) usages within the same function or block that are prefixed with f but contain no interpolation and remove the leading f.plugins/jira/commands/generate-feature-updates.md (1)
12-14: Add language specifier to synopsis code block.For consistency and to satisfy markdownlint MD040, specify a language (e.g.,
textorbash) for the synopsis block.-``` +```text /jira:generate-feature-updates [project-key] [--component <component-name>] [--label <label-name>] [user-filters...]<details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@plugins/jira/commands/generate-feature-updates.mdaround lines 12 - 14, The
synopsis code block in generate-feature-updates.md lacks a language specifier
which triggers markdownlint MD040; update the fenced code block that currently
contains "/jira:generate-feature-updates [project-key] [--component
] [--label ] [user-filters...]" to include a
language (e.g., change the opening fence to "text" or "bash") so the block
is explicitly tagged and linting passes.</details> </blockquote></details> <details> <summary>plugins/jira/skills/status-analysis/scripts/gather_status_data.py (1)</summary><blockquote> `1189-1189`: **Rename ambiguous variable `l` to `label`.** Ruff E741 flags single-letter `l` as ambiguous (easily confused with `1` or `I`). <details> <summary>Suggested fix</summary> ```diff - labels = [l for l in fields.get("labels", []) if isinstance(l, str)] + labels = [label for label in fields.get("labels", []) if isinstance(label, str)] ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@plugins/jira/skills/status-analysis/scripts/gather_status_data.py` at line 1189, Rename the ambiguous single-letter loop variable in the list comprehension that builds labels to a clear name: change the comprehension that currently reads using l over fields.get("labels", []) to use label (e.g., labels = [label for label in fields.get("labels", []) if isinstance(label, str)]), ensuring any other local references in the same expression are updated accordingly so the variable is no longer a single-letter ambiguous identifier. ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Inline comments:
In@plugins/jira/commands/generate-feature-updates.md:
- Around line 436-438: The environment variable names are inconsistent between
the docs and code: update the docs in generate-feature-updates.md to reference
the actual variable used in gather_status_data.py (JIRA_API_TOKEN) or,
alternatively, make gather_status_data.py accept JIRA_TOKEN/JIRA_PERSONAL_TOKEN
(e.g., check for those names before falling back to JIRA_API_TOKEN). Reference
the variable JIRA_API_TOKEN in gather_status_data.py and the markdown section in
generate-feature-updates.md to ensure both sides use the same name.
Nitpick comments:
In@plugins/jira/commands/generate-feature-updates.md:
- Around line 12-14: The synopsis code block in generate-feature-updates.md
lacks a language specifier which triggers markdownlint MD040; update the fenced
code block that currently contains "/jira:generate-feature-updates [project-key]
[--component ] [--label ] [user-filters...]" to
include a language (e.g., change the opening fence to "text" or "bash") so
the block is explicitly tagged and linting passes.In
@plugins/jira/skills/status-analysis/scripts/gather_status_data.py:
- Line 1189: Rename the ambiguous single-letter loop variable in the list
comprehension that builds labels to a clear name: change the comprehension that
currently reads using l over fields.get("labels", []) to use label (e.g., labels
= [label for label in fields.get("labels", []) if isinstance(label, str)]),
ensuring any other local references in the same expression are updated
accordingly so the variable is no longer a single-letter ambiguous identifier.In
@plugins/jira/skills/status-analysis/scripts/summarize_issue.py:
- Line 240: Remove the unnecessary f-string prefixes on print calls that contain
no placeholders in summarize_issue.py; specifically edit the print statements
(e.g., the print("Examples:", file=sys.stderr) call at the location currently
using print(f"Examples:", file=sys.stderr) and the similar print at line ~306)
so they use plain string literals instead of f-strings; look for print(...)
usages within the same function or block that are prefixed with f but contain no
interpolation and remove the leading f.In
@plugins/jira/skills/status-analysis/scripts/validate_feature_updates.py:declaration") behavior unchanged; target the block using the
- Around line 109-111: The DOCTYPE check contains a redundant case-insensitive
branch; replace the two-part condition with a single case-insensitive check by
calling text.upper() once and testing for "<!DOCTYPE" (i.e., change the if that
currently reads if "<!DOCTYPE" not in text.upper() and "<!doctype" not in text:
to a single check using text.upper()), leaving the errors.append("Missinglocal variable text and the errors list in validate_feature_updates.py.
</details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: Path: .coderabbit.yaml **Review profile**: CHILL **Plan**: Pro **Run ID**: `60130236-8309-428b-88a8-04b8b6cfcce0` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between a04bd6c85dcd08bfd42adda4fbb6a837d9a0f97f and 1b740b871c063a1fb224300713561d1bb9c9b216. </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/generate-feature-updates.md` * `plugins/jira/commands/status-rollup.md` * `plugins/jira/commands/update-weekly-status.md` * `plugins/jira/skills/status-analysis/SKILL.md` * `plugins/jira/skills/status-analysis/formatting.md` * `plugins/jira/skills/status-analysis/scripts/gather_status_data.py` * `plugins/jira/skills/status-analysis/scripts/summarize_issue.py` * `plugins/jira/skills/status-analysis/scripts/triage_issues.py` * `plugins/jira/skills/status-analysis/scripts/validate_feature_updates.py` </details> <details> <summary>✅ Files skipped from review due to trivial changes (4)</summary> * .claude-plugin/marketplace.json * plugins/jira/commands/status-rollup.md * plugins/jira/.claude-plugin/plugin.json * PLUGINS.md </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (3)</summary> * plugins/jira/commands/update-weekly-status.md * docs/data.json * plugins/jira/skills/status-analysis/scripts/triage_issues.py </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Comment on lines
+436
to
+438
| - **Environment variables**: | ||
| - `JIRA_TOKEN` or `JIRA_PERSONAL_TOKEN`: Jira API bearer token | ||
| - `GITHUB_TOKEN` or authenticated `gh` CLI |
Contributor
There was a problem hiding this comment.
Environment variable name mismatch.
Line 437 references JIRA_TOKEN or JIRA_PERSONAL_TOKEN, but gather_status_data.py (line 1362) uses JIRA_API_TOKEN. This inconsistency could confuse users setting up the environment.
Suggested fix
- **Environment variables**:
- - `JIRA_TOKEN` or `JIRA_PERSONAL_TOKEN`: Jira API bearer token
+ - `JIRA_API_TOKEN`: Atlassian API token (create at https://id.atlassian.com/manage-profile/security/api-tokens)
+ - `JIRA_USERNAME`: Atlassian account email
- `GITHUB_TOKEN` or authenticated `gh` CLI📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - **Environment variables**: | |
| - `JIRA_TOKEN` or `JIRA_PERSONAL_TOKEN`: Jira API bearer token | |
| - `GITHUB_TOKEN` or authenticated `gh` CLI | |
| - **Environment variables**: | |
| - `JIRA_API_TOKEN`: Atlassian API token (create at https://id.atlassian.com/manage-profile/security/api-tokens) | |
| - `JIRA_USERNAME`: Atlassian account email | |
| - `GITHUB_TOKEN` or authenticated `gh` CLI |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/jira/commands/generate-feature-updates.md` around lines 436 - 438,
The environment variable names are inconsistent between the docs and code:
update the docs in generate-feature-updates.md to reference the actual variable
used in gather_status_data.py (JIRA_API_TOKEN) or, alternatively, make
gather_status_data.py accept JIRA_TOKEN/JIRA_PERSONAL_TOKEN (e.g., check for
those names before falling back to JIRA_API_TOKEN). Reference the variable
JIRA_API_TOKEN in gather_status_data.py and the markdown section in
generate-feature-updates.md to ensure both sides use the same name.
|
PR needs rebase. 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. |
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
2 participants
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.
Summary
--label,--component, and user filters;--servemode converts to HTML on localhost for copy/paste into Google Docstriage_issues.py,summarize_issue.py, andvalidate_feature_updates.py(validates markdown for GitHub handles and HTML for proper UTF-8 charset to prevent emoji rendering issues)Test plan
/jira:generate-feature-updates OCPSTRAT --label control-plane-work --serveend-to-endmake lint)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores