Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a pdf-sg-review skill pointer, a detailed SKILL.md implementing a multi-phase PDF→text chunking, parallel agent analysis, report-merge and optional interactive remediation loop; updates .gitignore to ignore generated skill artifacts. (≤50 words) Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Skill as pdf-sg-review Skill
participant FS as Filesystem
participant PD as pdftotext (Poppler)
participant Task as Task/Agents
participant Repo as Git/Source
User->>Skill: invoke /pdf-sg-review (PR/commit/path/HEAD)
Skill->>FS: check `${CLAUDE_SKILL_DIR}/chunks/` existence
alt chunks missing
Skill->>PD: check availability
PD-->>Skill: available / not available
alt available
User->>Skill: upload PDFs
Skill->>PD: convert PDFs -> text
Skill->>FS: write numbered chunk files to `chunks/`
else not available
Skill->>User: instruct to install poppler-utils
end
end
Skill->>FS: create `temp/`, write `temp/commit-diff.txt`
Skill->>Task: spawn one agent per chunk (provide chunk + diff)
Task-->>Skill: structured findings per chunk
Skill->>FS: merge findings, dedupe, renumber, write `reports/` markdown
alt issues exist
Skill->>User: present issues for Apply/Skip/Modify
User->>Skill: choose action
Skill->>Repo: apply edits via Edit tool
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 8✅ Passed checks (8 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/ibm-sg-review/SKILL.md`:
- Around line 38-42: Replace all hardcoded home paths like
"~/redhat-docs-agent-tools/..." in SKILL.md with the appropriate environment
variables: use ${CLAUDE_SKILL_DIR} for paths that refer to scripts or files
co-located with this skill and use ${CLAUDE_PLUGIN_ROOT} for cross-skill or
plugin-root references; update every occurrence (including the other mentioned
occurrences corresponding to the same pattern) so commands, examples and file
listings point to the variable-based paths instead of the tilde-prefixed home
path.
- Around line 46-119: SKILL.md currently embeds procedural orchestration in the
"Phase 0: Verify Chunks Exist" section; extract that entire workflow into a
standalone script (e.g., prepare_style_guides) that performs: checking for
chunks, verifying pdftotext, creating the style-guides and chunks directories,
converting PDFs to text and splitting into chunks, and returning structured JSON
results (success/failure, absolute paths, and counts) on stdout and errors on
stderr; update SKILL.md to be declarative and simply dispatch to this script,
invoking the AskUserQuestion tool only based on the script's structured output
(use the same option labels: "Done - I uploaded the PDF(s)" and "Skip - proceed
without PDF review") and ensure SKILL.md no longer contains the conditional/loop
logic currently under "Phase 0: Verify Chunks Exist".
- Around line 10-14: The usage examples currently use the legacy slash-command
form (e.g., "/ibm-sg-review https://github.com/org/repo/pull/123",
"/ibm-sg-review `#123`", "/ibm-sg-review HEAD~1", "/ibm-sg-review
path/to/file.adoc", "/ibm-sg-review") — update them to the current skill
reference convention by removing the leading slash and phrasing them as plain
examples of invoking the skill (e.g., "ibm-sg-review
https://github.com/org/repo/pull/123", "ibm-sg-review 123", "ibm-sg-review
HEAD~1", "ibm-sg-review path/to/file.adoc", "ibm-sg-review" or reformat as
sentences explaining each invocation), and ensure any adjacent explanatory text
uses the new convention consistently (look for occurrences of the token
"ibm-sg-review" in SKILL.md to replace).
- Around line 60-63: Multiple fenced code blocks in SKILL.md are missing
language identifiers (MD040); update each triple-backtick block (the ``` blocks
that contain messages like "No PDF style guide chunks found. I need to set up
your style guides first.", "AI review report", and "You are analyzing
documentation content for style guide violations.") to include an appropriate
language tag (e.g., use ```text for plain text snippets or a more specific tag
if content is code/JSON/YAML) for all ranges flagged (around the listed ranges
60-63, 69-72, 81-86, 123-131, 134-136, 145-147, 154-180) so markdownlint MD040
is satisfied; ensure consistency across similar blocks and do not change the
block contents beyond adding the language identifier.
🪄 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: Pro Plus
Run ID: bb9c4319-b03b-49ce-8266-14d90055f118
📒 Files selected for processing (2)
.claude/skills/ibm-sg-reviewplugins/docs-tools/skills/ibm-sg-review/SKILL.md
max-cx
force-pushed
the
pdf-review
branch
2 times, most recently
from
d59daf7 to
574ae0e
Compare
max-cx
force-pushed
the
pdf-review
branch
3 times, most recently
from
ef7a5a4 to
07b2217
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
plugins/docs-tools/skills/pdf-sg-review/SKILL.md (2)
46-119: 🛠️ Refactor suggestion | 🟠 MajorMove orchestration logic out of
SKILL.mdinto scripts and keep this file declarative.The phased branching/looping workflow is embedded inline (chunk checks, install gating, conversion, merge, interactive resolution). Per repo rules, this should be dispatched to
scripts/*.py(JSON stdout/stderr) andSKILL.mdshould only orchestrate script calls + output checks.Refactor direction (shape only)
-### Phase 0: Verify Chunks Exist -...inline conditional workflow... -### Phase 1: Setup -...inline workflow... -### Phase 2: Parallel Analysis -...inline workflow... -### Phase 3: Merge Results -...inline workflow... -### Phase 4: Interactive Issue Resolution -...inline workflow... +### Execute workflow +1. Prepare PDF/style-guide inputs and chunk inventory: + `python3 ${CLAUDE_SKILL_DIR}/scripts/prepare_chunks.py "$ARGUMENTS" --json` +2. Build review input (target diff/files): + `python3 ${CLAUDE_SKILL_DIR}/scripts/prepare_review_input.py "$ARGUMENTS" --json` +3. Run parallel chunk analysis: + `python3 ${CLAUDE_SKILL_DIR}/scripts/run_parallel_review.py --json` +4. Merge findings and render final report: + `python3 ${CLAUDE_SKILL_DIR}/scripts/merge_report.py --json` +5. Optional interactive issue resolution: + `python3 ${CLAUDE_SKILL_DIR}/scripts/resolve_issues.py --interactive --json`Based on learnings: “Do NOT embed procedural logic inline in SKILL.md… defer that logic to a script that emits structured output (typically JSON on stdout, errors on stderr).”
Also applies to: 120-214, 257-277
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/docs-tools/skills/pdf-sg-review/SKILL.md` around lines 46 - 119, The SKILL.md currently embeds procedural orchestration (chunk checks, pdftotext gating, PDF-to-text conversion, splitting and interactive prompts) which must be moved into executable scripts; create one or more scripts under scripts/ (e.g., scripts/prepare_pdf_chunks.py and scripts/manage_pdf_review.py) that perform the phased workflow and emit structured JSON to stdout and errors to stderr, then replace the inline step-by-step shell logic in SKILL.md with declarative calls to those scripts and simple checks of their JSON output (use AskUserQuestion only to present choices and pass results into the scripts); ensure the scripts implement the checks described (pdftotext presence, creating style-guides and chunks, conversion/splitting, chunk-count verification, and the "Done/Skip" flow) and that SKILL.md only orchestrates invocation and output handling.
60-63:⚠️ Potential issue | 🟡 MinorAdd language identifiers to all fenced code blocks (MD040).
Several fenced blocks are missing a language tag and currently fail markdownlint.
Minimal fix pattern
-``` +```text No PDF style guide chunks found. I need to set up your style guides first.-
+text
AI review report
...-``` +```text You are analyzing documentation content for style guide violations. ...</details> As per coding guidelines, follow static-analysis-enforced markdown quality for changed `.md` files. Also applies to: 69-72, 81-86, 128-136, 139-141, 150-152, 159-185 <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/pdf-sg-review/SKILL.mdaround lines 60 - 63,
Several fenced code blocks in SKILL.md lack language identifiers (triggering
markdownlint MD040); locate the anonymous triple-backtick blocks such as the one
containing "No PDF style guide chunks found. I need to set up your style guides
first.", the blocks around "AI review report ..." and "You are analyzing
documentation content for style guide violations.", and the other ranges noted
(lines ~69-72, 81-86, 128-136, 139-141, 150-152, 159-185) and add appropriate
language tags (e.g.,text for plain text output,json for JSON, ```bash
for shell examples) so every fenced code block includes a language identifier.
Ensure consistency across SKILL.md by choosing the most semantically accurate
language tag for each block.</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/pdf-sg-review/SKILL.md:
- Around line 52-53: Replace every hardcoded home-directory path like
'/redhat-docs-agent-tools/plugins/docs-tools/skills/pdf-sg-review/chunks/' with/redhat-docs-agent-tools/...') to use these variables so paths
the skill/plugin substitution variables: use ${CLAUDE_SKILL_DIR}/chunks for
assets bundled with this skill and ${CLAUDE_PLUGIN_ROOT}/... (or
workspace-relative Cursor paths) for cross-skill calls; update the other
occurrences of the same pattern (the commands that call ls, cp, or run scripts
referencing '
remain portable and follow the skill conventions.- Around line 10-15: The examples in SKILL.md use the old slash-command syntax
(e.g., "/pdf-sg-review ..."); update them to the modern skill invocation format
by removing the leading slash and presenting usages like "pdf-sg-review
https://github.com/org/repo/pull/123", "pdf-sg-review 123" (or "pdf-sg-review
#123"), "pdf-sg-review HEAD~1", "pdf-sg-review path/to/file.adoc", and
"pdf-sg-review" for default behavior so all examples reference the pdf-sg-review
skill without a leading "/" while preserving the same argument forms and intent.
Duplicate comments:
In@plugins/docs-tools/skills/pdf-sg-review/SKILL.md:
- Around line 46-119: The SKILL.md currently embeds procedural orchestration
(chunk checks, pdftotext gating, PDF-to-text conversion, splitting and
interactive prompts) which must be moved into executable scripts; create one or
more scripts under scripts/ (e.g., scripts/prepare_pdf_chunks.py and
scripts/manage_pdf_review.py) that perform the phased workflow and emit
structured JSON to stdout and errors to stderr, then replace the inline
step-by-step shell logic in SKILL.md with declarative calls to those scripts and
simple checks of their JSON output (use AskUserQuestion only to present choices
and pass results into the scripts); ensure the scripts implement the checks
described (pdftotext presence, creating style-guides and chunks,
conversion/splitting, chunk-count verification, and the "Done/Skip" flow) and
that SKILL.md only orchestrates invocation and output handling.- Around line 60-63: Several fenced code blocks in SKILL.md lack language
identifiers (triggering markdownlint MD040); locate the anonymous
triple-backtick blocks such as the one containing "No PDF style guide chunks
found. I need to set up your style guides first.", the blocks around "AI review
report ..." and "You are analyzing documentation content for style guide
violations.", and the other ranges noted (lines ~69-72, 81-86, 128-136, 139-141,
150-152, 159-185) and add appropriate language tags (e.g.,text for plain text output,json for JSON, ```bash for shell examples) so every fenced code
block includes a language identifier. Ensure consistency across SKILL.md by
choosing the most semantically accurate language tag for each block.</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**: Pro Plus **Run ID**: `513d4b8f-c966-4144-ac48-45722a9d1dae` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between d15c10610d3030833ee13a0fc735abaa4c4b4f6d and 803e9a2dcc42be0f1fb03e5410337ff784e07a87. </details> <details> <summary>📒 Files selected for processing (3)</summary> * `.claude/skills/pdf-sg-review` * `.gitignore` * `plugins/docs-tools/skills/pdf-sg-review/SKILL.md` </details> <details> <summary>✅ Files skipped from review due to trivial changes (2)</summary> * .claude/skills/pdf-sg-review * .gitignore </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
♻️ Duplicate comments (3)
plugins/docs-tools/skills/pdf-sg-review/SKILL.md (3)
11-11:⚠️ Potential issue | 🟡 MinorRemove legacy slash-command example from skill usage docs.
/pdf-sg-review 123should not appear in skill invocation examples; keep only modern skill-style invocations.Suggested doc fix
-- pdf-sg-review `#123` or /pdf-sg-review 123 - review a PR by number +- pdf-sg-review `#123` or 123 - review a PR by numberBased on learnings: “Do not use old slash-command syntax (e.g.,
/skill-name).”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/docs-tools/skills/pdf-sg-review/SKILL.md` at line 11, Update the SKILL.md usage line to remove the legacy slash-command example: delete the `/pdf-sg-review 123` variant and leave only the modern skill-style invocation (e.g., `pdf-sg-review `#123`` or similar), ensuring the example reflects the current skill invocation pattern; locate the string "pdf-sg-review `#123` or /pdf-sg-review 123 - review a PR by number" and replace it so the slash command is omitted.
46-213:⚠️ Potential issue | 🟠 MajorMove workflow control logic out of
SKILL.mdand into scripts.This section is still procedural/orchestrator-heavy (branching, checks, loops, phase control). Keep
SKILL.mddeclarative and dispatch to script(s) that return structured output.Suggested structure (declarative dispatch)
-### Phase 0: Verify Chunks Exist -... (inline conditional workflow) -### Phase 1: Setup -... (inline orchestration) -### Phase 2: Parallel Analysis -... (inline orchestration) -### Phase 3: Merge Results -... (inline orchestration) -### Phase 4: Interactive Issue Resolution -... (inline orchestration) +## Execute workflow +1. Prepare inputs/chunks: + `python3 ${CLAUDE_SKILL_DIR}/scripts/prepare_chunks.py "$ARGUMENTS" --json` +2. Run parallel review: + `python3 ${CLAUDE_SKILL_DIR}/scripts/run_parallel_review.py --json` +3. Merge report: + `python3 ${CLAUDE_SKILL_DIR}/scripts/merge_report.py --json` +4. Optional interactive resolution: + `python3 ${CLAUDE_SKILL_DIR}/scripts/resolve_issues.py --interactive --json`Based on learnings: “Do NOT embed procedural logic inline in SKILL.md… defer that logic to a script that emits structured output.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/docs-tools/skills/pdf-sg-review/SKILL.md` around lines 46 - 213, The SKILL.md currently contains procedural/orchestrator logic in "Phase 0"–"Phase 3"; extract those procedures into one or more scripts (e.g., a setup/orchestrator script) that perform the checks, chunk creation, agent spawning and merge steps and emit structured machine-readable output (JSON) and clear user-facing messages; then shrink SKILL.md to a declarative dispatch that documents the expected inputs/outputs and shows the command to run the script(s) and the exact JSON schema they return (including how AskUserQuestion choices are surfaced), moving loop/branch logic (chunk existence, pdftotext check, mkdir/split, Task tool invocations, parsing of agent responses, deduping/renumbering and cleanup) out of SKILL.md and into named scripts so callers/tests can invoke them and validate structured results.
60-63:⚠️ Potential issue | 🟡 MinorAdd language identifiers to fenced code blocks (MD040).
Several fenced blocks are untyped; markdownlint flags these. Use
text,bash, or a specific language tag as appropriate.Example patch pattern
- ``` + ```text No PDF style guide chunks found. I need to set up your style guides first. ``` - ``` + ```text AI review report ... ``` -``` +```text You are analyzing documentation content for style guide violations. ...</details> As per coding guidelines, follow static-analysis-enforced markdown quality for changed `.md` files. Also applies to: 69-72, 81-86, 128-136, 139-141, 150-152, 159-185 <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/pdf-sg-review/SKILL.mdaround lines 60 - 63, The
markdown has several untyped fenced code blocks in SKILL.md (e.g., the blocks
containing "No PDF style guide chunks found. I need to set up your style guides
first.", "AI review report", and the assistant prompt examples); update each
triple-backtick block to include an appropriate language tag such as text or
bash (for plain messages usetext, for shell examples usebash, or a
specific language when showing code) so that markdownlint rule MD040 is
satisfied; search for all untypedblocks in SKILL.md (including the ranges referenced in the review) and replace the openingwith ```text or the
correct language tag while leaving the block contents unchanged.</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.
Duplicate comments:
In@plugins/docs-tools/skills/pdf-sg-review/SKILL.md:
- Line 11: Update the SKILL.md usage line to remove the legacy slash-command
example: delete the/pdf-sg-review 123variant and leave only the modern
skill-style invocation (e.g.,pdf-sg-review#123`` or similar), ensuring the
example reflects the current skill invocation pattern; locate the string
"pdf-sg-review#123or /pdf-sg-review 123 - review a PR by number" and replace
it so the slash command is omitted.- Around line 46-213: The SKILL.md currently contains procedural/orchestrator
logic in "Phase 0"–"Phase 3"; extract those procedures into one or more scripts
(e.g., a setup/orchestrator script) that perform the checks, chunk creation,
agent spawning and merge steps and emit structured machine-readable output
(JSON) and clear user-facing messages; then shrink SKILL.md to a declarative
dispatch that documents the expected inputs/outputs and shows the command to run
the script(s) and the exact JSON schema they return (including how
AskUserQuestion choices are surfaced), moving loop/branch logic (chunk
existence, pdftotext check, mkdir/split, Task tool invocations, parsing of agent
responses, deduping/renumbering and cleanup) out of SKILL.md and into named
scripts so callers/tests can invoke them and validate structured results.- Around line 60-63: The markdown has several untyped fenced code blocks in
SKILL.md (e.g., the blocks containing "No PDF style guide chunks found. I need
to set up your style guides first.", "AI review report", and the assistant
prompt examples); update each triple-backtick block to include an appropriate
language tag such as text or bash (for plain messages usetext, for shell examples usebash, or a specific language when showing code) so that
markdownlint rule MD040 is satisfied; search for all untypedblocks in SKILL.md (including the ranges referenced in the review) and replace the openingwith ```text or the correct language tag while leaving the block contents
unchanged.</details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: Path: .coderabbit.yaml **Review profile**: CHILL **Plan**: Pro Plus **Run ID**: `11621b92-d1a5-4b08-a942-82a1945d1e0e` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 803e9a2dcc42be0f1fb03e5410337ff784e07a87 and d3cbaeee92b4f6029d892d5a50740a1f93493332. </details> <details> <summary>📒 Files selected for processing (3)</summary> * `.claude/skills/pdf-sg-review` * `.gitignore` * `plugins/docs-tools/skills/pdf-sg-review/SKILL.md` </details> <details> <summary>✅ Files skipped from review due to trivial changes (2)</summary> * .claude/skills/pdf-sg-review * .gitignore </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
plugins/docs-tools/skills/pdf-sg-review/SKILL.md (2)
10-12:⚠️ Potential issue | 🟡 MinorRemove legacy slash-style invocation from usage examples.
The example
... or /pdf-sg-review 123reintroduces deprecated slash invocation style. Keep examples in modern skill invocation format only.Based on learnings: “Do not use old slash-command syntax (e.g.,
/skill-name).”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/docs-tools/skills/pdf-sg-review/SKILL.md` around lines 10 - 12, The usage examples include a deprecated slash-style invocation "/pdf-sg-review 123"; remove that legacy example and keep only modern invocation forms such as "pdf-sg-review `#123`" and "pdf-sg-review https://github.com/org/repo/pull/123" (and the commit form "pdf-sg-review HEAD~1") so the SKILL.md only shows current skill invocation syntax; update the example line to drop the "/pdf-sg-review 123" entry and ensure the remaining examples are formatted consistently.
46-119:⚠️ Potential issue | 🟠 MajorMove orchestration logic from
SKILL.mdinto scripts.This file still embeds conditional workflow, branching, and execution control directly in markdown. Keep
SKILL.mddeclarative and dispatch to scripts underskills/pdf-sg-review/scripts/that emit structured output.Suggested shape
-### Phase 0: Verify Chunks Exist -...inline conditional logic... -### Phase 1: Setup -...inline orchestration... -### Phase 2: Parallel Analysis -...inline orchestration... -### Phase 3: Merge Results -...inline orchestration... -### Phase 4: Interactive Issue Resolution -...inline orchestration... +### Execute workflow +1. Prepare/validate chunks: + `python3 ${CLAUDE_SKILL_DIR}/scripts/prepare_chunks.py "$ARGUMENTS" --json` +2. Collect review input: + `python3 ${CLAUDE_SKILL_DIR}/scripts/collect_review_input.py "$ARGUMENTS" --json` +3. Run parallel analysis: + `python3 ${CLAUDE_SKILL_DIR}/scripts/run_parallel_review.py --json` +4. Merge/dedupe/format report: + `python3 ${CLAUDE_SKILL_DIR}/scripts/merge_report.py --json` +5. Optional issue-resolution loop: + `python3 ${CLAUDE_SKILL_DIR}/scripts/resolve_issues.py --interactive --json`Based on learnings: “Do NOT embed procedural logic inline in SKILL.md… defer that logic to a script that emits structured output (typically JSON on stdout, errors on stderr).”
Also applies to: 120-213, 257-277
🧹 Nitpick comments (1)
plugins/docs-tools/skills/pdf-sg-review/SKILL.md (1)
4-4: Add a migration note for the claimed redundancy with existing skills.The PR states this skill makes multiple
ibm-sg-*skills redundant, but this file does not document deprecation/migration behavior. Add an explicit differentiation/consolidation note to prevent overlapping usage paths.Based on learnings: “When adding or significantly modifying plugin files (commands, skills, agents), check for overlapping functionality against existing plugins and open PRs. Flag HIGH similarity (>=85%) immediately with consolidation recommendation.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/docs-tools/skills/pdf-sg-review/SKILL.md` at line 4, The SKILL.md description currently claims this skill makes multiple ibm-sg-* skills redundant but lacks any migration/deprecation guidance; update SKILL.md to add an explicit "Migration and Consolidation" note that (1) lists the overlapping ibm-sg-* skills by name, (2) states recommended migration paths or deprecation timelines for those skills, and (3) documents the consolidation policy including the HIGH similarity flag (>=85%) and the action to open a consolidation PR or coordination ticket; reference the skill name/description text and the ibm-sg-* identifier so readers can find the affected items.
🤖 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/pdf-sg-review/SKILL.md`:
- Around line 138-141: The text claims “directory and subdirectories” but the
command shown (ls ${CLAUDE_SKILL_DIR}/chunks/) is non‑recursive; either update
the wording to say just “directory” to match the existing ls command, or change
the example to perform a recursive listing (use a recursive file listing such as
a recursive ls or find) so the command actually lists subdirectories; edit
SKILL.md replacing the mismatched wording or replacing the ls command
accordingly.
- Around line 149-152: Phase 1 Step 4 currently only shows the path
${CLAUDE_SKILL_DIR}/temp/commit-diff.txt and is not executable; update the code
block under "Phase 1 Step 4" to include concrete shell commands that write the
resolved diff into that file (for example, use the GitHub CLI to output a PR
diff or use git show/git diff to output a commit or range) and include commented
examples for PR vs commit/range cases, ensuring the commands redirect output
into "${CLAUDE_SKILL_DIR}/temp/commit-diff.txt".
---
Duplicate comments:
In `@plugins/docs-tools/skills/pdf-sg-review/SKILL.md`:
- Around line 10-12: The usage examples include a deprecated slash-style
invocation "/pdf-sg-review 123"; remove that legacy example and keep only modern
invocation forms such as "pdf-sg-review `#123`" and "pdf-sg-review
https://github.com/org/repo/pull/123" (and the commit form "pdf-sg-review
HEAD~1") so the SKILL.md only shows current skill invocation syntax; update the
example line to drop the "/pdf-sg-review 123" entry and ensure the remaining
examples are formatted consistently.
---
Nitpick comments:
In `@plugins/docs-tools/skills/pdf-sg-review/SKILL.md`:
- Line 4: The SKILL.md description currently claims this skill makes multiple
ibm-sg-* skills redundant but lacks any migration/deprecation guidance; update
SKILL.md to add an explicit "Migration and Consolidation" note that (1) lists
the overlapping ibm-sg-* skills by name, (2) states recommended migration paths
or deprecation timelines for those skills, and (3) documents the consolidation
policy including the HIGH similarity flag (>=85%) and the action to open a
consolidation PR or coordination ticket; reference the skill name/description
text and the ibm-sg-* identifier so readers can find the affected items.
🪄 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: Pro Plus
Run ID: 157981b0-b1c5-40c5-a2f1-b00177c95ef1
📒 Files selected for processing (1)
plugins/docs-tools/skills/pdf-sg-review/SKILL.md
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
plugins/docs-tools/skills/pdf-sg-review/SKILL.md (3)
25-33:⚠️ Potential issue | 🟠 MajorMove orchestration/branching out of
SKILL.mdinto scripts.Line 25 onward embeds argument parsing, branching, setup, and control-flow directly in markdown. This makes the skill brittle and conflicts with the skill architecture conventions.
Based on learnings: “Do NOT embed procedural logic (conditionals, path construction, validation) inline in SKILL.md… defer that logic to a script that emits structured output (typically JSON on stdout, errors on stderr).”
Also applies to: 46-119, 120-213, 257-277
149-152:⚠️ Potential issue | 🟠 MajorPhase 1 Step 4 is non-executable as written.
Line 151 is only a target path; it does not retrieve or write diff content. Add concrete commands that produce
${CLAUDE_SKILL_DIR}/temp/commit-diff.txt.Suggested fix
4. Retrieve the commit diff content and save it to a temporary file: ```bash - ${CLAUDE_SKILL_DIR}/temp/commit-diff.txt + # PR URL/number input + gh pr diff <number> > "${CLAUDE_SKILL_DIR}/temp/commit-diff.txt" + # Commit/range input examples: + # git show <ref> > "${CLAUDE_SKILL_DIR}/temp/commit-diff.txt" + # git diff <range> > "${CLAUDE_SKILL_DIR}/temp/commit-diff.txt" ```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/docs-tools/skills/pdf-sg-review/SKILL.md` around lines 149 - 152, Phase 1 Step 4 currently only lists the target path `${CLAUDE_SKILL_DIR}/temp/commit-diff.txt` without any command to produce it; update the step in SKILL.md to include concrete commands that write the diff into that file (e.g., use `gh pr diff <number> > "${CLAUDE_SKILL_DIR}/temp/commit-diff.txt"` for PRs and provide alternatives like `git show <ref> > "${CLAUDE_SKILL_DIR}/temp/commit-diff.txt"` or `git diff <range> > "${CLAUDE_SKILL_DIR}/temp/commit-diff.txt"`), ensuring the documentation around the `${CLAUDE_SKILL_DIR}/temp/commit-diff.txt` target explains which command to use depending on PR vs commit/range inputs.
11-11:⚠️ Potential issue | 🟡 MinorRemove legacy slash-command invocation example.
Line 11 still includes
/pdf-sg-review 123; keep only modern skill invocation examples.Based on learnings: “Do not use old slash-command syntax (e.g.,
/skill-name).”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/docs-tools/skills/pdf-sg-review/SKILL.md` at line 11, Remove the legacy slash-command example "/pdf-sg-review 123" from the SKILL.md line that currently reads "pdf-sg-review `#123` or /pdf-sg-review 123 - review a PR by number" and keep only the modern invocation examples (e.g., "pdf-sg-review `#123`" or "pdf-sg-review 123"); update the text so it no longer contains the leading slash variant and reads clearly like "pdf-sg-review `#123` - review a PR by number" (or include both modern forms without the slash).
🧹 Nitpick comments (1)
plugins/docs-tools/skills/pdf-sg-review/SKILL.md (1)
4-4: Add explicit consolidation/migration guidance for replaced skills.The description says this skill makes multiple existing skills redundant. Please document deprecation/replacement mapping (and planned cleanup) to prevent parallel, conflicting usage.
Based on learnings: “When adding or significantly modifying plugin files… check for overlapping functionality… Flag HIGH similarity (>=85%) immediately with consolidation recommendation.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/docs-tools/skills/pdf-sg-review/SKILL.md` at line 4, Update the SKILL.md description to include an explicit deprecation and replacement section listing which existing skills this PDF style-review skill supersedes (add mapping like "replaces: [skill-name-A, skill-name-B]") and a short planned cleanup timeline; also add a consolidation guidance note instructing operators to check for overlapping functionality and, per the project guideline, to immediately flag any HIGH similarity (>=85%) between skills with a consolidation recommendation; ensure these notes appear alongside the existing "description" paragraph so reviewers see the replacement mapping and the similarity threshold rule.
🤖 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/pdf-sg-review/SKILL.md`:
- Around line 37-42: The spec currently contradicts itself: the "spawn multiple
parallel agents" step describes that "each agent writes findings to a separate
temporary file" while later steps expect in-band responses; pick one contract
and make the orchestration and docs consistent. Either (A) keep the file-write
contract: update the agent behavior so each agent writes its report to a
uniquely named temp file (per "each agent writes findings to a separate
temporary file"), ensure the final merge step reads those temp files and deletes
them afterwards, and update the SKILL.md text to remove any references to
in-band responses; or (B) switch to in-band responses: remove the file-write
behavior from the agents, change the orchestration that spawns agents to collect
their returned outputs (IPC/promises) and pass them to the "final merge step",
and update SKILL.md to remove mentions of temp files—ensure only one contract
(file-based or in-band) is documented and implemented for the "spawn multiple
parallel agents", "each agent writes findings..." and "final merge step combines
all findings" flow.
---
Duplicate comments:
In `@plugins/docs-tools/skills/pdf-sg-review/SKILL.md`:
- Around line 149-152: Phase 1 Step 4 currently only lists the target path
`${CLAUDE_SKILL_DIR}/temp/commit-diff.txt` without any command to produce it;
update the step in SKILL.md to include concrete commands that write the diff
into that file (e.g., use `gh pr diff <number> >
"${CLAUDE_SKILL_DIR}/temp/commit-diff.txt"` for PRs and provide alternatives
like `git show <ref> > "${CLAUDE_SKILL_DIR}/temp/commit-diff.txt"` or `git diff
<range> > "${CLAUDE_SKILL_DIR}/temp/commit-diff.txt"`), ensuring the
documentation around the `${CLAUDE_SKILL_DIR}/temp/commit-diff.txt` target
explains which command to use depending on PR vs commit/range inputs.
- Line 11: Remove the legacy slash-command example "/pdf-sg-review 123" from the
SKILL.md line that currently reads "pdf-sg-review `#123` or /pdf-sg-review 123 -
review a PR by number" and keep only the modern invocation examples (e.g.,
"pdf-sg-review `#123`" or "pdf-sg-review 123"); update the text so it no longer
contains the leading slash variant and reads clearly like "pdf-sg-review `#123` -
review a PR by number" (or include both modern forms without the slash).
---
Nitpick comments:
In `@plugins/docs-tools/skills/pdf-sg-review/SKILL.md`:
- Line 4: Update the SKILL.md description to include an explicit deprecation and
replacement section listing which existing skills this PDF style-review skill
supersedes (add mapping like "replaces: [skill-name-A, skill-name-B]") and a
short planned cleanup timeline; also add a consolidation guidance note
instructing operators to check for overlapping functionality and, per the
project guideline, to immediately flag any HIGH similarity (>=85%) between
skills with a consolidation recommendation; ensure these notes appear alongside
the existing "description" paragraph so reviewers see the replacement mapping
and the similarity threshold rule.
🪄 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: Pro Plus
Run ID: fbe385b4-6271-4854-899b-13fff6d65d02
📒 Files selected for processing (1)
plugins/docs-tools/skills/pdf-sg-review/SKILL.md
|
why do we need to have a PDF in the mix here? |
2 participants
Summary by CodeRabbit
New Features
Chores