Skip to content

chore: Modularize impact-analysis scripts and add Zod validation requires changes#2520

Closed
google-labs-jules[bot] wants to merge 15 commits into
mainfrom
feat/issue-impact-scripts-submodules-6659404272263102890
Closed

chore: Modularize impact-analysis scripts and add Zod validation requires changes#2520
google-labs-jules[bot] wants to merge 15 commits into
mainfrom
feat/issue-impact-scripts-submodules-6659404272263102890

Conversation

@google-labs-jules

@google-labs-jules google-labs-jules Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

This PR refactors the impact-analysis tooling, organizing it into a modular structure while also implementing Zod for runtime validation. Includes test suite improvement and updated dependencies.

Scope Minimization Suggestions:

  • Evaluate the necessity of visual snapshots in scripts/__tests__/build.test.ts since these could contribute to code churn; consider removing any outdated snapshots that do not affect current tests.

Fixes #2492

…idation

- Extract preview-server, parser-sanitizer, metric-calculator, and report-generator into scripts/impact/ submodules.
- Implement Zod schemas for VisualRouteSummary and DomRouteSummary for runtime validation.
- Add unit tests for impact submodules in scripts/__tests__/build.test.ts.
- Update package.json: add zod@^3.24.1, move jsdom and @types/jsdom to dependencies.
- Update vite.config.ts to include script tests in vitest.
- Update knip.ts to include new script entry points.
@google-labs-jules

Copy link
Copy Markdown
Contributor Author

👋 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.

@github-actions

github-actions Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

🚀 Deployment Details (Last updated: Jun 19, 2026, 11:17 AM PST)

🚀 Pushed to gh-pages; publish in progress

@github-actions

github-actions Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

🚀 Impact Analysis Details (Last updated: Jun 18, 2026, 5:20 PM PST)

Impact Analysis Complete

Deployment Review

Summary

Impact Level: LOW

📝 Changed Files (12)
  • knip.ts
  • package.json
  • pnpm-lock.yaml
  • scripts/tests/build.test.ts
  • scripts/impact-dom-diff.ts
  • scripts/impact-review-utils.ts
  • scripts/impact-visual-diff.ts
  • scripts/impact/metric-calculator.ts
  • scripts/impact/parser-sanitizer.ts
  • scripts/impact/preview-server.ts
  • scripts/impact/report-generator.ts
  • scripts/vitest.config.scripts.ts

Routes Reviewed

No concrete routes required review.

…idation

- Extract preview-server, parser-sanitizer, metric-calculator, and report-generator into scripts/impact/ submodules.
- Implement Zod schemas for VisualRouteSummary and DomRouteSummary for runtime validation.
- Add unit tests for impact submodules in scripts/__tests__/build.test.ts.
- Implement a separate Vitest configuration for script tests to avoid matcher conflicts.
- Update package.json: add zod@^3.24.1, add test:scripts task.
- Simplify JSDoc in parser-sanitizer.ts to remove AI slop.
- Ensure the deployment report maintains rich HTML formatting for readability.

@arii arii left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

ANTI-AI-SLOP\n\n\n## FINDINGS\n\n\n## FINAL RECOMMENDATION\n<Approved | Approved with Minor Changes | Not Approved>\n\n

Inline Comments (Fallback due to Github line resolution errors)

  • :1:

@arii arii left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

ANTI-AI-SLOP\n\n\n## FINDINGS\n\n\n## FINAL RECOMMENDATION\n<Approved | Approved with Minor Changes | Not Approved>\n\n

Inline Comments (Fallback due to Github line resolution errors)

  • :1:

@arii arii left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

ANTI-AI-SLOP\n\n\n## FINDINGS\n\n\n## FINAL RECOMMENDATION\n<Approved | Approved with Minor Changes | Not Approved>\n\n

Inline Comments (Fallback due to Github line resolution errors)

  • :1:

@arii arii left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

ANTI-AI-SLOP\n\n\n## FINDINGS\n\n\n## FINAL RECOMMENDATION\n<Approved | Approved with Minor Changes | Not Approved>\n\n

