refactor(ci): enhance lint script with programmatic ESLint and line-level filtering

[kevmoo]

Description

Refactored scripts/lint-changed-files.ts to use the programmatic ESLint API instead of spawning execSync child processes. This improves performance and allows for robust line-level filtering of lint errors.

• Extracted monolithic main function into modular helpers: getChangedFiles, getChangedLines, runLint, reportStandard, and reportFiltered.
• Unified the ESLint execution path.
• Replaced inline process.exit with a custom LintError class handled at the top level.
• Updated .github/workflows/node-test.yml to use --only-changed-lines by default.

Scenarios Tested

• Ran standard mode to verify it reports all lints for modified files.
• Ran --only-changed-lines mode to verify it filters out pre-existing warnings on unchanged lines.
• Verified that syntax errors on changed lines are correctly reported.

Sample Commands

npm run lint:changed-files
npm run lint:changed-files -- --only-changed-lines

[gemini-code-assist]

Code Review

This pull request refactors the lint-changed-files.ts script to utilize the ESLint programmatic API and introduces a new --only-changed-lines flag for targeted linting. The review feedback highlights a potential buffer overflow risk in execSync when handling large diffs, a regression where ESLint CLI arguments are currently ignored, and a suggestion to clarify the success message when warnings are present.

[kevmoo]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors the lint-changed-files.ts script to use the programmatic ESLint API and introduces a new --only-changed-lines mode to filter linting results to only those lines modified in the current branch. Several issues were identified in the review: the new implementation ignores most ESLint CLI arguments (a regression), there is a potential command injection vulnerability when calling git diff, the manual formatting in reportFiltered should be replaced by the standard ESLint formatter for consistency, and the parsing of the --max-warnings flag is fragile and could lead to silent failures.

[kevmoo]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors the lint-changed-files.ts script to use the ESLint programmatic API and introduces a feature to filter linting results to only changed lines via the --only-changed-lines flag. It also adds support for --quiet and --max-warnings arguments. Feedback focuses on optimizing performance by combining multiple filter and reduce operations into single passes to avoid redundant array iterations. Additionally, a suggestion was made to ensure consistent error handling by throwing an error instead of calling process.exit directly, adhering to the repository's style guide.

[gemini-code-assist]

The messages array is filtered twice to calculate the messages property and the errorCount. Additionally, the filter condition r.messages.length > 0 || r.errorCount > 0 is redundant since they are equivalent after the mapping. This can be optimized by filtering once.

    processedResults = results
      .map((r) => {
        const errors = r.messages.filter((m) => m.severity === 2);
        return {
          ...r,
          messages: errors,
          errorCount: errors.length,
          warningCount: 0,
        };
      })
      .filter((r) => r.errorCount > 0);

[gemini-code-assist]

Multiple reduce calls on the same array can be combined into a single pass for better efficiency.

  let errorCount = 0;
  let warningCount = 0;
  for (const r of processedResults) {
    errorCount += r.errorCount;
    warningCount += r.warningCount;
  }

[gemini-code-assist]

The messages array is filtered multiple times within the map function. This can be optimized by calculating errorCount and warningCount during the initial filtering pass.

  const filteredResults: EslintResult[] = results
    .map((r) => {
      const relPath = relative(root, r.filePath);
      const changedLines = changedLinesByFile[relPath] || new Set<number>();
      let errorCount = 0;
      let warningCount = 0;
      const messages = r.messages.filter((msg) => {
        const lineMatch = changedLines.has(msg.line);
        const quietMatch = !quiet || msg.severity === 2;
        const keep = lineMatch && quietMatch;
        if (keep) {
          if (msg.severity === 2) errorCount++;
          else if (msg.severity === 1) warningCount++;
        }
        return keep;
      });
      return {
        ...r,
        messages,
        errorCount,
        warningCount,
      };
    })
    .filter((r) => r.messages.length > 0);

[gemini-code-assist]

These reduce calls can be combined into a single pass.

Suggested change

	const errorCount = filteredResults.reduce((acc, r) => acc + r.errorCount, 0);
	const warningCount = filteredResults.reduce((acc, r) => acc + r.warningCount, 0);
	let errorCount = 0;
	let warningCount = 0;
	for (const r of filteredResults) {
	errorCount += r.errorCount;
	warningCount += r.warningCount;
	}

[gemini-code-assist]

The PR description mentions replacing inline process.exit with a custom LintError handled at the top level. However, process.exit(1) is still used here. To maintain consistency and follow the repository's best practices regarding error handling, consider throwing an error instead.

Suggested change

	console.error("Error: --max-warnings requires a numeric value.");
	process.exit(1);
	throw new Error("Error: --max-warnings requires a numeric value.");

References

1. Throw errors for expected, user-facing errors instead of using process.exit directly. ^(link)