Skip to content

🧹 Refactor duplicated graph visualization logic in CLI#34

Closed
saurabhsharma2u wants to merge 3 commits into
mainfrom
refactor-visualization-logic-18012622951430391998
Closed

🧹 Refactor duplicated graph visualization logic in CLI#34
saurabhsharma2u wants to merge 3 commits into
mainfrom
refactor-visualization-logic-18012622951430391998

Conversation

@saurabhsharma2u

Copy link
Copy Markdown
Contributor

This PR addresses a code health issue by removing duplicated graph visualization logic in the CLI plugin.

Changes:

  • Refactored plugins/cli/src/utils/exportRunner.ts to use generateHtml from @crawlith/core for the visualize export format.
  • Removed the manual implementation of HTML generation that was duplicating logic found in plugins/core/src/report/html.ts.
  • Added a new test file plugins/cli/tests/exportRunner.test.ts to verify the runSitegraphExports function works correctly for both visualize and html formats.

Benefits:

  • Maintainability: Visualization logic is now centralized in @crawlith/core. Any changes to the template or data injection will automatically apply to the CLI export.
  • Safety: The generateHtml function uses safeJson to escape HTML characters in the JSON data, preventing potential XSS issues that were possible with the previous raw JSON.stringify implementation in the CLI.
  • Consistency: Both html and visualize formats now produce consistent output structure.

Verification:

  • Verified with new unit tests plugins/cli/tests/exportRunner.test.ts.
  • Ran full test suite for plugins/cli and plugins/core to ensure no regressions.

PR created automatically by Jules for task 18012622951430391998 started by @saurabhsharma2u

Consolidates the graph visualization HTML generation logic by reusing the robust `generateHtml` function from `@crawlith/core` in the `@crawlith/cli` export runner. This removes duplicated code and ensures consistent, safe JSON injection (using `safeJson`).

- Replaced inline HTML generation in `plugins/cli/src/utils/exportRunner.ts` with `generateHtml`.
- Removed unused `SITEGRAPH_HTML` import in `plugins/cli/src/utils/exportRunner.ts`.
- Added unit tests in `plugins/cli/tests/exportRunner.test.ts` to verify correct behavior.
@google-labs-jules

Copy link
Copy Markdown

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

…logic in CLI.

- Removed unused `FetchOptions` import in `plugins/core/src/crawler/crawler.ts`.
- Removed unused `http` import in `plugins/core/tests/audit/transport.test.ts`.
- (From previous commit) Refactored `plugins/cli/src/utils/exportRunner.ts` to use `generateHtml`.
- (From previous commit) Added `plugins/cli/tests/exportRunner.test.ts`.

This commit addresses code health issues and aims to resolve CI failures potentially caused by strict linting or transient network issues.
This commit addresses CI failures on Windows runners caused by registry access issues (403 Forbidden) and improves the robustness of the CI workflow. It also includes lint fixes and the refactoring of the CLI visualization logic.

**Infrastructure Changes:**
- Updated `.github/workflows/ci.yml` to use `pnpm/action-setup` instead of `npm install -g pnpm`. This is the recommended way to install pnpm in CI and should resolve the registry access issues seen with `npm`.
- Added `"packageManager": "pnpm@9.15.0"` to `package.json` to pin the pnpm version used by `pnpm/action-setup`.

**Lint Fixes (Core Plugin):**
- Removed unused `FetchOptions` import in `plugins/core/src/crawler/crawler.ts`.
- Removed unused `http` import in `plugins/core/tests/audit/transport.test.ts`.

**Refactor (CLI Plugin):**
- Refactored `plugins/cli/src/utils/exportRunner.ts` to use the shared `generateHtml` function from `@crawlith/core` for the `visualize` export format, removing duplicated logic.
- Added `plugins/cli/tests/exportRunner.test.ts` to verify the export functionality.
@saurabhsharma2u

Copy link
Copy Markdown
Contributor Author

Closing as this refactoring was already completed and merged in PR 37.

@saurabhsharma2u
saurabhsharma2u deleted the refactor-visualization-logic-18012622951430391998 branch February 25, 2026 19:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant