feat(ai): Foundation — Error Types + Constants [PR 1/7]

[ianwhitedeveloper]

Context

Part of the consolidation effort for draft PR #394. Per Eric's direction, the 80+ commit / 104-file feature branch is being decomposed into 7 small, focused PRs targeting ai-testing-framework-implementation-consolidation, which will eventually land as a single clean PR against master.

Consolidation order: 1 → 2 → 3 → 4 → 5 → 6 → 7 → master

---

What's in this PR

PR 1 of 7: Foundation — the leaf modules everything else depends on.

New source files

File	Description
source/ai-errors.js	9 named error types via error-causes: ParseError, ValidationError, SecurityError, TimeoutError, AgentProcessError, AITestError, OutputError, ExtractionParseError, ExtractionValidationError
source/ai-errors.test.js	31 tests — descriptor shape, named export identity, createError integration (2 types), and handleAIErrors routing (exhaustive handler map pattern)
source/constants.js	defaults, constraints, and Zod schemas for all AI runner config options: runsSchema, thresholdSchema, concurrencySchema (with concurrencyMax: 50), timeoutSchema, agentSchema, calculateRequiredPassesSchema, aiTestOptionsSchema
source/constants.test.js	34 tests — all schemas with boundary conditions + full aiTestOptionsSchema coverage including lazy cwd default

Config changes

File	Change
eslint.config.js	ecmaVersion: 2017 → 2022 (prerequisite for numeric separators + optional chaining used throughout the framework); Object.assign → spread
vitest.config.js	Exclude source/e2e.test.js (uses Riteway/TAP, not Vitest — sourced from feature branch)
package.json	error-causes moved from devDependencies → dependencies (runtime CLI dep); zod added to dependencies

---

Epic requirements addressed

From the original epic:

• ✅ Error handling infrastructure (error-causes pattern) — every downstream module uses createError from these descriptors
• ✅ Zod schema validation for CLI args — aiTestOptionsSchema is the primary validation boundary for riteway ai <file>

---

WIP fixes from the consolidation plan

No WIP fixes were scoped to this module. All issues in the scratch pad apply to later PRs.

---

Key correctness decisions made during consolidation

Lazy process.cwd() default — the original feature branch had:

cwd: z.string().default(process.cwd())  // evaluated once at module load

Fixed to:

cwd: z.string().default(() => process.cwd())  // evaluated at parse time

Prevents stale value if cwd changes after module import (common in test environments).

concurrencyMax: 50 — concurrencySchema on the feature branch had no upper bound, which would accept concurrency: 10000 and exhaust file descriptors. Added concurrencyMax: 50 to constraints and enforced in the schema.

test.each convention established — feature branch used for...of loops inside single test blocks. Replaced with Vitest test.each throughout so each case gets its own named test entry. This convention is documented in the consolidation plan for PRs 2–7.

handleAIErrors is exhaustive — requires all registered error types to have handlers or throws MissingHandler. Tests use an allNoop spread pattern to satisfy this while testing routing of a single type.

---

Verification

npm test     → 71 tests pass (31 ai-errors + 34 constants + 6 vitest)
npm run lint → Lint complete.
npm run ts   → TypeScript check complete.

---

What's next

PR	Module	Depends on
2/7	Utilities — Debug Logger + Concurrency Limiter + TAP YAML	This PR
3/7	Parsers + Execute Agent	PR 1 + 2
4/7	Config + Validation	PR 1–3
5/7	Test Extractor + Core Runner	PR 1–4
6/7	Test Output + CLI Integration	PR 1–5
7/7	E2E Tests + Fixtures + Config	All prior

Made with Cursor

[ianwhitedeveloper]

@cursor please /review

[cursor]

🔬 Code Review

✅ Verification Status

All checks pass successfully:

• Tests: 71/71 passing (31 ai-errors + 34 constants + 6 vitest)
• Lint: Clean
• TypeScript: No errors

---

Code Quality Assessment

source/ai-errors.js ✅

Strengths:

• Clean, focused module with single responsibility
• Excellent use of error-causes pattern for structured error handling
• Named exports properly destructured from aiErrors object
• All 9 error types have consistent descriptor shape (code, message)

Observations:

• Error messages are clear and descriptive
• Code follows functional programming principles from javascript.mdc
• Module is properly organized by feature

source/ai-errors.test.js ✅

Strengths:

• Exemplary TDD adherence per tdd.mdc:
  • Answers all 5 questions (unit under test, expected behavior, actual output, expected output, how to find bug)
  • Tests are isolated with no shared mutable state
  • Uses test.each for DRY test patterns (good convention established)
• Comprehensive coverage:
  • Descriptor shape validation (name + code) for all 9 error types
  • Named export identity checks
  • createError integration tests
  • handleAIErrors routing with exhaustive handler pattern
• Smart test helper pattern with allNoop spread to satisfy exhaustive handler requirement
• Clear given/should descriptions provide excellent documentation

Observation:
The errorTable array structure is clean and maintainable.

source/constants.js ✅

Strengths:

• Well-organized separation: defaults, constraints, schemas
• Critical fix applied: Lazy cwd default (() => process.cwd()) prevents stale values
• Security improvement: concurrencyMax: 50 prevents file descriptor exhaustion
• Numeric separators (300_000, 3_600_000) improve readability (enabled by ecmaVersion 2022)
• Zod schemas have helpful error messages
• Schemas are composable (calculateRequiredPassesSchema reuses sub-schemas)

Observations:

• Schema validation is comprehensive with proper min/max bounds
• aiTestOptionsSchema serves as the primary validation boundary (per PR description)
• Optional fields (agentConfigPath, projectRoot) properly handled

