From 4f32209f78299e03919453f441afd4cdc808c847 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 28 Aug 2026 08:12:27 -0400 Subject: [PATCH] enhance(#3267): reduce handleEvaluate complexity below refactor-trigger's own threshold (#3978) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 * docs: backfill changeset PR number for #3978 Co-Authored-By: Claude Sonnet 5 --------- Co-authored-by: sim Co-authored-by: Claude Sonnet 5 --- .changeset/plucky-pandas-wave.md | 5 + src/refactor-trigger-command-router.cts | 161 ++++++++++++++++-------- tests/refactor-trigger-cli.test.cjs | 19 +++ 3 files changed, 130 insertions(+), 55 deletions(-) create mode 100644 .changeset/plucky-pandas-wave.md diff --git a/.changeset/plucky-pandas-wave.md b/.changeset/plucky-pandas-wave.md new file mode 100644 index 000000000..7c773a285 --- /dev/null +++ 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) diff --git a/src/refactor-trigger-command-router.cts b/src/refactor-trigger-command-router.cts index 8c606ebc7..32de4f0b1 100644 --- 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 [--since ] [--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; + 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 { + 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; } diff --git a/tests/refactor-trigger-cli.test.cjs b/tests/refactor-trigger-cli.test.cjs index faab531c2..5970223d7 100644 --- 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)`, + ); + }); +});