feat: add automated core rpc help extraction and comparison tools #527
Conversation
- Reorganize report to show consolidated table before detailed sections - Enhance change detail rendering with grouped, nested bullet points - Add emoji indicators for change types (➕ added, ➖ removed,⚠️ deprecated, 🔁 replacement) - Replace verbose "Notes" column with concise "Change Types" in table - Add anchor links from table to detailed sections for navigation - Filter out docs-only changes from consolidated table - Improve spacing and markdown code block formatting
WalkthroughAdds documentation and two CLI tools: a Bash extractor that dumps dash-cli RPC help into text and optional JSONL, and a Python comparator that reads two JSONL dumps and produces a versioned markdown summary of RPC API changes. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Dump as dump-cli-help.sh
participant CLI as dash-cli
participant JSONL1 as "JSONL v1"
participant JSONL2 as "JSONL v2"
participant Diff as generate-rpc-change-summary.py
participant Report as "Markdown Report"
User->>Dump: Run for version 1
Dump->>CLI: Query top-level help
CLI-->>Dump: Top-level help text
Dump->>CLI: Query per-command help (and subcommands)
CLI-->>Dump: Per-command help texts
Dump->>JSONL1: Write metadata + command records
User->>Dump: Run for version 2
Dump->>CLI: Repeat queries
Dump->>JSONL2: Write metadata + command records
User->>Diff: Compare JSONL v1 & v2
Diff->>JSONL1: Read metadata & records
JSONL1-->>Diff: Records
Diff->>JSONL2: Read metadata & records
JSONL2-->>Diff: Records
rect rgb(235,245,235)
note over Diff: Analysis (signatures, args, results, deprecations)
Diff->>Diff: Categorize added/removed/modified
Diff->>Diff: Compute per-RPC diffs
end
Diff->>Report: Emit markdown summary with stats & categorized changes
Report-->>User: Deliver report
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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: 9
🧹 Nitpick comments (8)
scripts/core-rpc-tools/README.md (2)
29-32: Consider adding network sync verification.The setup instructions mention ensuring the node is synced, but the verification command
dash-cli -testnet getblockchaininfodoesn't demonstrate how to check sync status. Users unfamiliar with Dash might not know what output indicates a synced node.Consider adding a brief note about checking the
blocksvsheadersfields orverificationprogressto confirm sync status.
53-54: Example command references non-existent version.The example uses version
22.1.3but the PR title and objectives reference version22.0.0as the target branch. While this is just an example, using consistent version numbers would reduce confusion.Consider using placeholder text like
<old-version>or aligning the example with actual versions mentioned in the PR context.scripts/core-rpc-tools/dump-cli-help.sh (2)
52-55: Version extraction could fail silently with misleading results.The version extraction uses a non-fatal approach (
|| true) which is good, but the regex'.*v\([0-9][^ ]*\).*/\1/p'may not match all version formats (e.g., if there's no 'v' prefix, or if the format changes). An empty VERSION would result in non-versioned filenames.Consider adding a warning if version extraction fails:
VERSION="$(sed -n 's/.*v\([0-9][^ ]*\).*/\1/p' <<< "$full_version")" +if [[ -z "$VERSION" ]]; then + echo "warning: could not extract version, using timestamp-only filenames" >&2 +fi
201-201: Complex boolean expression for is_family check.Line 201 uses command substitution within jq's
--argjsonto determine if a command is a family RPC. The logic$([[ $(is_family "${CMD_TAIL[$cmd]}") ]] && echo true || echo false)is convoluted.Simplify by checking the result directly:
- --argjson isfam "$([[ $(is_family "${CMD_TAIL[$cmd]}") ]] && echo true || echo false)" \ + --argjson isfam "$(is_family "${CMD_TAIL[$cmd]}" && echo true || echo false)" \Actually,
is_familyreturns 0 (success) for family RPCs and non-zero otherwise, so you can test its exit code directly without capturing output.scripts/core-rpc-tools/generate-rpc-change-summary.py (4)
99-99: Unused loop variable.Static analysis correctly identifies that the loop index
ion Line 99 is unused. This is a minor code smell.Apply the static analysis suggestion:
- for i, line in enumerate(lines): + for line in lines:The
enumerateis unnecessary here since the indexiis never used.
296-299: Unused loop variable in replacement rendering.Line 296 has a loop
for line, repl in d.get("replacements", [])wherereplis not used. The code only useslineto format the output.Apply the static analysis suggestion:
- for line, repl in d.get("replacements", []): + for line, _repl in d.get("replacements", []): repl_parts.append(f"🔁 replacement hint: {line}")Or if
replwas intended to be used, consider showing the replacement target:for line, repl in d.get("replacements", []): - repl_parts.append(f"🔁 replacement hint: {line}") + repl_parts.append(f"🔁 {line} → {repl}")
312-330: Version resolution logic is clear but could fail silently.The function falls back to "old"/"new" if versions aren't found, which is reasonable. However, it prints diagnostic messages to stdout, which could interfere with piped output.
Use
sys.stderrfor diagnostic messages:- print(f"Parsing {old_file} (version: {meta_old_ver or 'n/a'})...") + print(f"Parsing {old_file} (version: {meta_old_ver or 'n/a'})...", file=sys.stderr) old_commands = parse_jsonl(old_file) - print(f"Found {len(old_commands)} commands in {old_version}") + print(f"Found {len(old_commands)} commands in {old_version}", file=sys.stderr)Apply similarly to lines 327-329.
399-400: Anchor link generation assumes GitHub markdown.Line 399 generates anchor links with
.replace(' ', '-').lower(). This works for GitHub-flavored markdown but may not work in other markdown renderers. GitHub also strips some special characters from anchors.For more robust anchor generation, consider removing or replacing special characters:
# More robust anchor generation import re anchor = re.sub(r'[^\w\s-]', '', key).strip().replace(' ', '-').lower() link = f"[`{key}`](#{anchor})"This ensures characters like backticks or parentheses don't break the anchor.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
scripts/core-rpc-tools/README.md(1 hunks)scripts/core-rpc-tools/dump-cli-help.sh(1 hunks)scripts/core-rpc-tools/generate-rpc-change-summary.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.14.3)
scripts/core-rpc-tools/generate-rpc-change-summary.py
94-94: Docstring contains ambiguous ’ (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
(RUF002)
99-99: Loop control variable i not used within loop body
Rename unused i to _i
(B007)
263-263: String contains ambiguous ➕ (HEAVY PLUS SIGN). Did you mean + (PLUS SIGN)?
(RUF001)
265-265: String contains ambiguous ➖ (HEAVY MINUS SIGN). Did you mean - (HYPHEN-MINUS)?
(RUF001)
272-272: String contains ambiguous ➕ (HEAVY PLUS SIGN). Did you mean + (PLUS SIGN)?
(RUF001)
274-274: String contains ambiguous ➖ (HEAVY MINUS SIGN). Did you mean - (HYPHEN-MINUS)?
(RUF001)
281-281: String contains ambiguous ➕ (HEAVY PLUS SIGN). Did you mean + (PLUS SIGN)?
(RUF001)
283-283: String contains ambiguous ➖ (HEAVY MINUS SIGN). Did you mean - (HYPHEN-MINUS)?
(RUF001)
296-296: Loop control variable repl not used within loop body
Rename unused repl to _repl
(B007)
🔇 Additional comments (11)
scripts/core-rpc-tools/README.md (1)
171-177: LGTM! Clear versioning guidance.The recommendation to keep JSONL files in version control is excellent for tracking historical RPC changes and provides a clear commit message template.
scripts/core-rpc-tools/dump-cli-help.sh (5)
38-43: Error handling captures stderr but may hide useful diagnostics.The
run_clifunction captures both stdout and stderr with2>&1, which is good for error reporting. However, legitimate stderr output (like progress indicators or warnings) will be mixed with stdout. For RPC help text, this is probably acceptable since help output should be on stdout only.
98-102: Regex pattern for family RPC detection is precise.The
is_familyfunction correctly detects family RPCs by checking for exactly one argument named "command". The regex^\"command\"([[:space:]]*)$properly handles trailing whitespace.
122-156: Subcommand discovery is robust with multiple parsing strategies.The
discover_subcommandsfunction handles various help text formats well:
- Stops at "Arguments:" section
- Handles multiple whitespace formats
- Splits on " - " delimiter with fallback
- Sorts and deduplicates results
The awk script properly handles edge cases like blank lines and trailing colons.
176-184: JSONL metadata generation is well-structured.The metadata header provides all necessary context (version, network, CLI path, timestamp, command count). The use of
jq -cnwith--argparameters properly escapes special characters, preventing JSON injection.
33-33: No issues found—review comment is incorrect.The bash implementation is sound. When
NET_ARGSis empty,read -r -a NET_ARR <<< ""creates an empty array (length 0), and the expansion"${NET_ARR[@]}"safely produces nothing—it doesn't introduce word splitting or other problems. This is the standard and correct bash idiom for optional arguments.Testing confirms:
- Empty array:
"${NET_ARR[@]}"safely expands to nothing- Non-empty single arg:
-testnetexpands correctly- Multiple args:
-testnet -rpcwaitboth captured and expanded correctlyThe script correctly uses this pattern across lines 38, 39, and 54 with no issues.
Likely an incorrect or invalid review comment.
scripts/core-rpc-tools/generate-rpc-change-summary.py (5)
54-58: Regex compilation is efficient.Compiling regex patterns at module level (lines 54-58) is a good practice for performance when these patterns are used repeatedly.
129-148: Result field parsing is well-scoped.The
parse_result_fieldsfunction correctly:
- Stops at "Examples:" section
- Uses a clear regex pattern for quoted field names
- Handles the typical JSON-style result format from RPC help
263-283: Unicode symbols are intentional for visual distinction.Static analysis flags the Unicode symbols (➕, ➖) as ambiguous, but these are clearly intentional for visual distinction in the markdown output. The warning is a false positive.
452-468: Key observations section provides valuable insights.The heuristic detection of Platform/EvoNode signatures and deprecated service fields provides helpful categorization for documentation maintainers. This is a nice touch for the executive summary.
90-128: Parser works correctly on tested RPC help formats; no false positives detected.The
parse_arguments_sectionfunction has been tested against 20+ real RPC commands from the Dash CLI help data (dash-cli-help-22.1.3.jsonl). All tested commands correctly extracted their arguments without false positives:
- Simple single arguments (e.g.,
txid,psbt)- Multiple arguments with varied types (e.g.,
nrequired,keys,label)- Optional arguments and complex signatures
Edge cases including JSON structures in descriptions, optional parameters, and multi-line argument blocks are properly handled. The numbered-list pattern correctly identifies arguments, the JSON-block filter (
not line.strip().startswith('{')) prevents false captures, and the function correctly terminates at empty lines or the "Result:" header.
Update readme with changes
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
scripts/core-rpc-tools/dump-cli-help.sh (1)
29-30: Remove debug echo statements.These debug statements should be removed to keep the output clean and avoid confusion.
Apply this diff:
-echo $CLI -echo $NET_ARGSscripts/core-rpc-tools/generate-rpc-change-summary.py (3)
13-30: Add file I/O error handling in read_metadata.The function doesn't handle
FileNotFoundErrororIOError, which will cause the script to crash with an unhandled exception if the file is missing or unreadable.Wrap the file operations in error handling:
def read_metadata(filepath): """ Read first JSONL line that contains {"metadata": {...}} and return that dict. Returns {} if not found. """ + try: - with open(filepath, 'r') as f: + with open(filepath, 'r') as f: - for line in f: + for line in f: - line = line.strip() + line = line.strip() - if not line: + if not line: - continue + continue - try: + try: - data = json.loads(line) + data = json.loads(line) - except json.JSONDecodeError: + except json.JSONDecodeError: - continue + continue - meta = data.get("metadata") + meta = data.get("metadata") - if isinstance(meta, dict): + if isinstance(meta, dict): - return meta + return meta + except (FileNotFoundError, IOError) as e: + print(f"Error reading {filepath}: {e}", file=sys.stderr) + return {} return {}Note: Add
import sysat the top if not already present.
32-50: Add file I/O error handling in parse_jsonl.Similar to
read_metadata, this function will crash if the file doesn't exist or contains I/O errors.Add error handling:
def parse_jsonl(filepath): """ Parse JSONL file and return dict: qualified -> record Skips metadata lines. """ cmds = {} + try: - with open(filepath, 'r') as f: + with open(filepath, 'r') as f: - for line in f: + for line in f: - if not line.strip(): + if not line.strip(): - continue + continue - data = json.loads(line) + try: + data = json.loads(line) + except json.JSONDecodeError as e: + print(f"Warning: Skipping malformed JSON in {filepath}: {e}", file=sys.stderr) + continue - if 'metadata' in data: + if 'metadata' in data: - continue + continue - # Use 'qualified' so subcommands aren't overwritten + # Use 'qualified' so subcommands aren't overwritten - key = data.get('qualified') or data.get('command') + key = data.get('qualified') or data.get('command') - if not key: + if not key: - continue + continue - cmds[key] = data + cmds[key] = data + except (FileNotFoundError, IOError) as e: + print(f"Error reading {filepath}: {e}", file=sys.stderr) + return {} return cmds
489-490: Add error handling for file write operation.The output file write doesn't handle potential I/O errors (disk full, permission denied, etc.), which could cause the script to crash after successfully generating the report.
Add error handling:
+ try: - with open(output_file, 'w') as f: + with open(output_file, 'w') as f: - f.write(report) + f.write(report) + except IOError as e: + print(f"Error writing output file {output_file}: {e}", file=sys.stderr) + sys.exit(1)
🧹 Nitpick comments (2)
scripts/core-rpc-tools/dump-cli-help.sh (1)
112-112: Unreachable error handler in append_help.The error handler
|| echo "(error retrieving help for: $*)"on Line 112 will never execute becauserun_cli(line 41) callsexit 1on failure, terminating the script before this fallback can run.If you want per-command error resilience, consider modifying
run_clito return errors instead of exiting, then handle them in the callers. Otherwise, this line can be simplified.scripts/core-rpc-tools/generate-rpc-change-summary.py (1)
90-127: Optional: Unused loop variable.Line 99 uses
enumeratebut never references the indexi. Consider using justfor line in lines:since the index isn't needed.Apply this diff if desired:
- for i, line in enumerate(lines): + for line in lines:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
scripts/core-rpc-tools/README.md(1 hunks)scripts/core-rpc-tools/dump-cli-help.sh(1 hunks)scripts/core-rpc-tools/generate-rpc-change-summary.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.14.3)
scripts/core-rpc-tools/generate-rpc-change-summary.py
94-94: Docstring contains ambiguous ’ (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCENT)?
(RUF002)
99-99: Loop control variable i not used within loop body
Rename unused i to _i
(B007)
263-263: String contains ambiguous ➕ (HEAVY PLUS SIGN). Did you mean + (PLUS SIGN)?
(RUF001)
265-265: String contains ambiguous ➖ (HEAVY MINUS SIGN). Did you mean - (HYPHEN-MINUS)?
(RUF001)
272-272: String contains ambiguous ➕ (HEAVY PLUS SIGN). Did you mean + (PLUS SIGN)?
(RUF001)
274-274: String contains ambiguous ➖ (HEAVY MINUS SIGN). Did you mean - (HYPHEN-MINUS)?
(RUF001)
281-281: String contains ambiguous ➕ (HEAVY PLUS SIGN). Did you mean + (PLUS SIGN)?
(RUF001)
283-283: String contains ambiguous ➖ (HEAVY MINUS SIGN). Did you mean - (HYPHEN-MINUS)?
(RUF001)
296-296: Loop control variable repl not used within loop body
Rename unused repl to _repl
(B007)
🔇 Additional comments (3)
scripts/core-rpc-tools/README.md (1)
1-176: Documentation is comprehensive and well-structured.The README provides clear explanations of the tooling workflow, prerequisites, configuration options, and output formats. The examples are practical and the file organization makes it easy to navigate. The filename references are consistent between the documentation and actual script files.
scripts/core-rpc-tools/dump-cli-help.sh (1)
186-258: Main loop implementation is correct for the chosen error handling strategy.The script uses an exit-on-first-error approach where any
dash-clifailure terminates the entire script (viarun_cliat line 41). This means the concerns raised in past review comments about "error text being written to JSONL" won't occur - the script exits cleanly before any JSONL write happens.This design is appropriate for a documentation extraction tool that expects a properly functioning
dash-cli. If more granular error handling is needed (e.g., skipping failed commands while continuing with others), it would require restructuringrun_clito return errors instead of exiting.scripts/core-rpc-tools/generate-rpc-change-summary.py (1)
164-238: Comparison logic is well-structured.The functions effectively categorize changes by comparing SHA256 hashes and performing detailed structural analysis of signatures, arguments, result fields, and deprecation notices. The logic is sound and comprehensive.
1 participant
Add Core RPC Documentation Tools
This PR introduces scripts for tracking and documenting Dash Core RPC changes between versions. These tools automate the extraction of RPC help text and generate detailed change summaries to streamline the documentation update process when new Dash Core versions are released.
Key Features
dump-cli-help.shscript captures all RPC command help text from dash-cli, including subcommands, and outputs both human-readable text and structured JSONL formatgenerate-rpc-change-summary.pyscript compares two versions and generates comprehensive change reports with detailed analysis of signature changes, deprecated fields, added/removed arguments, and result modificationsPreview build: https://dash-docs--527.org.readthedocs.build/en/527/
Summary by CodeRabbit
Documentation
Chores