feat: issue 337 code extraction writes through - #360

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/riviere-cli/src/__fixtures__/command-test-fixtures.ts`:
- Around line 127-178: Move the standalone fixture constants into a single
contiguous block so all component/link fixtures are grouped together and helper
functions/interfaces live separately; specifically, relocate domainOpComponent
and simpleUseCaseComponent to be adjacent with useCaseComponent, apiComponent,
eventHandlerComponent, validLink, and baseMetadata (and keep createGraphWith*
helpers and CustomTypeDefinition in a separate helpers section), ensuring the
file groups: 1) all fixture constants (baseMetadata, sourceLocation,
useCaseComponent, simpleUseCaseComponent, domainOpComponent, apiComponent,
eventHandlerComponent, validLink) followed by 2) all helper functions/interfaces
(createGraphWith*, CustomTypeDefinition) for improved readability.
- Around line 96-113: The afterEach teardown in setupCommandTest can mask
beforeEach failures because ctx.testDir and ctx.originalCwd may be empty; update
afterEach to only call process.chdir(ctx.originalCwd) and await rm(ctx.testDir,
{ recursive: true, force: true }) if those values are non-empty strings (e.g.
check ctx.testDir && ctx.originalCwd are truthy strings) and keep
vi.restoreAllMocks() as-is; this prevents chdir/rm from throwing on partial
setup and adds force: true to rm to silence "no such file" errors while
preserving the original setup error (references: setupCommandTest, beforeEach,
afterEach, ctx.testDir, ctx.originalCwd, ProcessExitError).

In
`@packages/riviere-cli/src/features/extract/domain/detect-connections-per-module.spec.ts`:
- Around line 53-90: The test title claims the "shared-core full result
unchanged" but only asserts result.kind and doesn't verify the transformed shape
produced by ExtractionProject.extractDraftComponents which calls
this.toBuilderBackedOutcome(builder.build(), outcome, writePort); update the
test (in detect-connections-per-module.spec.ts) to either rename it to reflect
it only preserves the kind and forwards options (e.g., "preserves the
shared-core full outcome kind and forwards options to extractInto") or add
assertions that check the full structure returned by toBuilderBackedOutcome for
the empty full outcome (use mockExtractInto return value and assert the
resulting object equals the expected builder-backed shape), and keep the
existing expectation that mockExtractInto was called with resolvedConfig and the
options object so you still verify forwarding.

In `@packages/riviere-cli/src/features/extract/domain/enrich-per-module.spec.ts`:
- Line 65: The top-level describe currently labeled
'ExtractionProject.enrichDraftComponents' is misleading because the spec also
tests extractDraftComponents and extractIntoWorkflowBuilder; rename or
restructure the suite so the describe accurately reflects the contained tests
(e.g., change the top-level describe name to 'ExtractionProject - extraction
helpers' or split into separate describes for
'ExtractionProject.extractDraftComponents' and
'ExtractionProject.extractIntoWorkflowBuilder'), and move the tests that
exercise extractDraftComponents and extractIntoWorkflowBuilder into their
corresponding describe blocks to ensure correct test discovery and clearer scope
for the functions extractDraftComponents, extractIntoWorkflowBuilder, and
enrichDraftComponents.
- Line 226: Export the DraftComponentModuleResolutionError class from
ExtractionProject (so it can be imported in tests) and update the test in
enrich-per-module.spec.ts to expect that specific error class instead of
matching the message string; specifically, export
DraftComponentModuleResolutionError from wherever
ExtractionProject/ExtractionProjectModule is defined and change the assertion in
the spec from toThrow("Unable to resolve module for draft component
'PlaceOrder'") to using expect(() =>
...).toThrowError(DraftComponentModuleResolutionError) (or the appropriate
matcher for that class).

In
`@packages/riviere-cli/src/features/extract/domain/extraction-project-builder-backed.spec.ts`:
- Around line 289-297: createModuleContext() is being called multiple times to
build duplicateModuleContext, which creates throwaway ts-morph Project
instances; instead call createModuleContext() once into a base variable, then
build duplicateModuleContext by spreading that single base (e.g., const base =
createModuleContext(); duplicateModuleContext = { ...base, module: {
...base.module, name: 'orders-duplicate' }, files: [...] }). Apply the same
single-call refactor for the second occurrence that similarly calls
createModuleContext() multiple times so you never instantiate more than one
Project per intended context.

In
`@packages/riviere-cli/src/features/extract/domain/extraction-project-presentation.spec.ts`:
- Around line 1-9: Add explicit Vitest globals by importing describe, it, and
expect from 'vitest' at the top of the spec file to match the project's test
patterns; update the imports near toOutcomeComponentId, toPresentedComponent,
and toPresentedLink so the file starts with "import { describe, it, expect }
from 'vitest'" alongside the existing imports, keeping existing named imports
(toOutcomeComponentId, toPresentedComponent, toPresentedLink) and type imports
unchanged.

In `@packages/riviere-cli/src/features/extract/domain/extraction-project.ts`:
- Around line 199-211: Hoist the defensive-copy-producing calls out of the
per-element maps: call writePort.missingFields() and writePort.uncertainLinks()
once into local constants (e.g., missingFieldsCopy and uncertainLinksCopy)
before using graph.components.map(...) and graph.links.map(...), then pass those
copies into toPresentedComponent and toPresentedLink instead of invoking
writePort.missingFields() / writePort.uncertainLinks() inside each map to avoid
repeated cloning and reduce complexity from O(N·D) to O(N).

In
`@packages/riviere-cli/src/features/extract/infra/external-clients/source-files/extraction-project-source-files.ts`:
- Around line 72-93: The code is re-globbing module file lists inside
createModuleContexts instead of reusing the file resolution done by
resolveSourceFilePaths; change resolveSourceFilePaths to return a per-module
mapping (e.g. Map or Record keyed by module identifier/path) of resolved
absolute file paths, then update createModuleContexts to accept that per-module
map and use it to intersect with sourceFileSet (or use the map's array directly)
rather than calling globSync again; keep existing uses of findModuleTsConfigDir
and createConfiguredProject, and also adjust the globSync options object
formatting in this file to match the multiline shape used elsewhere (replace
single-line {cwd: configDir,} with the multiline form).
- Around line 55-69: The function resolveSelectedSourceFilePaths should accept
the config directory and resolve requestedFiles against it so comparisons match
allSourceFiles; change the signature to
resolveSelectedSourceFilePaths(allSourceFiles: string[], requestedFiles:
string[], configDir: string): string[] and update the existence check and the
requestedAbsolute set to use path.resolve(configDir, filePath) (replace
path.resolve(filePath) with path.resolve(configDir, filePath)); keep throwing
ConfigValidationError for missingFiles and ensure callers that invoke
resolveSelectedSourceFilePaths are updated to pass parsedConfigState.configDir.

In
`@packages/riviere-cli/src/features/extract/infra/persistence/extraction-project/extraction-project-repository.ts`:
- Around line 53-64: The PackageResolveError currently mutates this.message
after calling super; instead build the full error message string up front
(including the packageName and cause.message if present) and pass it into super
along with the cause via ErrorOptions (i.e., super(fullMessage, { cause })) so
you don't reassign this.message or this.cause afterward; update the
PackageResolveError constructor to compute fullMessage first and call
super(fullMessage, { cause }) and set this.name = 'PackageResolveError'.

In `@packages/riviere-cli/src/platform/__fixtures__/command-test-fixtures.ts`:
- Line 1: Remove the dead transitional shim file that only re-exports fixtures:
delete the file whose sole content is "export * from
'../../fixtures/command-test-fixtures'"; ensure there are no remaining imports
referencing platform/__fixtures__/command-test-fixtures and run tests/build to
verify nothing breaks after removal.

In
`@packages/riviere-cli/src/platform/infra/cli/presentation/output-writer.spec.ts`:
- Line 78: The test hard-codes code: 'VALIDATION_ERROR' for filesystem failures;
update the assertions in output-writer.spec.ts (around the expectation at the
line referencing code: 'VALIDATION_ERROR' and the similar assertion at line 111)
so they follow the production decision: either import and assert against the
error code constant exported by output-writer (or the IO/runtime error constant)
or compute expectedErrorCode from the behavior of OutputWriter (or allow either
VALIDATION_ERROR or IO_ERROR via a flexible matcher), and ensure the tests
reference writeFileSync-related behavior in output-writer.ts rather than a
literal string.

In `@packages/riviere-extract-config/extraction-config.schema.json`:
- Around line 38-59: Add human-readable top-level "description" strings to the
$defs.sourceInfo and $defs.domainMetadata schema objects and add per-property
"description" fields for each property (sourceInfo: repository, commit,
extractedAt; domainMetadata: description, systemType) so the generated docs no
longer show "(no description)". Keep the existing constraints (type, minLength,
pattern, enum, additionalProperties) but populate each schema and property with
brief descriptive text that explains the field purpose (e.g., repository: source
repo URL or identifier; commit: commit SHA or tag; extractedAt: ISO timestamp;
domainMetadata.description: short summary of domain; systemType: one of
"domain"|"bff"|"ui"|"other").

In `@packages/riviere-extract-config/scripts/generate-docs.ts`:
- Around line 93-104: The function readInlinePropertyType currently only handles
$ref, enum, and primitive types; update it to recurse into nested schemas so
arrays, inline objects, and oneOf/anyOf/allOf render correctly: for type ===
'array' use the items schema and return a recursive
readInlinePropertyType(items) plus [] (handle items.$ref/inline), for type ===
'object' with properties build an inline shape by iterating prop.properties and
recursively calling readInlinePropertyType for each property (honor required to
include ?), and for oneOf/anyOf/allOf join the recursive results with ' | ' (or
' & ' for allOf as appropriate). Reuse getRefTypeName for $ref cases and ensure
SchemaProperty/Schema types passed into recursive calls are the same shape as
used by readInlinePropertyType.

In
`@packages/riviere-extract-config/src/validation-whitespace-required-fields.spec.ts`:
- Around line 4-17: The test currently mutates all module string fields to
whitespace at once, which hides per-field regressions; change the spec to an
it.each over the four field names ('name', 'domain', 'path', 'glob') and for
each case clone the minimal config from createMinimalConfig(), set only that
single field on config.modules[0] to a whitespace-only string, and assert
isValidExtractionConfig(config) returns false — this preserves use of
createMinimalConfig and isValidExtractionConfig while giving one failing
assertion per property.

In
`@packages/riviere-extract-ts/src/features/extraction/domain/extract-into.edge-coverage.spec.ts`:
- Around line 56-132: Extract the duplicated test helpers (createModule,
createProjectWithDispose, createWritePortRecorder, createDraftComponent,
createEnrichedComponent) into a single shared test-fixtures module (e.g., export
them from extract-into.test-fixtures.ts) and replace the duplicated definitions
in each spec (extract-into.edge-coverage.spec.ts,
extract-into.branch-coverage.spec.ts, extract-into.spec.ts) with imports from
that module; keep any vi.mock(...) calls in each spec file as-is (do not move
them) to respect Vitest hoisting rules, and update imports/usages of the helper
functions (e.g., createModule, createProjectWithDispose,
createWritePortRecorder, createDraftComponent, createEnrichedComponent) so tests
continue to compile.

In `@packages/riviere-extract-ts/src/features/extraction/domain/extract-into.ts`:
- Line 343: The inline object in the spread expression using
externalLink.sourceLocation is missing the preferred spacing and trailing-comma
style; update the snippet that builds the spread (the object literal containing
sourceLocation from externalLink) to use spaced braces and no extraneous comma —
i.e. change {sourceLocation: externalLink.sourceLocation,} to { sourceLocation:
externalLink.sourceLocation } and run the repo formatter so the rest of
extract-into.ts matches project style.
- Around line 117-120: The field-failure branch in extractInto returns early
when enrichDraftComponents yields enrichment.kind === 'fieldFailure', which
prevents per-field reasons from being reported via writePort.reportMissingField
in writeCompleteExtraction; update extractInto (around the
enrichDraftComponents/enrichment handling) to, before returning the fieldFailure
summary, iterate enrichment.failures and call the writePort.reportMissingField
(or invoke the same reporting logic used by writeCompleteExtraction) for each
failure so mergeWritePort callers and strictWritePort.missingFields() receive
the per-field diagnostics; ensure you reference enrichDraftComponents,
enrichment.failures, writePort.reportMissingField, and toBuilderBackedOutcome
(or reuse writeCompleteExtraction) so the failure branch emits the same
missing-field events as the success path.
- Around line 107-136: The field-failure branch in extractInto returns early
when enrichment.kind === 'fieldFailure', skipping writeCompleteExtraction and
thus never calling reportMissingFields/writePort.reportMissingField; modify
extractInto so that before returning the 'fieldFailure' result it invokes
reportMissingFields(writePort, enrichment.failedFields, /* any needed context
from enrichment or options */) (or call writeCompleteExtraction with the
enrichment and an empty connectionResult) so diagnostics from
enrichDraftComponents are forwarded; additionally make disposal of module
contexts opt-in by changing the finally disposeProjects(options.moduleContexts)
to respect a new boolean option (e.g., options.disposeProjects defaulting to
true) or by moving disposeProjects out of finally and only calling it when that
flag is true, referencing disposeProjects and options.moduleContexts to locate
the change.

In
`@packages/riviere-extract-ts/src/features/extraction/domain/extraction-write-port.ts`:
- Around line 166-179: In addComponent, move the duplicate identity check before
any side effects: compute componentId with toCanonicalComponentId(input) and if
emittedComponentIds.has(componentId) throw SameStepDuplicateComponentError
(using stepContext.step and stepContext.stepType) before calling
defineCustomTypeWhenNeeded; only after emitting add the id to
emittedComponentIds and call dispatchMergeComponentWrite(builder, input); keep
references to definedCustomTypes and builder for defineCustomTypeWhenNeeded but
ensure it runs after the duplicate guard.
- Around line 71-79: Change WorkflowDiagnosticEvent from an open shape with
optional properties into a discriminated union keyed by the kind field: define
one variant for kind 'missing-field' that requires componentId and field (and
optionally source/target/linkType if relevant), another for kind
'uncertain-link' that requires source, target, and linkType (and optionally
componentId/field if relevant), and keep reason required on each variant; update
the exported WorkflowDiagnosticEvent type to be the union of these specific
interfaces so consumers can narrow by event.kind and get non-optional fields
(refer to the WorkflowDiagnosticEvent type and the 'missing-field' and
'uncertain-link' kind strings when making the change).

---

Outside diff comments:
In
`@packages/riviere-cli/src/features/extract/infra/persistence/extraction-project/extraction-project-repository.spec.ts`:
- Around line 1-9: Add the missing vitest runtime import and convert the spy to
a module mock so the SUT's named binding is replaced: import vi from 'vitest'
(or add vi to the existing import list) and replace the vi.spyOn(ExtractConfig,
'parseExtractionConfig') usage with a top-level
vi.mock('@living-architecture/riviere-extract-config', () => ({
parseExtractionConfig: () => { throw 'bad-config' } })) so
extraction-project-repository.ts receives the mocked parseExtractionConfig;
finally tighten the assertion to check for a wrapped Error type (e.g. instance
of ConfigValidationError and message contains 'bad-config') instead of relying
on a regex-only .toThrow('/bad-config/') to prove stringification behavior.

In
`@packages/riviere-cli/src/features/extract/infra/persistence/extraction-project/extraction-project-repository.ts`:
- Around line 308-321: Update the test "loadFromFullProject loads config with
top-level extends using all defaults" to assert the converted module's domain
and name are correct: after calling
ExtractionProjectRepository().loadFromFullProject(...) assert result is defined
and that result.domain === 'extended' and result.name === 'extended'; this
verifies topLevelRulesToModule produces a Module with domain and name set to
'extended' rather than only checking for presence.

In `@packages/riviere-cli/src/platform/infra/cli/presentation/output-writer.ts`:
- Around line 21-31: The catch block around the file write is misclassifying
system-level I/O failures as CliErrorCode.ValidationError while exiting with
ExitCode.RuntimeError and also discards the original stack; fix by either
removing the try/catch so writeFileSync errors propagate (preferred) or, if you
must return structured JSON, map the error to a new runtime/io code (e.g.,
CliErrorCode.RuntimeError or CliErrorCode.IOError) and use that code in
formatError while keeping ExitCode.RuntimeError so JSON code and process exit
match; update the block that calls formatError/buildWriteFailureMessage (and any
references to options.output) accordingly so callers can distinguish validation
vs system errors.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/riviere-cli/src/__fixtures__/command-test-fixtures.ts`:
- Around line 376-383: Remove the helper function parseErrorOutputSchema and
inline the Zod parse call where it was used: delete parseErrorOutputSchema and
replace its usages (e.g., in parseErrorOutput) with a one-liner
errorOutputSchema.parse(value) (or errorOutputSchema.safeParse(value) + throw if
you need TestAssertionError); reference symbols: parseErrorOutputSchema,
errorOutputSchema, TestAssertionError, and parseErrorOutput to locate and update
the code.

