Implement review analysis next#100
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR continues the “review analysis next” refactor by splitting broad internal barrels (util.js, graphs.js, indexer.js) into leaf modules, adding new review-report phases (risk/tasks/graph delta/candidates), and improving resolution + CLI robustness (Composer autoload coverage, Rust module resolution, stricter numeric option parsing, and more consistent project-relative display paths).
Changes:
- Refactors internal imports away from broad barrels into focused modules (indexer build vs navigation vs types; util paths/comments/specifiers/concurrency; graphs queries/rendering).
- Adds/extends language and resolution coverage (PHP Composer PSR-0 / autoload-dev / classmap / files; Rust crate-relative module resolution; JS/TS specifier extraction for
import = requireanddeclare module). - Improves CLI behavior and ergonomics (centralized integer option parsing/validation; new command modules; consistent display paths; unresolved-import reporting can exclude graph-only docs by default).
Reviewed changes
Copilot reviewed 169 out of 169 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/session.test.ts | Updates spies/imports to split indexer build module. |
| tests/samples/php/legacy/Tools/Box.php | Adds PSR-0 fixture class. |
| tests/samples/php/dev-src/Tool.php | Adds autoload-dev PSR-4 fixture class. |
| tests/samples/php/dev-legacy/Tools/Box.php | Adds autoload-dev PSR-0 fixture class. |
| tests/samples/php/dev-classmap/DevSpecific.php | Adds autoload-dev classmap fixture class. |
| tests/samples/php/composer.json | Expands Composer autoload/autoload-dev sections for resolution tests. |
| tests/samples/php/composer-psr0-consumer.php | Adds PSR-0 consumer fixture. |
| tests/samples/php/composer-files-consumer.php | Adds Composer files consumer fixture. |
| tests/samples/php/composer-excluded-classmap-consumer.php | Adds fixture to validate exclude-from-classmap. |
| tests/samples/php/composer-dev-psr4-consumer.php | Adds autoload-dev PSR-4 consumer fixture. |
| tests/samples/php/composer-dev-psr0-consumer.php | Adds autoload-dev PSR-0 consumer fixture. |
| tests/samples/php/composer-dev-classmap-consumer.php | Adds autoload-dev classmap consumer fixture. |
| tests/samples/php/composer-classmap-consumer.php | Adds classmap consumer fixture. |
| tests/samples/php/classmap/Specific.php | Adds classmap fixture class. |
| tests/samples/php/classmap/Excluded/Hidden.php | Adds excluded classmap fixture class. |
| tests/samples/php/autoload/global_helper.php | Adds Composer files autoload fixture. |
| tests/samples/php/autoload/dev_helper.php | Adds Composer autoload-dev.files fixture. |
| tests/review.test.ts | Updates mocks/spies to split indexer build/navigation/types modules. |
| tests/references.test.ts | Adds reference-finding coverage for Composer autoload variants. |
| tests/path-normalization.test.ts | Adds POSIX-root resolution + toProjectDisplayPath tests. |
| tests/package-metadata.test.ts | Adds boundary checks (avoid broad barrels) + export/API doc assertions. |
| tests/native-query-scope.test.ts | Ensures native compact-imports ignores arbitrary string args. |
| tests/map-limit.test.ts | Moves mapLimit import and adds ordering + failure semantics tests. |
| tests/languages/php.test.ts | Adds PHP graph/symbol/goto/refs cases for Composer autoload variants. |
| tests/impact-context-large.test.ts | Updates mocks and imports to new graph/type module paths. |
| tests/impact-cli.test.ts | Adds CLI validation coverage for invalid numeric analysis options. |
| tests/graph-reports.test.ts | Adds unresolved-import behavior for graph-only docs + Rust crate-relative modules. |
| tests/goto.test.ts | Adds go-to-definition coverage for Composer autoload variants and exclusions. |
| tests/fallback-import-extraction.test.ts | Adds JS/TS fallback specifier extraction for import = require and declare module. |
| tests/coverage-targeted.test.ts | Updates concurrency imports and expands formatting in captured fixtures. |
| tests/cli-command-modules.test.ts | Adds validation coverage for chunk token bounds. |
| tests/cache-invalidation.test.ts | Asserts incremental build report counters/timings when cached. |
| src/util/symbolHash.ts | Switches indexer type imports to indexer/types. |
| src/util/specifiers.ts | Extends JS/TS specifier extraction for TS import = require and declare module. |
| src/util/semaphore.ts | Makes semaphore module a facade re-exporting from util/concurrency. |
| src/util/resolution/tsconfig.ts | Extracts TS config path mapping loader/cache into dedicated module. |
| src/util/resolution/rust.ts | Adds Rust module import resolution (crate/self/super + heuristics). |
| src/util/resolution/python.ts | Adds Python module resolver with caching and package-anchor heuristics. |
| src/util/resolution/projectSymbols.ts | Updates concurrency import to new module path. |
| src/util/resolution/phpComposer.ts | Adds Composer autoload parsing + PSR-0/PSR-4/classmap/files resolution helpers + caches. |
| src/util/resolution/node.ts | Adds node_modules resolution helper using package exports/main/index heuristics. |
| src/util/projectFiles.ts | Updates concurrency import to new module path. |
| src/util/paths.ts | Improves cross-platform root/file path handling and adds toProjectDisplayPath. |
| src/util/memberAccess.ts | Switches util import to leaf util/ast. |
| src/util/lazySymbols.ts | Switches indexer imports to leaf types + locals/exports module; minor formatting. |
| src/util/concurrency.ts | Introduces shared semaphore + mapLimit implementation. |
| src/util.ts | Re-exports reorganized util leaf modules; mapLimit now from util/concurrency. |
| src/triples.ts | Switches symbol-graph type import to leaf graph module. |
| src/sqlite/write.ts | Switches symbol-graph type import to leaf graph module. |
| src/sqlite/types.ts | Switches symbol-graph type import to leaf graph module. |
| src/sql/sourceGraph.ts | Uses mapLimit from util/concurrency. |
| src/sql/review.ts | Uses mapLimit from util/concurrency and minor formatting. |
| src/session.ts | Splits indexer imports (types vs build vs navigation) and uses leaf util paths import. |
| src/review/risk.ts | Adds risk scoring/task generation helpers for review reports. |
| src/review/report.ts | Adds review report assembly + graph delta and SQL context integration. |
| src/review/changes.ts | Adds git/file/diff change collection for review flow. |
| src/review/candidates.ts | Adds candidate test collection/merging + display-path normalization. |
| src/query/symbols.ts | Switches symbol-graph type import to leaf graph module. |
| src/query/parser.ts | Switches symbol-graph type import to leaf graph module. |
| src/presets.ts | Switches build options type import to indexer/types. |
| src/presentation/bounds.ts | Centralizes limits/bounds helpers for CLI/review/agent presentation. |
| src/native/runtime.ts | Switches util import to leaf util/ast. |
| src/native/nativeBackendReport.ts | Switches util import to leaf util/ast. |
| src/native/execution.ts | Switches util import to leaf util/ast. |
| src/mcp/server.ts | Switches to leaf graph query + indexer navigation + display path helper. |
| src/mcp/security.ts | Switches to leaf util paths module. |
| src/languages/importStatementParsers.ts | Switches to leaf util paths module. |
| src/languages/definitions/javascript.ts | Tweaks query grouping for require call capture. |
| src/indexer/types.ts | Switches project-file types import to leaf util/projectFiles. |
| src/indexer/shared.ts | Switches util import to leaf util paths module. |
| src/indexer/scope.ts | Switches util import to leaf util/ast. |
| src/indexer/reference-context.ts | Switches util import to leaf util/ast. |
| src/indexer/parsed-cache.ts | Adds parsed-context cache sizing + retention helpers. |
| src/indexer/parse-context.ts | Switches util import to leaf util/ast. |
| src/indexer/navigation.ts | Switches to leaf resolution + util/ast imports. |
| src/indexer/navigation-references.ts | Switches util import to leaf util/ast. |
| src/indexer/navigation-php.ts | Switches util import to leaf util/ast. |
| src/indexer/navigation-goto.ts | Switches util import to leaf util/ast. |
| src/indexer/locals-and-exports.ts | Splits util imports across util/comments and util/ast. |
| src/indexer/incremental-plan.ts | Adds incremental git diff option building + tracked manifest partitioning helpers. |
| src/indexer/imports/python.ts | Splits util imports across util/resolution and util/comments. |
| src/indexer/imports/nativeCaptures.ts | Switches util import to leaf util/ast; minor formatting. |
| src/indexer/imports/languageSpecific.ts | Switches Composer implicit files import to util/resolution; minor formatting. |
| src/indexer/imports/jsFallback.ts | Switches util import to leaf util/comments. |
| src/indexer/imports/graphOnly.ts | Splits util imports across util/resolution and util/workspace; minor formatting. |
| src/indexer/imports.ts | Splits util imports across util/resolution and util/workspace. |
| src/indexer/finalize.ts | Extracts index finalization into dedicated module (adjacency, projectFiles, parsed cache). |
| src/indexer/build-workers.ts | Adds native worker-pool setup/teardown and worker-based file preparation. |
| src/indexer/build-manifest.ts | Adds manifest snapshot writing helper around build-cache manifest logic. |
| src/indexer/build-cache/reports.ts | Switches util import to leaf util/ast. |
| src/indexer/build-cache/options.ts | Splits util imports across util/paths and util/projectFiles; minor formatting. |
| src/indexer/build-cache/manifest.ts | Splits util imports across util/projectFiles, util/paths, util/git, util/ast. |
| src/index.ts | Adds documentation clarifying root package entrypoint/public surface intent. |
| src/impact/types.ts | Switches imports to leaf indexer types + util projectFiles. |
| src/impact/transitive.ts | Switches to leaf indexer types. |
| src/impact/testPatterns.ts | Switches to leaf indexer types + util paths. |
| src/impact/suggestions.ts | Splits indexer imports (navigation vs parse-context) and util paths. |
| src/impact/streaming.ts | Switches to leaf indexer types + util projectFiles. |
| src/impact/severity.ts | Switches to leaf indexer types. |
| src/impact/reportFull.ts | Switches to leaf indexer types; minor formatting. |
| src/impact/reportCompact.ts | Switches to leaf indexer types; minor formatting. |
| src/impact/report.ts | Switches to leaf graphs modules + util projectFiles/paths. |
| src/impact/report-suggestions.ts | Switches to leaf imports; uses display path helper and new mapLimit module. |
| src/impact/providers/base.ts | Switches gitDiffArgs import to leaf util/git. |
| src/impact/path.ts | Uses toProjectDisplayPath for impact report file paths; minor formatting. |
| src/impact/map.ts | Switches indexer imports to leaf types + parse-context. |
| src/impact/index.ts | Switches indexer type import to leaf. |
| src/impact/direct.ts | Switches to leaf indexer types/navigation and concurrency semaphore. |
| src/impact/context.ts | Switches to leaf graphs modules and leaf indexer types. |
| src/impact/collect.ts | Switches to leaf indexer types and mapLimit from concurrency. |
| src/impact/analyzer.ts | Switches to leaf indexer types. |
| src/graphs/unresolved.ts | Adds option to exclude graph-only importers by default. |
| src/graphs/symbol-render.ts | Uses toProjectDisplayPath instead of ad-hoc relative path logic. |
| src/graphs/symbol-graph-detailed/memberChains.ts | Switches util import to leaf util/ast. |
| src/graphs/symbol-graph-detailed/edgePasses.ts | Switches util import to leaf util/ast. |
| src/graphs/symbol-graph-detailed/ast.ts | Switches util import to leaf util/ast. |
| src/graphs/specifiers.ts | Splits util imports across util/ast and util/specifiers. |
| src/graphs/grep.ts | Uses toProjectDisplayPath and leaf projectFiles imports. |
| src/graphs/edgeResolution.ts | Adjusts language routing (including Rust) and splits util imports by module. |
| src/graphs/angularjs.ts | Splits util imports across util/resolution and util/workspace. |
| src/graph-edge-collector.ts | Splits util imports across resolution/workspace/specifiers modules. |
| src/graph-builder.ts | Splits util imports across workspace/paths/concurrency; minor formatting. |
| src/documentLinks/shared.ts | Switches ModuleSpecifier type import to util/specifiers. |
| src/documentLinks/sfc.ts | Switches specifier imports to util/specifiers. |
| src/documentLinks/rst.ts | Switches specifier imports to util/specifiers; minor formatting. |
| src/documentLinks/markdown.ts | Switches specifier imports to util/specifiers. |
| src/documentLinks/html.ts | Switches specifier imports to util/specifiers. |
| src/documentLinks/asciidoc.ts | Switches specifier imports to util/specifiers. |
| src/documentLinks.ts | Switches ModuleSpecifier type import to util/specifiers. |
| src/config.ts | Normalizes discovery roots to slash form; switches discovery type import to leaf module. |
| src/cli/sql.ts | Switches util imports to leaf paths module. |
| src/cli/review.ts | Uses centralized bounds; validates numeric args via shared parsers; leaf imports. |
| src/cli/options.ts | Adds shared integer option parsing helpers (positive/non-negative/bounded/optional). |
| src/cli/navigation.ts | Adds extracted navigation command handlers (dumpmod/goto/refs). |
| src/cli/mcp.ts | Replaces custom port parsing with shared bounded integer parser. |
| src/cli/inspect.ts | Adds extracted inspect/hotspots handlers with scoping, cache reuse, and recommendations. |
| src/cli/index.ts | Adds extracted index command handler with reporting/verbosity support. |
| src/cli/impact.ts | Splits imports to leaf modules and validates numeric args via shared parsers. |
| src/cli/grep.ts | Adds extracted grep handler supporting AST or regex search and max-hits validation. |
| src/cli/graphQueries.ts | Splits imports to leaf modules; improves output path display and depth parsing. |
| src/cli/graphDelta.ts | Splits imports to leaf modules; validates threads arg via shared parser. |
| src/cli/context.ts | Adds CLI runtime/context utilities (arg parsing, progress, filtered discovery globs, reporting). |
| src/cli/chunk.ts | Validates token bounds and adds explicit min/max constraint. |
| src/agent/session.ts | Switches to leaf indexer + graphs modules and leaf projectFiles import. |
| src/agent/normalize.ts | Adds snapshot path resolution + SQL-kind helpers; uses display-path normalization. |
| src/agent/bounds.ts | Reuses shared presentation bounds helpers. |
| src/agent/artifact.ts | Switches to leaf imports and unifies relative file handling through normalize helpers. |
| src/agent-tools.ts | Switches to leaf indexer + graphs + util modules; reduces broad barrel dependence. |
| REVIEW_ANALYSIS_NEXT.md | Marks planned items completed and updates checklist state. |
| README.md | Documents public API boundary and links to library API section. |
| docs/scenario-catalog.md | Updates unresolved-import scenario note to reflect graph-only exclusion. |
| docs/library-api.md | Documents public API boundary and unresolved-import default behavior. |
| docs/language-parity.md | Notes unresolved-import default exclusion of graph-only link edges. |
| docs/cli.md | Documents numeric option validation and clarifies scan scope/root/include-root model. |
| codegraph-skill/codegraph/SKILL.md | Documents numeric option validation, scan scope model, and unresolved-import behavior. |
| docs/superpowers/plans/2026-05-17-read-performance.md | Removes an obsolete completed plan document. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+72
to
+88
| const index = nextIndex++; | ||
| const item = items[index]!; | ||
| activeCount++; | ||
|
|
||
| fn(item) | ||
| .then((result) => { | ||
| if (aborted) return; | ||
| results[index] = result; | ||
| activeCount--; | ||
| if (nextIndex < items.length) { | ||
| startNext(); | ||
| } else if (activeCount === 0 && resolveAll) { | ||
| resolveAll(); | ||
| } | ||
| }) | ||
| .catch((err) => { | ||
| if (aborted) return; |
Comment on lines
+231
to
+235
| for (const root of Array.from(composerConfig.psr4.values()).flat()) { | ||
| addRoot(root, true); | ||
| } | ||
| for (const root of Array.from(composerConfig.psr0.values()).flat()) { | ||
| addRoot(root, true); |
Comment on lines
112
to
+116
| } | ||
| to = await resolveImportSpecifierEdge(entry, context); | ||
| } else if (context.support.id === "go" || context.support.id === "php") { | ||
| } else if (context.support.id === "go" || context.support.id === "php" || context.support.id === "rust") { | ||
| to = await resolveImportSpecifierEdge(entry, context); | ||
| } else if (["csharp", "ruby", "rust"].includes(context.support.id)) { | ||
| const { resolvePathLikeModule } = await import("../util.js"); | ||
| } else if (["csharp", "ruby"].includes(context.support.id)) { |
Comment on lines
+34
to
+36
| const cacheKey = `${fromFile}::${".".repeat(importDotCount)}${moduleName ?? ""}`; | ||
| const cached = resolvePythonModuleCache.get(cacheKey); | ||
| if (cached) return cached; |
Comment on lines
+34
to
+35
| `discovery.includeGlobs` and `discovery.ignoreGlobs` are project-root-relative, even when a command scans child include roots. `discovery.ignoreGlobs` is useful for large fixture, generated, or vendored folders that should not be indexed for search, unresolved-import checks, graphing, impact, or review. CLI `--include-glob` and `--ignore-glob` values are added for a single run; with child include roots, CLI globs stay relative to each scanned root. `inspect` follow-up commands preserve the selected `--root` and include roots. `--no-gitignore` overrides `useGitignore`. | ||
|
|
Comment on lines
+76
to
+93
| Promise.resolve() | ||
| .then(() => fn(item)) | ||
| .then((result) => { | ||
| if (aborted) return; | ||
| results[index] = result; | ||
| activeCount--; | ||
| if (nextIndex < items.length) { | ||
| startNext(); | ||
| } else if (activeCount === 0 && resolveAll) { | ||
| resolveAll(); | ||
| } | ||
| }) | ||
| .catch((err) => { | ||
| if (aborted) return; | ||
| aborted = true; | ||
| activeCount--; | ||
| if (rejectAll) rejectAll(err); | ||
| }); |
Comment on lines
+20
to
+25
| export function resolveAgentSnapshotFile(snapshot: AgentFileSnapshot, candidate: string): string | null { | ||
| const normalizedFiles = new Map(snapshot.files.map((file) => [normalizePath(file), normalizePath(file)])); | ||
| const absoluteCandidate = path.isAbsolute(candidate) | ||
| ? normalizePath(candidate) | ||
| : normalizePath(path.resolve(snapshot.root, candidate)); | ||
| return normalizedFiles.get(absoluteCandidate) ?? null; |
Comment on lines
+311
to
+316
| export function isPhpComposerClassmapExcluded(filePath: string, composerConfig: PhpComposerConfig): boolean { | ||
| const normalizedFile = normalizePath(path.resolve(filePath)); | ||
| return composerConfig.excludeFromClassmap.some((entry) => { | ||
| const normalizedEntry = normalizePath(path.resolve(entry)).replace(/\/+$/, ""); | ||
| return normalizedFile === normalizedEntry || normalizedFile.startsWith(`${normalizedEntry}/`); | ||
| }); |
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.
No description provided.