chore: apply code review feedback for PR #2520 [approved]

[google-labs-jules]

This PR implements the changes recommended in code review for PR #2520. The updates include improving test script execution in package.json by ensuring that test failures are not masked, and refining the knip.ts entry points for more accurate dead code analysis by eliminating non-entry test and impact scripts.

Scope Minimization Suggestions:

1. Review test snapshots for removal if they are not needed for the current test cases.
2. Consider further reducing the entry points in knip.ts by assessing if all current paths are necessary.

[google-labs-jules]

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

🚀 Deployment Details (Last updated: Jun 18, 2026, 4:37 PM PST)

🚀 Pushed to gh-pages; publish in progress

• Preview URL: https://arii.github.io/tech-dancer/fix/pr-2520-code-review-feedback-6407712281537791444/
• Preview Dashboard: https://arii.github.io/tech-dancer/previews/index.html
• Commit SHA: 9166814
• Marker File: verify here

[github-actions]

🚀 Impact Analysis Details (Last updated: Jun 18, 2026, 4:36 PM PST)

✅ Impact Analysis Complete

• Commit SHA: 9166814
• Link to artifacts

Deployment Review

Summary

Impact Level: LOW

📝 Changed Files (19)

• .github/actions/setup-node-pnpm/action.yml
• .github/workflows/ai-chatops.yml
• .github/workflows/ci.yml
• .github/workflows/mass-audit-prs.yml
• .github/workflows/mergellama.yml
• .github/workflows/self-healing.yml
• .github/workflows/wcs_etl.yml
• 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.

[github-actions]

🐙 GitHub Models Code Review

Powered by GitHub Models

Reviewing: PR #2560

Code Review Feedback

Review of the provided code diff:

1. knip.ts

• Only a minor change: added 'gh' to ignoreBinaries. No high severity issues.

2. package.json

• Test script change:

  • "test": "run-s -c test:app test:scripts"
    This changes the test command from running only vitest run to running two scripts in parallel: test:app and test:scripts.
    • High Severity Issue:
      • run-s -c (from npm-run-all) runs scripts concurrently. If both scripts write to the same output, or if there are shared resources (e.g., global mocks, test setup/teardown), this can cause race conditions, flaky tests, or corrupted output.
      • If test:app and test:scripts are not designed to run concurrently, this is a blocking issue.
      • If either script depends on the other, or if there are global side effects, this can break CI or local testing.
    • Action:
      • Confirm that test:app and test:scripts are fully isolated and safe to run concurrently. If not, use run-s (sequential) instead of run-s -c (concurrent).
      • If unsure, revert to sequential execution or add documentation/guards.

• Dependency changes:

  • Added "zod": "^3.24.1": No high severity issue.
  • Various dependency upgrades: No high severity issue unless breaking changes are present, but this cannot be determined from the diff alone.

3. pnpm-lock.yaml

• Dependency upgrades and additions, including zod.
• No high severity issues visible in the lockfile changes.

---

Summary

Blocking Issue:

• The change to "test": "run-s -c test:app test:scripts" is potentially unsafe unless both test suites are fully isolated and do not interfere with each other. Running tests concurrently can cause race conditions, flaky tests, or corrupted output if not handled carefully.

Action Required:

• Revert to sequential execution or provide evidence that concurrent execution is safe.

[VERDICT: FAIL]

---

Generated by github-models-code-review

[arii]

The PR has been reviewed and approved as the changes effectively address the feedback and enhance the code. It is recommended to consider removing any updated visual snapshots that do not contribute to the tests to streamline the codebase.