enhance(#3267): reduce handleEvaluate complexity below refactor-trigger's own threshold (#3978)

* fix(#3267): reduce handleEvaluate complexity below refactor-trigger's own threshold handleEvaluate scored 26 (then 21 after later #3261 commits) against the complexity-triggered-refactor feature's own default threshold of 15. Extracts the read-and-analyze loop (analyzeTouchedFiles) and the artifact/baseline/ ledger write path (finalizeEvaluation) into named helpers, per the issue's suggested direction. Behavior-preserving: every existing test in tests/refactor-trigger-cli.test.cjs is unchanged, and every degrade-path reason code (REFACTOR_INVALID_PHASE, REFACTOR_GIT_UNAVAILABLE, REFACTOR_NO_TOUCHED_FILES, REFACTOR_FILE_UNREADABLE, REFACTOR_ANALYZER_UNSUPPORTED, REFACTOR_ANALYZER_UNPARSEABLE, REFACTOR_BASELINE_WRITE_FAILED, REFACTOR_STRICT_NOT_ENFORCING) keeps its current value and emission path. The four complexity-trigger.cts lexer functions (scanFunctions, stripLiterals, skipTypeExpr, skipGenericParamList) are deliberately left untouched, per ADR-1953 D5 — they are a hand-rolled lexer state machine, densely branchy by construction, and refactoring them to lower the metric would be exactly the "split a coherent function to satisfy a metric" behavior D5 exists to prevent. Closes #3267 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs: backfill changeset PR number for #3978 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: sim <sim@local> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-28 08:12:27 -04:00
parent d24e22b156
commit 4f32209f78
3 changed files with 130 additions and 55 deletions
--- a/.changeset/plucky-pandas-wave.md
+++ b/.changeset/plucky-pandas-wave.md
@@ -0,0 +1,5 @@
+---
+type: Fixed
+pr: 3978
+---
+**Reduced the complexity of the refactor-trigger evaluate handler.** `handleEvaluate` scored above the complexity-triggered-refactor feature's own default threshold; the read/analyze loop and the artifact/baseline/ledger write path are now separate named helpers, with no change to CLI behavior, output shape, or reason codes. (#3267)
--- a/src/refactor-trigger-command-router.cts
+++ b/src/refactor-trigger-command-router.cts
@@ -481,54 +481,22 @@ function resolveLedgerWindow(

 const EVALUATE_USAGE = 'Usage: gsd-tools refactor evaluate --phase <N> [--since <ref>] [--raw]';

-function handleEvaluate(
-  args: string[],
+/**
+ * Step 5 of `evaluate`: filter `touched` with `isAnalyzablePath`, read each
+ * survivor and classify it into `analyzed`. A read failure (or a path that
+ * escapes the project root) skips that one file with
+ * `REFACTOR_FILE_UNREADABLE` and the loop continues — never aborts the
+ * evaluation. Only files that were SUCCESSFULLY analyzed feed
+ * `successfullyAnalyzedFiles` — an unreadable file must never wipe that
+ * file's baseline history via nextBaseline's "file in analyzedFiles but
+ * function missing" prune rule.
+ */
+function analyzeTouchedFiles(
  cwd: string,
-  raw: boolean,
-  c: CoreModule,
+  touched: string[],
  complexity: ComplexityModule,
-  git: GitModule,
-  windowsOverride: WindowsModule | undefined,
-): unknown {
-  const rest = args.slice(2);
-  const phaseCheck = requirePhaseArg(rest, complexity, EVALUATE_USAGE);
-  if (!phaseCheck.ok) return phaseCheck.result;
-
-  // Step 3: resolve PHASE_DIR + anchor (phaseStartCommit, or --since override).
-  const resolved = resolvePhaseDirForArg(cwd, phaseCheck.phase);
-  if (resolved === null) {
-    c.output({ verdict: complexity.VERDICT.SKIPPED, reason: complexity.REASON.REFACTOR_INVALID_PHASE, phase: phaseCheck.phase }, raw);
-    return undefined;
-  }
-  const { phaseDir, padded } = resolved;
-
-  const sinceFlag = readFlag(rest, '--since');
-  const sinceOverride = sinceFlag.present && !sinceFlag.flagShaped ? sinceFlag.value.trim() : '';
-  const sinceRef = sinceOverride !== '' ? sinceOverride : git.phaseStartCommit(cwd, phaseDir);
-  if (sinceRef === null) {
-    c.output({ verdict: complexity.VERDICT.SKIPPED, reason: complexity.REASON.REFACTOR_GIT_UNAVAILABLE, phase: padded }, raw);
-    return undefined;
-  }
-
-  const touched = git.changedFilesSince(cwd, sinceRef);
-  if (touched === null) {
-    c.output({ verdict: complexity.VERDICT.SKIPPED, reason: complexity.REASON.REFACTOR_GIT_UNAVAILABLE, phase: padded }, raw);
-    return undefined;
-  }
-
-  // Step 4: empty touched set.
-  if (touched.length === 0) {
-    c.output({ verdict: complexity.VERDICT.BELOW_THRESHOLD, reason: complexity.REASON.REFACTOR_NO_TOUCHED_FILES, phase: padded }, raw);
-    return undefined;
-  }
-
-  // Step 5: filter with isAnalyzablePath; read each survivor. A read failure
-  // (or a path that escapes the project root) skips that one file with a
-  // reason and the run continues — never aborts the evaluation.
+): { analyzed: AnalyzedFile[]; successfullyAnalyzedFiles: string[] } {
  const analyzed: AnalyzedFile[] = [];
-  // Only files that were SUCCESSFULLY analyzed feed nextBaseline's prune set
-  // — an unreadable file must never wipe that file's baseline history via
-  // the "file in analyzedFiles but function missing" prune rule.
  const successfullyAnalyzedFiles: string[] = [];
  for (const relFile of touched) {
    if (!complexity.isAnalyzablePath(relFile)) continue;
@@ -552,17 +520,36 @@ function handleEvaluate(
      successfullyAnalyzedFiles.push(relFile);
    }
  }
+  return { analyzed, successfullyAnalyzedFiles };
+}

-  // Step 6.
-  const planningDirPath = planningDir(cwd);
-  const baselineRead = complexity.readBaseline(planningDirPath);
-  const evalConfig = readEvalConfig(cwd);
-  const evaluation: Evaluation = complexity.evaluateCandidates({
-    analyzed,
-    baseline: baselineRead.baseline,
-    threshold: evalConfig.threshold,
-    jumpDelta: evalConfig.jumpDelta,
-  });
+interface FinalizeEvaluationOptions {
+  cwd: string;
+  phaseDir: string;
+  padded: string;
+  planningDirPath: string;
+  complexity: ComplexityModule;
+  evaluation: Evaluation;
+  evalConfig: { threshold: unknown; jumpDelta: unknown; strict: boolean; windowsEnforce: boolean };
+  baselineRead: ReturnType<ComplexityModule['readBaseline']>;
+  analyzed: AnalyzedFile[];
+  successfullyAnalyzedFiles: string[];
+  windowsOverride: WindowsModule | undefined;
+}
+
+/**
+ * Steps 7-9 of `evaluate`: write the proposal artifact when TRIGGERED, write
+ * the next baseline (failure is reported but never fails the command),
+ * record the strict-mode ledger window when applicable, and assemble the
+ * final result object. Pure orchestration over the side-effecting helpers —
+ * every degrade path (artifact write throw, baseline write failure, ledger
+ * unavailable) keeps its existing reason code and result shape.
+ */
+function finalizeEvaluation(opts: FinalizeEvaluationOptions): Record<string, unknown> {
+  const {
+    cwd, phaseDir, padded, planningDirPath, complexity, evaluation, evalConfig,
+    baselineRead, analyzed, successfullyAnalyzedFiles, windowsOverride,
+  } = opts;

  // Step 7.
  let artifactWritten = false;
@@ -633,6 +620,70 @@ function handleEvaluate(
  if (ledgerNote !== undefined) result.ledger_note = ledgerNote;
  if (warnings.length > 0) result.warnings = warnings;

+  return result;
+}
+
+function handleEvaluate(
+  args: string[],
+  cwd: string,
+  raw: boolean,
+  c: CoreModule,
+  complexity: ComplexityModule,
+  git: GitModule,
+  windowsOverride: WindowsModule | undefined,
+): unknown {
+  const rest = args.slice(2);
+  const phaseCheck = requirePhaseArg(rest, complexity, EVALUATE_USAGE);
+  if (!phaseCheck.ok) return phaseCheck.result;
+
+  // Step 3: resolve PHASE_DIR + anchor (phaseStartCommit, or --since override).
+  const resolved = resolvePhaseDirForArg(cwd, phaseCheck.phase);
+  if (resolved === null) {
+    c.output({ verdict: complexity.VERDICT.SKIPPED, reason: complexity.REASON.REFACTOR_INVALID_PHASE, phase: phaseCheck.phase }, raw);
+    return undefined;
+  }
+  const { phaseDir, padded } = resolved;
+
+  const sinceFlag = readFlag(rest, '--since');
+  const sinceOverride = sinceFlag.present && !sinceFlag.flagShaped ? sinceFlag.value.trim() : '';
+  const sinceRef = sinceOverride !== '' ? sinceOverride : git.phaseStartCommit(cwd, phaseDir);
+  if (sinceRef === null) {
+    c.output({ verdict: complexity.VERDICT.SKIPPED, reason: complexity.REASON.REFACTOR_GIT_UNAVAILABLE, phase: padded }, raw);
+    return undefined;
+  }
+
+  const touched = git.changedFilesSince(cwd, sinceRef);
+  if (touched === null) {
+    c.output({ verdict: complexity.VERDICT.SKIPPED, reason: complexity.REASON.REFACTOR_GIT_UNAVAILABLE, phase: padded }, raw);
+    return undefined;
+  }
+
+  // Step 4: empty touched set.
+  if (touched.length === 0) {
+    c.output({ verdict: complexity.VERDICT.BELOW_THRESHOLD, reason: complexity.REASON.REFACTOR_NO_TOUCHED_FILES, phase: padded }, raw);
+    return undefined;
+  }
+
+  // Step 5: filter, read, and classify each touched file.
+  const { analyzed, successfullyAnalyzedFiles } = analyzeTouchedFiles(cwd, touched, complexity);
+
+  // Step 6.
+  const planningDirPath = planningDir(cwd);
+  const baselineRead = complexity.readBaseline(planningDirPath);
+  const evalConfig = readEvalConfig(cwd);
+  const evaluation: Evaluation = complexity.evaluateCandidates({
+    analyzed,
+    baseline: baselineRead.baseline,
+    threshold: evalConfig.threshold,
+    jumpDelta: evalConfig.jumpDelta,
+  });
+
+  // Steps 7-9: artifact write, baseline persistence, strict-mode ledger.
+  const result = finalizeEvaluation({
+    cwd, phaseDir, padded, planningDirPath, complexity, evaluation, evalConfig,
+    baselineRead, analyzed, successfullyAnalyzedFiles, windowsOverride,
+  });
+
  c.output(result, raw);
  return undefined;
 }
--- a/tests/refactor-trigger-cli.test.cjs
+++ b/tests/refactor-trigger-cli.test.cjs
@@ -41,6 +41,8 @@ const {
  VERDICT,
  REASON,
  PROPOSAL_SUFFIX,
+  DEFAULTS,
+  analyzeSource,
 } = require('../gsd-core/bin/lib/complexity-trigger.cjs');
 const gitBaseBranch = require('../gsd-core/bin/lib/git-base-branch.cjs');
 const windowsModule = require('../gsd-core/bin/lib/broken-windows.cjs');
@@ -979,3 +981,20 @@ describe('refactor-trigger: loop wiring', () => {
    }
  });
 });
+
+// ─── Regression: handleEvaluate self-complexity (#3267) ────────────────────
+
+describe('refactor-trigger-command-router: self-complexity', () => {
+  test('handleEvaluateStaysAtOrBelowThreshold', () => {
+    const routerPath = path.join(__dirname, '..', 'src', 'refactor-trigger-command-router.cts');
+    const source = fs.readFileSync(routerPath, 'utf8');
+    const result = analyzeSource(source);
+    assert.strictEqual(result.ok, true, 'router source must analyze cleanly');
+    const handleEvaluateFn = result.functions.find((f) => f.name === 'handleEvaluate');
+    assert.ok(handleEvaluateFn, 'handleEvaluate must still be found by the analyzer');
+    assert.ok(
+      handleEvaluateFn.score <= DEFAULTS.threshold,
+      `handleEvaluate scores ${handleEvaluateFn.score}, must be <= ${DEFAULTS.threshold} (refactor.complexity_threshold)`,
+    );
+  });
+});