From 2c8898c318f345c58a47130b5e0446cf6e7cd7c1 Mon Sep 17 00:00:00 2001 From: Christopher Bays Date: Mon, 16 Mar 2026 22:38:24 -0400 Subject: [PATCH] refactor: inline remaining thin wrappers and colocate saved-kata tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Inline shouldRecordBetOutcomes, shouldWriteDojoDiary, and shouldWriteDojoSession — same trivial-wrapper pattern as the 8 helpers removed in PR #390. Simplify checkExpiry type guard. Fix test names referencing removed hasObservations helper. Create colocated saved-kata-store.test.ts and remove duplicated tests from execute.test.ts. Fix import alias in session-bridge helpers test. Mutation score holds at 90.92%. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/cli/commands/execute.test.ts | 139 +----------------- src/cli/commands/saved-kata-store.test.ts | 137 +++++++++++++++++ .../cooldown-session.helpers.test.ts | 20 --- .../cooldown-session.helpers.ts | 12 -- .../cycle-management/cooldown-session.ts | 13 +- .../cooldown-session.unit.test.ts | 6 +- .../execution/session-bridge.helpers.test.ts | 2 +- vitest.test-groups.ts | 1 + 8 files changed, 148 insertions(+), 182 deletions(-) create mode 100644 src/cli/commands/saved-kata-store.test.ts diff --git a/src/cli/commands/execute.test.ts b/src/cli/commands/execute.test.ts index 89ee5af..1955a00 100644 --- a/src/cli/commands/execute.test.ts +++ b/src/cli/commands/execute.test.ts @@ -3,7 +3,7 @@ import { mkdirSync, rmSync, writeFileSync, existsSync, readFileSync, readdirSync import { tmpdir } from 'node:os'; import { randomUUID } from 'node:crypto'; import { Command } from 'commander'; -import { registerExecuteCommands, listSavedKatas, loadSavedKata, saveSavedKata, deleteSavedKata } from './execute.js'; +import { registerExecuteCommands } from './execute.js'; import { CycleManager } from '@domain/services/cycle-manager.js'; import { JsonStore } from '@infra/persistence/json-store.js'; import { SessionExecutionBridge } from '@infra/execution/session-bridge.js'; @@ -2137,140 +2137,3 @@ describe('registerExecuteCommands', () => { }); }); -// --------------------------------------------------------------------------- -// Saved kata CRUD — direct function tests for mutation hardening -// --------------------------------------------------------------------------- - -describe('saved kata CRUD functions', () => { - let tmpBase: string; - - beforeEach(() => { - tmpBase = join(tmpdir(), `kata-crud-${randomUUID()}`); - mkdirSync(tmpBase, { recursive: true }); - }); - - afterEach(() => { - rmSync(tmpBase, { recursive: true, force: true }); - }); - - describe('listSavedKatas', () => { - it('returns empty when katas dir does not exist', () => { - expect(listSavedKatas(tmpBase)).toEqual([]); - }); - - it('returns valid katas and skips non-json files', () => { - const katasDir = join(tmpBase, 'katas'); - mkdirSync(katasDir, { recursive: true }); - writeFileSync(join(katasDir, 'valid.json'), JSON.stringify({ name: 'valid', stages: ['build'] })); - writeFileSync(join(katasDir, 'notes.txt'), 'not json'); - writeFileSync(join(katasDir, 'broken.json'), '{ broken }'); - - const result = listSavedKatas(tmpBase); - expect(result).toHaveLength(1); - expect(result[0]!.name).toBe('valid'); - expect(result[0]!.stages).toEqual(['build']); - }); - - it('skips files with invalid schema data', () => { - const katasDir = join(tmpBase, 'katas'); - mkdirSync(katasDir, { recursive: true }); - writeFileSync(join(katasDir, 'bad-schema.json'), JSON.stringify({ name: 123 })); - - expect(listSavedKatas(tmpBase)).toEqual([]); - }); - }); - - describe('loadSavedKata', () => { - it('loads a valid saved kata', () => { - const katasDir = join(tmpBase, 'katas'); - mkdirSync(katasDir, { recursive: true }); - writeFileSync(join(katasDir, 'my-kata.json'), JSON.stringify({ - name: 'my-kata', stages: ['research', 'build'], - })); - - const result = loadSavedKata(tmpBase, 'my-kata'); - expect(result.stages).toEqual(['research', 'build']); - }); - - it('throws when kata does not exist', () => { - expect(() => loadSavedKata(tmpBase, 'missing')).toThrow( - 'Kata "missing" not found.', - ); - }); - - it('throws for invalid JSON content with cause', () => { - const katasDir = join(tmpBase, 'katas'); - mkdirSync(katasDir, { recursive: true }); - writeFileSync(join(katasDir, 'broken.json'), '{ broken }'); - - try { - loadSavedKata(tmpBase, 'broken'); - expect.fail('Should have thrown'); - } catch (err) { - expect(err).toBeInstanceOf(Error); - expect((err as Error).message).toContain('Kata "broken" has invalid JSON:'); - expect((err as Error).cause).toBeDefined(); - } - }); - - it('throws for valid JSON with invalid schema and includes cause', () => { - const katasDir = join(tmpBase, 'katas'); - mkdirSync(katasDir, { recursive: true }); - writeFileSync(join(katasDir, 'bad.json'), JSON.stringify({ name: 123 })); - - try { - loadSavedKata(tmpBase, 'bad'); - expect.fail('Should have thrown'); - } catch (err) { - expect(err).toBeInstanceOf(Error); - expect((err as Error).message).toContain('Kata "bad" has invalid structure.'); - expect((err as Error).cause).toBeDefined(); - } - }); - }); - - describe('saveSavedKata', () => { - it('creates the katas directory and writes the file', () => { - saveSavedKata(tmpBase, 'new-kata', ['build', 'review']); - - const filePath = join(tmpBase, 'katas', 'new-kata.json'); - expect(existsSync(filePath)).toBe(true); - const data = JSON.parse(readFileSync(filePath, 'utf-8')); - expect(data.name).toBe('new-kata'); - expect(data.stages).toEqual(['build', 'review']); - }); - }); - - describe('deleteSavedKata', () => { - it('deletes an existing saved kata', () => { - saveSavedKata(tmpBase, 'del-kata', ['build']); - const filePath = join(tmpBase, 'katas', 'del-kata.json'); - expect(existsSync(filePath)).toBe(true); - - deleteSavedKata(tmpBase, 'del-kata'); - expect(existsSync(filePath)).toBe(false); - }); - - it('throws when kata does not exist', () => { - expect(() => deleteSavedKata(tmpBase, 'nonexistent')).toThrow( - 'Kata "nonexistent" not found.', - ); - }); - }); - - describe('listSavedKatas with non-json files', () => { - it('filters out non-json files from the katas directory', () => { - const dir = join(tmpBase, 'katas'); - mkdirSync(dir, { recursive: true }); - writeFileSync(join(dir, 'readme.txt'), 'not a kata'); - writeFileSync(join(dir, 'valid.json'), JSON.stringify({ - name: 'valid', - stages: ['build'], - })); - - const katas = listSavedKatas(tmpBase); - expect(katas).toHaveLength(1); - expect(katas[0]!.name).toBe('valid'); - }); - }); -}); diff --git a/src/cli/commands/saved-kata-store.test.ts b/src/cli/commands/saved-kata-store.test.ts new file mode 100644 index 0000000..c02a81f --- /dev/null +++ b/src/cli/commands/saved-kata-store.test.ts @@ -0,0 +1,137 @@ +import { join } from 'node:path'; +import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { randomUUID } from 'node:crypto'; +import { listSavedKatas, loadSavedKata, saveSavedKata, deleteSavedKata } from '@cli/commands/saved-kata-store.js'; + +describe('saved-kata-store', () => { + let tmpBase: string; + + beforeEach(() => { + tmpBase = join(tmpdir(), `kata-saved-store-${randomUUID()}`); + mkdirSync(tmpBase, { recursive: true }); + }); + + afterEach(() => { + rmSync(tmpBase, { recursive: true, force: true }); + }); + + describe('listSavedKatas', () => { + it('returns empty when katas dir does not exist', () => { + expect(listSavedKatas(tmpBase)).toEqual([]); + }); + + it('returns valid katas and skips non-json files', () => { + const katasDir = join(tmpBase, 'katas'); + mkdirSync(katasDir, { recursive: true }); + writeFileSync(join(katasDir, 'valid.json'), JSON.stringify({ name: 'valid', stages: ['build'] })); + writeFileSync(join(katasDir, 'notes.txt'), 'not json'); + writeFileSync(join(katasDir, 'broken.json'), '{ broken }'); + + const result = listSavedKatas(tmpBase); + expect(result).toHaveLength(1); + expect(result[0]!.name).toBe('valid'); + expect(result[0]!.stages).toEqual(['build']); + }); + + it('skips files with invalid schema data', () => { + const katasDir = join(tmpBase, 'katas'); + mkdirSync(katasDir, { recursive: true }); + writeFileSync(join(katasDir, 'bad-schema.json'), JSON.stringify({ name: 123 })); + + expect(listSavedKatas(tmpBase)).toEqual([]); + }); + + it('filters out non-json files from the katas directory', () => { + const dir = join(tmpBase, 'katas'); + mkdirSync(dir, { recursive: true }); + writeFileSync(join(dir, 'readme.txt'), 'not a kata'); + writeFileSync(join(dir, 'valid.json'), JSON.stringify({ + name: 'valid', + stages: ['build'], + })); + + const katas = listSavedKatas(tmpBase); + expect(katas).toHaveLength(1); + expect(katas[0]!.name).toBe('valid'); + }); + }); + + describe('loadSavedKata', () => { + it('loads a valid saved kata', () => { + const katasDir = join(tmpBase, 'katas'); + mkdirSync(katasDir, { recursive: true }); + writeFileSync(join(katasDir, 'my-kata.json'), JSON.stringify({ + name: 'my-kata', stages: ['research', 'build'], + })); + + const result = loadSavedKata(tmpBase, 'my-kata'); + expect(result.stages).toEqual(['research', 'build']); + }); + + it('throws when kata does not exist', () => { + expect(() => loadSavedKata(tmpBase, 'missing')).toThrow( + 'Kata "missing" not found.', + ); + }); + + it('throws for invalid JSON content with cause', () => { + const katasDir = join(tmpBase, 'katas'); + mkdirSync(katasDir, { recursive: true }); + writeFileSync(join(katasDir, 'broken.json'), '{ broken }'); + + try { + loadSavedKata(tmpBase, 'broken'); + expect.fail('Should have thrown'); + } catch (err) { + expect(err).toBeInstanceOf(Error); + expect((err as Error).message).toContain('Kata "broken" has invalid JSON:'); + expect((err as Error).cause).toBeDefined(); + } + }); + + it('throws for valid JSON with invalid schema and includes cause', () => { + const katasDir = join(tmpBase, 'katas'); + mkdirSync(katasDir, { recursive: true }); + writeFileSync(join(katasDir, 'bad.json'), JSON.stringify({ name: 123 })); + + try { + loadSavedKata(tmpBase, 'bad'); + expect.fail('Should have thrown'); + } catch (err) { + expect(err).toBeInstanceOf(Error); + expect((err as Error).message).toContain('Kata "bad" has invalid structure.'); + expect((err as Error).cause).toBeDefined(); + } + }); + }); + + describe('saveSavedKata', () => { + it('creates the katas directory and writes the file', () => { + saveSavedKata(tmpBase, 'new-kata', ['build', 'review']); + + const filePath = join(tmpBase, 'katas', 'new-kata.json'); + expect(existsSync(filePath)).toBe(true); + const data = JSON.parse(readFileSync(filePath, 'utf-8')); + expect(data.name).toBe('new-kata'); + expect(data.stages).toEqual(['build', 'review']); + }); + }); + + describe('deleteSavedKata', () => { + it('deletes an existing saved kata', () => { + saveSavedKata(tmpBase, 'del-kata', ['build']); + const filePath = join(tmpBase, 'katas', 'del-kata.json'); + expect(existsSync(filePath)).toBe(true); + + deleteSavedKata(tmpBase, 'del-kata'); + expect(existsSync(filePath)).toBe(false); + }); + + it('throws when kata does not exist', () => { + expect(() => deleteSavedKata(tmpBase, 'nonexistent')).toThrow( + 'Kata "nonexistent" not found.', + ); + }); + }); +}); diff --git a/src/features/cycle-management/cooldown-session.helpers.test.ts b/src/features/cycle-management/cooldown-session.helpers.test.ts index 9a1d49b..fc50a66 100644 --- a/src/features/cycle-management/cooldown-session.helpers.test.ts +++ b/src/features/cycle-management/cooldown-session.helpers.test.ts @@ -19,10 +19,7 @@ import { mapBridgeRunStatusToSyncedOutcome, resolveAppliedProposalIds, selectEffectiveBetOutcomes, - shouldRecordBetOutcomes, shouldWarnOnIncompleteRuns, - shouldWriteDojoDiary, - shouldWriteDojoSession, } from './cooldown-session.helpers.js'; describe('cooldown-session helpers', () => { @@ -34,13 +31,6 @@ describe('cooldown-session helpers', () => { }); }); - describe('shouldRecordBetOutcomes', () => { - it('returns true only for non-empty explicit outcomes', () => { - expect(shouldRecordBetOutcomes([])).toBe(false); - expect(shouldRecordBetOutcomes([{ betId: 'bet-1', outcome: 'complete' }])).toBe(true); - }); - }); - describe('selectEffectiveBetOutcomes', () => { it('prefers explicit outcomes over auto-synced outcomes', () => { expect(selectEffectiveBetOutcomes( @@ -70,16 +60,6 @@ describe('cooldown-session helpers', () => { }); }); - describe('dojo helpers', () => { - it('requires dojoDir to write diary and session outputs', () => { - expect(shouldWriteDojoDiary(undefined)).toBe(false); - expect(shouldWriteDojoDiary('/tmp/dojo')).toBe(true); - expect(shouldWriteDojoSession(undefined, { build: vi.fn() })).toBe(false); - expect(shouldWriteDojoSession('/tmp/dojo', undefined)).toBe(false); - expect(shouldWriteDojoSession('/tmp/dojo', { build: vi.fn() })).toBe(true); - }); - }); - describe('clampConfidenceWithDelta', () => { it('clamps confidence updates to the valid 0..1 range', () => { expect(clampConfidenceWithDelta(0.9, 0.2)).toBe(1); diff --git a/src/features/cycle-management/cooldown-session.helpers.ts b/src/features/cycle-management/cooldown-session.helpers.ts index 98e681e..c4209b6 100644 --- a/src/features/cycle-management/cooldown-session.helpers.ts +++ b/src/features/cycle-management/cooldown-session.helpers.ts @@ -52,10 +52,6 @@ export function shouldWarnOnIncompleteRuns(incompleteRunsCount: number, force: b return incompleteRunsCount > 0 && !force; } -export function shouldRecordBetOutcomes(outcomes: readonly CooldownHelperBetOutcome[]): boolean { - return outcomes.length > 0; -} - export function selectEffectiveBetOutcomes( explicitBetOutcomes: readonly CooldownHelperBetOutcome[], syncedBetOutcomes: readonly CooldownHelperBetOutcome[], @@ -76,14 +72,6 @@ export function buildDiaryBetOutcomesFromCycleBets(bets: readonly CooldownDiaryB })); } -export function shouldWriteDojoDiary(dojoDir: string | undefined): boolean { - return Boolean(dojoDir); -} - -export function shouldWriteDojoSession(dojoDir: string | undefined, dojoSessionBuilder: unknown): boolean { - return Boolean(dojoDir && dojoSessionBuilder); -} - export function clampConfidenceWithDelta(existingConfidence: number, confidenceDelta: number): number { return Math.min(1, Math.max(0, existingConfidence + confidenceDelta)); } diff --git a/src/features/cycle-management/cooldown-session.ts b/src/features/cycle-management/cooldown-session.ts index 9bdeb7a..9b27e2c 100644 --- a/src/features/cycle-management/cooldown-session.ts +++ b/src/features/cycle-management/cooldown-session.ts @@ -49,10 +49,7 @@ import { mapBridgeRunStatusToSyncedOutcome, resolveAppliedProposalIds, selectEffectiveBetOutcomes, - shouldRecordBetOutcomes, shouldWarnOnIncompleteRuns, - shouldWriteDojoDiary, - shouldWriteDojoSession, isJsonFile, isSynthesisPendingFile, isSyncableBet, @@ -397,7 +394,7 @@ export class CooldownSession { } private recordExplicitBetOutcomes(cycleId: string, betOutcomes: BetOutcomeRecord[]): void { - if (!shouldRecordBetOutcomes(betOutcomes)) return; + if (betOutcomes.length === 0) return; this.recordBetOutcomes(cycleId, betOutcomes); } @@ -454,7 +451,7 @@ export class CooldownSession { humanPerspective?: string; }): void { // Stryker disable next-line ConditionalExpression: guard redundant with catch in writeDiaryEntry - if (!shouldWriteDojoDiary(this.deps.dojoDir)) return; + if (!this.deps.dojoDir) return; this.writeDiaryEntry({ cycleId: input.cycleId, @@ -486,7 +483,7 @@ export class CooldownSession { ruleSuggestions?: RuleSuggestion[]; synthesisProposals?: SynthesisProposal[]; }): void { - if (!shouldWriteDojoDiary(this.deps.dojoDir)) return; + if (!this.deps.dojoDir) return; this.writeDiaryEntry({ cycleId: input.cycleId, @@ -501,7 +498,7 @@ export class CooldownSession { } private writeOptionalDojoSession(cycleId: string, cycleName?: string): void { - if (!shouldWriteDojoSession(this.deps.dojoDir, this.deps.dojoSessionBuilder)) return; + if (!this.deps.dojoDir || !this.deps.dojoSessionBuilder) return; this.writeDojoSession(cycleId, cycleName); } @@ -1224,7 +1221,7 @@ export class CooldownSession { private runExpiryCheck(): void { try { // Stryker disable next-line ConditionalExpression: guard redundant with catch — checkExpiry absence is swallowed - if (typeof (this.deps.knowledgeStore as unknown as Record)?.['checkExpiry'] !== 'function') return; + if (typeof (this.deps.knowledgeStore as { checkExpiry?: unknown }).checkExpiry !== 'function') return; const result = this.deps.knowledgeStore.checkExpiry(); for (const message of buildExpiryCheckMessages(result)) { logger.debug(message); diff --git a/src/features/cycle-management/cooldown-session.unit.test.ts b/src/features/cycle-management/cooldown-session.unit.test.ts index 08ee095..3ffeec7 100644 --- a/src/features/cycle-management/cooldown-session.unit.test.ts +++ b/src/features/cycle-management/cooldown-session.unit.test.ts @@ -1306,7 +1306,7 @@ describe('CooldownSession unit seams', () => { } }); - it('collectSynthesisObservations uses hasObservations to filter empty observation lists', async () => { + it('collectSynthesisObservations filters out runs with no observations', async () => { const fixture = createFixture(); try { @@ -1331,7 +1331,7 @@ describe('CooldownSession unit seams', () => { const result = await session.prepare(cycle.id); const input = JSON.parse(readFileSync(result.synthesisInputPath, 'utf-8')); - // Empty observations should be filtered out by hasObservations + // Empty observations should be filtered out expect(input.observations).toHaveLength(0); } finally { fixture.cleanup(); @@ -1676,7 +1676,7 @@ describe('CooldownSession follow-up pipeline', () => { } }); - it('hasObservations filters out runs with empty observations from synthesis input', async () => { + it('synthesis input excludes runs with empty observations', async () => { const fixture = createFixture(); try { diff --git a/src/infrastructure/execution/session-bridge.helpers.test.ts b/src/infrastructure/execution/session-bridge.helpers.test.ts index 7385b80..29ccf54 100644 --- a/src/infrastructure/execution/session-bridge.helpers.test.ts +++ b/src/infrastructure/execution/session-bridge.helpers.test.ts @@ -9,7 +9,7 @@ import { matchesCycleRef, resolveAgentId, -} from './session-bridge.helpers.js'; +} from '@infra/execution/session-bridge.helpers.js'; describe('session-bridge helpers', () => { describe('canTransitionCycleState', () => { diff --git a/vitest.test-groups.ts b/vitest.test-groups.ts index 6075507..76a805a 100644 --- a/vitest.test-groups.ts +++ b/vitest.test-groups.ts @@ -9,6 +9,7 @@ export const integrationTestFiles = [ export const mutationTestFiles = [ 'src/cli/commands/execute.test.ts', 'src/cli/commands/execute.helpers.test.ts', + 'src/cli/commands/saved-kata-store.test.ts', 'src/features/execute/workflow-runner.test.ts', 'src/features/cycle-management/cooldown-session.test.ts', 'src/features/cycle-management/cooldown-session-prepare.test.ts',