refactor(analysis): add an Analyzer parent for lineage + graph#13
Open
burrows99 wants to merge 1 commit into
Open
refactor(analysis): add an Analyzer parent for lineage + graph#13burrows99 wants to merge 1 commit into
burrows99 wants to merge 1 commit into
Conversation
…through it Introduce `Analyzer<In, Out>` as the shared parent of every analysis: a stable `name` plus `analyze(input)` that normalizes raw signal into a domain shape. Both analyses now extend it: - LineageAnalyzer extends Analyzer<TraceEvent[], Lineage[]> — the runtime value analysis (sync). `compute` becomes the instance `analyze`; the pure `summary` formatter stays a static helper. - GraphAnalyzer (new) extends Analyzer<GraphAnalysisInput, CodeGraph> — the graphical analysis (async). It delegates the call-hierarchy walk to the existing pluggable CodeGraphProvider, so the provider stays the swappable backend while `graph` now depends on an Analyzer, like a sibling to lineage. Call sites updated: RunCommand uses `new LineageAnalyzer().analyze(...)`; GraphCommand builds a `GraphAnalyzer` over the chosen provider and reads `analyzer.name` for the `graph.<name>` envelope. Exports + tests follow, with an instanceof-Analyzer assertion on both sides to pin the contract. Verified: tsc --noEmit clean, full build green, 77/78 tests pass (1 Postgres test skipped — needs a live DB). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces a shared Analyzer<In, Out> abstract base class for analysis components and refactors existing lineage analysis to conform to that contract, while adding a new GraphAnalyzer that wraps the code-graph provider behind the same interface. This aligns lineage + graph under a common “analysis” abstraction that commands can depend on.
Changes:
- Added
src/analysis/Analyzer.tsand refactoredLineageAnalyzerto extend it (compute→ instanceanalyze, plus stablename). - Added
GraphAnalyzeras anAnalyzeroverCodeGraphProviderand updatedGraphCommandto use it. - Updated exports and tests to validate
instanceof Analyzer, analyzer naming, and graph delegation.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/domain.test.js | Updates lineage tests to use new LineageAnalyzer().analyze(...) and asserts Analyzer inheritance/name. |
| test/codegraph.test.js | Adds GraphAnalyzer delegation test + asserts Analyzer inheritance/name. |
| src/index.ts | Exposes Analyzer, GraphAnalyzer, and GraphAnalysisInput from the package entrypoint. |
| src/cli/commands/RunCommand.ts | Switches lineage computation call site to new LineageAnalyzer().analyze(...). |
| src/cli/commands/GraphCommand.ts | Refactors graph command to depend on GraphAnalyzer and use analyzer.name for provenance. |
| src/analysis/LineageAnalyzer.ts | Converts lineage analysis into an Analyzer subclass with stable name + instance analyze. |
| src/analysis/GraphAnalyzer.ts | Introduces graph analysis layer delegating to a CodeGraphProvider. |
| src/analysis/Analyzer.ts | Adds shared abstract base defining name + analyze contract (sync or async). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+38
to
40
| const analyzer = new GraphAnalyzer(createCodeGraphProvider(request.provider)); | ||
| const diagnostics: Diagnostic[] = []; | ||
| let data = new TraceData({}); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Introduces
Analyzer<In, Out>as the shared parent of every analysis in trace-cli, and routes both existing analyses through it.The parent (
src/analysis/Analyzer.ts) fixes one contract: a stablename+analyze(input): Out | Promise<Out>that normalizes raw signal into a domain shape. Abstract, so subclasses narrowIn/Outand pick sync or async.Both analyses now extend it:
analyzeLineageAnalyzerAnalyzer<TraceEvent[], Lineage[]>computebodyGraphAnalyzer(new)Analyzer<GraphAnalysisInput, CodeGraph>CodeGraphProviderDesign notes
static compute→ instanceanalyze; the puresummaryformatter stays a static helper (it's a view util, not part of the analysis contract).graphcommand depends on anAnalyzer(a sibling to lineage), while the existingCodeGraphProvider/LspCodeGraphProviderstays the swappable backend it delegates to.namemirrors the provider, so thegraph.lspenvelope command is unchanged.RunCommand→new LineageAnalyzer().analyze(...);GraphCommandbuilds aGraphAnalyzerover the chosen provider and readsanalyzer.name.Verification
tsc --noEmitclean (abstract implementations need nooverride; return-type narrowing holds)instanceof Analyzerassertion on both sides + aGraphAnalyzerdelegation test that drives a real language server🤖 Generated with Claude Code