feat(docs-tools): add commit-driven documentation workflow#107
Open
feat(docs-tools): add commit-driven documentation workflow#107
Conversation
Add commit-analyst skill and commit-driven pipeline for analyzing commits, PRs, and MRs for documentation impact. Supports standalone reports (/commit-analyst) and full pipeline mode (/docs-orchestrator --commit <url>). New files: - commit-analyst skill (SKILL.md, gather_changes.py, impact-criteria.md) - docs-workflow-commit-analysis step skill - commit-driven-workflow.yaml (9-step pipeline) Modified files: - docs-orchestrator: --commit flag, identifier derivation, auto-select workflow - docs-workflow-start: "Analyze a commit/PR" interactive option - step-result-schema: commit-analysis sidecar schema - plugin.json: version bump 0.0.47 → 0.0.48 Co-authored by Claude Code, under the supervision of Alex McLeod
WalkthroughAdds a commit-driven documentation workflow: new commit-analyst skill (spec, impact criteria, and gather_changes implementation), docs-orchestrator wiring and commit-driven workflow, a workflow step to invoke/parse commit analysis, interactive launcher updates, and a plugin manifest version bump. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Start as docs-workflow-start
participant Orchestrator as docs-orchestrator
participant Analysis as commit-analyst
participant Planning as planning
participant Evidence as code-evidence
participant Writing as writing
participant ReviewTech as technical-review
participant ReviewStyle as style-review
participant MR as create-mr
rect rgba(220,240,255,0.5)
User->>Start: provide commit/PR/MR URL + optional JIRA
Start->>Orchestrator: invoke with --commit <url>
end
rect rgba(200,255,220,0.5)
Orchestrator->>Analysis: run commit-analysis (gather_changes.py)
Analysis->>Analysis: extract changes, detect signals, grade impact
Analysis-->>Orchestrator: emit requirements.md + step-result.json
end
rect rgba(255,240,220,0.5)
Orchestrator->>Planning: run planning using requirements
alt has_source_repo
Orchestrator->>Evidence: fetch code-evidence
Evidence-->>Orchestrator: code samples
end
Orchestrator->>Writing: generate documentation
Writing-->>Orchestrator: draft content
Orchestrator->>ReviewTech: technical review
ReviewTech-->>Orchestrator: feedback
Orchestrator->>ReviewStyle: style review
ReviewStyle-->>Orchestrator: feedback
Orchestrator->>MR: commit and create MR
MR-->>User: MR created
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 9 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (9 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Comment |
Add direct GitLab REST API extraction for commit URLs and merge request URLs, removing the dependency on glab CLI. Supports GITLAB_TOKEN env var for private repos, falls back to unauthenticated access for public ones. Co-authored by Claude Code, under the supervision of Alex McLeod
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
plugins/docs-tools/skills/commit-analyst/scripts/gather_changes.py (2)
159-161: Ambiguous variable namel.Per PEP 8 (flagged by Ruff E741), single-letter
lis ambiguous because it resembles1. Use a descriptive name.♻️ Suggested fix
metadata["labels"] = [ - l.get("name", "") for l in data.get("labels", []) + label.get("name", "") for label in data.get("labels", []) ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/docs-tools/skills/commit-analyst/scripts/gather_changes.py` around lines 159 - 161, The list comprehension that builds metadata["labels"] uses an ambiguous single-letter variable name "l"; change it to a descriptive name like "label" (or "lbl") in the expression that iterates over data.get("labels", []) so the comprehension becomes [label.get("name", "") for label in data.get("labels", [])], updating any reference to the loop variable in that statement to the new name to satisfy PEP8/Ruff E741 without changing the logic that extracts "name" from each label dict.
236-237: Consider unpacking instead of concatenation.Ruff RUF005 suggests using
[*list(...), "source_code"]for clarity.♻️ Optional improvement
- for name in list(CATEGORIES.keys()) + ["source_code"]: + for name in [*CATEGORIES.keys(), "source_code"]:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/docs-tools/skills/commit-analyst/scripts/gather_changes.py` around lines 236 - 237, Replace the list concatenation used to iterate categories with iterable unpacking for clarity: in the loop that currently reads the category iteration (the for name in list(CATEGORIES.keys()) + ["source_code"]: block that initializes cats[name] = {"files": [], "total_added": 0, "total_removed": 0}), change the iteration to use unpacking (e.g., [*CATEGORIES.keys(), "source_code"] or [*list(CATEGORIES.keys()), "source_code"]) so the combined sequence is created more clearly.plugins/docs-tools/skills/docs-workflow-commit-analysis/SKILL.md (1)
26-28: Add language identifiers to fenced code blocks.The markdownlint tool flagged missing language identifiers on code blocks at lines 26, 50, and 61. While the content is clear, adding language identifiers improves syntax highlighting and accessibility.
♻️ Suggested improvements
Line 26: Add
textor leave empty (it's a path)-``` +```text <base-path>/requirements/requirements.mdLine 50: Add `text` (it's a command template) ```diff -``` +```text --commit <COMMIT_URL> --base-path <BASE_PATH>Line 61: Add `text` (it's a skill invocation example) ```diff -``` +```text Skill: docs-tools:commit-analyst, args: "<constructed args>"</details> Also applies to: 50-52, 61-63 <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@plugins/docs-tools/skills/docs-workflow-commit-analysis/SKILL.mdaround
lines 26 - 28, The fenced code blocks in SKILL.md are missing language
identifiers (markdownlint warning); update the three code fences containing
"/requirements/requirements.md", the command template starting with
"--commit <COMMIT_URL> --base-path <BASE_PATH>", and the skill invocation line
"Skill: docs-tools:commit-analyst, args: """ to include a
language tag (use "text") on each opening triple-backtick so they becometext ...for proper syntax highlighting and accessibility.</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/docs-tools/skills/commit-analyst/SKILL.md:
- Around line 44-49: Update the documented command blocks in SKILL.md to include
Cursor-compatible (workspace-root relative) variants alongside the existing
Claude environment variables: add lines showing commands like "python3
plugins/docs-tools/skills/commit-analyst/scripts/gather_changes.py ..." next to
the Claude form using ${CLAUDE_SKILL_DIR} or ${CLAUDE_PLUGIN_ROOT}; do the same
for the jira-reader commands (jira_reader.py --issue and --graph) and the
article-extractor command (article_extractor.py --url), and apply identical
changes to the other affected blocks mentioned (around lines 109-117 and
136-139) so every script call has both the Claude variable-based path and a
repo-root relative "plugins/..." path for Cursor.
Nitpick comments:
In@plugins/docs-tools/skills/commit-analyst/scripts/gather_changes.py:
- Around line 159-161: The list comprehension that builds metadata["labels"]
uses an ambiguous single-letter variable name "l"; change it to a descriptive
name like "label" (or "lbl") in the expression that iterates over
data.get("labels", []) so the comprehension becomes [label.get("name", "") for
label in data.get("labels", [])], updating any reference to the loop variable in
that statement to the new name to satisfy PEP8/Ruff E741 without changing the
logic that extracts "name" from each label dict.- Around line 236-237: Replace the list concatenation used to iterate categories
with iterable unpacking for clarity: in the loop that currently reads the
category iteration (the for name in list(CATEGORIES.keys()) + ["source_code"]:
block that initializes cats[name] = {"files": [], "total_added": 0,
"total_removed": 0}), change the iteration to use unpacking (e.g.,
[*CATEGORIES.keys(), "source_code"] or [*list(CATEGORIES.keys()),
"source_code"]) so the combined sequence is created more clearly.In
@plugins/docs-tools/skills/docs-workflow-commit-analysis/SKILL.md:
- Around line 26-28: The fenced code blocks in SKILL.md are missing language
identifiers (markdownlint warning); update the three code fences containing
"/requirements/requirements.md", the command template starting with
"--commit <COMMIT_URL> --base-path <BASE_PATH>", and the skill invocation line
"Skill: docs-tools:commit-analyst, args: """ to include a
language tag (use "text") on each opening triple-backtick so they becometext ...for proper syntax highlighting and accessibility.</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**: `1b8abfbf-6615-4d5b-8de0-c89516491331` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between cbc717640d4414d957cd72987cc149e7c4090872 and 0d89fe4318b070043f4a106eb71c071e15a7b4b7. </details> <details> <summary>📒 Files selected for processing (9)</summary> * `plugins/docs-tools/.claude-plugin/plugin.json` * `plugins/docs-tools/skills/commit-analyst/SKILL.md` * `plugins/docs-tools/skills/commit-analyst/reference/impact-criteria.md` * `plugins/docs-tools/skills/commit-analyst/scripts/gather_changes.py` * `plugins/docs-tools/skills/docs-orchestrator/SKILL.md` * `plugins/docs-tools/skills/docs-orchestrator/defaults/commit-driven-workflow.yaml` * `plugins/docs-tools/skills/docs-orchestrator/schema/step-result-schema.md` * `plugins/docs-tools/skills/docs-workflow-commit-analysis/SKILL.md` * `plugins/docs-tools/skills/docs-workflow-start/SKILL.md` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Comment on lines
+44
to
+49
| ```bash | ||
| python3 ${CLAUDE_SKILL_DIR}/scripts/gather_changes.py \ | ||
| --commit <ref> \ | ||
| [--repo <path>] \ | ||
| [--base-branch <branch>] | ||
| ``` |
There was a problem hiding this comment.
Add Cursor-compatible command variants for script calls.
The command blocks currently document Claude-only path variables. Please add Cursor equivalents (repo-root relative paths) so the skill is executable in both environments.
Suggested doc patch
```bash
+# Claude Code
python3 ${CLAUDE_SKILL_DIR}/scripts/gather_changes.py \
--commit <ref> \
[--repo <path>] \
[--base-branch <branch>]
+
+# Cursor (workspace-root relative)
+python3 plugins/docs-tools/skills/commit-analyst/scripts/gather_changes.py \
+ --commit <ref> \
+ [--repo <path>] \
+ [--base-branch <branch>]@@
+# Claude Code
python3 ${CLAUDE_PLUGIN_ROOT}/skills/jira-reader/scripts/jira_reader.py --issue <TICKET>
+
+# Cursor (workspace-root relative)
+python3 plugins/docs-tools/skills/jira-reader/scripts/jira_reader.py --issue <TICKET>@@
+# Claude Code
python3 ${CLAUDE_PLUGIN_ROOT}/skills/jira-reader/scripts/jira_reader.py --graph <TICKET>
+
+# Cursor (workspace-root relative)
+python3 plugins/docs-tools/skills/jira-reader/scripts/jira_reader.py --graph <TICKET>@@
+# Claude Code
python3 ${CLAUDE_PLUGIN_ROOT}/skills/article-extractor/scripts/article_extractor.py \
--url <url> --format markdown
+
+# Cursor (workspace-root relative)
+python3 plugins/docs-tools/skills/article-extractor/scripts/article_extractor.py \
+ --url <url> --format markdown</details>
As per coding guidelines: “In Cursor, use paths relative to the repository root (workspace) for all script calls, both internal and cross-skill.”
Also applies to: 109-117, 136-139
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @plugins/docs-tools/skills/commit-analyst/SKILL.md around lines 44 - 49,
Update the documented command blocks in SKILL.md to include Cursor-compatible
(workspace-root relative) variants alongside the existing Claude environment
variables: add lines showing commands like "python3
plugins/docs-tools/skills/commit-analyst/scripts/gather_changes.py ..." next to
the Claude form using ${CLAUDE_SKILL_DIR} or ${CLAUDE_PLUGIN_ROOT}; do the same
for the jira-reader commands (jira_reader.py --issue and --graph) and the
article-extractor command (article_extractor.py --url), and apply identical
changes to the other affected blocks mentioned (around lines 109-117 and
136-139) so every script call has both the Claude variable-based path and a
repo-root relative "plugins/..." path for Cursor.
</details>
<!-- fingerprinting:phantom:triton:hawk:fddd047c-5505-43b5-8b80-0e3266d0eef9 -->
<!-- This is an auto-generated comment by CodeRabbit -->
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
plugins/docs-tools/skills/commit-analyst/scripts/gather_changes.py (2)
345-345: Consider more idiomatic dict unpacking.Dict iteration yields keys directly, so you can simplify this.
✏️ Suggested fix
- for name in list(CATEGORIES.keys()) + ["source_code"]: + for name in [*CATEGORIES, "source_code"]:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/docs-tools/skills/commit-analyst/scripts/gather_changes.py` at line 345, The loop currently uses list(CATEGORIES.keys()) which is verbose; replace it with direct dict iteration (e.g., for name in list(CATEGORIES) + ["source_code"]: or for name in itertools.chain(CATEGORIES, ["source_code"]):) so the code iterates keys idiomatically — update the for loop that references CATEGORIES to remove .keys() and use the simpler form.
168-170: Use a descriptive variable name instead ofl.The single-letter
lis easily confused with1orIin many fonts.✏️ Suggested fix
metadata["labels"] = [ - l.get("name", "") for l in data.get("labels", []) + label.get("name", "") for label in data.get("labels", []) ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/docs-tools/skills/commit-analyst/scripts/gather_changes.py` around lines 168 - 170, Replace the ambiguous single-letter loop variable `l` in the list comprehension that builds metadata["labels"] with a descriptive name (e.g., `label`), updating the expression from [l.get("name", "") for l in data.get("labels", [])] to use the new name so it reads [label.get("name", "") for label in data.get("labels", [])]; ensure you change only the loop variable and any direct references to it in that comprehension within gather_changes.py so callers and surrounding logic (metadata, data.get("labels")) remain unchanged.
🤖 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/docs-tools/skills/commit-analyst/scripts/gather_changes.py`:
- Around line 192-196: The try/except around urlopen(req, timeout=30) and
json.loads(resp.read()) currently catches a bare Exception and hides failures;
change it to catch specific exceptions (e.g., urllib.error.URLError and
urllib.error.HTTPError for the urlopen call, and json.JSONDecodeError for
json.loads) and log the caught exception to stderr (or via logging) before
returning None so failures are visible; also add the required import for
urllib.error at the top of the module.
---
Nitpick comments:
In `@plugins/docs-tools/skills/commit-analyst/scripts/gather_changes.py`:
- Line 345: The loop currently uses list(CATEGORIES.keys()) which is verbose;
replace it with direct dict iteration (e.g., for name in list(CATEGORIES) +
["source_code"]: or for name in itertools.chain(CATEGORIES, ["source_code"]):)
so the code iterates keys idiomatically — update the for loop that references
CATEGORIES to remove .keys() and use the simpler form.
- Around line 168-170: Replace the ambiguous single-letter loop variable `l` in
the list comprehension that builds metadata["labels"] with a descriptive name
(e.g., `label`), updating the expression from [l.get("name", "") for l in
data.get("labels", [])] to use the new name so it reads [label.get("name", "")
for label in data.get("labels", [])]; ensure you change only the loop variable
and any direct references to it in that comprehension within gather_changes.py
so callers and surrounding logic (metadata, data.get("labels")) remain
unchanged.
🪄 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: d5308fda-bc1d-43c4-81a2-5f6df8a4f571
📒 Files selected for processing (1)
plugins/docs-tools/skills/commit-analyst/scripts/gather_changes.py
Comment on lines
+192
to
+196
| try: | ||
| with urlopen(req, timeout=30) as resp: | ||
| return json.loads(resp.read()) | ||
| except Exception: | ||
| return None |
There was a problem hiding this comment.
Catching bare Exception hides failures silently.
This makes debugging difficult when API calls fail unexpectedly. Consider catching specific exceptions (e.g., urllib.error.URLError, json.JSONDecodeError) or at minimum logging the error to stderr before returning None.
🛡️ Suggested improvement
+ except (urllib.error.URLError, json.JSONDecodeError, TimeoutError) as e:
+ print(f"GitLab API error for {endpoint}: {e}", file=sys.stderr)
- except Exception:
return NoneYou'll also need to add at the top:
import urllib.error📝 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
| try: | |
| with urlopen(req, timeout=30) as resp: | |
| return json.loads(resp.read()) | |
| except Exception: | |
| return None | |
| try: | |
| with urlopen(req, timeout=30) as resp: | |
| return json.loads(resp.read()) | |
| except (URLError, json.JSONDecodeError, TimeoutError): | |
| return None |
🧰 Tools
🪛 Ruff (0.15.11)
[error] 193-193: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
[warning] 195-195: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/docs-tools/skills/commit-analyst/scripts/gather_changes.py` around
lines 192 - 196, The try/except around urlopen(req, timeout=30) and
json.loads(resp.read()) currently catches a bare Exception and hides failures;
change it to catch specific exceptions (e.g., urllib.error.URLError and
urllib.error.HTTPError for the urlopen call, and json.JSONDecodeError for
json.loads) and log the caught exception to stderr (or via logging) before
returning None so failures are visible; also add the required import for
urllib.error at the top of the module.
…-start Replace non-existent AskUserQuestion textInput references with conversational prompts for commit URL and JIRA ticket inputs. Add commit-driven invocation examples to Step 5. Co-authored by Claude Code, under the supervision of Alex McLeod
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
plugins/docs-tools/skills/docs-workflow-start/SKILL.md (1)
238-252: Consider adding language hints to fenced code blocks.The fenced code blocks at lines 238, 244, and 250 lack language specifiers, which triggers markdownlint warnings. While these blocks represent skill invocation syntax (not a standard programming language), you could add a generic identifier for clarity or consistency with other markdown in the repository.
♻️ Optional fix to satisfy markdownlint
-``` +```text Skill: docs-orchestrator, args: "PROJ-123 --mkdocs --pr https://github.com/org/repo/pull/42 --draft"Commit-driven (with JIRA):
-
+text
Skill: docs-orchestrator, args: "PROJ-123 --commit https://github.com/org/repo/pull/42"Commit-driven (without JIRA): -``` +```text Skill: docs-orchestrator, args: "--commit https://gitlab.com/org/repo/-/merge_requests/5"</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@plugins/docs-tools/skills/docs-workflow-start/SKILL.mdaround lines 238 -
252, Add a language hint to the three fenced code blocks that show Skill
invocation so markdownlint stops warning; locate the blocks containingSkill: docs-orchestrator, args: "PROJ-123 --mkdocs --pr https://github.com/org/repo/pull/42 --draft",Skill: docs-orchestrator, args: "PROJ-123 --commit https://github.com/org/repo/pull/42", andSkill: docs-orchestrator, args: "--commit https://gitlab.com/org/repo/-/merge_requests/5"and change the opening fences
fromtotext (or another generic identifier) to annotate them as plain
text/code examples.</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/docs-tools/skills/docs-workflow-start/SKILL.md:
- Around line 160-164: Replace the conversational JIRA prompt with the same
AskUserQuestion (textInput: true) pattern used for other free-text fields in
Step 4: use AskUserQuestion with textInput set to true and map the response to
positional arg $1 (the JIRA ticket ID) so the JIRA input is consistent with PR
URLs, source repo, docs repo path, and JIRA project fields.
Nitpick comments:
In@plugins/docs-tools/skills/docs-workflow-start/SKILL.md:
- Around line 238-252: Add a language hint to the three fenced code blocks that
show Skill invocation so markdownlint stops warning; locate the blocks
containingSkill: docs-orchestrator, args: "PROJ-123 --mkdocs --pr https://github.com/org/repo/pull/42 --draft",Skill: docs-orchestrator, args: "PROJ-123 --commit https://github.com/org/repo/pull/42", andSkill: docs-orchestrator, args: "--commit https://gitlab.com/org/repo/-/merge_requests/5"and change the opening fences
fromtotext (or another generic identifier) to annotate them as plain
text/code examples.</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**: `6e362f4d-6890-4cc3-91b8-9343a8df1908` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between f4d54853211c98118fd755f1add315aae9dc55cb and 726b55c5d356f76c40a0937c3306411ae6223226. </details> <details> <summary>📒 Files selected for processing (1)</summary> * `plugins/docs-tools/skills/docs-workflow-start/SKILL.md` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Comment on lines
+160
to
+164
| **If "Yes" was selected for JIRA ticket (commit-driven path)**: | ||
|
|
||
| Ask the user conversationally: "Enter the JIRA ticket ID (e.g., PROJ-123):" | ||
|
|
||
| Maps to `$1` (positional arg before flags). |
There was a problem hiding this comment.
Inconsistent input method for JIRA ticket collection.
Line 158 establishes the pattern: "Use AskUserQuestion with textInput: true for each value" in Step 4. All other free-text inputs in Step 4 (PR URLs at line 168, source repo at line 180, docs repo path at line 195, JIRA project at line 201) follow this pattern by using "Ask via AskUserQuestion (textInput)".
However, line 162 says "Ask the user conversationally" for the commit-driven JIRA ticket, breaking this established UX pattern.
♻️ Suggested fix for consistency
-**If "Yes" was selected for JIRA ticket (commit-driven path)**:
-
-Ask the user conversationally: "Enter the JIRA ticket ID (e.g., PROJ-123):"
-
-Maps to `$1` (positional arg before flags).
+**If "Yes" was selected for JIRA ticket (commit-driven path)**:
+
+Ask via AskUserQuestion (textInput): "Enter the JIRA ticket ID (e.g., PROJ-123):"
+
+Maps to `$1` (positional arg before flags).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/docs-tools/skills/docs-workflow-start/SKILL.md` around lines 160 -
164, Replace the conversational JIRA prompt with the same AskUserQuestion
(textInput: true) pattern used for other free-text fields in Step 4: use
AskUserQuestion with textInput set to true and map the response to positional
arg $1 (the JIRA ticket ID) so the JIRA input is consistent with PR URLs, source
repo, docs repo path, and JIRA project fields.
… gather_changes.py Replace custom urllib-based GitLab MR API code with CLI delegation to git-pr-reader, matching the existing GitHub PR pattern. GitLab commit URL fetching retained locally since git-pr-reader doesn't handle bare commit URLs. Co-authored by Claude Code, under the supervision of Alex McLeod
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
plugins/docs-tools/skills/commit-analyst/scripts/gather_changes.py (4)
168-170: Rename ambiguous variableltolabel.Single-letter
lis easily confused with1orI. Use a descriptive name.♻️ Suggested fix
metadata["labels"] = [ - l.get("name", "") for l in data.get("labels", []) + label.get("name", "") for label in data.get("labels", []) ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/docs-tools/skills/commit-analyst/scripts/gather_changes.py` around lines 168 - 170, In metadata["labels"] list comprehension in gather_changes.py (the assignment that currently uses "l" inside the comprehension), replace the ambiguous single-letter variable "l" with a descriptive name such as "label" so the comprehension reads something like [label.get("name", "") for label in data.get("labels", [])]; update only the iterator variable name to "label" to avoid confusion while preserving the existing .get logic and behavior.
185-211:extract_gitlab_mrduplicatesextract_pr— consolidate into a shared helper.Both functions perform identical operations: verify reader existence, invoke
info/files/diffsubcommands, parse JSON, build file list, and call_build_pr_metadata. The only difference is the function name.♻️ Suggested refactor
Rename or keep
extract_pras the shared implementation and removeextract_gitlab_mr:-def extract_gitlab_mr(ref): - """Extract files, diff, and metadata from a GitLab MR URL via git-pr-reader.""" - if not GIT_PR_READER.exists(): - _err(f"git_pr_reader.py not found at {GIT_PR_READER}") - - reader = str(GIT_PR_READER) - - info_raw = _run(["python3", reader, "info", ref, "--json"]) - info = json.loads(info_raw) if info_raw else {} - - files_raw = _run(["python3", reader, "files", ref, "--json"]) - raw_files = json.loads(files_raw) if files_raw else [] - - files = [ - { - "path": f.get("path", ""), - "status": f.get("status", "modified"), - "added": f.get("additions", 0), - "removed": f.get("deletions", 0), - } - for f in raw_files - ] - - diff_text = (_run(["python3", reader, "diff", ref]) or "").strip() - - metadata = _build_pr_metadata(ref, info) - return files, diff_text, metadataThen in
main():if ref_type == "github_pr": files, diff_text, metadata = extract_pr(args.commit) elif ref_type == "gitlab_mr": - files, diff_text, metadata = extract_gitlab_mr(args.commit) + files, diff_text, metadata = extract_pr(args.commit) # same handler🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/docs-tools/skills/commit-analyst/scripts/gather_changes.py` around lines 185 - 211, extract_gitlab_mr duplicates extract_pr; consolidate by removing extract_gitlab_mr and reusing extract_pr as the single implementation. Replace callers (e.g., in main()) that call extract_gitlab_mr to call extract_pr instead, ensuring extract_pr continues to perform the GIT_PR_READER existence check, invoke _run with "info"/"files"/"diff", parse JSON into raw_files, map to the files dict (path/status/added/removed), and call _build_pr_metadata(ref, info); keep references to GIT_PR_READER and _run/_build_pr_metadata intact so behavior does not change.
264-265: Add defensiveNonecheck for regex match.Although the current call flow guarantees
GITLAB_COMMIT_REmatched, adding a guard prevents crypticAttributeErrorif this function is called from other contexts.🛡️ Suggested fix
def extract_gitlab_commit(ref): """Extract files, diff, and metadata from a GitLab commit URL.""" m = GITLAB_COMMIT_RE.match(ref) + if not m: + _err(f"Invalid GitLab commit URL: {ref}") host, project_path, sha = m.group(1), m.group(2), m.group(3)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/docs-tools/skills/commit-analyst/scripts/gather_changes.py` around lines 264 - 265, The code assumes GITLAB_COMMIT_RE.match(ref) always returns a match and immediately calls m.group(...), which can raise AttributeError if called elsewhere; modify the block around the lines with GITLAB_COMMIT_RE.match(ref) and the unpack host, project_path, sha = m.group(1), m.group(2), m.group(3) to first check that m is not None, and if it is None raise a clear ValueError (or log and return) that includes the problematic ref; keep the same variable names (m, host, project_path, sha) and ensure the error message provides the ref and indicates the regex did not match.
347-347: Consider using unpacking instead of concatenation.Minor style improvement per Ruff suggestion.
♻️ Suggested fix
- for name in list(CATEGORIES.keys()) + ["source_code"]: + for name in [*CATEGORIES, "source_code"]:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/docs-tools/skills/commit-analyst/scripts/gather_changes.py` at line 347, Replace the concatenation in the for loop that builds the iteration list from list(CATEGORIES.keys()) + ["source_code"] with unpacking to improve style: update the loop that iterates over CATEGORIES and the literal "source_code" (the for name in ... line) to use an unpacked list/tuple (e.g., using [*CATEGORIES.keys(), "source_code"] or (*CATEGORIES.keys(), "source_code")) so the loop still iterates the same items but follows Ruff's suggested style.
🤖 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/docs-tools/skills/commit-analyst/scripts/gather_changes.py`:
- Around line 133-137: The calls to json.loads on info_raw and files_raw can
raise JSONDecodeError if _run returns malformed or non-JSON output; wrap each
json.loads(...) in a try/except catching json.JSONDecodeError (and ValueError
for older Python) around the parsing of info_raw and files_raw in
gather_changes.py, log or warn the parse error including the raw output (using
your existing logger or print), and fall back to the empty defaults (info = {}
and raw_files = []) so the script continues instead of crashing; update the
blocks that handle info_raw/files_raw and reference the variables info_raw,
files_raw, json.loads, and the _run call in your fix.
---
Nitpick comments:
In `@plugins/docs-tools/skills/commit-analyst/scripts/gather_changes.py`:
- Around line 168-170: In metadata["labels"] list comprehension in
gather_changes.py (the assignment that currently uses "l" inside the
comprehension), replace the ambiguous single-letter variable "l" with a
descriptive name such as "label" so the comprehension reads something like
[label.get("name", "") for label in data.get("labels", [])]; update only the
iterator variable name to "label" to avoid confusion while preserving the
existing .get logic and behavior.
- Around line 185-211: extract_gitlab_mr duplicates extract_pr; consolidate by
removing extract_gitlab_mr and reusing extract_pr as the single implementation.
Replace callers (e.g., in main()) that call extract_gitlab_mr to call extract_pr
instead, ensuring extract_pr continues to perform the GIT_PR_READER existence
check, invoke _run with "info"/"files"/"diff", parse JSON into raw_files, map to
the files dict (path/status/added/removed), and call _build_pr_metadata(ref,
info); keep references to GIT_PR_READER and _run/_build_pr_metadata intact so
behavior does not change.
- Around line 264-265: The code assumes GITLAB_COMMIT_RE.match(ref) always
returns a match and immediately calls m.group(...), which can raise
AttributeError if called elsewhere; modify the block around the lines with
GITLAB_COMMIT_RE.match(ref) and the unpack host, project_path, sha = m.group(1),
m.group(2), m.group(3) to first check that m is not None, and if it is None
raise a clear ValueError (or log and return) that includes the problematic ref;
keep the same variable names (m, host, project_path, sha) and ensure the error
message provides the ref and indicates the regex did not match.
- Line 347: Replace the concatenation in the for loop that builds the iteration
list from list(CATEGORIES.keys()) + ["source_code"] with unpacking to improve
style: update the loop that iterates over CATEGORIES and the literal
"source_code" (the for name in ... line) to use an unpacked list/tuple (e.g.,
using [*CATEGORIES.keys(), "source_code"] or (*CATEGORIES.keys(),
"source_code")) so the loop still iterates the same items but follows Ruff's
suggested style.
🪄 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: bb27211a-10ff-4a3a-bb66-e42b796fe428
📒 Files selected for processing (1)
plugins/docs-tools/skills/commit-analyst/scripts/gather_changes.py
Comment on lines
+133
to
+137
| info_raw = _run(["python3", reader, "info", ref, "--json"]) | ||
| info = json.loads(info_raw) if info_raw else {} | ||
|
|
||
| files_raw = _run(["python3", reader, "files", ref, "--json"]) | ||
| raw_files = json.loads(files_raw) if files_raw else [] |
There was a problem hiding this comment.
Unhandled JSONDecodeError if subprocess returns malformed output.
If git_pr_reader.py outputs non-JSON text (e.g., error messages on stdout), json.loads() will raise JSONDecodeError, crashing the script. Consider wrapping in try/except.
🛡️ Suggested fix
- info_raw = _run(["python3", reader, "info", ref, "--json"])
- info = json.loads(info_raw) if info_raw else {}
-
- files_raw = _run(["python3", reader, "files", ref, "--json"])
- raw_files = json.loads(files_raw) if files_raw else []
+ info_raw = _run(["python3", reader, "info", ref, "--json"])
+ try:
+ info = json.loads(info_raw) if info_raw else {}
+ except json.JSONDecodeError:
+ info = {}
+
+ files_raw = _run(["python3", reader, "files", ref, "--json"])
+ try:
+ raw_files = json.loads(files_raw) if files_raw else []
+ except json.JSONDecodeError:
+ raw_files = []🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/docs-tools/skills/commit-analyst/scripts/gather_changes.py` around
lines 133 - 137, The calls to json.loads on info_raw and files_raw can raise
JSONDecodeError if _run returns malformed or non-JSON output; wrap each
json.loads(...) in a try/except catching json.JSONDecodeError (and ValueError
for older Python) around the parsing of info_raw and files_raw in
gather_changes.py, log or warn the parse error including the raw output (using
your existing logger or print), and fall back to the empty defaults (info = {}
and raw_files = []) so the script continues instead of crashing; update the
blocks that handle info_raw/files_raw and reference the variables info_raw,
files_raw, json.loads, and the _run call in your fix.
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.
Add commit-analyst skill and commit-driven pipeline for analyzing commits, PRs, and MRs for documentation impact. Supports standalone reports (/commit-analyst) and full pipeline mode
(/docs-orchestrator --commit ).
New files:
Modified files:
Co-authored by Claude Code, under the supervision of Alex McLeod
Summary by CodeRabbit
New Features
Documentation
Chores