Inline Comments (Fallback due to Github line resolution errors)

  • :1:

@arii arii left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

ANTI-AI-SLOP\n\n\n## FINDINGS\n\n\n## FINAL RECOMMENDATION\n<Approved | Approved with Minor Changes | Not Approved>\n\n

Inline Comments (Fallback due to Github line resolution errors)

  • :1:

@arii arii left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

ANTI-AI-SLOP\n\n\n## FINDINGS\n\n\n## FINAL RECOMMENDATION\n<Approved | Approved with Minor Changes | Not Approved>\n\n

Inline Comments (Fallback due to Github line resolution errors)

  • :1:

@arii

arii commented Jun 18, 2026

Copy link
Copy Markdown
Owner

Automated Agent Review

ℹ️ Tooling/Scripts: Please ensure any Python script changes are covered by pytest and that typescript scripts pass pnpm run test:scripts.


This is an automated review generated by Jules.

…idation (final)

- Extract preview-server, parser-sanitizer, metric-calculator, and report-generator into scripts/impact/ submodules.
- Implement Zod schemas for VisualRouteSummary and DomRouteSummary for runtime validation.
- Add unit tests for impact submodules in scripts/__tests__/build.test.ts.
- Implement a separate Vitest configuration for script tests to avoid matcher conflicts.
- Update package.json: add zod@^3.24.1, add test:scripts task.
- Simplify JSDoc in parser-sanitizer.ts to remove AI slop.
- Ensure the deployment report maintains rich HTML formatting for readability.
- Move jsdom to devDependencies.

@arii arii left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

ANTI-AI-SLOP\n\n\n## FINDINGS\n\n\n## FINAL RECOMMENDATION\n<Approved | Approved with Minor Changes | Not Approved>\n\n

Inline Comments (Fallback due to Github line resolution errors)

  • :1:

…idation

- Extract preview-server, parser-sanitizer, metric-calculator, and report-generator into scripts/impact/ submodules.
- Implement Zod schemas for VisualRouteSummary and DomRouteSummary for runtime validation.
- Add unit tests for impact submodules in scripts/__tests__/build.test.ts.
- Implement a separate Vitest configuration for script tests to avoid matcher conflicts.
- Update package.json: add zod@^3.24.1, add test:scripts task.
- Simplify JSDoc in parser-sanitizer.ts to remove AI slop.
- Ensure the deployment report maintains rich HTML formatting for readability.
- Keep jsdom in devDependencies.
@arii

arii commented Jun 18, 2026

Copy link
Copy Markdown
Owner

🔍 PR #2520 Feedback — Modularize impact-analysis scripts and add Zod validation

CI Status

✅ All checks passed.

Code Audit Findings

No violations found.

Impact Analysis

  • Impact: No UI Files Touched.

Next Steps

  • All checks passed — ready to merge

…idation

- Extract preview-server, parser-sanitizer, metric-calculator, and report-generator into scripts/impact/ submodules.
- Implement Zod schemas for VisualRouteSummary and DomRouteSummary for runtime validation.
- Add unit tests for impact submodules in scripts/__tests__/build.test.ts.
- Implement a separate Vitest configuration for script tests to avoid matcher conflicts.
- Update package.json: add zod@^3.24.1, add test:scripts task.
- Simplify JSDoc in parser-sanitizer.ts to remove AI slop.
- Ensure the deployment report maintains rich HTML formatting for readability.
- Keep jsdom in devDependencies.
…idation

- Extract preview-server, parser-sanitizer, metric-calculator, and report-generator into scripts/impact/ submodules.
- Implement Zod schemas for VisualRouteSummary and DomRouteSummary for runtime validation.
- Add unit tests for impact submodules in scripts/__tests__/build.test.ts.
- Implement a separate Vitest configuration for script tests to avoid matcher conflicts.
- Update package.json: add zod@^3.24.1, add test:scripts task.
- Simplify JSDoc in parser-sanitizer.ts to remove AI slop.
- Ensure the deployment report maintains rich HTML formatting for readability.
- Keep jsdom in devDependencies.

@arii arii left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

ANTI-AI-SLOP\n\n\n## FINDINGS\n\n\n## FINAL RECOMMENDATION\n<Approved | Approved with Minor Changes | Not Approved>\n\n

Inline Comments (Fallback due to Github line resolution errors)

  • :1:

- Extract preview-server, parser-sanitizer, metric-calculator, and report-generator into scripts/impact/ submodules.
- Implement Zod schemas for VisualRouteSummary and DomRouteSummary for runtime validation.
- Add unit tests for impact submodules in scripts/__tests__/build.test.ts.
- Implement a separate Vitest configuration for script tests.
- Update knip.ts to ignore 'gh' binary and track new script entry points.
- Restore rich HTML formatting in deployment reports.
- Ensure jsdom remains in devDependencies.
- Extract preview-server, parser-sanitizer, metric-calculator, and report-generator into scripts/impact/ submodules.
- Implement Zod schemas for VisualRouteSummary and DomRouteSummary for runtime validation.
- Add unit tests for impact submodules in scripts/__tests__/build.test.ts.
- Implement a separate Vitest configuration for script tests.
- Update knip.ts to ignore 'gh' binary and track new script entry points.
- Restore rich HTML formatting in deployment reports.
- Ensure jsdom remains in devDependencies.
- Update package.json with test:scripts task.

@arii arii left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

ANTI-AI-SLOP\n\n\n## FINDINGS\n\n\n## FINAL RECOMMENDATION\n<Approved | Approved with Minor Changes | Not Approved>\n\n

Inline Comments (Fallback due to Github line resolution errors)

  • :1:

@arii arii left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

⚠️ BLOCKING CI FAILURE: Approval overridden to COMMENT because the following checks are failing: Build & E2E. Please resolve CI issues before approval.

AUTO-AUDIT

FINAL RECOMMENDATION

Approved

@arii arii left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

⚠️ BLOCKING CI FAILURE: Approval overridden to COMMENT because the following checks are failing: Build & E2E. Please resolve CI issues before approval.

AUTO-AUDIT

FINAL RECOMMENDATION

Approved

@arii arii left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

⚠️ BLOCKING CI FAILURE: Approval overridden to COMMENT because the following checks are failing: Build & E2E. Please resolve CI issues before approval.

AUTO-AUDIT

FINAL RECOMMENDATION

Approved

@Logic-gate

Copy link
Copy Markdown

Why am I being mentioned here?

@github-actions

github-actions Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

🐙 GitHub Models Code Review

Powered by GitHub Models

Reviewing: PR #2520

Code Review Feedback

Review of the provided code diff:

1. knip.ts

  • Change: Expanded entry globs, added gh to ignoreBinaries.
  • High Severity Issues: None. The changes are configuration updates and do not introduce bugs or anti-patterns.

2. package.json

  • Change:
    • Modified test script to run both vitest run and pnpm run test:scripts.
    • Added test:scripts script.
    • Added zod to dependencies.
  • High Severity Issues:
    • Test Script Chaining: The new test script is:
      "test": "vitest run && pnpm run test:scripts"
      • Issue: If vitest run fails, test:scripts will not run due to the && operator. This is standard, but if you expect both test suites to always run (even if one fails), you should use ; instead of &&. However, if you want to fail fast, this is fine.
      • Blocking? Not blocking, but worth confirming intent.
    • Missing Types: No missing types detected.
    • Performance: No performance issues.

3. pnpm-lock.yaml

  • Change: Dependency upgrades, new packages, and version bumps.
  • High Severity Issues:
    • Dependency Upgrades: Several major upgrades (e.g., firebase, react, vite, hyparquet, etc.).
      • Issue: Major upgrades can introduce breaking changes. There is no evidence in this diff that the codebase has been updated to accommodate these breaking changes.
      • Blocking? If these upgrades are not accompanied by code changes to handle breaking changes, this is a high severity issue. For example, upgrading react from 19.2.5 to 19.2.7 is minor, but react-router-dom from 7.14.1 to 7.18.0 could have breaking changes. Similarly, firebase from 12.12.1 to 12.15.0 may have breaking changes. If the codebase is not updated accordingly, this can break production.
      • Lockfile Only: If this diff is only updating the lockfile and not the actual code, this is a blocking issue unless you have verified compatibility.

Summary of High Severity Issues:

  • Dependency Upgrades Without Code Changes: Upgrading dependencies in the lockfile without corresponding code changes or compatibility checks is a blocking issue. This can cause runtime errors, unexpected behavior, or build failures.

Actionable Feedback:

  • Verify Compatibility: Before merging, ensure that all upgraded dependencies are compatible with your codebase. Run all tests and check for breaking changes in the changelogs of upgraded packages.
  • Test Script Intent: Confirm that the && in the test script is intentional (fail fast). If you want both test suites to run regardless of failures, use ; instead.
  • Lockfile Consistency: If you are only updating the lockfile, ensure that this is intentional and that the codebase is compatible with the new versions.

[VERDICT: FAIL]


Generated by github-models-code-review

@arii arii changed the title Modularize impact-analysis scripts and add Zod validation chore: Modularize impact-analysis scripts and add Zod validation requires changes Jun 18, 2026
@arii

arii commented Jun 18, 2026

Copy link
Copy Markdown
Owner

The PR shows significant improvements with modularization and validation. However, to reduce code churn, it is essential to review and potentially remove visual snapshots that are no longer relevant. Please address this issue before we proceed with approval.

arii pushed a commit that referenced this pull request Jun 18, 2026
- Modify `package.json` test script to use `run-s -c` (npm-run-all) instead of `&&` to run app tests and script tests sequentially but guaranteeing both run even if the first fails without short-circuiting.
- Update `knip.ts` entry paths to exclude `scripts/__tests__/*.ts` and `scripts/impact/*.ts` from the entry array to improve dead code analysis accuracy.
arii and others added 3 commits June 18, 2026 17:04
@arii

arii commented Jun 19, 2026

Copy link
Copy Markdown
Owner

@jules-fix-ci

@github-actions

Copy link
Copy Markdown
Contributor

🤖 Jules is on it!

Initialized autonomous repair session (sessions/4637785506721206380) for branch feat/issue-impact-scripts-submodules-6659404272263102890.

@arii arii left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

ANTI-AI-SLOP\n\n\n## FINDINGS\n\n\n## FINAL RECOMMENDATION\n<Approved | Approved with Minor Changes | Not Approved>\n\n

Inline Comments (Fallback due to Github line resolution errors)

  • :1:

@arii arii left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

ANTI-AI-SLOP\n\n\n## FINDINGS\n\n\n## FINAL RECOMMENDATION\n<Approved | Approved with Minor Changes | Not Approved>\n\n

Inline Comments (Fallback due to Github line resolution errors)

  • :1:

@arii arii left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Automated Review for PR #2520

CI Status: Failing checks detected.

Failing Checks:

  • Lint & Type Check
  • Oxlint Scan

Recommendation: Please review the failing CI logs and apply fixes.

FINAL RECOMMENDATION

Not Approved

@arii arii left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Automated Review for PR #2520

CI Status: Failing checks detected.

Failing Checks:

  • Lint & Type Check
  • Oxlint Scan

Recommendation: Please review the failing CI logs and apply fixes.

FINAL RECOMMENDATION

Not Approved

@arii arii left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Comprehensive Review for PR #2520

CI Status: Failing checks detected.

Failing Checks:

  • Lint & Type Check
  • Oxlint Scan

Recommendation: Please review the failing CI logs and apply fixes before requesting another review.

FINAL RECOMMENDATION

Not Approved

google-labs-jules Bot and others added 2 commits June 19, 2026 11:12
Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
Co-authored-by: Ariel Anders <anders.ariel@gmail.com>
@arii arii closed this Jun 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor(scripts): extract impact-analysis helpers into scripts/impact/ submodules with Zod schemas

2 participants