Migrate TS plugin's build_dep_graph onto shared ts_build_dep_graph; align framework-file graph-node semantics

[elfensky]

The problem

The TypeScript plugin has its own dependency-graph builder, build_dep_graph, at desloppify/languages/typescript/detectors/deps/__init__.py:52-108. The shared infrastructure has another one, ts_build_dep_graph, at desloppify/languages/_framework/treesitter/imports/graph.py. They do almost the same job — parse imports, build edges, scan framework files — with slightly different implementations.

When PR #614 landed a cross-CWD bug fix, the fix had to be applied twice — once in the shared builder, once in the TypeScript-only copy. Same bug, two patches.

The next dep-graph bug will hit the same wall, and the wall will keep growing as more frameworks land via the FrameworkSpec registry from #418.

Why this isn't just "fix the outlier"

I asked four AI advisors to investigate before recommending an approach (debate synthesis at the end of this issue). Codex's codebase archaeology surfaced a useful nuance:

TS isn't the only deep plugin with its own build_dep_graph. Rust, Python, C++, Dart, and Go all have one too:

Plugin	Builder location
Go	go/detectors/deps.py:13-19 (stub returning {})
Rust	rust/detectors/deps.py:21-37 (workspace/package index)
Python	python/detectors/deps.py:46 (AST-based)
C++	cxx/detectors/deps.py:199-207 (compile-commands graph)
Dart	dart/detectors/deps.py:73-87 (package graph)
Java/Ruby/PHP/Swift/Kotlin	go through generic_lang(...) → make_ts_dep_builder

But Rust/Python/C++/Dart are solving different problems (workspace indexes, compile commands, AST analysis, package graphs). They wouldn't share TS's pattern anyway. TS is uniquely tree-sitter-aligned among the deep plugins — its build_dep_graph is doing the same JS-style tree-sitter import work that the shared ts_build_dep_graph already does for JS and every other generic_lang plugin.

So this isn't "establish a universal pattern across all deep plugins." It's "TS is the one deep plugin whose dep-graph implementation can fold cleanly into the shared one."

The solution

The shared ts_build_dep_graph already delegates all import resolution to spec.resolve_import(...) at graph.py:107. TypeScript's three "TS-specific" capabilities all fit cleanly inside a TS-flavored resolve_import closure:

