mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-05 02:43:32 +00:00
2 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Copilot
|
fa06c5610b
|
fix: resolve cross-file type propagation stall on large repos (#1626)
* Initial plan * fix: add time-based deadline to cross-file type propagation to prevent stalling on large repos Adds a 2-minute wall-clock time limit (DEFAULT_CROSS_FILE_ELAPSED_MS) to runCrossFileBindingPropagation. When exceeded, the phase gracefully stops and logs a warning. Users can override via GITNEXUS_CROSS_FILE_TIMEOUT_MS env var. This prevents the analyze command from stalling for hours on very large repositories where per-file re-resolution is expensive. Fixes the reported issue where gitnexus analyze stalls at "Cross-file type propagation" for several hours on repos with 15000+ files. Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/b8341947-557c-4111-a3a8-991ba455ab01 Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * fix: root cause - cache tree-sitter queries across files, add live progress reporting Root cause: cross-file propagation called processCalls() with 1 file at a time, causing Parser.Query to be recompiled from the query string for every single file (O(N) compilations vs O(1) for the whole phase). Additionally, progress was only reported once at the start, making the phase appear completely frozen. Fixes: - Add optional `compiledQueryCache` parameter to `processCalls` so callers that invoke it with single-file batches can share compiled query objects across calls. The cross-file phase now compiles each language's query string exactly once and reuses it for all files of that language (e.g. 1 TypeScript compile for 595+ files). - Pre-count candidate files and emit onProgress every 25 files showing "Cross-file type propagation (N/M files)..." so the UI shows real movement instead of a frozen bar. - Keep the wall-clock deadline (GITNEXUS_CROSS_FILE_TIMEOUT_MS) as a safety net for pathological inputs. Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/f5028cc8-4bc9-4309-8ffb-798fe2bd7a0a Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * fix: address code review - use SupportedLanguages key type, rename queryCache to compiledQueryCache Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/f5028cc8-4bc9-4309-8ffb-798fe2bd7a0a Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * chore(autofix): apply prettier + eslint fixes via /autofix command * fix(cross-file): remove wall-clock timeout from type propagation The query compilation cache and live progress reporting address the original stall; the 2-minute deadline could truncate cross-file work on large repos. MAX_CROSS_FILE_REPROCESS (2000) remains as the only cap. * test(cross-file): verify compiledQueryCache is shared across all processCalls invocations Finding 1: O(N) query recompilation was fixed by sharing a compiledQueryCache Map across all processCalls invocations in runCrossFileBindingPropagation. This test verifies the fix is correctly wired: the same Map instance is passed as the 12th argument to every call, proving queries are compiled once per language, not once per file. Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/3ab768d9-3993-4882-9d8f-17f7fcbd086e Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * test(cross-file): verify live progress events are emitted with N/M format Finding 2: frozen progress display was fixed by emitting onProgress every 25 files with "Cross-file type propagation (N/M files)..." messages instead of calling it once at phase start. This test verifies the fix with 50 candidate files: expects onProgress called 3 times (1 initial + at 25 + at 50) with correct N/M counters. Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/3ab768d9-3993-4882-9d8f-17f7fcbd086e Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * fix(cross-file): skip registry-primary language files before readFileContents Finding 3 (from comment 4466231612): cross-file-impl was calling processCalls for every candidate file even when that file's language is registry-primary (TypeScript, C++, Python, Go, C#, PHP, C — since AGENTS.md v1.7.0). processCalls would immediately skip those files via its own isRegistryPrimary guard, but cross-file-impl still paid the full cost: readFileContents I/O, buildImportedReturnTypes, buildImportedRawReturnTypes, and Map allocation — all discarded. Fix: check isRegistryPrimary(lang) in both the totalCandidates pre-count loop and the levelCandidates builder, before any file I/O or map building. This eliminates 595+ no-op processCalls invocations on large TypeScript repos. Test: mocks isRegistryPrimary to always return true and verifies that processCalls is never invoked and result is 0. The mock also defaults to false in beforeEach so existing tests using .ts files are unaffected. Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/3ab768d9-3993-4882-9d8f-17f7fcbd086e Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * refactor(test): address code review - simplify mock factory, name the arg index constant Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/3ab768d9-3993-4882-9d8f-17f7fcbd086e Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> |
||
|
Copilot
|
26ff700e37
|
refactor(pipeline): DAG-based phase architecture + container-logic extraction to LanguageProvider (#809)
* Initial plan * refactor: move language-specific container node logic into LanguageProvider - Add resolveEnclosingOwner hook to LanguageProviderConfig - Add staticOwnerTypes to MethodExtractionConfig - Implement Ruby resolveEnclosingOwner (singleton_class → class/module) - Replace hardcoded STATIC_OWNER_TYPES with config.staticOwnerTypes - Move Ruby static types to rubyMethodConfig - Move Kotlin static types to kotlinMethodConfig - Remove Ruby singleton_class branch from findEnclosingClassInfo - Collapse seqFindEnclosingClassNode/seqFindRawEnclosingContainerNode into single provider-aware seqFindEnclosingOwnerNode - Update worker path to pass provider.resolveEnclosingOwner Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/bc9f9d4d-f749-4872-9ff2-17fc86e08787 Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * test: add regression tests for config-driven staticOwnerTypes and resolveEnclosingOwner hook Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/bc9f9d4d-f749-4872-9ff2-17fc86e08787 Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * refactor: implement DAG-based pipeline architecture with phase extraction Restructure the ingestion pipeline from a ~1800-line monolithic orchestrator into a DAG (Directed Acyclic Graph) of named phases with explicit dependencies. New files under pipeline-phases/: - types.ts: PipelinePhase, PipelineContext, PhaseResult contracts - runner.ts: DAG runner with topological sort validation - scan.ts, structure.ts, markdown.ts, cobol.ts: early phases - parse.ts + parse-impl.ts: chunked parse + resolve (the core) - routes.ts, tools.ts, orm.ts: post-parse enrichment phases - cross-file.ts + cross-file-impl.ts: cross-file binding propagation - mro.ts, communities.ts, processes.ts: graph analysis phases - index.ts: barrel export pipeline.ts reduced from ~1960 lines to ~184 lines: - DAG phase array declaration - runPipelineFromRepo as thin orchestrator - topologicalLevelSort retained for backward compat Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/136bf9c3-2f4f-449b-9fff-001332c8371c * test: add DAG runner unit tests, update ARCHITECTURE.md with phase DAG docs Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/136bf9c3-2f4f-449b-9fff-001332c8371c * fix: address code review - pass resolutionContext through parse output, fix worker URL path Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/136bf9c3-2f4f-449b-9fff-001332c8371c * fix: declare transitive parse dependency explicitly in mro/communities/processes phases Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/136bf9c3-2f4f-449b-9fff-001332c8371c * refactor: improve pipeline-phases clean code and folder structure - Extract synthesizeWildcardImportBindings to wildcard-synthesis.ts - Extract extractORMQueriesInline to orm-extraction.ts - Create shared constants.ts for AST_CACHE_CAP - Fix inline type import in orm.ts (use proper top-level import) - Add comprehensive JSDoc to getPhaseOutput explaining type safety - Move isDev to module level in cross-file.ts (consistency) - Improve module-level documentation across files - Organize barrel exports in index.ts with section comments Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/2bd6d4aa-6271-4009-8dd2-332ea8ec73ab Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * address review feedback: fix circular dep, allFetchCalls mutation, progress bugs, remove DAG naming, extract isDev, fix _item naming, fix O(n²) line calc Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/6cf53c9b-d55d-4c6f-bf3d-7bfb82d512b6 Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * improve JSDoc on lineNumberAtOffset binary search Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/6cf53c9b-d55d-4c6f-bf3d-7bfb82d512b6 Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * address review: filter deps in runner, move totalFiles to ctx, fix cycle JSDoc, centralize isDev, remove DAG naming Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/b388424f-b939-4a94-97de-3855f9465564 Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * fix doc consistency in graph-sort.ts module-level and function-level JSDoc Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/b388424f-b939-4a94-97de-3855f9465564 Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * fix(pipeline): wrap phase errors with phase name and emit terminal error progress event Restores phase diagnostics at CLI/MCP boundary. runPipeline now wraps phase.execute() in try/catch and rethrows with 'Phase <name> failed: ...' preserving the original via { cause }. Also emits a terminal { phase: 'error' } progress event so subscribers see the failure before the rejection propagates. Handler errors during error reporting are swallowed to keep the original cause authoritative. Plan: docs/plans/2026-04-13-001-fix-pipeline-dag-refactor-review-findings-plan.md (U1) * fix(pipeline): move bindingAccumulator dispose into crossFile try/finally; make single-use crossFile.execute() now wraps its body in try/finally so the accumulator is released on both the happy path and when runCrossFileBindingPropagation throws. Dev-mode telemetry stays inside the try block before dispose (all three counters return 0 after dispose clears internal maps). BindingAccumulator becomes single-use: appendFile after dispose now throws 'BindingAccumulator: use after dispose' instead of silently re-animating via the old _disposed auto-clear. Docs updated; the only production construction site (parse-impl) always creates a fresh instance per run, so no caller relied on the re-use contract. Residual risk documented in crossFile module JSDoc: a future phase inserted between parse and crossFile that throws would still leak the accumulator. Any such phase must manage accumulator lifetime explicitly. Plan: docs/plans/2026-04-13-001-fix-pipeline-dag-refactor-review-findings-plan.md (U2) * docs(pipeline): explain why importCtx teardown is safe before crossFile Investigation (plan U3) confirms: `importCtx` (ImportResolutionContext) is a scratch workspace with no downstream consumer after parse. `resolutionContext` (returned to crossFile) is a distinct object that owns importMap / namedImportMap / packageMap / moduleAliasMap / model, and never closes over importCtx. cross-file-impl consumes only that ctx via processCalls. The two confusingly-similar "context" names were the root of the adversarial reviewer's concern — comment locks in the invariant so the next reader sees it. No behavioral change. Plan: docs/plans/2026-04-13-001-fix-pipeline-dag-refactor-review-findings-plan.md (U3) * refactor(pipeline): remove ctx.totalFiles side-channel; promote to ParseOutput totalFiles was a hidden mutable field on PipelineContext written by parse and read by mro/communities/processes — five reviewers flagged this as a violation of the immutable-context invariant. Removed from PipelineContext, which is now fully readonly, and made the implicit temporal dep explicit: mro/communities/processes now declare 'parse' as a dep and read totalFiles via getPhaseOutput<ParseOutput>(...). No behavior change. Topo-sort unchanged because parse was already a transitive dep through crossFile. Plan: docs/plans/2026-04-13-001-fix-pipeline-dag-refactor-review-findings-plan.md (U4) * feat(method-extractor): runtime staticOwnerTypes guard at factory chokepoint createMethodExtractor now rejects MethodExtractionConfigs that list companion_object / singleton_class / object_declaration in typeDeclarationNodes but omit the matching entry from staticOwnerTypes. Fails loudly at provider construction time instead of producing silent isStatic=false on the 50000th file analyzed. Opt-out convention preserved: an explicit `new Set()` (empty Set) signals intentional exclusion and passes the guard (memory obs #30588). All 13 existing language configs pass the guard; the new negative test fails without it. Test-first. Plan: docs/plans/2026-04-13-001-fix-pipeline-dag-refactor-review-findings-plan.md (U5) * fix(pipeline): wrap sequential-fallback in try/finally so cleanup survives throws The sequential-fallback block in runChunkedParseAndResolve now runs inside a try/finally that guarantees astCache.clear(), accumulator finalize, and enrichExportedTypeMap execute even if readFileContents or processCalls throws mid-fallback. Cleanup failures are caught inside the finally so they can't mask the original error. Accumulator disposal ownership remains with crossFile (U2) — U6 only adds astCache cleanup and preserves finalize ordering on the error path. Plan: docs/plans/2026-04-13-001-fix-pipeline-dag-refactor-review-findings-plan.md (U6) * test(pipeline): direct unit coverage for wildcard-synthesis and cross-file-impl Both modules previously had zero direct unit coverage — branches were exercised only through integration tests' happy paths. wildcard-synthesis.test.ts covers: Go graph-IMPORTS fallback, Python moduleAliasMap build, MAX_SYNTHETIC_BINDINGS_PER_FILE cap, dedup against existing namedImportMap entries, and empty-exportedSymbols early return. cross-file-impl.test.ts covers: gapRatio below threshold no-op, MAX_CROSS_FILE_REPROCESS cap, graph-only exportedTypeMap fallback, and empty namedImportMap short-circuit. Tests assert current behavior — any future regression flips them. Plan: docs/plans/2026-04-13-001-fix-pipeline-dag-refactor-review-findings-plan.md (U7) * test(pipeline): golden-file graph-parity regression guard on mini-repo fixture Pins the current post-P1/P2 graph output (57 symbols, 92 relationships, 4 processes, deterministic edge digest) so future silent refactors cannot drift behavior unnoticed. If any count changes or any edge rewires, the test fails with a readable diff listing what changed and a copy-pasteable UPDATE_GOLDEN=1 regen command. Edge digest keyed by symbolic (label, name, filePath) triples rather than raw generateId output — stays meaningful across id-encoding refactors while still catching real semantic rewiring. Plan: docs/plans/2026-04-13-001-fix-pipeline-dag-refactor-review-findings-plan.md (U8) * fix(pipeline): minimal cycle reporting + resolveEnclosingOwner loop safeguards U9: runner cycle detection now reports only the SCC members via DFS back-edge trace ('Cycle detected: A -> B -> C -> A') rather than everything with inDegree > 0 (which mixed cycle members with blocked dependents). Also emits the 'error' progress event for graph- validation failures, symmetric with U1's runtime-error path. U16: findEnclosingClassInfo now defends against language-provider hooks that return non-container nodes — visitedContainers Set breaks repeat-visit loops, MAX_ENCLOSING_WALK_ITERATIONS is belt-and-braces. Documented the hook contract invariant so future provider authors know the walk-continues-upward expectation. Plan: docs/plans/2026-04-13-001-fix-pipeline-dag-refactor-review-findings-plan.md (U9, U16) * refactor(pipeline): type hygiene, dead code cleanup, shared allPathSet, graph-sort naming Bundles plan units U10, U11, U12, U14, U15: U10 — Type hygiene: readonly ParseOutput arrays (allExtractedRoutes, allDecoratorRoutes, allToolDefs, allORMQueries, allPaths); removed redundant 'as string[] | undefined' cast in routes.ts and 'as URL' in parse-impl.ts; WorkerPool is now 'import type'. Readonly contract propagated into processORMQueries (only iterates). U11 — Dead code & shims: deleted constants.ts shim (AST_CACHE_CAP inlined into its sole real consumer cross-file-impl.ts; isDev consumers now import directly from ../utils/env.js). Removed internal utility re-exports from pipeline-phases/index.ts (no external consumers). Removed topologicalLevelSort re-export from pipeline.ts; updated topological-sort.test.ts to import from the canonical utils/graph-sort.js. Stripped 'Phase 3+4:' stale JSDoc from parse-impl.ts. U12 — Perf: StructureOutput now carries allPathSet (ReadonlySet<string>) built once; cobol, markdown, and cross-file-impl consume the shared set instead of allocating their own. Parse forwards it via ParseOutput.allPathSet; processCobol/processMarkdown widened to ReadonlySet<string>. U14 — graph-sort.ts: renamed local 'inDegree' to 'pendingImportsPerFile' with expanded JSDoc explaining the reverse- graph Kahn's formulation and warning future maintainers not to 'correct' it to standard in-degree semantics. Added self-edge test. U15 — Unconditional worker-fallback logging: removed isDev guard on the worker-pool-creation-failure console.warn so operators can diagnose perf degradations in production. No behavior change. U8 golden-file test confirms pipeline output is byte-identical. Plan: docs/plans/2026-04-13-001-fix-pipeline-dag-refactor-review-findings-plan.md (U10, U11, U12, U14, U15) * docs: fix ARCHITECTURE.md table integrity; bump AGENTS.md/CLAUDE.md to 1.3.0 U13 — documentation fixes: ARCHITECTURE.md: the prior insertion of the 'Pipeline Phase DAG' section orphaned 7 rows from the 'Where to change what' header. Moved those 7 rows back up under their header so the table reads contiguously; DAG section now follows the completed table. AGENTS.md + CLAUDE.md: bumped version 1.2.0 -> 1.3.0, updated Last reviewed to 2026-04-13, added matching Changelog row documenting the GitNexus index stats refresh after the DAG refactor. Stat bumps (symbols/relationships/execution flows) that were sitting uncommitted in the working tree are now landed under a proper changelog entry per each file's own documented schema. Plan: docs/plans/2026-04-13-001-fix-pipeline-dag-refactor-review-findings-plan.md (U13) * refactor(pipeline): drop spurious parse deps, true-readonly ParseOutput.exportedTypeMap, skip redundant wildcard synth - mro/communities/processes: switch redundant `parse` dep to `structure` — totalFiles originates in structure, so depending on parse for it was a spurious data dep that obscured the real DAG. - ParseOutput.exportedTypeMap: typed as truly ReadonlyMap<...,ReadonlyMap>>; graph→exports enrichment moved into parse-impl so the snapshot is fully populated at parse return. crossFile builds its own local mutable working copy for per-file re-resolution writes — no cast at the boundary. - parse-impl: hasSynthesized flag guards the unconditional final synthesizeWildcardImportBindings call when per-chunk/fallback synthesis already ran (graph-global + idempotent across chunks). - cross-file-impl: documented the intentional `phase: 'parsing'` progress label so telemetry bucketing stays consistent with the parse phase. - cross-file-impl test: replaced the now-moved fallback-enrichment assertion with a stronger one — crossFile must not mutate the parse-supplied map. Addresses PR #809 review pass 5 carry-overs. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> Co-authored-by: Gergo Magyar <gergomagyar@icloud.com> |