[Feature]: Remove export from 19 internal implementation details

[sjnims]

Feature Type

Pipeline architecture

Problem or Need

19 functions across the pipeline are exported solely so unit tests can import them directly. These are internal implementation details — not called from any other production module — and their export keyword falsely signals they're part of the module's public contract. This couples tests to implementation details, making refactors harder.

Proposed Solution

Remove the export keyword from 19 functions and refactor their tests to either:

• (a) Test through the parent module's public function (preferred for tightly-coupled helpers), or
• (b) Continue importing from the module file directly (TypeScript allows importing non-exported members for tests via path imports in some setups, or keep the export but gate behind @internal)

If (a) isn't practical for a given function, fall back to adding @internal instead of removing export.

Stage 1 — Analysis (2 exports):

Export	File	Line
analyzeHooks	src/stages/1-analysis/hook-analyzer.ts	287
readRawManifest	src/stages/1-analysis/plugin-parser.ts	89

Stage 2 — Generation (4 exports):

Export	File	Line
createBaseScenario	src/stages/2-generation/diversity-manager.ts	83
baseToTestScenario	src/stages/2-generation/diversity-manager.ts	106
createBatchConfig	src/stages/2-generation/batch-calculator.ts	128
wouldExceedTokenLimit	src/stages/2-generation/batch-calculator.ts	187

Stage 3 — Execution (9 exports):

Export	File	Line
filterTriggerCaptures	src/stages/3-execution/hook-capture.ts	164
analyzeHookResponses	src/stages/3-execution/hook-capture.ts	438
isFilesPersistedEvent	src/stages/3-execution/sdk-client.ts	342
isTaskNotificationMessage	src/stages/3-execution/sdk-client.ts	355
isAuthStatusMessage	src/stages/3-execution/sdk-client.ts	368
extractSessionId	src/stages/3-execution/transcript-builder.ts	255
countAssistantTurns	src/stages/3-execution/transcript-builder.ts	292
isSuccessfulExecution	src/stages/3-execution/transcript-builder.ts	271
parseCustomId	src/stages/4-evaluation/batch-evaluator.ts	163

Stage 4 — Evaluation (1 export):

Export	File	Line
isLowVariance	src/stages/4-evaluation/multi-sampler.ts	293

State (3 exports):

Export	File	Line
updateStateWithPartialExecutions	src/state/updates.ts	146
updateStateWithError	src/state/updates.ts	166
getIncompleteScenarios	src/state/queries.ts	117

Steps:

1. For each export, determine if the test can be refactored to test through the parent module's public interface
2. If yes: remove export, refactor tests
3. If no: add @internal JSDoc tag instead (knip already excludes @internal)
4. Update barrel re-exports as needed
5. Verify: npm run build && npm run typecheck && npm test && npm run knip

Pipeline Stage Affected

General / Multiple stages

Component Type (if applicable)

Not component-specific

Alternatives Considered

• Keeping all exports as-is with documentation — rejected because it misleads about API surface
• Moving functions to test utility files — rejected because they're genuinely called internally by production code; duplicating logic would be worse

How important is this feature to you?

Medium - Would be nice to have

Additional Context

Identified during comprehensive dead code analysis (2026-01-31). Related: #371 (dead types), #372 (stdlib wrappers). The 3 state exports (updateStateWithPartialExecutions, updateStateWithError, getIncompleteScenarios) are scaffolding for an unimplemented checkpointing/resume feature — consider whether to keep or delete entirely.

---

🤖 Created with Claude Code

[claude]

Analysis: Remove export from 19 internal implementation details

1. Problem Assessment

Clearly defined? Yes — the issue provides an exhaustive inventory of all 19 functions with file paths and line numbers. The problem statement is precise: these exports exist solely for test access, falsely signaling public API surface.

Root cause: Tests were written against internal helpers directly rather than through the module's public interface. This is a common pattern that accumulates during rapid development but creates coupling between tests and implementation details.

