feat(docs-tools): maximize code-evidence usage across all workflow steps#137
Merged
DonaghBr merged 1 commit intoredhat-documentation:mainfrom May 1, 2026
Merged
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (14)
WalkthroughUpdates documentation and configs to add repo-aware enrichment, multi-query evidence classification with optional API-surface cross-checking, API-surface extraction in pre-flight, and new artifacts/flags for code-grounded technical review. Agent turn limit increased from 8 to 10. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Requirement Agent
participant NLSearch as Natural-Language Search
participant APISurf as API Surface Artifact
participant Classifier as Evidence Classifier
Client->>Classifier: submit requirement (+ optional API_SURFACE_FILE)
Classifier->>NLSearch: run Query A (implementation-focused)
NLSearch-->>Classifier: results A (top_score)
Classifier->>NLSearch: run Query B (term-focused)
NLSearch-->>Classifier: results B (top_score)
Classifier->>NLSearch: optional Query C (alternate phrasing)
NLSearch-->>Classifier: results C (top_score)
Classifier->>Classifier: select best result by top_score
alt API_SURFACE_FILE provided
Classifier->>APISurf: cross-check selected results vs API entities
APISurf-->>Classifier: exact match?
Classifier->>Classifier: if exact -> set top_score >= grounded threshold
end
Classifier-->>Client: return evidence classification + top_score
sequenceDiagram
participant ReqAgent as Requirements Analyst
participant RepoReader as Repo Reader
participant WebSearch as Web Search
participant Merger as Evidence Merger
ReqAgent->>RepoReader: REPO_PATH present?
alt REPO_PATH provided
RepoReader->>RepoReader: read/glob/grep codebase
RepoReader-->>ReqAgent: code references, README/CHANGELOG/docs, repo_metadata
end
ReqAgent->>WebSearch: web search (step 3)
WebSearch-->>ReqAgent: web results
ReqAgent->>Merger: merge repo + web evidence into references
Merger-->>ReqAgent: enriched requirement output
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (8 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 40 minutes and 59 seconds.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: 5
🧹 Nitpick comments (4)
plugins/docs-tools/skills/docs-orchestrator/scripts/resolve_source.py (1)
355-368: ⚡ Quick winSilent exception handling loses diagnostic information.
When
jira_reader.pyfails due toJSONDecodeError,TimeoutExpired, orIndexError, the exceptions are silently swallowed. This makes debugging difficult when JIRA discovery unexpectedly returns no results despite the ticket having git links.Consider logging a warning to stderr before the
pass:🔧 Proposed fix to add warning on failure
except (json.JSONDecodeError, subprocess.TimeoutExpired, IndexError): - pass + print(f"WARNING: Failed to fetch git_links from JIRA ticket {ticket}", file=sys.stderr)And similarly for the
--graphcall at line 382-383.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/docs-tools/skills/docs-orchestrator/scripts/resolve_source.py` around lines 355 - 368, The try/except around the subprocess.run call that invokes jira_reader (the ["python3", str(jira_script), "--issue", ticket] invocation) silently swallows JSONDecodeError, subprocess.TimeoutExpired, and IndexError; modify the except to capture the exception as e and write a concise warning including the ticket, jira_script, and exception details to stderr (or via logging) before continuing so failures are visible; apply the same change to the parallel subprocess.run that calls jira_reader with the "--graph" flag (the other run invocation) so both failure paths emit diagnostic warnings rather than silently passing.plugins/docs-tools/skills/docs-workflow-requirements/SKILL.md (1)
50-65: 💤 Low valueAdd language specifier to Agent pseudocode block.
Static analysis flagged missing language specifier. Since this is pseudocode for the Agent tool invocation pattern, consider using a generic specifier or adding a comment.
📝 Suggested fix
-``` +```yaml Agent: subagent_type: docs-tools:requirements-discoverer🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/docs-tools/skills/docs-workflow-requirements/SKILL.md` around lines 50 - 65, Update the fenced pseudocode block for the Agent invocation to include a language specifier (e.g., change the opening fence from ``` to ```yaml) so static analysis recognizes the block as YAML; specifically modify the block that begins with "Agent:" and contains "subagent_type: docs-tools:requirements-discoverer" to use a language tag (or add a brief inline comment) while preserving the existing keys and prompt content.plugins/docs-tools/skills/docs-workflow-tech-review/SKILL.md (1)
27-39: 💤 Low valueMinor: Add language specifiers to fenced code blocks.
Static analysis flagged missing language specifiers on the code blocks at lines 27 and 34. This is a minor documentation hygiene issue.
📝 Suggested fix
## Input -``` +```text <base-path>/writing/ <repo-path>/ (optional — source code repo for code-grounded validation)Output
-
+text
/technical-review/review.md</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-tech-review/SKILL.mdaround lines 27
- 39, The fenced code blocks containing "/writing/ ... /"
and the "## Output" block (e.g., lines showing
"/technical-review/review.md", "step-result.json", etc.) in SKILL.md
are missing language specifiers; update both triple-backtick fences to include a
language (use "text") so they becometext ...to satisfy static analysis
and documentation hygiene.</details> </blockquote></details> <details> <summary>plugins/docs-tools/skills/docs-workflow-writing/scripts/build_writing_args.sh (1)</summary><blockquote> `26-26`: _💤 Low value_ **`IMPORT_FROM` is initialized but never assigned a value.** The variable is set to empty string on line 26 and emitted as `null` in the JSON output (line 201), but there's no CLI flag or logic to populate it. If this is intentional scaffolding for a future `--import-from` flag, consider adding a comment. Otherwise, remove the dead variable. <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-writing/scripts/build_writing_args.sh` at line 26, The variable IMPORT_FROM in build_writing_args.sh is defined as an empty string but never set or wired to a CLI flag, causing it to emit null in the generated JSON; either remove the unused IMPORT_FROM variable and any references to it in the JSON-building logic, or implement a CLI flag/assignment to populate it (e.g., add parsing for a --import-from option and assign to IMPORT_FROM before the JSON is produced). Update the JSON emission code that currently references IMPORT_FROM so it only includes the field when the variable is non-empty, or remove that field entirely if you choose to delete IMPORT_FROM. ``` </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/agents/evidence-classifier.md:
- Around line 45-77: The classification incorrectly treats empty NL results as
Absent before honoring exact API-surface matches; update the logic that handles
API_SURFACE_FILE and top_score so an exact API surface entity match immediately
sets top_score (or a flag) to at least GROUNDED_THRESHOLD and bypasses the
"empty NL result => Absent" check; ensure the classifier evaluates API-surface
matches prior to the absent-rule and that the subsequent classification uses
top_score, GROUNDED_THRESHOLD, and ABSENT_THRESHOLD (and the count of results
above ABSENT_THRESHOLD) to decide Grounded/Partial/Absent per the described
order.In
@plugins/docs-tools/skills/docs-workflow-requirements/SKILL.md:
- Line 4: Update the SKILL.md metadata so the argument-hint includes the
undocumented --repo flag; specifically edit the argument-hint key (currently
"argument-hint: --base-path [--pr ]...") to add "[--repo
<repo_path>]" (or similar name used elsewhere) so it matches references to
--repo on lines that call this step (see references to --repo on lines 105 and
113); ensure the hint syntax matches the orchestrator docs' "[--pr ]...
[--repo <repo_path>]" pattern.In
@plugins/docs-tools/skills/docs-workflow-writing/SKILL.md:
- Around line 24-49: The JSON example contains duplicate keys (e.g., "mode",
"ticket", "format", "input_file", "evidence_file", "has_evidence", "output_dir",
"output_file") causing confusion; remove the first duplicated block and keep the
consolidated schema that includes the extended fields ("evidence_status",
"has_evidence_status", "source_repo", "repo_path", "fix_from", "import_from",
"verify_output"), ensuring only one occurrence of each key in the JSON example
so readers see the final, authoritative schema.In
@scripts/test-upstream-plugin.sh:
- Around line 122-130: The branch detection currently uses the caller's CWD;
update the logic to detect the branch from the marketplace repo by running git
in MARKETPLACE_DIR (e.g., git -C "$MARKETPLACE_DIR" branch --show-current or
pushd/popd into MARKETPLACE_DIR) when $branch is empty, and ensure
MARKETPLACE_DIR exists and is a git repository before any git commands by
checking [[ -d "$MARKETPLACE_DIR" ]] and [[ -d "$MARKETPLACE_DIR/.git" ]] (or
git -C "$MARKETPLACE_DIR" rev-parse --is-inside-work-tree) and fail with a clear
error if not; modify the block that sets branch (the branch variable resolution)
and any subsequent git operations that rely on MARKETPLACE_DIR to use the
validated path (or git -C) so fetch/checkout operate on the marketplace repo,
not the caller's CWD.- Around line 83-90: The --branch and --plugin case arms currently read $2
directly which can trigger an unbound-variable exit under set -u when the option
value is omitted; update the case branches in the while loop handling to
validate there is a next argument (e.g., check that$# -ge 2 or test $ {2:-} is
non-empty) before assigning branch="$2" or plugin="$2", and if the value is
missing call usage (or print an error) and exit instead of reading an unset
variable; reference the while loop and the case arms for --branch/--plugin and
the variables branch, plugin, and usage when making the change.
Nitpick comments:
In@plugins/docs-tools/skills/docs-orchestrator/scripts/resolve_source.py:
- Around line 355-368: The try/except around the subprocess.run call that
invokes jira_reader (the ["python3", str(jira_script), "--issue", ticket]
invocation) silently swallows JSONDecodeError, subprocess.TimeoutExpired, and
IndexError; modify the except to capture the exception as e and write a concise
warning including the ticket, jira_script, and exception details to stderr (or
via logging) before continuing so failures are visible; apply the same change to
the parallel subprocess.run that calls jira_reader with the "--graph" flag (the
other run invocation) so both failure paths emit diagnostic warnings rather than
silently passing.In
@plugins/docs-tools/skills/docs-workflow-requirements/SKILL.md:
- Around line 50-65: Update the fenced pseudocode block for the Agent invocation
to include a language specifier (e.g., change the opening fence from ``` tothe block that begins with "Agent:" and contains "subagent_type: docs-tools:requirements-discoverer" to use a language tag (or add a brief inline comment) while preserving the existing keys and prompt content. In `@plugins/docs-tools/skills/docs-workflow-tech-review/SKILL.md`: - Around line 27-39: The fenced code blocks containing "<base-path>/writing/ ... <repo-path>/" and the "## Output" block (e.g., lines showing "<base-path>/technical-review/review.md", "step-result.json", etc.) in SKILL.md are missing language specifiers; update both triple-backtick fences to include a language (use "text") so they become ```text ... ``` to satisfy static analysis and documentation hygiene. In `@plugins/docs-tools/skills/docs-workflow-writing/scripts/build_writing_args.sh`: - Line 26: The variable IMPORT_FROM in build_writing_args.sh is defined as an empty string but never set or wired to a CLI flag, causing it to emit null in the generated JSON; either remove the unused IMPORT_FROM variable and any references to it in the JSON-building logic, or implement a CLI flag/assignment to populate it (e.g., add parsing for a --import-from option and assign to IMPORT_FROM before the JSON is produced). Update the JSON emission code that currently references IMPORT_FROM so it only includes the field when the variable is non-empty, or remove that field entirely if you choose to delete IMPORT_FROM.🪄 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:
f797efd1-67e4-4877-91f5-6080c61f1265📒 Files selected for processing (19)
plugins/docs-tools/.claude-plugin/plugin.jsonplugins/docs-tools/agents/evidence-classifier.mdplugins/docs-tools/agents/requirements-analyst.mdplugins/docs-tools/agents/requirements-discoverer.mdplugins/docs-tools/skills/docs-orchestrator/SKILL.mdplugins/docs-tools/skills/docs-orchestrator/defaults/docs-workflow-code-evidence.yamlplugins/docs-tools/skills/docs-orchestrator/schema/step-result-schema.mdplugins/docs-tools/skills/docs-orchestrator/scripts/resolve_source.pyplugins/docs-tools/skills/docs-workflow-code-evidence/SKILL.mdplugins/docs-tools/skills/docs-workflow-planning/SKILL.mdplugins/docs-tools/skills/docs-workflow-requirements/SKILL.mdplugins/docs-tools/skills/docs-workflow-scope-req-audit/SKILL.mdplugins/docs-tools/skills/docs-workflow-start/SKILL.mdplugins/docs-tools/skills/docs-workflow-style-review/SKILL.mdplugins/docs-tools/skills/docs-workflow-tech-review/SKILL.mdplugins/docs-tools/skills/docs-workflow-writing/SKILL.mdplugins/docs-tools/skills/docs-workflow-writing/scripts/build_writing_args.shscripts/test-upstream-plugin.shscripts/toggle-plugin-dev.sh💤 Files with no reviewable changes (1)
- scripts/toggle-plugin-dev.sh
DonaghBr
force-pushed
the
improve-code-evidence
branch
2 times, most recently
from
f737820 to
c6792fe
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
plugins/docs-tools/skills/docs-workflow-requirements/SKILL.md (1)
4-4:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winThe
--repoflag remains undocumented in the argument-hint.Lines 105 and 113 reference
--repobeing passed to this step, but the argument-hint on line 4 only shows[--pr <url>].... The orchestrator documentation confirms requirements receives[--pr <url>]... [--repo <repo_path>].📝 Suggested fix
-argument-hint: <ticket> --base-path <path> [--pr <url>]... +argument-hint: <ticket> --base-path <path> [--pr <url>]... [--repo <path>]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/docs-tools/skills/docs-workflow-requirements/SKILL.md` at line 4, The argument-hint entry is missing the --repo flag referenced elsewhere; update the argument-hint string (the line starting with "argument-hint:") to include [--repo <repo_path>] alongside [--pr <url>] so it reads something like: argument-hint: <ticket> --base-path <path> [--pr <url>]... [--repo <repo_path>], ensuring the documented CLI signature matches the uses of --repo in the SKILL.md content.
🧹 Nitpick comments (1)
plugins/docs-tools/skills/docs-workflow-scope-req-audit/SKILL.md (1)
152-152: 💤 Low valueClarify the conditional inclusion of
API_SURFACE_FILE.The instruction says "or omit this line if empty" but shows the line in the agent prompt template. Consider making it explicit: when
API_SURFACE_FILE="", omit the entire line from the prompt rather than including- API_SURFACE_FILE:with no value.✍️ Suggested clarification
- REPO_PATH: <absolute repo path> - GROUNDED_THRESHOLD: <threshold> - ABSENT_THRESHOLD: <threshold> - - API_SURFACE_FILE: <API_SURFACE_FILE or omit this line if empty> + [Include only if API_SURFACE_FILE is not empty:] + - API_SURFACE_FILE: <API_SURFACE_FILE>Or more explicitly in the prose:
If API_SURFACE_FILE is not empty, add: - API_SURFACE_FILE: <path> Otherwise, omit this line entirely.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/docs-tools/skills/docs-workflow-scope-req-audit/SKILL.md` at line 152, Update the SKILL.md prompt template text to explicitly state the conditional behavior for API_SURFACE_FILE: when API_SURFACE_FILE is non-empty include the line "- API_SURFACE_FILE: <path>", and when API_SURFACE_FILE == "" omit the entire "- API_SURFACE_FILE:" line; reference the API_SURFACE_FILE token in the prose so readers and any template-rendering code know to remove the line rather than render an empty value.
🤖 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/agents/requirements-analyst.md`:
- Line 77: The "first requirement" guard that skips repo metadata extraction
based on RELATED_TICKETS is racy in the fan-out architecture; remove that
conditional in the requirements-analyst logic so each agent always produces a
repo_metadata field (the requirements skill/orchestrator will deduplicate during
merge), or alternatively move the extraction out of requirements-analyst into
the single-run requirements-discoverer component so metadata is produced exactly
once; update the code paths that reference RELATED_TICKETS and repo_metadata to
reflect the chosen approach.
In `@plugins/docs-tools/skills/docs-workflow-code-evidence/SKILL.md`:
- Around line 217-233: The doc must explicitly describe reading
`${OUTPUT_DIR}/api-surface.json`, parsing the existing `$EVIDENCE_FILE` JSON,
adding an `api_surface` property whose value is the parsed API surface object,
and writing the modified JSON back to `$EVIDENCE_FILE`; update the SKILL.md text
around the mention of `api-surface.json` and `$EVIDENCE_FILE` to show the three
steps (read API surface, read/parse evidence.json, merge/append `api_surface`,
write back) and give a concrete command example using `jq` (or equivalent)
referencing `api-surface.json`, `$EVIDENCE_FILE`, and the `api_surface` field so
readers can implement the multi-step modification reliably.
In `@plugins/docs-tools/skills/docs-workflow-tech-review/SKILL.md`:
- Line 119: The review text references API_SURFACE_FILE even when api_surface
extraction failed; update the logic to check HAS_API_SURFACE before emitting the
"Cross-reference the API surface at <API_SURFACE_FILE>" instruction—mirror the
pattern used for the HAS_GROUNDED check (wrap the instruction in a conditional
that only outputs the API surface guidance when HAS_API_SURFACE=true), using the
HAS_API_SURFACE variable and API_SURFACE_FILE symbol to locate and guard that
block.
---
Duplicate comments:
In `@plugins/docs-tools/skills/docs-workflow-requirements/SKILL.md`:
- Line 4: The argument-hint entry is missing the --repo flag referenced
elsewhere; update the argument-hint string (the line starting with
"argument-hint:") to include [--repo <repo_path>] alongside [--pr <url>] so it
reads something like: argument-hint: <ticket> --base-path <path> [--pr <url>]...
[--repo <repo_path>], ensuring the documented CLI signature matches the uses of
--repo in the SKILL.md content.
---
Nitpick comments:
In `@plugins/docs-tools/skills/docs-workflow-scope-req-audit/SKILL.md`:
- Line 152: Update the SKILL.md prompt template text to explicitly state the
conditional behavior for API_SURFACE_FILE: when API_SURFACE_FILE is non-empty
include the line "- API_SURFACE_FILE: <path>", and when API_SURFACE_FILE == ""
omit the entire "- API_SURFACE_FILE:" line; reference the API_SURFACE_FILE token
in the prose so readers and any template-rendering code know to remove the line
rather than render an empty value.
🪄 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: 36ebe346-b545-4530-9a71-61638824b7c9
📒 Files selected for processing (12)
plugins/docs-tools/agents/evidence-classifier.mdplugins/docs-tools/agents/requirements-analyst.mdplugins/docs-tools/skills/docs-orchestrator/SKILL.mdplugins/docs-tools/skills/docs-orchestrator/defaults/docs-workflow-code-evidence.yamlplugins/docs-tools/skills/docs-orchestrator/schema/step-result-schema.mdplugins/docs-tools/skills/docs-workflow-code-evidence/SKILL.mdplugins/docs-tools/skills/docs-workflow-planning/SKILL.mdplugins/docs-tools/skills/docs-workflow-requirements/SKILL.mdplugins/docs-tools/skills/docs-workflow-scope-req-audit/SKILL.mdplugins/docs-tools/skills/docs-workflow-tech-review/SKILL.mdplugins/docs-tools/skills/docs-workflow-writing/SKILL.mdplugins/docs-tools/skills/docs-workflow-writing/scripts/build_writing_args.sh
✅ Files skipped from review due to trivial changes (1)
- plugins/docs-tools/skills/docs-orchestrator/defaults/docs-workflow-code-evidence.yaml
🚧 Files skipped from review as they are similar to previous changes (5)
- plugins/docs-tools/skills/docs-workflow-planning/SKILL.md
- plugins/docs-tools/skills/docs-orchestrator/schema/step-result-schema.md
- plugins/docs-tools/skills/docs-workflow-writing/SKILL.md
- plugins/docs-tools/agents/evidence-classifier.md
- plugins/docs-tools/skills/docs-workflow-writing/scripts/build_writing_args.sh
DonaghBr
force-pushed
the
improve-code-evidence
branch
from
c6792fe to
034ab62
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
plugins/docs-tools/skills/docs-workflow-writing/scripts/build_writing_args.sh (1)
37-87:⚠️ Potential issue | 🟠 Major | ⚡ Quick winWire
--import-fromthrough the parser before exportingimport_from.The script now publishes an
import_fromfield, but there is no--import-fromcase in the CLI parser, so the value is alwaysnulland the new import contract cannot actually be reached. Either add the missing flag handling here or remove the field until import mode is implemented.Also applies to: 175-208
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/docs-tools/skills/docs-workflow-writing/scripts/build_writing_args.sh` around lines 37 - 87, The parser is missing handling for the --import-from flag so the exported import_from is always null; add a case in the argument-parsing loop to accept --import-from by calling require_arg "$1" "${2:-}", assign IMPORT_FROM="$2", and shift 2 (mirror the pattern used for --repo/--base-path/--format), and ensure the variable used when exporting import_from later references IMPORT_FROM; also apply the same addition in the second parsing block around lines 175-208 where duplicated parsing occurs so both code paths populate IMPORT_FROM consistently.plugins/docs-tools/skills/docs-workflow-tech-review/SKILL.md (1)
40-155: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftMove the pre-scan out of
SKILL.md.This block is doing argument handling, path setup, file discovery, command selection, and output wiring inline. That conflicts with the repo rule that docs-workflow step skills should keep procedural logic in
skills/<skill-name>/scripts/and leaveSKILL.mddeclarative. Based on learnings: step skills must defer argument parsing, mode determination, input validation, path computation, and directory creation to a script.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/docs-tools/skills/docs-workflow-tech-review/SKILL.md` around lines 40 - 155, The SKILL.md contains procedural pre-scan logic (argument parsing, path setup, HAS_REPO/HAS_GROUNDED/HAS_API_SURFACE flags, DRAFTS_DIR, OUTPUT_DIR/GROUNDED_FILE/API_SURFACE_FILE generation, drafts-batch.json creation, and invoking grounded_review.py and api_surface.py) that must be moved into a script; extract all runtime logic into a new script under skills/docs-workflow-tech-review/scripts/ (e.g., pre_scan.sh or pre_scan.py) that accepts the ticket ID, --base-path, and optional --repo and performs: argument parsing, directory creation, mode detection (reading writing/step-result.json and _index.md), building drafts-batch.json, invoking grounded_review.py and api_surface.py with the same flags and setting HAS_GROUNDED/HAS_API_SURFACE, and writing GROUNDED_FILE/API_SURFACE_FILE and CODE_EVIDENCE_SUMMARY; then replace the SKILL.md block with a short declarative description that calls the script and documents its outputs (GROUNDED_FILE, API_SURFACE_FILE, CODE_EVIDENCE_SUMMARY) without inline shell logic.
♻️ Duplicate comments (1)
plugins/docs-tools/skills/docs-workflow-tech-review/SKILL.md (1)
173-194:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGuard the API-surface reference on
HAS_API_SURFACEtoo.
<API_SURFACE_FILE>is still mentioned wheneverHAS_GROUNDED=true, even if API extraction failed. That can send the reviewer to a file that does not exist and makes the fallback path inconsistent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/docs-tools/skills/docs-workflow-tech-review/SKILL.md` around lines 173 - 194, The SKILL.md grounded-review block currently always mentions <API_SURFACE_FILE> when HAS_GROUNDED is true; update the template to only include or reference <API_SURFACE_FILE> when HAS_API_SURFACE is true as well (i.e., guard the API surface cross-reference behind both HAS_GROUNDED and HAS_API_SURFACE), so modify the conditional logic that renders the "Cross-reference the API surface at `<API_SURFACE_FILE>`" line to check HAS_API_SURFACE before inserting <API_SURFACE_FILE>; ensure related symbols in the block such as HAS_REPO, HAS_GROUNDED, GROUNDED_FILE, and CODE_EVIDENCE_SUMMARY 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/docs-workflow-writing/SKILL.md`:
- Around line 24-42: The SKILL contract advertises mode: import and import_from
but no implementation exists; either implement an import execution branch or
remove it from the public schema. If you implement it, add an "import" branch in
the skill dispatcher (the code path that switches on the mode), implement a
handler (e.g., handleImport / execute import branch) that consumes import_from,
defines prompt and placement rules consistent with other modes (use the same
buildPrompt/placement logic), and wire verify_output/has_evidence handling for
imports; otherwise remove "import" and "import_from" from the SKILL JSON block
and any related validation so callers cannot request a non-existent mode. Ensure
references to "mode" and "import_from" in the dispatcher, prompt generation, and
validation are kept consistent.
---
Outside diff comments:
In `@plugins/docs-tools/skills/docs-workflow-tech-review/SKILL.md`:
- Around line 40-155: The SKILL.md contains procedural pre-scan logic (argument
parsing, path setup, HAS_REPO/HAS_GROUNDED/HAS_API_SURFACE flags, DRAFTS_DIR,
OUTPUT_DIR/GROUNDED_FILE/API_SURFACE_FILE generation, drafts-batch.json
creation, and invoking grounded_review.py and api_surface.py) that must be moved
into a script; extract all runtime logic into a new script under
skills/docs-workflow-tech-review/scripts/ (e.g., pre_scan.sh or pre_scan.py)
that accepts the ticket ID, --base-path, and optional --repo and performs:
argument parsing, directory creation, mode detection (reading
writing/step-result.json and _index.md), building drafts-batch.json, invoking
grounded_review.py and api_surface.py with the same flags and setting
HAS_GROUNDED/HAS_API_SURFACE, and writing GROUNDED_FILE/API_SURFACE_FILE and
CODE_EVIDENCE_SUMMARY; then replace the SKILL.md block with a short declarative
description that calls the script and documents its outputs (GROUNDED_FILE,
API_SURFACE_FILE, CODE_EVIDENCE_SUMMARY) without inline shell logic.
In
`@plugins/docs-tools/skills/docs-workflow-writing/scripts/build_writing_args.sh`:
- Around line 37-87: The parser is missing handling for the --import-from flag
so the exported import_from is always null; add a case in the argument-parsing
loop to accept --import-from by calling require_arg "$1" "${2:-}", assign
IMPORT_FROM="$2", and shift 2 (mirror the pattern used for
--repo/--base-path/--format), and ensure the variable used when exporting
import_from later references IMPORT_FROM; also apply the same addition in the
second parsing block around lines 175-208 where duplicated parsing occurs so
both code paths populate IMPORT_FROM consistently.
---
Duplicate comments:
In `@plugins/docs-tools/skills/docs-workflow-tech-review/SKILL.md`:
- Around line 173-194: The SKILL.md grounded-review block currently always
mentions <API_SURFACE_FILE> when HAS_GROUNDED is true; update the template to
only include or reference <API_SURFACE_FILE> when HAS_API_SURFACE is true as
well (i.e., guard the API surface cross-reference behind both HAS_GROUNDED and
HAS_API_SURFACE), so modify the conditional logic that renders the
"Cross-reference the API surface at `<API_SURFACE_FILE>`" line to check
HAS_API_SURFACE before inserting <API_SURFACE_FILE>; ensure related symbols in
the block such as HAS_REPO, HAS_GROUNDED, GROUNDED_FILE, and
CODE_EVIDENCE_SUMMARY 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: fd9b107b-e37e-442d-b88b-b24b750ad346
📒 Files selected for processing (12)
plugins/docs-tools/agents/evidence-classifier.mdplugins/docs-tools/agents/requirements-analyst.mdplugins/docs-tools/skills/docs-orchestrator/SKILL.mdplugins/docs-tools/skills/docs-orchestrator/defaults/docs-workflow-code-evidence.yamlplugins/docs-tools/skills/docs-orchestrator/schema/step-result-schema.mdplugins/docs-tools/skills/docs-workflow-code-evidence/SKILL.mdplugins/docs-tools/skills/docs-workflow-planning/SKILL.mdplugins/docs-tools/skills/docs-workflow-requirements/SKILL.mdplugins/docs-tools/skills/docs-workflow-scope-req-audit/SKILL.mdplugins/docs-tools/skills/docs-workflow-tech-review/SKILL.mdplugins/docs-tools/skills/docs-workflow-writing/SKILL.mdplugins/docs-tools/skills/docs-workflow-writing/scripts/build_writing_args.sh
✅ Files skipped from review due to trivial changes (4)
- plugins/docs-tools/skills/docs-orchestrator/defaults/docs-workflow-code-evidence.yaml
- plugins/docs-tools/skills/docs-workflow-requirements/SKILL.md
- plugins/docs-tools/agents/requirements-analyst.md
- plugins/docs-tools/skills/docs-workflow-code-evidence/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (4)
- plugins/docs-tools/skills/docs-orchestrator/schema/step-result-schema.md
- plugins/docs-tools/skills/docs-workflow-planning/SKILL.md
- plugins/docs-tools/skills/docs-workflow-scope-req-audit/SKILL.md
- plugins/docs-tools/agents/evidence-classifier.md
Comment on lines
24
to
42
| ```json | ||
| { | ||
| "mode": "update-in-place | draft | fix", | ||
| "ticket": "PROJ-123", | ||
| "format": "adoc | mkdocs", | ||
| "input_file": "<base-path>/planning/plan.md", | ||
| "evidence_file": "<base-path>/code-evidence/evidence.json | null", | ||
| "has_evidence": true | false, | ||
| "output_dir": "<base-path>/writing", | ||
| "output_file": "<base-path>/writing/_index.md", | ||
| "docs_repo_path": "<path> | null", | ||
| "source_repo_path": "<path> | null", | ||
| "fix_from": "<path> | null", | ||
| "verify_output": true | false | ||
| "mode": "update-in-place | draft | fix | import", | ||
| "ticket": "PROJ-123", | ||
| "format": "adoc | mkdocs", | ||
| "input_file": "<base-path>/planning/plan.md", | ||
| "evidence_file": "<base-path>/code-evidence/evidence.json | null", | ||
| "has_evidence": true | false, | ||
| "evidence_status": "<base-path>/scope-req-audit/evidence-status.json | null", | ||
| "has_evidence_status": true | false, | ||
| "output_dir": "<base-path>/writing", | ||
| "output_file": "<base-path>/writing/_index.md", | ||
| "docs_repo_path": "<path> | null", | ||
| "source_repo_path": "<path> | null", | ||
| "fix_from": "<path> | null", | ||
| "import_from": "<path> | null", | ||
| "verify_output": true | false | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Add an import execution branch or drop it from the public contract.
The JSON schema now advertises mode: import and import_from, but the rest of the skill never defines import behavior. As written, callers can request a mode that has no prompt/placement rules, so the contract and dispatcher will diverge.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/docs-tools/skills/docs-workflow-writing/SKILL.md` around lines 24 -
42, The SKILL contract advertises mode: import and import_from but no
implementation exists; either implement an import execution branch or remove it
from the public schema. If you implement it, add an "import" branch in the skill
dispatcher (the code path that switches on the mode), implement a handler (e.g.,
handleImport / execute import branch) that consumes import_from, defines prompt
and placement rules consistent with other modes (use the same
buildPrompt/placement logic), and wire verify_output/has_evidence handling for
imports; otherwise remove "import" and "import_from" from the SKILL JSON block
and any related validation so callers cannot request a non-existent mode. Ensure
references to "mode" and "import_from" in the dispatcher, prompt generation, and
validation are kept consistent.
ba80223 to
2f9a0f3
Compare
Extend code-evidence grounding from 2 of 7 steps to all steps that benefit from it. The repo is already cloned and indexed — these changes make every step smarter about using that evidence. Changes by step: 1. Technical review: run grounded_review.py and api_surface.py as a pre-scan when source repo is available. Pass verdicts to the technical-reviewer agent as pre-computed evidence. 2. Writing: pass evidence-status.json (grounded/partial/absent per requirement), source repo path, and anti-fabrication guardrails. Writer uses [NEEDS VERIFICATION] for partial requirements, skips absent ones. 3. Code-evidence step: run api_surface.py alongside find_evidence.py and include API surface in evidence.json. Seed additional queries from scope-req-audit key_files for better retrieval quality. 4. Requirements: requirements-analyst verifies features exist in code, identifies existing docs, extracts project metadata, and notes code references when repo is available. 5. Planning: planner references key_files from evidence-status as content source pointers in module specifications. 6. Scope-req-audit: evidence-classifier uses 2-3 query reformulations to reduce false-absent classifications. API surface extracted in pre-flight and passed to classifiers as supplementary evidence. Also: code-evidence workflow no longer hard-fails without explicit --source-code-repo. Pre-flight attempts JIRA auto-discovery first, then fails with clear actionable options if no repo is found. Made-with: Cursor Co-authored-by: Cursor <cursoragent@cursor.com>
DonaghBr
force-pushed
the
improve-code-evidence
branch
from
2f9a0f3 to
4cc3c62
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
grounded_review.pyandapi_surface.pyto the technical review step as a pre-scan, giving the reviewer concrete code verdicts instead of relying solely on engineering judgment[NEEDS VERIFICATION]markers for partial requirements and skip absent ones--source-code-repo(but hard-stops if no repo is found from any source)Changes by step
grounded_review.py+api_surface.pypre-scan; pass verdicts totechnical-revieweragentevidence-status.json, source repo path; per-requirement fabrication guardrailsapi_surface.py, seed queries from scope-req-auditkey_filesrequirements-analystverifies features in code, identifies existing docs, extracts metadatakey_filesas content source pointers in module specsDepends on
Test plan
docs-orchestrator RHAISTRAT-1084 --workflow code-evidence --source-code-repo https://github.com/opendatahub-io/model-registry --docs-repo-path <docs-fork>and verify each step's outputbuild_writing_args.shemitsevidence_status,has_evidence_status,source_repofields (unit test passes)--source-code-repoattempts JIRA auto-discovery and fails with clear message if no repo foundMade with Claude Opus 4.6
Summary by CodeRabbit
New Features
Documentation