Skip to content

Add scriptsInfo metadata and CI validation for script documentation - #135

Merged
Ryan-Millard merged 21 commits into
mainfrom
refactor/help-script/issue-124
Dec 17, 2025
Merged

Add scriptsInfo metadata and CI validation for script documentation#135
Ryan-Millard merged 21 commits into
mainfrom
refactor/help-script/issue-124

Conversation

@Ryan-Millard

@Ryan-Millard Ryan-Millard commented Dec 17, 2025

Copy link
Copy Markdown
Owner

🧹 Chore / Maintenance Pull Request

Code refactoring, dependency updates, etc.

📌 What’s Changing

Refactor help scripts in both main app and docs app to:

  • reuse the same logic
  • use a source-of-truth scriptsInfo field in package.json that contains metadata about custom scripts
  • improve fzf functionality
  • improve layout & styles (format, colors, whatever)
  • add ci.yml to enforce this so scripts don't get forgotten

🔗 Context

Fixes #124

Only changing

  • scripts & docs/scripts
  • package.json & docs/package.json
  • github/workflows/ci.yml

Added some lib scripts to root for reuse inside docs scripts

🔍 Impact

New:
CI builds - ci.yml
root scripts lib folder - shared js scripts between main & docs apps

🧓 Pre-existing:
package.json files for both apps
help.js scripts for both apps

🧪 Verification

Numerous local runs for both apps - if one works, the other should, too.

✔️ Checklist

  • I’ve read the contribution guidelines.
  • CI/CD passes.
    • There is no CI/CD on main yet.
  • No unintended changes to production code.

Summary by CodeRabbit

  • New Features

    • Centralized scripts metadata, a new validate-scripts command, and an improved interactive, colorized fuzzy-search help CLI.
  • Chores

    • CI workflow added to run script validation on pushes and PRs; failing PR validations post a comment.
    • Reusable workflow added to post comments from workflows.
  • Documentation

    • New project-scripts docs, updated getting-started links, and removal of an older help page.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai

coderabbitai Bot commented Dec 17, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

Adds centralized scriptsInfo metadata to root and docs package.json, a validator script, refactored help CLIs with shared libraries, a CI workflow that validates script-related changes, and a reusable commenter workflow that posts validation failures to PRs; documentation pages updated accordingly.

Changes

