Repository navigation
feat(extract): add scoped call graph with sync connection detection (#237) - #246
Conversation
Implements connection detection for riviere-extract-ts: ComponentIndex, type resolver, interface resolution, call graph DFS with non-component transparency, and detectConnections orchestrator. 287 tests, 100% coverage. Closes #237 Co-Authored-By: Claude Opus 4.5 <[email protected]>
Deploying living-architecture with
|
| Latest commit: |
459b26b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://edfad062.living-architecture.pages.dev |
| Branch Preview URL: | https://issue-237-m1-d1-1-d1-2-d1-3.living-architecture.pages.dev |
📝 WalkthroughWalkthroughAdds a connection-detection subsystem to riviere-extract-ts: component indexing, receiver type resolution, interface resolution, call-graph construction with non-component tracing and deduplication, error type, orchestrator (detectConnections), fixtures/tests, and new public exports; also adds a workspace dependency and tsconfig project reference. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Detect as detectConnections
participant Index as ComponentIndex
participant Build as buildCallGraph
participant Resolver as resolveCallExpressionReceiverType
participant Interface as resolveInterface
participant Dedup as deduplicateLinks
Client->>Detect: project, components, options, globMatcher
Detect->>Index: build ComponentIndex from components
Index-->>Detect: ComponentIndex
Detect->>Build: project, components, ComponentIndex, sourceFilePaths, options
Build->>Build: iterate components & methods
Build->>Resolver: resolve receiver type for call
alt resolver returns concrete type
Resolver-->>Build: typeName
else resolver uncertain
Resolver->>Interface: resolve interface within sourceFilePaths
Interface-->>Resolver: implementationName or unresolved(reason)
Resolver-->>Build: resolved typeName or uncertain
end
Build->>Build: trace into non-component methods recursively (visited set avoids cycles)
Build->>Dedup: rawLinks[], uncertainLinks[]
Dedup-->>Build: ExtractedLink[] (deduplicated, normalized)
Build-->>Detect: ExtractedLink[]
Detect-->>Client: ExtractedLink[]
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Fix all issues with AI agents
In
`@packages/riviere-extract-ts/src/domain/connection-detection/call-graph/build-call-graph.ts`:
- Around line 27-105: resolveTypeThroughInterface currently forces strict: false
and drops interface resolution results; update it to pass options.strict into
resolveInterface and return both the found component (if any) and the interface
resolution result (e.g., { component, interfaceResult }) instead of just a
component. Then update processCallExpression to use that returned pair: if
interfaceResult.resolved is true but component is undefined, call
traceNonComponent using interfaceResult.typeName (so non-component
implementations are traced), and if interfaceResult has a non-empty reason and
options.strict is false (lenient) push an uncertainLinks entry with that reason
and currentCallSite; ensure when a component is found you keep the existing
rawLinks logic and when resolveInterface failed you still push uncertainLinks as
before.
In
`@packages/riviere-extract-ts/src/domain/connection-detection/call-graph/call-graph-fixtures.ts`:
- Around line 5-12: The sharedProject constant duplicates the same Project setup
(useInMemoryFileSystem and identical compilerOptions) used elsewhere; replace
duplication by extracting a reusable factory or shared fixture function (e.g.,
createSharedProject or getSharedProject) and use that from both
call-graph-fixtures.ts and type-resolver-fixtures.ts, or if test isolation is
required, add a short comment explaining why a separate sharedProject instance
is intentional; update references to the new function (look for sharedProject
and Project) and ensure imports are updated accordingly.
In
`@packages/riviere-extract-ts/src/domain/connection-detection/call-graph/call-graph-types.spec.ts`:
- Around line 9-17: Add edge-case unit tests for stripGenericArgs in the
existing spec: add cases asserting that stripGenericArgs('') returns '' (empty
input), stripGenericArgs('Map<K, List<V>>') returns 'Map' (nested generics), and
stripGenericArgs('Pair<A, B>') returns 'Pair' (multiple type parameters) so the
function's robustness is validated alongside the existing tests.
In
`@packages/riviere-extract-ts/src/domain/connection-detection/call-graph/deduplicate-links.spec.ts`:
- Around line 22-70: Add three edge-case tests for deduplicateLinks: (1) assert
deduplicateLinks([], []) returns an empty array to cover empty inputs; (2)
provide multiple UncertainRawLink entries and assert all are present in result
(e.g., two different uncertain links with distinct callSite entries and reasons)
to ensure uncertain links are fully included; (3) create duplicate RawLink
entries with the same source/target and identical sourceLocation.lineNumber and
assert deterministic/stable outcome (e.g., resulting length is 1 and the
preserved link equals the first input) to verify behavior when line numbers are
equal. Reference the deduplicateLinks function and existing test helpers like
buildRawLink/buildComponent to construct inputs.
In
`@packages/riviere-extract-ts/src/domain/connection-detection/call-graph/trace-calls.ts`:
- Around line 38-130: resolveCallExpressionReceiverType and resolveInterface are
being called with strict: false and interface-resolution uncertainty is dropped;
update resolveTypeThroughInterface to accept options.strict and pass
options.strict into resolveInterface so strict mode is honored, return any
uncertainty (e.g., change resolveTypeThroughInterface signature to return
{component, uncertain, resolvedTypeName?}), then in traceCallsInBody call
resolveCallExpressionReceiverType with { strict: options.strict }, call
resolveTypeThroughInterface with the same options, if
resolveTypeThroughInterface returns an uncertain reason push an UncertainRawLink
into uncertainResults (include originCallSite and the uncertainty string), and
if interface resolution succeeded but component is undefined, replace typeName
with the resolved implementation name from resolveTypeThroughInterface before
forming visitKey and before calling resolveMethodDeclaration so tracing
continues against the concrete type even when it's not a registered component.
In
`@packages/riviere-extract-ts/src/domain/connection-detection/component-index.spec.ts`:
- Around line 7-19: The test defines a local buildComponent helper that largely
duplicates the one in call-graph-fixtures.ts; extract this into a shared test
fixture (e.g., tests/fixtures/component-fixtures.ts) and import it into
component-index.spec.ts and call-graph-fixtures.ts, keeping EnrichedComponent as
the shared type; if the differing defaults (type: 'service' and name:
'OrderService') are intentional, expose buildComponent with optional parameters
or an overload/variant (e.g., buildComponent({ type?, name? })) so callers can
pass differing defaults rather than maintaining two copies.
In
`@packages/riviere-extract-ts/src/domain/connection-detection/component-index.ts`:
- Around line 3-9: Duplicate stripGenericArgs function found in
component-index.ts; remove this local implementation and reuse the shared helper
instead. Replace the local function by importing the shared stripGenericArgs
from the common utilities module (where the canonical implementation lives),
update any references in component-index.ts to use that imported symbol, and
ensure the import statement is added/updated and any local tests still pass.
In
`@packages/riviere-extract-ts/src/domain/connection-detection/detect-connections.spec.ts`:
- Around line 9-18: Replace the magic numeric targets in createProject by using
ts-morph enums: set compilerOptions.target to ScriptTarget.ESNext and
compilerOptions.module to ModuleKind.ESNext (import these enums from ts-morph)
to make intent clear; also remove the duplicated local buildComponent helper and
import the shared buildComponent fixture used elsewhere (the duplicate helper in
this spec should be replaced with the shared fixture import) so tests reuse the
common fixture.
In
`@packages/riviere-extract-ts/src/domain/connection-detection/detect-connections.ts`:
- Around line 13-25: The function filterSourceFiles currently mutates the shared
Project by calling project.removeSourceFile which causes side effects; change it
to be pure by returning a filtered list (e.g., array of SourceFile or file
paths) instead of removing files. Update filterSourceFiles to compute the keep
list using project.getSourceFiles(), moduleGlobs and globMatcher and return that
list; remove any calls to project.removeSourceFile inside it and update callers
to traverse only the returned files (or file paths) when performing connection
detection (references: filterSourceFiles, project.getSourceFiles,
project.removeSourceFile, moduleGlobs, globMatcher).
In
`@packages/riviere-extract-ts/src/domain/connection-detection/interface-resolution/resolve-interface.ts`:
- Around line 39-46: The current name comparisons in implementsInterface and
extendsAbstractClass use getText() which breaks on aliased imports; change both
to resolve the referenced symbol name first (use
impl.getExpression().getType().getSymbol()?.getName() or
extendsClause.getExpression().getType().getSymbol()?.getName()) and fall back to
getExpression().getText() if the symbol lookup is undefined, following the
pattern in type-resolver.ts; apply the same symbol-first matching fix to the
analogous checks in evaluate-predicate.ts and
evaluate-extraction-rule-generic.ts so aliased interface/class names are
correctly detected.
…no mutation Wire strict/lenient mode through interface resolution and type resolver. Extract shared functions from build-call-graph and trace-calls. Remove project mutation in detectConnections. Add symbol-based interface matching. Co-Authored-By: Claude Opus 4.5 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In
`@packages/riviere-extract-ts/src/domain/connection-detection/call-graph/build-call-graph.spec.ts`:
- Around line 39-70: The test hardcodes the call site lineNumber (10) which is
fragile because the template literal's leading newline and any formatting
changes will shift line numbers; update the spec to compute the expected line
number dynamically from the generated source instead of a magic constant. Use
the same file string produced by nextFile(...) and derive the line index of the
call (e.g., find the line containing "this.repo.save()" or compute line offset
from the class/method declaration) and assert that sourceLocation.lineNumber
equals that computed value; adjust references in the test around buildComponent,
buildCallGraph, ComponentIndex and defaultOptions to use the computed value so
the assertion remains correct if the template formatting changes.
In
`@packages/riviere-extract-ts/src/domain/connection-detection/call-graph/call-graph-shared.spec.ts`:
- Around line 55-76: The test for resolveTypeThroughInterface uses a fragile
hardcoded import './shared-test-${counter.value - 1}' in implFile; instead build
the impl import from the actual generated interface file name returned by
nextFile (e.g. derive the import path from interfaceFile's filename/path) so the
import always points to the interface created just above; update implFile
creation in the spec to reference that derived filename (affecting the
implFile/nextFile usage and the import string) so resolveTypeThroughInterface
and symbols like SharedGateway, interfaceFile, implFile, nextFile, and counter
no longer rely on counter ordering.
- Around line 15-23: The spec duplicates the test fixture pattern
(sharedProject, counter, nextFile) already defined in call-graph-fixtures.ts;
remove the local declarations (sharedProject, counter, nextFile) and import the
equivalent fixtures from call-graph-fixtures.ts (use the exported Project
instance, counter helper, and nextFile helper) so this spec reuses the shared
utilities—if test isolation is required, instead export a factory from
call-graph-fixtures.ts (e.g., createSharedProject or nextFileFactory) and call
that from this spec to get isolated instances.
In
`@packages/riviere-extract-ts/src/domain/connection-detection/call-graph/call-graph-shared.ts`:
- Around line 20-23: getCalledMethodName currently calls asKindOrThrow and will
throw when the CallExpression's expression is not a PropertyAccessExpression;
add a short JSDoc to document this precondition and thrown behavior and state
that callers must ensure
Node.isPropertyAccessExpression(callExpr.getExpression()) (or equivalent) before
calling getCalledMethodName; update the comment above the getCalledMethodName
function to describe that it extracts the method name from a property access
call expression and will throw if the expression is not a property access.
- Around line 56-76: findMethodInProject currently does a full scan of
project.getSourceFiles() each call which is costly; implement a module-level
cache (e.g., Map<string, MethodLookup> or Map<string, ClassDeclaration>) and
change findMethodInProject to check the cache first and only do the full scan
once to populate the cache on first miss; update the cache key by class name
(typeName) and return cached MethodLookup (method and classFound) thereafter;
alternatively replace the scan with ts-morph type resolution if you prefer, but
the quick fix is to add the cache and populate it inside the findMethodInProject
function so repeated lookups are O(1).
In
`@packages/riviere-extract-ts/src/domain/connection-detection/detect-connections.spec.ts`:
- Around line 115-160: Update the two tests to exercise unresolved-type
behavior: create a class that references a missing interface/type (e.g., use a
parameter or property typed to MissingInterface with no implementation) instead
of only defining execute(): void; then in the strict test (StrictComp) call
detectConnections(project, [comp], { moduleGlobs: [...] }, matchesGlob) and
assert it throws (or returns the strict-mode error) due to the unresolved type;
in the lenient test (LenientComp) pass allowIncomplete: true and assert
detectConnections returns a result entry for that component with status
'_uncertain' (or the lenient unresolved marker your code uses). Reference
detectConnections, StrictComp, LenientComp, and allowIncomplete to locate where
to change the test.
In
`@packages/riviere-extract-ts/src/domain/connection-detection/detect-connections.ts`:
- Around line 32-34: The double-negative mapping for strict is confusing: change
the mapping so strict is set directly from a non-negated flag or rename the
option; specifically, update the call to buildCallGraph in detect-connections
(the strict: options.allowIncomplete !== true line) to use strict:
!options.allowIncomplete for clarity, or alternatively rename the option in
ConnectionDetectionOptions from allowIncomplete to strict and propagate that
rename through callers so buildCallGraph receives strict directly.
In
`@packages/riviere-extract-ts/src/domain/connection-detection/interface-resolution/resolve-interface.ts`:
- Around line 71-78: createError currently constructs ConnectionDetectionError
with empty file/line, losing call-site context; change
createError(interfaceName: string, reason: string) to accept and forward caller
file and line (e.g., createError(interfaceName: string, reason: string, file:
string, line: number)) and update every call site in this module that invokes
createError to pass the actual source context (the file path and line number
available where the unresolved interface is detected). Ensure the returned
ConnectionDetectionError uses the provided file and line instead of ''/0 so
callers of resolve/interface-resolution logic receive actionable location info.
…ion mapping Strict/lenient tests now trigger on unresolvable `any` types. Simplified double negative in options mapping. Co-Authored-By: Claude Opus 4.5 <[email protected]>
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@packages/riviere-extract-ts/src/domain/connection-detection/detect-connections.ts`:
- Around line 24-35: In detectConnections, validate the incoming
ConnectionDetectionOptions before using moduleGlobs and allowIncomplete: ensure
options.moduleGlobs exists and is a non-empty array of non-empty strings (filter
out invalid/malformed entries) and ensure options.allowIncomplete is a boolean
(coerce or set a safe default if missing); if validation fails, either throw a
clear error or fall back to a safe default and log/warn; perform this check
right before calling computeFilteredFilePaths and then pass the sanitized
moduleGlobs and boolean into computeFilteredFilePaths/buildCallGraph
(referencing detectConnections, ConnectionDetectionOptions, moduleGlobs,
allowIncomplete, computeFilteredFilePaths, buildCallGraph, and ComponentIndex to
locate where to add the guard).
…nent reflection storage Process improvements from post-merge reflection on PR #246: - Post-merge-completion: pipeline timeline, inefficiency diagnosis, permanent storage, failure descriptions - Task-check: behavioral correctness verification - SD-022: never mutate function parameters - CR-007: assertion-to-title alignment in tests - CR-008: edge case checklist enforcement on test files - CR-004: duplication checks apply equally to test files - RFC-010: symbol-based AST name resolution - BS-015: inconsistent patterns across feature files Closes #247 Co-Authored-By: Claude Opus 4.5 <[email protected]>



Closes #237
Deliverable: Scoped call graph construction, non-component transparency, interface resolution, and sync connection detection
Context
PRD: `docs/project/PRD/active/PRD-phase-12-connection-detection.md` — M1 D1.1, D1.2, D1.3, D1.4
Connection detection requires building a method-level call graph from known components, tracing through non-components, resolving single-implementation interfaces, and detecting sync connections via method calls on component-typed instances. These four deliverables share the same state dependencies (ts-morph Project, EnrichedComponent[], ComponentIndex) and form a single vertical slice through the connection detection capability.
Traceability
Acceptance Criteria
Edge Case Scenarios
Implementation Guidelines
Embedded Reasoning
Why: Components are identified and enriched. Detecting how they connect via method calls is the foundation of flow-based architecture extraction. Call graph + sync detection is the core capability everything else builds on.
What: ComponentIndex (immutable lookup), ScopedCallGraph (DFS traversal with type-based resolution), interface resolution (single-impl auto-resolve), sync connection detection (method calls → ExtractedLink[]).
How: Build ComponentIndex from EnrichedComponent[] + AST. For each component method, DFS through call sites resolving receiver types via declared types. Collapse non-component nodes. Produce one ExtractedLink per unique (source, target, type) tuple.
Testing Strategy
Dependencies
None — this is the foundational task.
Verification
```bash
pnpm nx test riviere-extract-ts -- --testNamePattern "connection-detection"
pnpm nx test riviere-extract-ts -- --coverage
```
Summary by CodeRabbit
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.