source/constants.test.js ✅

Strengths:

• Exhaustive boundary testing:
  • Min/max values for all numeric schemas
  • NaN/Infinity edge cases for thresholdSchema
  • Empty string validation for filePath
  • Supported/unsupported enum values for agentSchema
• Tests for lazy cwd default evaluation
• Multiple validation error test for calculateRequiredPassesSchema
• Full aiTestOptionsSchema integration tests with default application
• Error message content validation (ensures helpful UX)

Observations:

• Tests answer all 5 TDD questions
• Coverage includes both positive and negative cases
• Clear test organization by schema

eslint.config.js ✅

Changes:

• ecmaVersion: 2017 → 2022 (prerequisite for numeric separators, optional chaining)
• Object.assign → spread operator (aligns with javascript.mdc preference for immutability)

Observation:
Config modernization is justified and minimal.

vitest.config.js ✅

Changes:

• Added source/e2e.test.js to exclusions (uses Riteway/TAP, not Vitest)

Observation:
Proper test framework separation.

package.json ✅

Changes:

• error-causes moved from devDependencies → dependencies ✅ (runtime CLI dependency)
• zod added to dependencies ✅ (runtime validation)

Observation:
Dependency changes are correct and justified.

---

Architecture & Design Decisions

1. Error Handling Strategy ⭐

• error-causes pattern provides structured error handling
• handleAIErrors exhaustive handler pattern ensures all error types are handled
• Clear separation between error types enables precise error routing

2. Validation Strategy ⭐

• Zod schemas provide runtime type safety
• Composable schema design enables reuse
• Helpful error messages improve developer experience
• Lazy defaults prevent common bugs

3. Test Design ⭐

• test.each convention (replacing for...of loops) provides granular test reporting
• Exhaustive handler testing pattern with allNoop spread is elegant
• Boundary testing is thorough

---

Adherence to Project Standards

javascript.mdc compliance: ✅

• Functional programming style throughout
• Const usage, no mutation
• Concise syntax (destructuring, arrow functions)
• Clear, active naming
• No classes, pure functions and data structures

tdd.mdc compliance: ✅

• All tests answer the 5 questions
• Tests are isolated and explicit
• Thorough edge case coverage
• given/should pattern consistently applied

Naming conventions: ✅

• Functions are verbs or clear nouns (runsSchema, handleAIErrors)
• Predicates would be yes/no questions (not applicable in this PR)
• No redundant naming

---

Security & Performance

Security:

• ✅ concurrencyMax: 50 prevents resource exhaustion
• ✅ Input validation via Zod schemas
• ✅ No code injection vectors

Performance:

• ✅ Lazy cwd default prevents unnecessary process.cwd() calls
• ✅ Schemas are defined once at module level
• ✅ No unnecessary allocations

---

Recommendations

None. This PR represents excellent engineering discipline:

1. Code quality: Clean, functional, well-organized
2. Test quality: Comprehensive, isolated, well-documented
3. Design decisions: All critical fixes documented in PR description
4. Standards adherence: Perfect alignment with project guidelines
5. Documentation: PR description is thorough and clear

The established conventions (test.each, exhaustive handlers, lazy defaults) will benefit subsequent PRs in the consolidation effort.

---

Summary

Status: ✅ APPROVED

This is a well-crafted foundation PR. The error infrastructure and validation schemas are solid, tests are exemplary, and the correctness fixes (lazy cwd, concurrencyMax) demonstrate careful engineering. Ready to merge.

 

[janhesters]

I don't know if it makes sense to test constants. CC: @ericelliott

Tests should only cover behavior, shouldn't they? So for example here, if there is a default of 4 runs, we should test that in the function by invoking it without arguments, don't we?

[ericelliott]

Yes, agreed. Test behaviors, not values, not types.

[ianwhitedeveloper]

Agreed - I admit I merged this too hastily and manually reviewed after the fact and saw this as well. Will clean up in a PR to the consolidated branch referencing these comments accordingly.

[ianwhitedeveloper]

Remediated in PR #412: the defaults and constraints value-testing describe blocks have been deleted from constants.test.js, and the three tautological expected: defaults.X assertions in the aiTestOptionsSchema test have been replaced with literals (4, 75, 'claude'). See: #412

[janhesters]

Similar comment to this testing file. We're kind just testing the error causes API.

If you want to test these at all, you should probably test them as pairs of getters and setters:

test('ParseError round-trips through handleAIErrors', () => {
  const err = createError(ParseError);
  let result;

  handleAIErrors({
    ...allNoop,
    ParseError: (cause) => { result = cause; }
  })(err);

  assert({
    given: 'an error created from ParseError',
    should: 'route to the correct handler with the right cause name',
    actual: result.name,
    expected: 'ParseError'
  });
});

The createError is the setter, handleAIErrors is the getter. That's the only contract worth testing — that errors you create actually route to the right handler with the right cause. The descriptor shape tests (name, code, named export equality) are just testing that errorCauses returns what you passed in.

[ericelliott]

This is just testing error causes. Don't do that. Do test that code that uses these throws the correct errors, and that functions handle them correctly. In other words, test how your consuming code USES errors, instead of testing the already tested error-causes API

[ianwhitedeveloper]

Agreed

[ianwhitedeveloper]

Remediated in PR #412: the error descriptors describe block (27 tests over 9 error types testing descriptor shape) and the createError integration describe block have been deleted. Only the handleAIErrors behavioral routing tests remain — createError as setter, handleAIErrors as getter. See: #412