Cohort / File(s) Summary
GitHub Actions
**.github/workflows/ci.yml**, **.github/workflows/commenter.yml**
New CI workflow triggers on PRs/pushes to main, detects script-related changes via git diff, runs validate-scripts when relevant, captures validation.log, and calls the reusable commenter workflow to post PR comments on failure. commenter.yml posts issue/PR comments via workflow_call.
Root package manifest
**package.json**
Adds top-level scriptsInfo metadata and new script "validate-scripts": "node scripts/validate-scripts.js".
Docs package manifest
**docs/package.json**
Adds top-level scriptsInfo, a docs-level validate-scripts entry (npm run --prefix .. validate-scripts), and related metadata additions.
Validation tool
**scripts/validate-scripts.js**
New validator that reads ./package.json and ./docs/package.json, flattens scriptsInfo, enforces strict 1:1 mapping with scripts, logs mismatches and exits non‑zero on errors.
Help CLIs (refactor)
**scripts/help.js**, **docs/scripts/help.js**
Converted to ES module style; now read scripts/scriptsInfo via helper, delegate listing/search to the fuzzy CLI orchestrator, accept CLI args for initial search, and add error handling.
CLI utilities
**scripts/lib/cli-fuzzy.js**, **scripts/lib/read-packageJson-scripts.js**, **scripts/lib/colors.js**
New modules: cli-fuzzy.js exports runFuzzyCli({...}); read-packageJson-scripts.js exports readPackageJsonScripts(fileUrl) which flattens scriptsInfo and returns basicItems; colors.js exports Colors, colorText, logColor.
Docs content
**docs/docs/project-scripts/***, **docs/docs/introduction/getting-started.md**
Added help pages and a detailed scripts guide, removed an older help doc, and appended a "Further Information" subsection to Getting Started.

Sequence Diagram(s)

sequenceDiagram
    participant GH as GitHub
    participant CI as CI Workflow
    participant Runner as Job Runner
    participant Validator as validate-scripts
    participant PKG as package.json(s)
    participant Commenter as commenter workflow
    participant PR as Pull Request

    GH->>CI: trigger on push/main or pull_request
    CI->>Runner: checkout (fetch-depth:0), setup Node, npm ci
    Runner->>Runner: compute common ancestor & changed files
    alt scripts changed
        Runner->>Validator: run validate-scripts (capture validation.log)
        Validator->>PKG: read & parse root and docs package.json
        Validator->>Validator: flatten scriptsInfo and compare with scripts
        alt validation fails
            Validator->>Runner: exit non-zero (validation.log)
            Runner->>Commenter: workflow_call(issue_number, message=validation.log)
            Commenter->>PR: create comment with validation output
        else validation passes
            Validator->>Runner: exit 0
        end
    else no script changes
        Runner->>CI: skip validation
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~30 minutes

  • Inspect flattening and _meta handling in scripts/validate-scripts.js.
  • Verify JSON/file reading and error handling in scripts/lib/read-packageJson-scripts.js.
  • Review interactive lifecycle and prompt teardown in scripts/lib/cli-fuzzy.js.
  • Validate CI git-diff detection logic, fetch-depth semantics, and the commenter.yml REST call and permissions.

Poem

🐰 I nibbled JSON keys and lined each name,
I hopped through scripts and gave each one a frame.
Fuzzy hops in CLI glow, validation bells ring,
CI watches, comments — then we merge with a spring.
Hooray for tidy scripts; I twitch and claim the fling!

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: adding scriptsInfo metadata and CI validation for script documentation, which aligns with the primary objectives of the PR.
Linked Issues check ✅ Passed The PR fully meets all coding requirements from issue #124: scriptsInfo metadata added to both package.json files [#124], help scripts refactored to use package.json as source of truth [#124], and CI validation workflow implemented for 1:1 mapping enforcement [#124].
Out of Scope Changes check ✅ Passed All changes are directly aligned with issue #124 objectives: scriptsInfo metadata, refactored help scripts, CI workflows, and documentation updates. No unrelated modifications detected.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch refactor/help-script/issue-124

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e4dd3f1 and c63c6d7.

📒 Files selected for processing (4)
  • scripts/lib/cli-fuzzy.js (1 hunks)
  • scripts/lib/colors.js (1 hunks)
  • scripts/lib/read-packageJson-scripts.js (1 hunks)
  • scripts/validate-scripts.js (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • scripts/lib/read-packageJson-scripts.js
  • scripts/lib/colors.js
  • scripts/validate-scripts.js
  • scripts/lib/cli-fuzzy.js

Comment @coderabbitai help to get the list of available commands and usage tips.

@Ryan-Millard
Ryan-Millard force-pushed the refactor/help-script/issue-124 branch 2 times, most recently from 81431bb to 643c839 Compare December 17, 2025 15:13

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (8)
scripts/lib/colors.js (2)

26-45: Consider freezing the codes object for consistency.

While the ANSI mappings are correct, freezing the codes object would align with the immutability pattern used for the Colors enum and prevent accidental runtime modifications.

Apply this diff if desired:

-const codes = {
+const codes = Object.freeze({
   reset: "\x1b[0m",
   bold: "\x1b[1m",
   dim: "\x1b[2m",
   red: "\x1b[31m",
   green: "\x1b[32m",
   yellow: "\x1b[33m",
   blue: "\x1b[34m",
   magenta: "\x1b[35m",
   cyan: "\x1b[36m",
   white: "\x1b[37m",
   bgRed: "\x1b[41m",
   bgGreen: "\x1b[42m",
   bgYellow: "\x1b[43m",
   bgBlue: "\x1b[44m",
   bgMagenta: "\x1b[45m",
   bgCyan: "\x1b[46m",
   bgWhite: "\x1b[47m",
-};
+});

47-51: Consider adding input validation for the text parameter.

While the color support and enum validation are handled correctly, non-string inputs (null, undefined, objects) will be coerced to strings, which may produce unexpected output like "undefined" or "[object Object]".

For more robust behavior, consider:

 export function colorText(text, colorEnum) {
+  if (text == null) return '';
   if (!supportsColor || !codes[colorEnum]) return text;
   return `${codes[colorEnum]}${text}${codes.reset}`;
 }
package.json (1)

79-84: Inconsistent args format in lint:fix.

The args array for lint:fix contains "Equivalent to: eslint . --fix", which differs from the standard format used elsewhere (e.g., "-- fix Automatically fix fixable issues"). For consistency, either remove this entry or reformat it to match the pattern used by other scripts.

Apply this diff to align with the established pattern:

     "lint:fix": {
       "desc": "Run ESLint and automatically fix issues",
       "args": [
-        "Equivalent to: eslint . --fix"
+        "Note: Runs 'eslint . --fix'"
       ]
     },

Or simply remove the args field if no additional context is needed:

     "lint:fix": {
-      "desc": "Run ESLint and automatically fix issues",
-      "args": [
-        "Equivalent to: eslint . --fix"
-      ]
+      "desc": "Run ESLint and automatically fix issues"
     },
scripts/validate-scripts.js (1)

5-7: Add error handling for file operations.

The function lacks error handling for file read or JSON parse failures. If the file doesn't exist or contains invalid JSON, the error message won't be helpful.

Apply this diff to add better error handling:

 function loadPackageJson(filePath) {
-  return JSON.parse(fs.readFileSync(filePath, "utf-8"));
+  try {
+    return JSON.parse(fs.readFileSync(filePath, "utf-8"));
+  } catch (error) {
+    console.error(`❌ Failed to load ${filePath}: ${error.message}`);
+    process.exit(1);
+  }
 }
.github/workflows/commenter.yml (1)

61-77: Remove redundant conditional logic.

Both branches of the if (inputs.is_pr) statement perform identical createComment calls on the issues API. The distinction between PR and issue is unnecessary here since GitHub's API treats PR comments as issue comments.

Apply this diff to simplify:

-            if (inputs.is_pr) {
-              // Pull Request comment
-              await github.rest.issues.createComment({
-                owner,
-                repo,
-                issue_number: number,
-                body
-              });
-            } else {
-              // Issue comment
-              await github.rest.issues.createComment({
-                owner,
-                repo,
-                issue_number: number,
-                body
-              });
-            }
+            // GitHub API treats PR comments as issue comments
+            await github.rest.issues.createComment({
+              owner,
+              repo,
+              issue_number: number,
+              body
+            });

You can keep the is_pr input for documentation purposes or remove it entirely if not needed.

.github/workflows/ci.yml (1)

13-16: Update GitHub Actions to latest versions.

Static analysis tools report that actions/checkout@v3 and actions/setup-node@v3 are outdated. Update to v4 for both actions to ensure compatibility with current GitHub Actions runners.

Apply this diff:

-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
       - name: Setup Node.js
-        uses: actions/setup-node@v3
+        uses: actions/setup-node@v4
         with:
           node-version: 22
scripts/lib/cli-fuzzy.js (1)

51-54: Review early return logic for non-interactive mode.

When skipIfInitialSearch is true, the function closes the readline interface and returns immediately. However, the line and close event handlers are registered after this check (lines 56-69). Consider moving the early return before setting up the handlers, or restructure the flow to avoid registering unused handlers.

   rl.setPrompt(colorText("> ", Colors.CYAN));
-  rl.prompt();
 
   // If initialSearch was provided, and we just want one-shot results, skip the interactive prompt
   if (skipIfInitialSearch) {
     return rl.close();
   }
 
+  rl.prompt();
+
   rl.on("line", line => {
scripts/help.js (1)

9-9: Add optional error handling for robustness.

The data reading logic is correct and uses the proper ESM pattern with new URL(). However, the code lacks error handling if readPackageJsonScripts fails (e.g., missing or malformed package.json).

Consider adding a try-catch block for better error messages:

+try {
   const { flat: items, basicItems } = readPackageJsonScripts(new URL("../package.json", import.meta.url));
+} catch (error) {
+  console.error("Failed to read package.json scripts:", error.message);
+  process.exit(1);
+}
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f5106cb and f8f35da.

📒 Files selected for processing (10)
  • .github/workflows/ci.yml (1 hunks)
  • .github/workflows/commenter.yml (1 hunks)
  • docs/package.json (2 hunks)
  • docs/scripts/help.js (1 hunks)
  • package.json (2 hunks)
  • scripts/help.js (1 hunks)
  • scripts/lib/cli-fuzzy.js (1 hunks)
  • scripts/lib/colors.js (1 hunks)
  • scripts/lib/read-packageJson-scripts.js (1 hunks)
  • scripts/validate-scripts.js (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
docs/scripts/help.js (2)
scripts/lib/read-packageJson-scripts.js (1)
  • readPackageJsonScripts (3-23)
scripts/lib/cli-fuzzy.js (1)
  • runFuzzyCli (5-19)
scripts/lib/read-packageJson-scripts.js (1)
scripts/lib/cli-fuzzy.js (2)
  • groups (85-85)
  • group (88-88)
scripts/validate-scripts.js (2)
scripts/lib/read-packageJson-scripts.js (1)
  • JSON (4-4)
scripts/lib/cli-fuzzy.js (1)
  • group (88-88)
scripts/help.js (3)
docs/scripts/help.js (3)
  • title (4-7)
  • items (9-9)
  • initialSearch (12-12)
scripts/lib/read-packageJson-scripts.js (1)
  • readPackageJsonScripts (3-23)
scripts/lib/cli-fuzzy.js (1)
  • runFuzzyCli (5-19)
🪛 actionlint (1.7.9)
.github/workflows/ci.yml

13-13: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)


15-15: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)

🔇 Additional comments (11)
scripts/lib/colors.js (3)

1-3: LGTM!

The TTY-based color detection is the standard approach for Node.js terminal utilities.


5-24: LGTM!

The frozen enum pattern provides immutability and a clear contract for allowed color values. The comprehensive palette covers common terminal styling needs.


53-56: LGTM!

Clean convenience wrapper that delegates appropriately to colorText.

docs/package.json (1)

1-137: LGTM! scriptsInfo structure is complete and consistent.

The scriptsInfo metadata is well-structured and covers all scripts defined in the scripts section. The delegation to the root validate-scripts via npm run --prefix .. is a clean approach to reuse validation logic.

docs/scripts/help.js (1)

1-19: LGTM! Clean refactoring to use shared utilities.

The refactored help script elegantly delegates to the new shared libraries. The use of import.meta.url for path resolution is appropriate for ES modules.

scripts/lib/cli-fuzzy.js (1)

1-115: LGTM! Well-structured interactive CLI with fuzzy search.

The implementation provides a clean API with good separation of concerns. The fuzzy search integration, colored output, and grouped display are well-executed. The defensive handling of info.desc as both array and string (lines 106-112) is good practice.

scripts/validate-scripts.js (1)

55-59: Verify path resolution for different working directories.

The script uses path.resolve("./package.json") which resolves relative to the current working directory. Ensure the validation script is always invoked from the repository root, or adjust to use import.meta.url or process.cwd() with explicit directory checks.

Run this script to confirm the validation script is always invoked from the repository root:

#!/bin/bash
# Check where validate-scripts is invoked from in package.json

rg -n "validate-scripts" package.json docs/package.json
scripts/help.js (4)

4-7: LGTM!

The title constant is clear and informative, providing both a header for the CLI and a link to additional documentation.


11-12: LGTM!

The CLI argument capture is implemented correctly using the standard Node.js pattern. The comment explains the purpose clearly, and process.argv.slice(2) will correctly capture arguments passed via npm run help -- search-term.


14-19: Excellent refactor improving modularity!

The orchestration is clean and well-structured. By delegating to runFuzzyCli, the code achieves better separation of concerns and reusability, which aligns perfectly with the PR objectives to unify help scripts across the repository.


1-2: Imports are correctly structured and reference valid modules.

Both cli-fuzzy.js and read-packageJson-scripts.js exist in scripts/lib/ with the expected exports: runFuzzyCli and readPackageJsonScripts respectively. The imports at lines 1-2 are properly configured for ESM modules.

Comment thread .github/workflows/ci.yml Outdated
Comment thread .github/workflows/ci.yml
Comment thread scripts/lib/read-packageJson-scripts.js
Comment thread scripts/lib/read-packageJson-scripts.js
@Ryan-Millard
Ryan-Millard force-pushed the refactor/help-script/issue-124 branch from f8f35da to dee4913 Compare December 17, 2025 15:38
@Ryan-Millard Ryan-Millard changed the title Refactor/help script/issue 124 Add scriptsInfo metadata and CI validation for script documentation Dec 17, 2025

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

♻️ Duplicate comments (2)
.github/workflows/ci.yml (2)

34-35: Add fallback for initial push scenario.

The git diff command uses ${{ github.event.before }} which will be all-zeros (0000000000000000000000000000000000000000) on the first push to a branch or after a force push, causing the diff to fail or compare against an invalid commit.

Apply this diff to add a fallback:

-          files=$(git diff --name-only ${{ github.event.before }} ${{ github.sha }})
+          if [[ "${{ github.event.before }}" == "0000000000000000000000000000000000000000" ]]; then
+            files=$(git diff --name-only HEAD~1 HEAD 2>/dev/null || git ls-files)
+          else
+            files=$(git diff --name-only ${{ github.event.before }} ${{ github.sha }})
+          fi
           echo "Changed files: $files"

63-74: Move reusable workflow call to a separate job.

Reusable workflows must be invoked as separate jobs using uses: at the job level, not as steps within a job. The current syntax at line 65 is invalid and will cause the workflow to fail.

Apply this diff to fix the workflow structure:

       - name: Run script validation
         id: validate
         if: steps.check_changes.outputs.scripts_changed == 'true'
+        continue-on-error: true
         shell: bash
         run: |
           set -o pipefail
           npm run validate-scripts 2>&1 | tee validation.log
           echo "validation_exit_code=${PIPESTATUS[0]}" >> $GITHUB_OUTPUT
+          echo "validation_log<<EOF" >> $GITHUB_OUTPUT
+          cat validation.log >> $GITHUB_OUTPUT
+          echo "EOF" >> $GITHUB_OUTPUT

-      # Comment on PR if validation fails
-      - name: Comment on PR if validation fails
-        if: steps.validate.outputs.validation_exit_code != '0' && github.event_name == 'pull_request'
-        uses: ./.github/workflows/commenter.yml
-        with:
-          issue_number: ${{ github.event.pull_request.number }}
-          message: |
-            ⚠️ **Script validation failed!**
-            The following issues were found with your PR (latest 100 lines):
-            ```
-            $(tail -n 100 validation.log)
-            ```
-          is_pr: true
+
+  comment-on-failure:
+    needs: validate-scripts
+    if: needs.validate-scripts.outputs.validation_exit_code != '0' && github.event_name == 'pull_request'
+    uses: ./.github/workflows/commenter.yml
+    with:
+      issue_number: ${{ github.event.pull_request.number }}
+      message: |
+        ⚠️ **Script validation failed!**
+        The following issues were found with your PR:
+        ```
+        ${{ needs.validate-scripts.outputs.validation_log }}
+        ```
+      is_pr: true

Additionally, add outputs to the validate-scripts job:

 jobs:
   validate-scripts:
     runs-on: ubuntu-latest
+    outputs:
+      validation_exit_code: ${{ steps.validate.outputs.validation_exit_code }}
+      validation_log: ${{ steps.validate.outputs.validation_log }}
     steps:
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f8f35da and 5b3f7b1.

📒 Files selected for processing (7)
  • .github/workflows/ci.yml (1 hunks)
  • .github/workflows/commenter.yml (1 hunks)
  • package.json (2 hunks)
  • scripts/help.js (1 hunks)
  • scripts/lib/cli-fuzzy.js (1 hunks)
  • scripts/lib/colors.js (1 hunks)
  • scripts/validate-scripts.js (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • scripts/lib/cli-fuzzy.js
  • scripts/lib/colors.js
🧰 Additional context used
🧬 Code graph analysis (1)
scripts/validate-scripts.js (1)
scripts/lib/read-packageJson-scripts.js (1)
  • JSON (4-4)
🪛 Biome (2.1.2)
package.json

[error] 81-81: Expected a property but instead found '}'.

Expected a property here.

(parse)

🔇 Additional comments (2)
scripts/validate-scripts.js (1)

1-64: LGTM!

The validation script is well-structured and correctly implements two-way validation between scripts and scriptsInfo in both root and docs package.json files. The error handling is appropriate, and the logic correctly handles the _meta exclusion when flattening scriptsInfo.

package.json (1)

81-81: Static analysis false positive.

The Biome error at line 81 appears to be a false positive. The syntax }, correctly closes the lint:fix object within the Linting category. This is valid JSON.

Comment thread .github/workflows/commenter.yml
Comment thread package.json
Comment thread scripts/help.js Outdated
Fix input access in github-script.
The inputs object is not directly accessible in the JavaScript context of github-script. Inputs must be passed through the with: section using ${{ inputs.* }} syntax or as environment variables.
@Ryan-Millard

Copy link
Copy Markdown
Owner Author

@CodeRabbit help

@coderabbitai

coderabbitai Bot commented Dec 17, 2025

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

@Ryan-Millard, here's what I can help you with:

🤖 CodeRabbit Capabilities

Review Commands

I can perform various actions on your PR. Simply mention me with these commands:

  • @coderabbit review - Trigger a full review of the PR
  • @coderabbit pause - Pause automatic reviews for this PR
  • @coderabbit resume - Resume automatic reviews for this PR
  • @coderabbit resolve - Resolve all my review comments in this PR
  • @coderabbit approve - Approve the PR changes

Interactive Review

You can ask me questions about:

  • Code explanations - Ask me to explain specific code sections
  • Generate tests - Request unit tests for new functionality
  • Suggest improvements - Ask for optimization or best practice recommendations
  • Generate diagrams - Request architecture or sequence diagrams
  • Analyze patterns - Discuss design patterns or code structure
  • Documentation - Help with code comments or documentation

Advanced Features

  • Running scripts - I can analyze your codebase using shell commands (ripgrep, ast-grep, git, etc.)
  • Web search - I can search for latest documentation or library information
  • Issue creation - I can create follow-up issues on GitHub when requested
  • Long-term memory - I remember project-specific patterns and preferences when you share them

Configuration

For repository-wide settings:

  • Path-based instructions: Configure review behavior per file/directory
  • Path filters: Exclude specific paths from review
  • Custom guidelines: Add project-specific coding standards

Visit CodeRabbit Documentation for detailed configuration options and guides.

Example Interactions

  • "Can you explain how the validation works in scripts/validate-scripts.js?"
  • "Generate unit tests for the readPackageJsonScripts function"
  • "Create a sequence diagram showing the CI workflow"
  • "Review the changes" (triggers a full review)

Feel free to ask me anything about this PR or request specific analyses! 🚀

@Ryan-Millard

Copy link
Copy Markdown
Owner Author

@CodeRabbit review

@coderabbitai

coderabbitai Bot commented Dec 17, 2025

Copy link
Copy Markdown
Contributor
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@Ryan-Millard

Copy link
Copy Markdown
Owner Author

@CodeRabbit full review

@coderabbitai

coderabbitai Bot commented Dec 17, 2025

Copy link
Copy Markdown
Contributor
✅ Actions performed

Full review triggered.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
.github/workflows/commenter.yml (2)

36-40: Unused is_pr input.

The is_pr input is defined but never used in the workflow. The script always uses issues.createComment regardless of this flag. Either implement the differentiation logic or remove the unused input to avoid confusion.


55-59: Consider escaping the message to prevent syntax errors.

If inputs.message contains backticks or ${...} patterns, the template literal on line 59 could cause JavaScript syntax errors or unintended interpolation. Consider using a safer approach:

         script: |
           const owner = context.repo.owner;
           const repo = context.repo.repo;
           const number = ${{ inputs.issue_number }};
-          const body = `${{ inputs.message }}`;
+          const body = String.raw`${{ inputs.message }}`;

Alternatively, pass the message via environment variables:

          env:
            COMMENT_BODY: ${{ inputs.message }}
          script: |
            const body = process.env.COMMENT_BODY;
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5b3f7b1 and 2028f24.

📒 Files selected for processing (4)
  • .github/workflows/commenter.yml (1 hunks)
  • docs/scripts/help.js (1 hunks)
  • package.json (2 hunks)
  • scripts/help.js (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
docs/scripts/help.js (3)
scripts/help.js (3)
  • title (4-7)
  • items (10-10)
  • initialSearch (13-13)
scripts/lib/read-packageJson-scripts.js (1)
  • readPackageJsonScripts (3-23)
scripts/lib/cli-fuzzy.js (1)
  • runFuzzyCli (5-19)
🔇 Additional comments (4)
docs/scripts/help.js (1)

1-24: Clean orchestrator pattern with proper error handling.

The refactored structure correctly delegates to shared utilities while keeping all dependent logic inside the try block, avoiding variable scope issues. The relative import paths correctly reference the shared lib in the parent project.

package.json (2)

7-116: Well-structured scriptsInfo metadata with complete 1:1 mapping.

The categorized structure with _meta.basic for commonly-used scripts and detailed args schemas for commands with options provides excellent discoverability. All 25 scripts have corresponding scriptsInfo entries, fulfilling the PR objective of strict 1:1 mapping.


141-142: New validate-scripts entry completes the CI enforcement objective.

The addition of validate-scripts ensures the 1:1 mapping can be enforced in CI, aligning with the PR's goal of preventing undocumented or stale scripts.

scripts/help.js (1)

9-24: Scope issue resolved by keeping dependent code inside try block.

The past review flagged that items and basicItems were declared in the try block but referenced outside. The current implementation resolves this by keeping runFuzzyCli inside the try block, which is a valid alternative to the suggested fix of declaring variables outside. Both approaches prevent the ReferenceError.

@Ryan-Millard
Ryan-Millard force-pushed the refactor/help-script/issue-124 branch from 146fffd to 43bd0b1 Compare December 17, 2025 19:36

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
scripts/lib/cli-fuzzy.js (1)

114-130: Complete the defensive validation for info.args.

The past review comment has been partially addressed—description and command are now checked. However, line 121 still doesn't validate that info.args is actually an array before iterating. If args is a non-array object with a length property (like a string), the for-of loop on line 122 could produce unexpected results.

Apply this diff to complete the fix:

-  if (info.args && info.args.length) {
+  if (Array.isArray(info.args) && info.args.length) {
     for (const arg of info.args) {
       console.log(`\t\t  ${colorText(arg, Colors.YELLOW)}`);
     }
   }
🧹 Nitpick comments (1)
scripts/lib/cli-fuzzy.js (1)

31-37: Consider extracting magic constants.

The hardcoded values (line width of 80 and instruction text) could be extracted as constants at the module level for easier maintenance.

+const LINE_WIDTH = 80;
+const INSTRUCTIONS = "Type 'a' to list all, 'q' to quit.";
+
 function printHeader(title) {
-  const line = colorText("─".repeat(80), Colors.BLUE);
+  const line = colorText("─".repeat(LINE_WIDTH), Colors.BLUE);
   console.log(line);
   console.log(colorText(title, Colors.BOLD));
-  console.log("Type 'a' to list all, 'q' to quit.");
+  console.log(INSTRUCTIONS);
   console.log(line);
 }
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 146fffd and 43bd0b1.

📒 Files selected for processing (1)
  • scripts/lib/cli-fuzzy.js (1 hunks)
🔇 Additional comments (5)
scripts/lib/cli-fuzzy.js (5)

5-29: LGTM!

The input validation is thorough and the orchestration logic correctly handles the different CLI modes (basic display, initial search, interactive).


39-45: LGTM!

The function correctly validates that each basic item exists in the items object before attempting to print it.


47-81: Verify readline cleanup behavior in one-shot mode.

When skipIfInitialSearch is true, line 62 calls rl.close() and immediately returns. The close event handler (lines 77-80) will fire asynchronously, but the function has already returned. While this may work for the one-shot use case, ensure that this pattern doesn't cause issues if the calling code expects synchronous cleanup.

If you encounter issues, consider making the cleanup explicit:

 function startInteractive(items, skipIfInitialSearch = false) {
   const rl = readline.createInterface({
     input: process.stdin,
     output: process.stdout,
     completer(line) {
       const names = Object.keys(items);
       const hits = fuzzy.filter(line, names).map(x => x.original);
       return [hits, line];
     },
   });

   rl.setPrompt(colorText("> ", Colors.CYAN));

   // If initialSearch was provided, and we just want one-shot results, skip the interactive prompt
   if (skipIfInitialSearch) {
-    return rl.close();
+    rl.close();
+    return;
   }

83-93: LGTM!

The search logic correctly handles empty results and prints matching items.


95-112: LGTM!

The grouping logic correctly handles missing group properties with a sensible default and prints items in an organized manner.

Comment thread scripts/lib/cli-fuzzy.js
@Ryan-Millard
Ryan-Millard force-pushed the refactor/help-script/issue-124 branch from e6ea791 to 5fcfd1a Compare December 17, 2025 20:44
@Ryan-Millard
Ryan-Millard force-pushed the refactor/help-script/issue-124 branch from e4dd3f1 to 2e2a4ce Compare December 17, 2025 20:55

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
scripts/lib/cli-fuzzy.js (1)

2-2: Verify that the fuzzy dependency has been added to package.json.

This was previously flagged as a critical issue. The fuzzy library is imported but must be declared in the package.json manifest to avoid runtime failures.

Run the following script to verify the dependency was added:

#!/bin/bash
# Description: Check if fuzzy dependency exists in package.json

cat package.json | jq -r '.dependencies.fuzzy // .devDependencies.fuzzy // "NOT_FOUND"'
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5fcfd1a and e4dd3f1.

📒 Files selected for processing (1)
  • scripts/lib/cli-fuzzy.js (1 hunks)
🔇 Additional comments (5)
scripts/lib/cli-fuzzy.js (5)

31-45: LGTM!

The header printing and basic items display logic is well-structured. The defensive check at line 42 ensures printItem is only called for items that exist in the items object.


47-81: LGTM!

The interactive mode implementation is well-designed. The one-shot mode (lines 60-63) correctly handles the case when initial search terms were provided, and the command handlers ('q', 'a', and search) provide good UX.


83-93: LGTM!

The search implementation correctly uses fuzzy filtering and provides clear feedback when no matches are found.


95-112: LGTM!

The grouping logic effectively organizes items by their group property with a sensible "Other" default, and the output formatting with colored group headers enhances readability.


114-130: Defensive checks properly implemented!

The function now correctly handles missing or undefined properties:

  • Line 116 handles both string and array descriptions
  • Line 117 checks if description exists before printing
  • Line 121 validates args is an array before iteration
  • Line 127 checks if command exists before printing

This addresses the concerns raised in the previous review.

Comment thread scripts/lib/cli-fuzzy.js
@Ryan-Millard

Copy link
Copy Markdown
Owner Author

@CodeRabbit generate unit tests

@Ryan-Millard

Copy link
Copy Markdown
Owner Author

@coderabbitai generate docstrings

@coderabbitai

coderabbitai Bot commented Dec 17, 2025

Copy link
Copy Markdown
Contributor
✅ Actions performed

Initiated docstring generation; will generate only if new commits exist.

@coderabbitai

coderabbitai Bot commented Dec 17, 2025

Copy link
Copy Markdown
Contributor

Note

Docstrings generation - SUCCESS
Generated docstrings for this pull request at #136

coderabbitai Bot added a commit that referenced this pull request Dec 17, 2025
Docstrings generation was requested by @Ryan-Millard.

* #135 (comment)

The following files were modified:

* `scripts/lib/cli-fuzzy.js`
* `scripts/lib/colors.js`
* `scripts/lib/read-packageJson-scripts.js`
* `scripts/validate-scripts.js`
Docstrings generation was requested by @Ryan-Millard.

* #135 (comment)

The following files were modified:

* `scripts/lib/cli-fuzzy.js`
* `scripts/lib/colors.js`
* `scripts/lib/read-packageJson-scripts.js`
* `scripts/validate-scripts.js`

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@Ryan-Millard
Ryan-Millard merged commit 494fd64 into main Dec 17, 2025
2 checks passed
@Ryan-Millard
Ryan-Millard deleted the refactor/help-script/issue-124 branch December 17, 2025 21:12
@coderabbitai

coderabbitai Bot commented Dec 17, 2025

Copy link
Copy Markdown
Contributor

An unexpected error occurred during Unit Test Generation.

@coderabbitai

coderabbitai Bot commented Dec 17, 2025

Copy link
Copy Markdown
Contributor
✅ Actions performed

Unit Test Generation started...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor: help.js CLI scripts - main app & docs site

1 participant