OCPEDGE-2781: Add /release-planning skill (KR 4.2)#219
Merged
openshift-merge-bot[bot] merged 1 commit intoJul 13, 2026
Merged
Conversation
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:
WalkthroughThis PR adds the Edge Scrum release-planning workflow, including new skills, Jira normalization, deterministic checks, report assembly, DOCX rendering, and test coverage. It also updates plugin version metadata and adds symlinks for the new skills. ChangesEdge Scrum release-planning workflow
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 9 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (9 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 6
🧹 Nitpick comments (4)
plugins/edge-scrum/bin/transform-bugs.py (1)
42-42: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winDefault priority to
"Unknown"instead of"Major"for data-quality visibility.Silently mapping missing priority data to
"Major"masks the gap inbugs_by_priorityaggregates, unlikecomponent, which correctly surfaces missing data as"Unknown".- priority = get_nested(raw, "priority", "name") or "Major" + priority = get_nested(raw, "priority", "name") or "Unknown"🤖 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/edge-scrum/bin/transform-bugs.py` at line 42, The default fallback in transform-bugs.py currently maps missing priority values to "Major", which hides missing data in priority aggregates. Update the logic in the priority assignment near get_nested(raw, "priority", "name") so that absent or empty priority values default to "Unknown" instead, matching the existing handling used for missing component data and preserving visibility in bugs_by_priority.plugins/edge-scrum/skills/release-planning/SKILL.md (1)
345-355: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFold "Edge Cases" into their originating steps instead of a standalone section.
Several entries here duplicate rules already stated inline at the step that owns them (e.g., "No Features found" restates Phase 2c's stop condition at line 186; "Version format varies" restates the JQL already shown in Phase 4b at line 266). Based on learnings, edge-case/failure rules should be co-located inline with the step that needs them rather than split into a separate top-level section, since the agent reads steps linearly and won't reliably re-consult a reference section mid-execution — this creates two sources of truth without improving compliance.
🤖 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/edge-scrum/skills/release-planning/SKILL.md` around lines 345 - 355, Remove the standalone “Edge Cases” section from SKILL.md and fold each rule into the step it belongs to, so the guidance lives inline with the relevant phase instead of creating a second source of truth. Update the sections that own the behavior, such as the Phase 2c feature discovery flow, the data-quality checks for epics/stories, the Phase 4b bug JQL handling, and the sprint-data fallback path, using those step names as anchors. Keep only the canonical instructions where the action is executed, and avoid repeating the same failure/edge-case rule in a separate top-level reference block.Source: Learnings
plugins/edge-scrum/bin/tests/test_md_to_docx.py (1)
75-78: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUnused
headerunpacked variable (Ruff RUF059).As per coding guidelines, "Python code must pass ruff validation." This line fails that check.
🔧 Proposed fix
- header, rows = parse_table(lines) + _header, rows = parse_table(lines) assert len(rows) == 1🤖 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/edge-scrum/bin/tests/test_md_to_docx.py` around lines 75 - 78, The test_separator_row_filtered test is unpacking an unused header value from parse_table, which triggers Ruff RUF059. Update the unpacking in that test to avoid binding the unused variable, using a throwaway name or otherwise restructuring the assertion so only the needed rows value is captured. Keep the change localized to test_separator_row_filtered and parse_table usage.Sources: Coding guidelines, Linters/SAST tools
plugins/edge-scrum/skills/release-planning-analysis/SKILL.md (1)
125-125: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded "3-week sprints" magic number.
weeks_latebakes in a literal 3-week sprint length. Sprint duration is Laws-owned data (09-sprint-policies.mdis already loaded in Step 1). Hardcoding it here risks silent drift if sprint length ever changes, and duplicates a value that should come from the loaded Laws context.♻️ Proposed fix
-7. `weeks_late` = (`sprints_needed` - `{REMAINING_SPRINT_COUNT}`) × 3 (3-week sprints) +7. `weeks_late` = (`sprints_needed` - `{REMAINING_SPRINT_COUNT}`) × `sprint_length_weeks` (from `09-sprint-policies.md`, loaded in Step 1)🤖 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/edge-scrum/skills/release-planning-analysis/SKILL.md` at line 125, The `weeks_late` calculation in the release-planning analysis still hardcodes a 3-week sprint length, so update that logic to read the sprint duration from the loaded Laws context instead of multiplying by a literal. Use the sprint policy data already loaded in Step 1 (from `09-sprint-policies.md`) and thread that value into the `weeks_late` computation alongside `sprints_needed` and `REMAINING_SPRINT_COUNT`, so the formula stays aligned with the configured sprint length.
🤖 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 @.claude/skills/edge-scrum-release-health:
- Line 1: The `.claude/skills/edge-scrum-release-health` entry is a symlinked
duplicate source of truth; remove the symlink-based skill entry and move or
reference the skill from the canonical plugin location under
`plugins/edge-scrum/skills/release-health` instead. Update the skill
registration/lookup so `edge-scrum-release-health` is served as a plugin skill,
and ensure there is only one maintained copy of the skill definition.
In @.claude/skills/edge-scrum-release-planning:
- Line 1: The edge-scrum release-planning entry under .claude/skills is a
symlink, which creates a duplicated source of truth. Replace the symlinked skill
with a real plugin-backed skill by moving or defining the release-planning
content under the existing plugins/edge-scrum/skills/release-planning location
and updating any references so the skill is loaded from the plugin tree rather
than .claude/skills.
In `@plugins/edge-scrum/bin/md-to-docx.py`:
- Around line 145-161: The hyperlink text in add_hyperlink is being inserted
directly into the OOXML <w:t> element without escaping, which can break
parse_xml when markdown link labels contain special characters. Update
add_hyperlink to XML-escape the text before constructing the run element, and
keep the fix localized to this helper since parse_inline feeds link segments
into it.
In `@plugins/edge-scrum/bin/tests/test_transform_bugs.py`:
- Around line 61-91: Add tests in TestTransformBug to cover the default fallback
paths in transform_bug: create one case where the raw bug omits priority so the
transform uses its default value, and another where updated/created are missing
or null so the stale-date fallback chain is exercised. Use transform_bug and the
existing _make_raw helper in test_transform_bugs.py to assert the fallback
behavior directly.
In `@plugins/edge-scrum/bin/transform-bugs.py`:
- Around line 8-17: The import setup in transform-bugs.py triggers Ruff E402
because sys.path.insert(0, os.path.dirname(__file__)) precedes the
_jira_transforms import. Update the module-level import block to keep it
Ruff-clean by either adding an explicit E402 noqa to the affected import
statement or replacing the absolute import with a package-relative import. Use
the sys.path.insert and from _jira_transforms import block as the target
location.
In `@plugins/edge-scrum/skills/release-planning-analysis/SKILL.md`:
- Around line 1-6: The SKILL.md metadata for release-planning-analysis is
missing the required disable-model-invocation flag for a non-user-invocable
skill that still has side-effecting tools. Update the frontmatter in
release-planning-analysis to add disable-model-invocation: true alongside
user-invocable: false, keeping the allowed-tools list unchanged so this skill
can only run through the orchestrator’s controlled flow.
---
Nitpick comments:
In `@plugins/edge-scrum/bin/tests/test_md_to_docx.py`:
- Around line 75-78: The test_separator_row_filtered test is unpacking an unused
header value from parse_table, which triggers Ruff RUF059. Update the unpacking
in that test to avoid binding the unused variable, using a throwaway name or
otherwise restructuring the assertion so only the needed rows value is captured.
Keep the change localized to test_separator_row_filtered and parse_table usage.
In `@plugins/edge-scrum/bin/transform-bugs.py`:
- Line 42: The default fallback in transform-bugs.py currently maps missing
priority values to "Major", which hides missing data in priority aggregates.
Update the logic in the priority assignment near get_nested(raw, "priority",
"name") so that absent or empty priority values default to "Unknown" instead,
matching the existing handling used for missing component data and preserving
visibility in bugs_by_priority.
In `@plugins/edge-scrum/skills/release-planning-analysis/SKILL.md`:
- Line 125: The `weeks_late` calculation in the release-planning analysis still
hardcodes a 3-week sprint length, so update that logic to read the sprint
duration from the loaded Laws context instead of multiplying by a literal. Use
the sprint policy data already loaded in Step 1 (from `09-sprint-policies.md`)
and thread that value into the `weeks_late` computation alongside
`sprints_needed` and `REMAINING_SPRINT_COUNT`, so the formula stays aligned with
the configured sprint length.
In `@plugins/edge-scrum/skills/release-planning/SKILL.md`:
- Around line 345-355: Remove the standalone “Edge Cases” section from SKILL.md
and fold each rule into the step it belongs to, so the guidance lives inline
with the relevant phase instead of creating a second source of truth. Update the
sections that own the behavior, such as the Phase 2c feature discovery flow, the
data-quality checks for epics/stories, the Phase 4b bug JQL handling, and the
sprint-data fallback path, using those step names as anchors. Keep only the
canonical instructions where the action is executed, and avoid repeating the
same failure/edge-case rule in a separate top-level reference block.
🪄 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: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 4b642985-c122-4daf-ae9e-521a46dc12be
📒 Files selected for processing (15)
.claude-plugin/marketplace.json.claude/skills/edge-scrum-release-health.claude/skills/edge-scrum-release-planningplugins/edge-scrum/.claude-plugin/plugin.jsonplugins/edge-scrum/README.mdplugins/edge-scrum/bin/md-to-docx.pyplugins/edge-scrum/bin/tests/test_md_to_docx.pyplugins/edge-scrum/bin/tests/test_transform_bugs.pyplugins/edge-scrum/bin/tests/test_transform_stories.pyplugins/edge-scrum/bin/transform-bugs.pyplugins/edge-scrum/bin/transform-stories.pyplugins/edge-scrum/references/Edge-Scrum-Laws.mdplugins/edge-scrum/references/release-planning-report-template.mdplugins/edge-scrum/skills/release-planning-analysis/SKILL.mdplugins/edge-scrum/skills/release-planning/SKILL.md
coderabbitai
Bot
added
the
ready-for-human-review
Contributor
Author
|
@coderabbitai perform full review |
Contributor
|
✅ Action performedFull review finished. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
plugins/edge-scrum/skills/release-planning-analysis/SKILL.md (1)
62-77: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove the edge-case rules next to the step that uses them.
unknown_contributorsandFAILhandling are buried in a later top-level section. Based on learnings, failure policies and edge-case rules should live inline with the relevant step so the agent reads them in execution order.🤖 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/edge-scrum/skills/release-planning-analysis/SKILL.md` around lines 62 - 77, The edge-case and failure guidance is separated from the step-by-step writing flow, which makes the instructions harder to follow in execution order. Move the `unknown_contributors` and `FAIL` handling guidance out of the later top-level notes and place it directly alongside the relevant recommendation step in the “Writing Guidelines” section so the agent sees these rules inline when deciding what to write. Keep the existing wording intent, but reorganize the guidance around the specific checks and recommendation behavior rather than as detached global notes.Sources: Path instructions, Learnings
plugins/edge-scrum/skills/release-planning/SKILL.md (1)
361-374: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInline the edge cases with the relevant phase.
This separate top-level section makes the stop conditions easy to miss during execution. Based on learnings,
SKILL.mdworkflows should keep failure policies and edge-case rules immediately next to the step they apply to, not in a standalone "Edge Cases" section.Based on learnings, "For any Claude/agent workflow skill documentation file named SKILL.md, co-locate failure policies and edge-case rules inline with the specific step that needs them ... Do not move these rules to a separate top-level 'Edge Cases' (or similar) section."
🤖 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/edge-scrum/skills/release-planning/SKILL.md` around lines 361 - 374, Move the edge-case rules out of the standalone “Edge Cases” section and inline them next to the relevant workflow phases in SKILL.md. Update the sections that define the execution steps so each stop condition, fallback, and data-quality rule appears with the phase it applies to (for example, Phase 2b, Phase 3, Phase 4b), keeping the policy close to the codepath it governs. Use the existing phase headings and rule names to relocate each bullet so reviewers can find failure handling immediately where it is used.Source: Learnings
plugins/edge-scrum/bin/assemble-report.py (1)
169-178: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winComposite risk table columns for assignment/sizing/bugs never get risk shading.
RISK_COLORSonly covers HIGH/MEDIUM/LOW/OK/PASS/WARN/FAIL/OVER, butsignal_detailsvalues for assignment ("SPOF", "SPOF+Unassigned", "Unassigned") and sizing ("Mismatch") aren't in that map, so those columns render unstyled even when they represent real risk — inconsistent with the "risk-aware formatting" goal for this layer.♻️ Proposed fix
RISK_COLORS = { "HIGH": (HIGH_RED_BG, HIGH_RED_TEXT), "OVER": (HIGH_RED_BG, HIGH_RED_TEXT), "FAIL": (HIGH_RED_BG, HIGH_RED_TEXT), + "SPOF": (HIGH_RED_BG, HIGH_RED_TEXT), + "SPOF+UNASSIGNED": (HIGH_RED_BG, HIGH_RED_TEXT), + "UNASSIGNED": (MED_YELLOW_BG, MED_YELLOW_TEXT), + "MISMATCH": (MED_YELLOW_BG, MED_YELLOW_TEXT), "MEDIUM": (MED_YELLOW_BG, MED_YELLOW_TEXT),Also applies to: 310-315
🤖 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/edge-scrum/bin/assemble-report.py` around lines 169 - 178, The risk shading map in `assemble-report.py` is missing several `signal_details` values used by composite risk columns, so assignment, sizing, and bug-related cells can render without color. Update `RISK_COLORS` and the formatting path that consumes it so `signal_details` entries like SPOF, SPOF+Unassigned, Unassigned, and Mismatch resolve to the appropriate risk colors, keeping the logic consistent with the existing risk-aware formatting in the report assembly flow.
🤖 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/edge-scrum/bin/assemble-report.py`:
- Around line 45-57: The remaining_sprint_count entry in assemble-report.py is
using a truthy and expression instead of the actual sprint count, so it can
incorrectly become 0 when assessed_features is 0. Update the header_map
construction in the report assembly logic to set remaining_sprint_count directly
from params.get("remaining_sprints", "?"), and keep the surrounding keys in the
same header_map block unchanged.
In `@plugins/edge-scrum/bin/run-checks.py`:
- Line 475: The `--spikes` CLI argument in `main()` is currently required but
never used, so either remove it from the parser in `run-checks.py` or wire
`args.spikes` through the relevant planning/check flow so it actually affects
behavior. Locate the `parser.add_argument("--spikes", required=True)` entry and
ensure any corresponding spike handling is either passed into the check
functions or deleted cleanly from the interface.
- Around line 392-394: The bug signal is collected in the bug_features loop but
never influences the final score, so unassigned blocker/critical bugs are
effectively ignored. Update the risk aggregation in run-checks.py so the
unassigned_blocker_critical results affect signal_details["bugs"], signal_count,
and composite_risk, using the existing bug_features collection or replacing it
if unnecessary. If bugs are not meant to contribute, remove the dead collection
instead, and add a test covering a non-empty unassigned_blocker_critical result.
- Around line 143-151: The global SP calculation in run_timeline_check is still
counting work from FAIL-gated features, which skews velocity for the PASS/WARN
timeline. Update the feature/story aggregation loop that builds global_sp so it
only includes stories from features that are actually eligible for the timeline
output (PASS/WARN), and keep the same filtering logic used by the output path.
Use the existing run_timeline_check flow and the per-feature iteration over
features/passed_keys to scope the SP totals consistently.
- Around line 42-49: The fallback in the component filtering logic is too broad
because the USHIFT story check in the run-checks.py matching block applies to
every component. Update the has_match logic in the component_filter handling so
the USHIFT- story fallback only applies when the requested component is
MicroShift, while other component values rely solely on their label match. Use
the existing component_filter branch and the epic/story scan in the
run-checks.py script to keep the change localized.
In `@plugins/edge-scrum/bin/tests/test_run_checks.py`:
- Around line 10-19: Add missing test coverage for build_hierarchy,
run_bug_load_check, and run_sizing_check in test_run_checks.py by creating
dedicated test classes/functions with both positive and negative cases. Cover
the correctness paths that were previously buggy: component-filter behavior in
build_hierarchy, gate filtering in run_sizing_check, and ensuring bug SP totals
are not leaked into sizing calculations. Use the existing run-checks module
imports to locate the targets and validate expected outputs and edge cases.
---
Nitpick comments:
In `@plugins/edge-scrum/bin/assemble-report.py`:
- Around line 169-178: The risk shading map in `assemble-report.py` is missing
several `signal_details` values used by composite risk columns, so assignment,
sizing, and bug-related cells can render without color. Update `RISK_COLORS` and
the formatting path that consumes it so `signal_details` entries like SPOF,
SPOF+Unassigned, Unassigned, and Mismatch resolve to the appropriate risk
colors, keeping the logic consistent with the existing risk-aware formatting in
the report assembly flow.
In `@plugins/edge-scrum/skills/release-planning-analysis/SKILL.md`:
- Around line 62-77: The edge-case and failure guidance is separated from the
step-by-step writing flow, which makes the instructions harder to follow in
execution order. Move the `unknown_contributors` and `FAIL` handling guidance
out of the later top-level notes and place it directly alongside the relevant
recommendation step in the “Writing Guidelines” section so the agent sees these
rules inline when deciding what to write. Keep the existing wording intent, but
reorganize the guidance around the specific checks and recommendation behavior
rather than as detached global notes.
In `@plugins/edge-scrum/skills/release-planning/SKILL.md`:
- Around line 361-374: Move the edge-case rules out of the standalone “Edge
Cases” section and inline them next to the relevant workflow phases in SKILL.md.
Update the sections that define the execution steps so each stop condition,
fallback, and data-quality rule appears with the phase it applies to (for
example, Phase 2b, Phase 3, Phase 4b), keeping the policy close to the codepath
it governs. Use the existing phase headings and rule names to relocate each
bullet so reviewers can find failure handling immediately where it is used.
🪄 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: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: f094bfc9-de61-4e2c-8fb0-1f103984ab0f
📒 Files selected for processing (5)
plugins/edge-scrum/bin/assemble-report.pyplugins/edge-scrum/bin/run-checks.pyplugins/edge-scrum/bin/tests/test_run_checks.pyplugins/edge-scrum/skills/release-planning-analysis/SKILL.mdplugins/edge-scrum/skills/release-planning/SKILL.md
coderabbitai
Bot
removed
the
ready-for-human-review
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
plugins/edge-scrum/bin/tests/test_transform_stories.py (1)
146-151: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
test_ocpbugs_verified_is_donedoesn't actually test the OCPBUGS/Verified done-status logic.The story is given an assignee, so it can never land in
unassigned_storiesregardless of status/done-set logic — the assertion is vacuously true. Sincesp=0, checkingdone_sp_by_epic/remaining_sp_by_epicwouldn't help either. To actually validate thatDONE_STATUSES_OCPBUGStreats"Verified"as done, use a non-zero-SP OCPBUGS story and assert ondone_sp_by_epic/remaining_sp_by_epic.✅ Suggested fix
- def test_ocpbugs_verified_is_done(self): - stories = [ - self._story("OCPBUGS-1", assignee="alice@redhat.com", sp=0, status="Verified", issue_type="Bug"), - ] - agg = compute_aggregates(stories) - assert "OCPBUGS-1" not in agg.get("unassigned_stories", []) + def test_ocpbugs_verified_is_done(self): + stories = [ + self._story("OCPBUGS-1", sp=5, epic="E-1", status="Verified"), + ] + agg = compute_aggregates(stories) + assert agg["done_sp_by_epic"]["E-1"] == 5 + assert agg["remaining_sp_by_epic"]["E-1"] == 0🤖 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/edge-scrum/bin/tests/test_transform_stories.py` around lines 146 - 151, The test test_ocpbugs_verified_is_done is vacuous because the story is assigned and has zero story points, so it never exercises DONE_STATUSES_OCPBUGS or the done-status aggregation logic. Update this test in compute_aggregates/_story usage to create an unassigned OCPBUGS story with non-zero sp and assert against done_sp_by_epic and remaining_sp_by_epic so “Verified” is actually validated as a done status.plugins/edge-scrum/bin/tests/test_md_to_docx.py (1)
1-153: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a regression test for hyperlink XML-escaping.
add_hyperlinkinmd-to-docx.pywas previously fixed to XML-escape link text (special chars like&,<,>) to avoid crashingparse_xml(). This test file coversparse_md_lines/parse_table/parse_inline/get_risk_levelbut nothing exercisesadd_hyperlinkitself, so a regression here would go undetected.✅ Suggested regression test
from docx import Document add_hyperlink = _mod.add_hyperlink class TestAddHyperlink(unittest.TestCase): def test_special_chars_escaped(self): doc = Document() p = doc.add_paragraph() # Should not raise even with XML-special characters in the label add_hyperlink(p, "https://example.com", "A & B <report>")🤖 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/edge-scrum/bin/tests/test_md_to_docx.py` around lines 1 - 153, Add a regression test for the add_hyperlink helper in md-to-docx.py to cover XML-escaping of hyperlink text. Import add_hyperlink (and Document from docx) in test_md_to_docx.py, add a focused test that creates a paragraph and calls add_hyperlink with link text containing special XML characters like &, <, and >, and assert it does not raise. This keeps the existing parse_md_lines/parse_table/parse_inline/get_risk_level coverage intact while directly exercising the previously fixed hyperlink path.
🤖 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/edge-scrum/bin/run-checks.py`:
- Around line 329-373: In run_sizing_check, use gate_map to skip any feature
whose data-quality gate failed before computing sizing results, so incomplete
FAIL-gated features are not assessed. Also align total_sp with the same scope
rules used elsewhere by excluding Bug stories when summing story points, and
keep the logic inside run_sizing_check consistent with the existing
is_story_done filtering and feature key lookups.
In `@plugins/edge-scrum/bin/tests/test_transform_stories.py`:
- Around line 102-105: Add a negative-path test for transform_story covering the
priority fallback: create a raw story without a priority value or with a falsy
nested priority.name, then assert the result uses the default "Major" from the
get_nested(raw, "priority", "name") or "Major" logic. Keep the existing
test_priority_extracted positive case and add a companion test in
test_transform_stories.py to verify the fallback behavior explicitly.
In `@plugins/edge-scrum/bin/transform-bugs.py`:
- Around line 91-98: The exception handling in the raw_issues processing loop is
too broad and can mask real failures in transform_bug, so narrow the catch in
transform-bugs.py to only the expected per-item parsing/validation errors.
Update the loop around transform_bug(raw, args.today) to let unexpected
exceptions surface, while still recording skipped_issues and printing a warning
for the known recoverable case; keep the existing raw.get("key", "unknown") and
skipped_issues bookkeeping in that branch.
In `@plugins/edge-scrum/skills/release-planning/SKILL.md`:
- Around line 124-130: The work directory setup currently uses a date-based path
that can collide across reruns or parallel sessions. Update the WORKDIR creation
step to use a truly unique temporary directory for each run, and keep the
resulting WORKDIR value recorded for reuse in the later agent prompts and
cleanup steps. Locate the shell snippet in the release-planning instructions
that initializes WORKDIR and replace the date-derived naming approach there.
- Around line 327-333: The Phase 5b sub-agent setup in release-planning
currently reads another skill’s SKILL.md at runtime, which leaks peer
instructions and triggers the AS3 skill-enumeration warning. Update the logic
around the Phase 5b recommendation step to use a dedicated agent definition or
inline prompt instead of loading release-planning-analysis/SKILL.md, while
keeping the existing checks.json input, recommendations.json output, and
executive_summary verification behavior intact.
---
Nitpick comments:
In `@plugins/edge-scrum/bin/tests/test_md_to_docx.py`:
- Around line 1-153: Add a regression test for the add_hyperlink helper in
md-to-docx.py to cover XML-escaping of hyperlink text. Import add_hyperlink (and
Document from docx) in test_md_to_docx.py, add a focused test that creates a
paragraph and calls add_hyperlink with link text containing special XML
characters like &, <, and >, and assert it does not raise. This keeps the
existing parse_md_lines/parse_table/parse_inline/get_risk_level coverage intact
while directly exercising the previously fixed hyperlink path.
In `@plugins/edge-scrum/bin/tests/test_transform_stories.py`:
- Around line 146-151: The test test_ocpbugs_verified_is_done is vacuous because
the story is assigned and has zero story points, so it never exercises
DONE_STATUSES_OCPBUGS or the done-status aggregation logic. Update this test in
compute_aggregates/_story usage to create an unassigned OCPBUGS story with
non-zero sp and assert against done_sp_by_epic and remaining_sp_by_epic so
“Verified” is actually validated as a done status.
🪄 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: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: b2d95d06-bc85-4611-9873-669f3f3bded5
📒 Files selected for processing (19)
.claude-plugin/marketplace.json.claude/skills/edge-scrum-release-health.claude/skills/edge-scrum-release-planningplugins/edge-scrum/.claude-plugin/plugin.jsonplugins/edge-scrum/README.mdplugins/edge-scrum/bin/_jira_transforms.pyplugins/edge-scrum/bin/assemble-report.pyplugins/edge-scrum/bin/md-to-docx.pyplugins/edge-scrum/bin/run-checks.pyplugins/edge-scrum/bin/tests/test_md_to_docx.pyplugins/edge-scrum/bin/tests/test_run_checks.pyplugins/edge-scrum/bin/tests/test_transform_bugs.pyplugins/edge-scrum/bin/tests/test_transform_stories.pyplugins/edge-scrum/bin/transform-bugs.pyplugins/edge-scrum/bin/transform-stories.pyplugins/edge-scrum/references/Edge-Scrum-Laws.mdplugins/edge-scrum/references/release-planning-report-template.mdplugins/edge-scrum/skills/release-planning-analysis/SKILL.mdplugins/edge-scrum/skills/release-planning/SKILL.md
coderabbitai
Bot
added
the
ready-for-human-review
Contributor
Author
|
@coderabbitai resolve |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
plugins/edge-scrum/bin/run-checks.py (1)
395-412: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the unused
bug_load_resultsparameterplugins/edge-scrum/bin/run-checks.py:395-412—run_composite_check()recomputes the bug signal fromf["all_stories"], sobug_load_resultsnever affects composite scoring. Either wire it into the bug check or drop it from the signature/call to avoid the misleading dependency.🤖 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/edge-scrum/bin/run-checks.py` around lines 395 - 412, The run_composite_check() helper currently accepts bug_load_results but never uses it because the bug signal is recomputed from each feature’s all_stories, so the parameter is misleading. Update run_composite_check() to either consume bug_load_results as the source of truth for the bug-related signal or remove it from the function signature and all call sites, keeping the composite scoring logic aligned with the actual inputs used.
🤖 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/edge-scrum/bin/run-checks.py`:
- Line 57: Rename the ambiguous loop variable in the labels normalization logic
to satisfy Ruff E741. Update the list comprehension in run-checks.py where
labels are lowercased so the variable currently named l uses a clearer name, and
keep the behavior of labels_lower unchanged.
In `@plugins/edge-scrum/bin/tests/test_run_checks.py`:
- Line 284: The unpacked variable in test_run_checks.py from build_hierarchy is
unused, triggering Ruff RUF059. Update the assignment at the build_hierarchy
call so the second returned value is clearly marked as intentionally unused by
prefixing its name with an underscore, while keeping the result handling in
test_run_checks unchanged.
---
Nitpick comments:
In `@plugins/edge-scrum/bin/run-checks.py`:
- Around line 395-412: The run_composite_check() helper currently accepts
bug_load_results but never uses it because the bug signal is recomputed from
each feature’s all_stories, so the parameter is misleading. Update
run_composite_check() to either consume bug_load_results as the source of truth
for the bug-related signal or remove it from the function signature and all call
sites, keeping the composite scoring logic aligned with the actual inputs used.
🪄 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: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: fd8c3737-41ec-45de-a9e0-b2a35b50e45c
📒 Files selected for processing (5)
plugins/edge-scrum/bin/assemble-report.pyplugins/edge-scrum/bin/run-checks.pyplugins/edge-scrum/bin/tests/test_run_checks.pyplugins/edge-scrum/skills/release-planning-analysis/SKILL.mdplugins/edge-scrum/skills/release-planning/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (2)
- plugins/edge-scrum/skills/release-planning-analysis/SKILL.md
- plugins/edge-scrum/bin/assemble-report.py
Contributor
✅ Action performedComments resolved and changes approved. |
Contributor
|
/hold so I can review and run these changes locally |
openshift-ci
Bot
added
the
do-not-merge/hold
jeff-roche
suggested changes
Jul 9, 2026
| TNF: "Two Node Fencing" | ||
| LVMS: "Logical Volume Manager Storage" | ||
| topolvm: "Logical Volume Manager Storage" | ||
| MicroShift: "Installer / Single Node OpenShift" |
Contributor
There was a problem hiding this comment.
The OCPBUGS components for microshift are MicroShift MicroShift / Networking and MicroShift / Storage
Contributor
Author
There was a problem hiding this comment.
Fixed in 4dd6d45. Updated the component mapping, Phase 4b JQL, TEAM_COMPONENTS, and COMPONENT_SHORT_NAMES to use the correct MicroShift OCPBUGS components (MicroShift, MicroShift / Networking, MicroShift / Storage). SNO stays as Installer / Single Node OpenShift.
|
|
||
| 1. **Release version** — e.g., `5.0` | ||
| 2. **Sprint range** — first sprint number through last | ||
| 3. **Branch cut sprint** — which sprint is the last dev sprint |
Contributor
There was a problem hiding this comment.
Something I've been missing in my skills: Branch Cut vs Pencil's Down. We should account for both in these skills since they have their own unique meaning in the release cycle
Contributor
Author
There was a problem hiding this comment.
Fixed in 4dd6d45. Added pd:<sprint> (pencils-down) as an optional argument. Feature timeline risk is now measured against pencils down — the deadline for feature code to be merged. Branch cut remains for when the release branch is created. TOTAL_DEV_SPRINTS is computed against pencils down, not branch cut. Defaults to branch cut if not provided (backwards compatible).
…y (KR 4.2) New Claude Code skill that assesses whether the team can deliver planned scope within remaining time. Runs a data-quality gate followed by 6 deterministic planning risk checks (capacity, timeline, assignment, bug load, sizing, composite progress), then delegates narrative recommendations to an LLM sub-agent. Outputs both markdown and DOCX reports. Architecture: all arithmetic is deterministic Python — the LLM only writes narrative recommendations around pre-computed numbers. MCP fetch → transforms → run-checks.py → sub-agent → assemble-report.py Key design decisions: - Deterministic checks in run-checks.py (testable, reproducible) - parent field (not Epic Link) for child-only issue fetching - Proportional velocity across features - DOCX rendered from structured data (not parsed from markdown) - Per-type done states matching Laws - Pencils down vs branch cut milestones (pd:<sprint>) - MicroShift OCPBUGS components with prefix matching - SPOF + Unassigned merged as one composite signal Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Neilhamza
force-pushed
the
OCPEDGE-2781/release-planning-skill
branch
from
July 12, 2026 10:32
fd1995e to
3506fea
Compare
jeff-roche
approved these changes
Jul 13, 2026
openshift-ci
Bot
added
lgtm
Contributor
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jeff-roche, Neilhamza The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Contributor
Author
|
/unhold |
openshift-ci
Bot
removed
the
do-not-merge/hold
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
Implements OCPEDGE-2781 — a new
/release-planningClaude Code skill that assesses whether the team can deliver planned scope within remaining time. This is OKR Key Result 4.2: Planning Risk Visibility.The skill pulls live Jira data, runs a data-quality gate followed by 6 planning risk checks deterministically in Python, then delegates narrative recommendations to an LLM sub-agent. Outputs both markdown and DOCX reports.
Architecture
All arithmetic is deterministic Python — the LLM only writes narrative:
run-checks.py— reads 6 JSON files + roster, outputschecks.jsonwith all computed valueschecks.json, writesrecommendations.json(narrative only, no arithmetic)assemble-report.py— renders tables from structured data, adds Jira links, produces both.mdand.docxWhat it checks
Data Quality Gate — validates story-level breakdown before running checks. Features without stories are flagged explicitly and excluded from numeric projections.
6 Checks:
How to use
Output:
.reports/release_planning_{version}_{date}.mdand.reports/release_planning_{version}_{date}.docxKey design decisions
pd:<sprint>argument. Feature timeline risk is measured against pencils down (when feature code must be merged), not branch cut (when the release branch is created). Defaults to branch cut if not provided.MicroShift,MicroShift / Networking,MicroShift / Storage(notInstaller / Single Node OpenShift, which is SNO only).run-checks.pyhandles all arithmetic. The LLM sub-agent only interprets pre-computed numbers.parentfield (not Epic Link) — fetches only direct child issueschecks.json, not by parsing markdownClosed. OCPBUGS:Closed+Verified. Matches the Laws.Files added (all under
plugins/edge-scrum/)skills/release-planning/SKILL.mdskills/release-planning-analysis/SKILL.mdbin/run-checks.pybin/assemble-report.pybin/transform-stories.pybin/transform-bugs.pybin/md-to-docx.pyreferences/release-planning-report-template.mdKnown deferrals
parent) fall through both queries — edge case, deferred.Prerequisites
pip3 install python-docxTest plan
/release-planning 5.0 287-292 bc:292--component TNA🤖 Generated with Claude Code
Summary by CodeRabbit
release-planning/release-planning-analysisskills with persisted check artifacts and optional component filtering.release-planningusage, plus new report template and laws entries.