Verification: I confirmed all 19 functions are only imported by test files — zero production cross-module consumers. However, 9 of the 19 are also re-exported from barrel index files (src/stages/2-generation/index.ts, src/stages/3-execution/index.ts, src/stages/4-evaluation/index.ts, src/state/index.ts), which amplifies the false public-API signal.

Pipeline impact: All 4 stages plus state management are affected, but Stage 3 (Execution) has the highest concentration with 9 of the 19 exports.

2. Proposed Solution(s)

The issue proposes two strategies:

• (a) Preferred: Remove export, refactor tests to go through parent public functions
• (b) Fallback: Keep export but add @internal JSDoc tag

Evaluation of approach (a):

Most of these functions are pure utility/predicate functions that are tightly coupled to their parent module's logic. Testing through the parent is feasible for many:

Difficulty	Functions	Rationale
Easy — test via parent	isFilesPersistedEvent, isTaskNotificationMessage, isAuthStatusMessage	Type guard predicates used only inside processStreamEvents; test by asserting parent behavior with different event types
Easy — test via parent	isSuccessfulExecution, extractSessionId, countAssistantTurns	Used inside buildTranscript; testable through transcript output assertions
Easy — test via parent	filterTriggerCaptures	Used inside captureHookTriggers; test via parent
Medium — test via parent	createBaseScenario, baseToTestScenario	Used by generateDiverseScenarios; test by asserting output scenario properties
Medium — test via parent	createBatchConfig, wouldExceedTokenLimit	Used by calculateBatches; test via parent with different token limits
Medium — test via parent	readRawManifest	Used by parsePlugin; test by providing different manifest fixtures
Medium — requires mocking	analyzeHooks, analyzeHookResponses	Complex inputs; testing via parent is possible but may need more elaborate test fixtures
Medium — test via parent	parseCustomId	Used by processBatchResults; test through batch evaluation output
Medium — test via parent	isLowVariance	Used inside runMultiSample; test via multi-sample output behavior
Special case	updateStateWithPartialExecutions, updateStateWithError, getIncompleteScenarios	Scaffolding for unimplemented checkpointing — see below

Evaluation of approach (b) — @internal:

The @internal tag approach is a reasonable fallback but has a key drawback: the functions remain importable and tests remain coupled to implementation. It's cosmetic rather than structural. That said, it's pragmatic for cases where testing through the parent would require disproportionate test refactoring.

Tradeoffs:

• (a) improves test resilience to refactors but requires meaningful test rewriting effort
• (b) is lower effort but doesn't actually decouple tests from internals
• Risk with (a): some tests may become less precise (testing larger units), potentially making failure diagnosis harder

3. Alternative Approaches

Option C: Use Vitest's vi.hoisted + dynamic import tricks.
TypeScript doesn't allow importing non-exported members, and this project uses isolatedModules: true. There's no clean way to test non-exported functions without either exporting them or using the parent interface. This rules out the "TypeScript allows importing non-exported members" claim in the issue for this project setup.

Option D: Move internal functions to __internal named exports.
Convention-based: export const __internal = { filterTriggerCaptures, ... }. Tests import __internal, knip ignores __ prefixed exports. This is explicit about intent but adds boilerplate.

Option E: Hybrid — remove export for easy cases, @internal for complex ones.
This is the most pragmatic path: get the structural benefit where it's cheap, use @internal where test refactoring cost is high.

Recommendation on state functions: The 3 state exports (updateStateWithPartialExecutions, updateStateWithError, getIncompleteScenarios) are described as scaffolding for an unimplemented checkpointing feature. Per the issue's additional context, these should be evaluated separately — if the checkpointing feature isn't planned for near-term, consider deleting these functions entirely rather than just removing exports. They're trivial to recreate when needed.

4. Implementation Considerations

Files affected (source):