In `@packages/riviere-cli/src/features/extract/domain/enrich-per-module.spec.ts`:
- Around line 229-257: The test title claims it verifies a fallback "extracted"
domain when module domains are blank, but because mockExtractInto returns {
kind: 'draftOnly' } and the builder pathway (toBuilderBackedOutcome ->
builder.build().domains) is never exercised, the assertion only checks that
mockExtractInto was called with mode: 'extract' and doesn't prove the fallback
domain; fix by either removing or adjusting the mock so the real builder path
runs and then assert on builder.build().domains (referencing
extractDraftComponents, mockExtractInto, toBuilderBackedOutcome and
builder.build().domains) to validate the fallback extracted domain, or rename
the test to reflect the current assertion (e.g., "passes mode 'extract' for
draft-only extraction") and update the test name accordingly.

In
`@packages/riviere-cli/src/features/extract/infra/external-clients/source-files/extraction-project-source-files.ts`:
- Line 110: The exported declaration extractionProjectGlobMatcher is missing the
required JSDoc `@riviere-role` tag; add a JSDoc line immediately above the export
like /** `@riviere-role` <role-name> */ using the same role used by the other
exported functions in this module (the four above) so the role-enforcement lint
passes, leaving the export as extractionProjectGlobMatcher =
ExtractTs.matchesGlob.
- Around line 42-53: resolveChangedSourceFilePaths currently calls
detectChangedTypeScriptFiles(process.cwd(), …) and resolves returned paths
against process.cwd(), which mismatches allSourceFiles (which are resolved
against parsedConfigState.configDir); change resolveChangedSourceFilePaths
signature to accept configDir: string, call
detectChangedTypeScriptFiles(configDir, gitOptions), and normalize the returned
file paths with path.resolve(configDir, filePath) before building
changedAbsolute; also update the caller in extraction-project-repository (where
resolveChangedSourceFilePaths is invoked) to pass parsedConfigState.configDir so
both git detection and path normalization use the same configDir (and keep
baseBranch handling as-is).

In
`@packages/riviere-cli/src/features/extract/infra/persistence/extraction-project/extraction-project-repository.ts`:
- Around line 110-123: In loadFromChangedProject update the call to
resolveChangedSourceFilePaths to pass the project configDir so path resolution
uses the parsed config directory: retrieve parsedConfigState.configDir and
forward it as the additional argument to resolveChangedSourceFilePaths (which
will have been updated to accept configDir) when computing sourceFilePaths,
keeping the rest of the call and subsequent
createExtractionProject(parsedConfigState, sourceFilesByModule, sourceFilePaths,
params.useTsConfig) unchanged.
- Around line 336-342: The createExtractionProject signature uses the namespaced
type ExtractConfig.Module for sourceFilesByModule; change that to the imported
Module type (i.e. use Module instead of ExtractConfig.Module) in the
createExtractionProject parameter list to match the direct import from
'@living-architecture/riviere-extract-config' and avoid the undefined namespace.

In
`@packages/riviere-cli/src/platform/infra/cli/presentation/global-error-handler.spec.ts`:
- Around line 17-78: The file duplicates test harness code (TestAssertionError,
TestContext, createTestContext, setupCommandTest and ProcessExitSignal) that
already exists in the shared fixture; remove the local duplicates and import the
shared utilities from the common fixture (e.g., fixtures/command-test-fixtures)
instead, keeping only the local UnexpectedGlobalError if it is truly unique; if
you need a different name for the process-exit error, import the shared
ProcessExitError/fixture symbol and either alias it or re-export it locally
rather than reimplementing ProcessExitSignal; update uses of setupCommandTest,
createTestContext, TestContext, and TestAssertionError in this spec to use the
imported symbols.

In
`@packages/riviere-cli/src/platform/infra/cli/presentation/output-writer.spec.ts`:
- Around line 22-25: Replace the afterEach cleanup that calls vi.clearAllMocks()
and vi.restoreAllMocks() with a single vi.resetAllMocks() so hoisted mocks'
implementations are cleared between tests; specifically update the afterEach
block surrounding mockWriteFileSync (the vi.hoisted(vi.fn()) mock) to call
vi.resetAllMocks() to ensure any mockImplementation() (including the throwing
implementations set in tests 2 and 3) does not leak into subsequent tests.

In
`@packages/riviere-cli/src/platform/infra/cli/presentation/query-graph-load-error-handler.spec.ts`:
- Around line 9-14: Replace the locally defined TestAssertionError class with
the shared exported one: remove the local class declaration in
query-graph-load-error-handler.spec.ts and add an import for TestAssertionError
from the shared fixture module (e.g., fixtures/command-test-fixtures) so tests
use the same TestAssertionError identity across specs and avoid duplicated code.

In `@packages/riviere-extract-config/scripts/generate-docs.ts`:
- Around line 133-138: The array item parenthesis guard in formatArrayType
incorrectly checks for unescaped ' | ' (and ' & ') while readInlinePropertyType
emits escaped pipes (' \\| ') for markdown, so union item types don't get
parenthesized; update formatArrayType to detect the escaped pipe produced by
readInlinePropertyType (and still accept unescaped forms) when deciding to wrap
the itemType in parentheses — i.e., adjust the conditional that builds
arrayItemType in formatArrayType to check for ' \\| ' (and keep checks for ' | '
and ' & ' as needed) so `(module \| moduleRef)[]` renders correctly.

In
`@packages/riviere-extract-ts/src/features/extraction/domain/extract-into-fixtures.ts`:
- Around line 36-74: The recorder currently types its arrays as unknown[],
losing type information; update createWritePortRecorder to type each array using
the concrete parameter/event types from ExtractionWritePort (e.g., use
Parameters<ExtractionWritePort['addComponent']>[0][] for components,
Parameters<ExtractionWritePort['addLink']>[0][] for links,
Parameters<ExtractionWritePort['addExternalLink']>[0][] for externalLinks,
Parameters<ExtractionWritePort['reportMissingField']>[0][] for missingFields,
and Parameters<ExtractionWritePort['reportUncertainLink']>[0][] for
uncertainLinks) and adjust the returned signature accordingly so consumers get
full compile-time types when asserting recorded entries.

In
`@packages/riviere-extract-ts/src/features/extraction/domain/extract-into.branch-coverage.spec.ts`:
- Around line 213-323: The test currently ignores the return value of
extractIntoModule.extractInto; capture its return into a variable (e.g., result)
and assert that result.kind === 'full' and that result.links and
result.externalLinks (and optionally result.timings) match the aggregated output
produced by mockDetectPerModuleConnections and mockDetectCrossModuleConnections;
update the test to reference extractIntoModule.extractInto and verify
result.links/result.externalLinks (and timings) alongside the existing
recorder.* and mockDetectCrossModuleConnections invocation assertions.
- Around line 1-58: Add an explicit Vitest import at the top of the file: import
{ describe, it, expect, beforeEach, vi } from 'vitest' so the test file follows
the package's import convention; keep the existing vi.hoisted() and vi.mock(...)
usages but reference the named vi from that import. Optionally, for conventional
grouping, move the import * as extractIntoModule from './extract-into' so it
appears before the vi.mock(...) blocks (or ensure mocks run before the module
import as required by the tests).

In
`@packages/riviere-extract-ts/src/features/extraction/domain/extraction-write-port.ts`:
- Around line 196-203: The toCanonicalComponentId function currently uses
toComponentIdTypeSegment(input.type) which yields the literal "custom" for
input.type === 'custom' and causes mismatched IDs; update toCanonicalComponentId
so that when input.type === 'custom' it uses input.customTypeName (falling back
to toComponentIdTypeSegment(input.type) only if customTypeName is missing) to
build the type segment, ensuring the canonical ID matches builder-generated IDs
(e.g., generateComponentId) and toExtractionComponentId used elsewhere (affects
mergeWritePort.addComponent and enrichment/connection detection).