refactor: remove standalone validate CLI command #8

boneskull · 2025-10-19T06:49:10Z

Remove validate command implementation and tests
Remove command registration from CLI
Remove ValidateCommandArgs interface
Update documentation (README.md and ARCHITECTURE.md)

Internal validation methods (engine.validate, loader.validate, config.validate)
remain intact for use by other commands like run.

Copilot

Pull Request Overview

This PR refactors the codebase by removing the standalone validate command from the CLI interface while preserving internal validation functionality for use by other commands like run. The change simplifies the CLI surface area by eliminating a redundant command that duplicated validation logic already present in the execution flow.

Key changes:

Removed the validate command implementation and associated test suite
Cleaned up CLI command registration and type definitions
Updated documentation to reflect the removed command

Reviewed Changes

Copilot reviewed 14 out of 15 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
`test/contract/validate-command.test.ts`	Removed comprehensive test suite for the validate command
`src/types/cli.ts`	Removed `ValidateCommandArgs` interface definition
`src/cli/index.ts`	Removed validate command registration and import
`src/cli/commands/validate.ts`	Removed entire validate command implementation (~683 lines)
`README.md`	Removed validation command documentation section
`ARCHITECTURE.md`	Removed validation command from removal candidates section

Copilot

Pull Request Overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Copilot

Pull Request Overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Copilot

Pull Request Overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Copilot

Pull Request Overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

Copilot · 2025-10-19T21:48:38Z

test/integration/iterations.test.ts

      // (though we can't be too strict due to warmup, process startup, etc)
-      expect(duration1, 'to be less than', duration2);
+      // Allow for 50% variance to account for system overhead and variability
+      expect(duration1, 'to be less than', duration2 * 1.5);


[nitpick] The magic number 1.5 should be extracted as a named constant like TIMING_VARIANCE_TOLERANCE to improve code readability and maintainability.

Copilot · 2025-10-19T21:48:39Z

src/types/core.ts

  readonly exclude: string[];
  /** Default number of iterations per task */
  readonly iterations: number;
+  /** How to limit benchmark execution: 'time', 'iterations', 'any', or 'all' */


[nitpick] The union type values are reordered alphabetically which is good, but the comment should also reflect this order for consistency: 'all', 'any', 'iterations', or 'time'.

Suggested change

/** How to limit benchmark execution: 'time', 'iterations', 'any', or 'all' */

/** How to limit benchmark execution: 'all', 'any', 'iterations', or 'time' */

Copilot · 2025-10-19T21:48:39Z

src/types/cli.ts

  readonly i?: number;
  /** Number of iterations */
  readonly iterations?: number;
+  /** How to limit benchmark execution */


[nitpick] The comment is less descriptive than the corresponding comment in core.ts. For consistency, it should include the specific options: 'How to limit benchmark execution: 'all', 'any', 'iterations', or 'time''.

Suggested change

/** How to limit benchmark execution */

/** How to limit benchmark execution: 'all', 'any', 'iterations', or 'time' */

Copilot · 2025-10-19T21:48:39Z

src/core/engine.ts

+          // Both must be met - tinybench default behavior
          effectiveTime = Math.min(config.time || 1000, 2000);
-          effectiveIterations = 1; // Minimal iterations so time is the limiting factor
-          break;
-
-        case 'iterations':
-          // Iterations is the limit, use minimal time
-          effectiveTime = 1;
          effectiveIterations = config.iterations;
          break;


[nitpick] The magic numbers 1000 and 2000 should be extracted as named constants like DEFAULT_TIME_MS and MAX_TIME_MS to improve code maintainability and make the logic clearer.

Copilot

Pull Request Overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Copilot · 2025-10-19T22:08:46Z

src/reporters/csv.ts

-    }
-
+    // Always write to stdout when no output path is specified
+    // The quiet flag only affects progress messages (stderr), not data output


The comment should clarify that this behavior only applies when no output path is specified, since the function name and context suggest this is specifically for stdout output.

Suggested change

// The quiet flag only affects progress messages (stderr), not data output

// When no output path is specified (i.e., writing to stdout), the quiet flag only affects progress messages (stderr), not CSV data output

- Remove validate command implementation and tests - Remove command registration from CLI - Remove ValidateCommandArgs interface - Update documentation (README.md and ARCHITECTURE.md) Internal validation methods (engine.validate, loader.validate, config.validate) remain intact for use by other commands like run. # Conflicts: # README.md

# Conflicts: # test/integration/quiet-mode.test.ts

Copilot AI review requested due to automatic review settings October 19, 2025 06:49

github-actions bot assigned boneskull Oct 19, 2025

boneskull force-pushed the feature/remove-validate-command branch from 1b3d607 to 4adce12 Compare October 19, 2025 06:49