1. Deno external prefix filtering (http://, https://, npm:, jsr:) — the closure returns None for these, the shared builder treats unresolved as "skip edge."
2. tsconfig path-alias resolution (@/ → src/, etc.) — the closure runs the existing _resolve_module + _load_tsconfig_paths logic before falling back to the default JS-style resolver.
3. .js specifier resolving to .ts file — already handled by iter_resolve_candidates at resolve.py:136-143; just keep it inside the closure.

The closure approach means:

• Zero new fields on TreeSitterLangSpec. The existing resolve_import: Callable field absorbs everything. No God-object risk.
• No new strategy classes. This is the existing pattern in the codebase — see GO_SPEC at specs/compiled.py:28-32 and TYPESCRIPT_SPEC at specs/scripting.py:244-249, both of which already bind language-specific resolvers via the same resolve_import field.
• The 90+ lines of hand-rolled TS code collapses to ~10 lines. typescript/detectors/deps/__init__.py:build_dep_graph becomes:

def build_dep_graph(path: Path, roslyn_cmd: str | None = None) -> dict[str, dict[str, Any]]:
    del roslyn_cmd
    return ts_build_dep_graph(
        path,
        TYPESCRIPT_SPEC,  # whose resolve_import is the TS closure
        find_ts_and_tsx_files(path),
        framework_extensions=framework_source_extensions(ecosystem="node"),
    )

The policy question (needs maintainer input before code lands)

This is the only judgment call in the work, and it deserves an explicit decision before any branch exists.

Current TS behavior: .svelte/.vue/.astro files are graph nodes. graph[svelte_key] exists with imports/importers populated.

Shared builder behavior (used by JS today): .svelte/.vue/.astro files contribute importer edges only. They are not graph nodes. graph[svelte_key] is absent. This is documented behavior — see _framework/treesitter/imports/graph.py:61-67 and graph.py:174-181.

Unifying TS onto the shared builder means picking one. The case for the shared-builder behavior:

• Framework files are not host-language source. Including them as nodes muddles "orphan" and "coupling" reports — a .svelte file isn't really a TypeScript orphan.
• JS already chose this semantics. The orphan detector at desloppify/engine/detectors/graph.py is semantics-agnostic (it filters by extension, not graph membership), so there's no production regression risk from the change.
• The maintainer's closing comment on Framework detection architecture: spec-driven horizontal layer (like tree-sitter) #418 documented FrameworkSpec semantics explicitly before implementation; this is the equivalent decision for the dep-graph layer.

If we agree on the shared-builder semantics, the test contract changes by exactly two lines:

• test_ts_deps.py:448 — assert utils_key in graph[svelte_key]["imports"] → delete (svelte_key no longer in graph)
• test_ts_deps.py:566-567 — assert graph[svelte_key]["import_count"] == 1 → delete

The other framework-file tests at test_ts_deps.py:434-535 keep passing because they check importer-edge membership (assert svelte_key in graph[utils_key]["importers"]), which is preserved.

What's NOT in scope

Three follow-up items deliberately left for separate work:

1. Migrating Rust/Python/C++/Dart build_dep_graph implementations. They solve different problems and don't benefit from this work.
2. Splitting the overloaded frameworks: bool flag into framework_phases + framework_imports. Pure cleanup; do it when something needs one without the other.
3. Opening framework_source_extensions to non-Node ecosystems. Speculative until a Python framework with source extensions appears.

Investigation list (before opening the PR)

These are concrete things to verify on a real codebase before committing to the migration:

1. Closure capture for tsconfig. Build the TS resolve_import inside make_ts_dep_builder(path) per-scan, not at module-import time on TYPESCRIPT_SPEC. Otherwise monorepo scans silently fall back to a default tsconfig. Pin with a test fixture: a monorepo with packages/foo/tsconfig.json carrying custom baseUrl: "src" + paths: { "@app/*": ["app/*"] }. Confirm aliases resolve against the right tsconfig root.
2. external_imports tracking. TS's hand-rolled builder sets graph[node]["external_imports"] for Deno URLs (http://, npm:, etc.). Grep downstream — does any detector read this? If yes, add a small optional flag on ts_build_dep_graph and turn it on for TS only. If no, dropping it is fine.
3. build_dynamic_import_targets external use. It's used at phases_coupling.py:130 and commands.py:127 for orphan detection (dynamic_import_finder= kwarg). Confirm it can stay as a free function exposed from typescript/detectors/deps/runtime.py without being folded into the shared builder.
4. Monorepo regression. Find a real TS monorepo with non-default tsconfig (e.g. nextauthjs/next-auth-example or t3-oss/create-t3-turbo). Manually run desloppify scan before+after with the new wrapper. Confirm import edge count and orphan findings match.
5. .js-specifier-resolving-to-.ts-file edge case. iter_resolve_candidates at resolve.py:136-143 handles this. Confirm it lives inside the TS resolve_import closure and isn't silently lost.
6. TS tree-sitter import_query coverage. Verify the existing query at scripting.py:244-249 captures: type-only imports, side-effect imports, export … from "…" re-exports, dynamic import(). If any are missed, the migration could silently drop edges.

Estimated blast radius

• Files modified: ~5 (delete ~150 lines, add ~30)
• Tests that break: exactly 2 assertion lines (test_ts_deps.py:448 + test_ts_deps.py:566-567)
• Behavior change: desloppify deps src/App.svelte no longer returns a node entry (framework files aren't tracked as nodes); orphan detection on TS+Svelte/Vue/Astro projects unchanged
• Effort estimate: ~11h dev + 2h manual testing on a real monorepo + maintainer review round-trip ≈ 3-4 days end-to-end

Debate synthesis (rationale for the chosen approach)

I asked four AI advisors with different specialties to weigh in:

Advisor	Recommended	Filed issue or branch?
Gemini (architectural patterns from peer tools)	B — Strategy pattern object	Issue first
Codex (codebase archaeology)	A with grouped ImportGraphCapabilities field	Branch-and-code
Sonnet (pragmatic implementer, read the actual code)	A — closure-based, no new spec fields	Issue first
Opus (strategic framing, synthesis)	initially C-but-bigger → revised to A after Sonnet's evidence	Issue first

The decisive findings:

• Sonnet's code-grounded read that the existing resolve_import: Callable already absorbs all TS quirks — meaning the spec needs zero new fields. This neutralizes Gemini's "God object" concern, which is correct in the abstract but doesn't apply here.
• Codex's archaeology that the deep plugins with their own builders are solving different problems and wouldn't share TS's pattern — meaning this isn't "establish a universal pattern," it's "TS folds into the tree-sitter import domain that already has a shared builder."
• The 3:1 issue-first majority is driven by the graph-node semantics decision, which is a behavior change worth confirming with the maintainer before code lands.

Related

• PR feat: Astro/Svelte/Vue project support via FrameworkSpec registry #614 — the immediate motivation, which made source_extensions registry-driven on FrameworkSpec
• Issue Framework detection architecture: spec-driven horizontal layer (like tree-sitter) #418 — the architectural precedent for spec-driven, capability-bearing dataclasses (the pattern this work extends)

---

🤖 Issue drafted with input from a 4-way AI debate (Gemini / Codex / Sonnet / Opus). Full debate transcript available on request.

[elfensky]

Investigation 1 — closure capture for tsconfig (RESOLVED)

Finding: Closure capture is not required. A plain free function works correctly, because:

• spec.resolve_import(import_text, filepath, scan_path) already receives scan_path per call (see _framework/treesitter/imports/graph.py:107).
• load_tsconfig_paths_cached is decorated @lru_cache(maxsize=32) (detectors/deps/resolve.py:19-22), keyed on str(project_root.resolve()). The "expensive" disk reads only happen once per project root for the process lifetime.
• find_tsconfig_root(scan_path, project_root) already walks up from scan_path (resolve.py:30-51), which is exactly the monorepo-aware behavior we want.

Proposed shape for the TS resolve_import:

def resolve_typescript_import(import_text: str, filepath: str, scan_path: str) -> str | None:
    if import_text.startswith(("http://", "https://", "npm:", "jsr:")):
        return None
    scan_root = Path(scan_path)
    tsconfig_root = find_tsconfig_root(scan_root, get_project_root())
    tsconfig_paths = load_tsconfig_paths(tsconfig_root)  # cached
    # Try alias resolve first, fall back to JS-style resolution
    aliased = resolve_alias(import_text, tsconfig_paths, tsconfig_root)
    if aliased is not None:
        for candidate in iter_resolve_candidates(aliased):
            if candidate.is_file():
                return str(candidate)
    return resolve_js_import(import_text, filepath, scan_path)

No state captured at module-import time → no 2am gotcha.

No maintainer decision needed. The pattern is mechanically derivable from the existing helpers.

[elfensky]

Investigation 2 — external_imports downstream readers (RESOLVED — safe to drop)

Finding: graph[node]["external_imports"] has zero production readers. The field is written in three places inside typescript/detectors/deps/__init__.py (lines 74, 88, 108) and read in exactly one place — a test assertion at typescript/tests/test_ts_deps.py:169.

Full grep:

$ grep -rn "external_imports" desloppify/ --include="*.py"
desloppify/languages/typescript/tests/test_ts_deps.py:169:    in graph[main_key]["external_imports"]
desloppify/languages/typescript/detectors/deps/__init__.py:74:  lambda: {"imports": set(), "importers": set(), "external_imports": set()}
desloppify/languages/typescript/detectors/deps/__init__.py:88:  graph[source_resolved]["external_imports"].add(module_path)
desloppify/languages/typescript/detectors/deps/__init__.py:108: graph[source_resolved]["external_imports"].add(module_path)

No detector, no command, no engine code reads it.

Decision needed:

• Option A (recommended): Drop external_imports entirely on migration. Delete the test assertion at test_ts_deps.py:169 (the surrounding test can keep asserting that Deno URLs don't show up as graph edges, which is the meaningful behavior).
• Option B: Add external_imports: bool = False flag on ts_build_dep_graph for forward-compat; TS turns it on. Adds plumbing for no current user.

I'd go A. The field has been a write-only data structure for its whole life — keeping it perpetuates the smell.

@peteromallet — your call on A vs B.

[elfensky]

Investigation 3 — build_dynamic_import_targets callers (RESOLVED — stays as free function)

Finding: build_dynamic_import_targets is not in scope for the dep-graph migration. It's a separate concern (dynamic imports are not captured by tree-sitter static import_statement queries; they need a runtime walk).

Callers:

• typescript/phases_coupling.py:130 — passes it as dynamic_import_finder= kwarg to the orphan detector
• typescript/commands.py:127 — same kwarg in cmd_orphaned
• Sibling: python/phases_runtime.py:169 passes Python's own find_python_dynamic_imports to the same kwarg — confirming the orphan detector's API is language-agnostic

The kwarg signature lives at engine/detectors/orphaned.py:90:

dynamic_import_finder: Callable[[Path, list[str]], set[str]] | None = None

Plan: build_dynamic_import_targets and ts_alias_resolver stay where they are (typescript/detectors/deps/runtime.py, exported via __all__ at line 56). The TS plugin keeps importing them. Migration touches build_dep_graph only — the orphan-detection integration is unchanged.

No maintainer decision needed. Confirms scope.

[elfensky]

Investigation 4 — monorepo regression test plan (DESIGN ONLY)

This one can't be fully resolved before code lands — it requires running the migrated wrapper against a real codebase. But here's the concrete test plan, sized to fit a single PR's verification effort:

Synthetic fixture (unit test, runs in CI):

tmp_path/
├── packages/
│   └── foo/
│       ├── tsconfig.json          # baseUrl: "src", paths: {"@app/*": ["app/*"]}
│       └── src/
│           ├── app/utils.ts
│           └── index.ts           # import { x } from "@app/utils"
└── tsconfig.json                  # workspace root, different paths

Assertion: desloppify deps packages/foo/src/index.ts resolves the @app/ alias against packages/foo/tsconfig.json, not the root tsconfig. This pins the per-scan tsconfig walk (find_tsconfig_root).

Real-world manual test (one-off pre-merge):

1. Clone t3-oss/create-t3-turbo (or vercel/next.js apps/docs)
2. Run desloppify scan --path apps/nextjs on the current main and capture desloppify scan --json output
3. Run the same scan on the PR2 branch
4. Diff:
   • orphaned count must match (±0 acceptable)
   • cycles count must match
   • Import edges per file: spot-check 5-10 files for parity
5. Specific test: desloppify deps src/components/Page.svelte returns "framework file, not a graph node" message instead of node data (the deliberate semantics change)

Acceptance criteria: zero unexplained drift in orphan/cycle/import counts on the manual test.

No maintainer decision needed here, but the synthetic monorepo fixture is the concrete contract this PR ships with.

[elfensky]

Investigation 5 — .js-specifier-resolving-to-.ts-file edge case (RESOLVED — no work needed)

Finding: Already lives in the right place and the migration plan preserves it.

iter_resolve_candidates at detectors/deps/resolve.py:121-146 handles this:

if target.suffix in _JS_SPECIFIER_EXTENSIONS:  # {".js", ".mjs", ".cjs"}
    stem = target.with_suffix("")
    yield from _emit(Path(str(stem) + ".ts"))
    yield from _emit(Path(str(stem) + ".tsx"))
    yield from _emit(Path(str(stem) + "/index.ts"))
    yield from _emit(Path(str(stem) + "/index.tsx"))
    yield from _emit(target)  # fall back to literal .js if .ts/.tsx not present

The proposed TS resolve_typescript_import function (Investigation 1) calls iter_resolve_candidates via resolve_module, so this behavior carries over verbatim. resolve.py keeps its current callers and the migration introduces no new path here.

No maintainer decision needed. Behavior preserved.

[elfensky]

Investigation 6 — TS import_query coverage (⚠️ REAL GAP FOUND)

Finding: The current tree-sitter query at _framework/treesitter/specs/scripting.py:244-247 misses export … from "…" re-exports. The TS hand-rolled regex catches them; the migration would silently drop these edges unless the query is extended.

Current query:

(import_statement
    source: (string (string_fragment) @path)) @import

What it catches:

• ✅ import X from 'foo'
• ✅ import 'foo' (side-effect)
• ✅ import type { X } from 'foo' (the type modifier doesn't change the import_statement node type)

What it misses:

• ❌ export { X } from 'foo' — tree-sitter parses this as export_statement, not import_statement
• ❌ export * from 'foo' — same
• ❌ import('foo') (dynamic import) — these are call_expression and handled separately by build_dynamic_import_targets (Investigation 3), so missing them statically is intentional

The current TS regex _IMPORT_SPEC_RE = r"""(?:from\s+|import\s+)(?:type\s+)?['"]([^'"]+)['"]""" catches export-from via the from\s+ alternative — so today's TS dep graph does include export-from edges. The migration would lose them silently.

Fix: extend the query in scripting.py:244-247 to also capture export_statement source strings:

[
  (import_statement
      source: (string (string_fragment) @path))
  (export_statement
      source: (string (string_fragment) @path))
] @import

This change also benefits JS (which uses the same query today via JS_SPEC), so it's a net improvement for the codebase — JS has been silently dropping export-from edges too. Worth verifying with a quick test: a TS file with export { foo } from './bar' whose ./bar export is currently classified as orphan when it shouldn't be.

Decision needed:

• Option A: Extend the query in this PR. Slightly broadens scope but earns a real bug fix for JS too.
• Option B: Add the query extension in a separate prep PR before TS migration, so the migration is a clean refactor.

I'd go A (one bundled PR is easier to review than a 2-PR sequence), but B is cleaner from a bisect/revert standpoint.

@peteromallet — your call on A vs B.

Test gap to fix either way: there's no existing test in test_ts_deps.py covering export { x } from './bar' — the hand-rolled regex's coverage of this case has been untested for as long as it's existed. Adding one is part of the migration.