• src/stages/1-analysis/hook-analyzer.ts:287 — remove export from analyzeHooks
• src/stages/1-analysis/plugin-parser.ts:89 — remove export from readRawManifest
• src/stages/2-generation/diversity-manager.ts:83,106 — remove export from createBaseScenario, baseToTestScenario
• src/stages/2-generation/batch-calculator.ts:128,187 — remove export from createBatchConfig, wouldExceedTokenLimit
• src/stages/3-execution/hook-capture.ts:164,438 — remove export from filterTriggerCaptures, analyzeHookResponses
• src/stages/3-execution/sdk-client.ts:342,355,368 — remove export from 3 type guards
• src/stages/3-execution/transcript-builder.ts:255,271,292 — remove export from 3 functions
• src/stages/4-evaluation/batch-evaluator.ts:163 — remove export from parseCustomId
• src/stages/4-evaluation/multi-sampler.ts:293 — remove export from isLowVariance
• src/state/updates.ts:146,166 — remove/delete 2 functions
• src/state/queries.ts:117 — remove/delete 1 function

Barrel files requiring cleanup (9 re-exports to remove):

• src/stages/2-generation/index.ts — remove createBatchConfig re-export
• src/stages/3-execution/index.ts — remove filterTriggerCaptures, extractSessionId, countAssistantTurns, isSuccessfulExecution re-exports
• src/stages/4-evaluation/index.ts — remove parseCustomId re-export
• src/state/index.ts — remove updateStateWithPartialExecutions, updateStateWithError, getIncompleteScenarios re-exports

Test files requiring refactoring (12 files):

• tests/unit/stages/1-analysis/hook-analyzer.test.ts
• tests/unit/stages/1-analysis/plugin-parser.test.ts
• tests/unit/stages/2-generation/hook-scenario-generator.test.ts
• tests/unit/stages/2-generation/diversity-manager.test.ts
• tests/unit/stages/2-generation/batch-calculator.test.ts
• tests/unit/stages/3-execution/hook-capture.test.ts
• tests/unit/stages/3-execution/sdk-client.test.ts
• tests/unit/stages/3-execution/transcript-builder.test.ts
• tests/unit/stages/4-evaluation/batch-evaluator.test.ts
• tests/unit/stages/4-evaluation/multi-sampler.test.ts
• tests/unit/state/state-manager.test.ts

Breaking changes: None for external consumers — all 19 functions have zero production cross-module imports. The barrel re-exports also have no production consumers.

State/resume impact: If the 3 state functions are deleted rather than just de-exported, the partial_executions field handling would need review when checkpointing is eventually implemented. No impact on current resume functionality since these functions are unused in production.

Testing: npm run build && npm run typecheck && npm test && npm run knip as specified in the issue.

5. Recommendation

Approach: Hybrid (Option E) — Remove export and test through parent for straightforward cases; use @internal for complex ones.

Suggested implementation order:

1. Batch 1 — Easy wins (6 functions): Remove export from the 3 sdk-client type guards and 3 transcript-builder functions. Refactor tests to exercise these through processStreamEvents and buildTranscript respectively. These are pure predicates with simple test paths through parents.

2. Batch 2 — Medium cases (7 functions): Remove export from filterTriggerCaptures, createBaseScenario, baseToTestScenario, createBatchConfig, wouldExceedTokenLimit, readRawManifest, isLowVariance. Refactor tests to go through parent functions. May require additional test fixtures.

3. Batch 3 — Complex cases (3 functions): For analyzeHooks, analyzeHookResponses, and parseCustomId, evaluate case-by-case. If test refactoring is disproportionate, apply @internal instead.

4. Batch 4 — State scaffolding (3 functions): Decide whether to delete updateStateWithPartialExecutions, updateStateWithError, and getIncompleteScenarios entirely (if checkpointing isn't near-term) or apply @internal. Consider filing a separate issue to track the checkpointing feature decision.

5. Cleanup: Remove all 9 barrel re-exports from index files. Run full verification suite.

Follow-up issues to consider:

• Checkpointing feature decision: keep/delete the 3 state scaffolding functions (could be tracked in a dedicated issue)
• The issue references chore: remove unused type interfaces (ScenarioVariation, MetaJudgmentResult) #371 (dead types) and [Feature]: Delete 7 stdlib wrapper functions and their tests #372 (stdlib wrappers) — coordinate to avoid merge conflicts if these are worked in parallel