Skip to content

feat: resolve ES/TS package specifiers for IMPORTS edges (#180)#184

Open
dLo999 wants to merge 1 commit intoDeusData:mainfrom
dLo999:feat/es-ts-package-map
Open

feat: resolve ES/TS package specifiers for IMPORTS edges (#180)#184
dLo999 wants to merge 1 commit intoDeusData:mainfrom
dLo999:feat/es-ts-package-map

Conversation

@dLo999
Copy link
Copy Markdown

@dLo999 dLo999 commented Mar 29, 2026

Closes #180

Summary

Adds ES/TS package specifier resolution so that import { foo } from '@myorg/storage-utils' produces real IMPORTS edges. Previously, fqn_module() treated npm package specifiers as file paths, producing QNs that matched no graph nodes — result: zero IMPORTS edges for all package imports.

How it works

  1. Package map build (new Phase 1.5 in pipeline_run()): Walks the repo for package.json files, parses name + entry point (exports["."] → main → src/index.ts fallback). Builds a CBMHashTable mapping package names to resolved module QNs.

  2. cbm_pipeline_resolve_module(ctx, module_path): New wrapper called instead of fqn_module() at IMPORTS-edge creation sites. If the specifier is a package reference and found in the map, returns the mapped QN. Handles subpath imports (@myorg/pkg/utils) by resolving relative to the package directory. Falls through to fqn_module() for relative paths and unknown packages.

  3. Zero behavior change for non-JS repos: When no package.json files exist, pkg_map is NULL and all resolution falls through to fqn_module(). 0ms overhead.

Changes

  • src/pipeline/pass_pkgmap.c (new) — package map build, free, and resolve functions
  • src/pipeline/pipeline_internal.h — pkg_map field on ctx, cbm_pkg_entry_t struct, prototypes
  • src/pipeline/pipeline.c — Phase 1.5 build step, ctx initialization, cleanup
  • src/pipeline/pass_definitions.c:297 — resolve_module() instead of fqn_module()
  • src/pipeline/pass_calls.c:91 — same
  • src/pipeline/pass_usages.c:96 — same
  • src/pipeline/pass_semantic.c:81 — same
  • src/pipeline/pass_parallel.c:647 — same (in cbm_build_registry_from_cache)
  • Makefile.cbm — added pass_pkgmap.c and test_pkgmap.c
  • tests/test_pkgmap.c (new) — 20 unit tests

Test results

Build: Compiles cleanly on macOS (Apple Clang, arm64) with -Wall -Wextra -Werror

Test suite: 2761 passed, 0 failed (was 2741 — 20 new pkgmap tests)

Behavioral verification (monorepo matching issue scenario):

Created pnpm-like monorepo:

packages/storage-utils/package.json   {"name": "@myorg/storage-utils", "main": "src/index.ts"}
packages/storage-utils/src/index.ts   (exports validateDatasetName, buildSelectQuery)
apps/server/src/index.ts              (import { validateDatasetName } from '@myorg/storage-utils')
Query Before fix After fix
MATCH (a)-[r:IMPORTS]->(b) RETURN a, b 0 rows 1 row: apps/server/src/__file__ → packages/storage-utils/src/index
MATCH (a)-[r:CALLS]->(b) RETURN a, b 2 rows (via unique_name) 2 rows (unchanged — no regression)
pkg_map phase timing N/A 0ms (negligible)

Edge cases tested (unit tests):

Test Result
NULL pkg_map → falls through to fqn_module PASS
Relative import ./utils → falls through PASS
Scoped package @test/utils → exact match PASS
Subpath @test/utils/helpers → resolves relative to pkg dir PASS
Unknown package react → falls through PASS
Package without "name" → skipped PASS
Package without entry point → skipped PASS
exports["."]["import"] conditional → resolved PASS
No package.json in repo → NULL map, 0ms PASS

Scope boundary (not addressed)

  • Relative import resolution (./utils) — already works
  • tsconfig paths (@/components) — separate issue
  • Barrel re-export resolution — separate enhancement
  • External npm deps (in node_modules/) — outside project graph by design

Generated with agent-team via /issue

@dLo999 dLo999 mentioned this pull request Mar 29, 2026
Open
dLo999
dLo999 commented Mar 30, 2026
View reviewed changes
Copy link
Copy Markdown
Author

@dLo999 dLo999 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

Adds ES/TS package specifier resolution via a package map built from package.json files. New pass_pkgmap.c (build/free/resolve), pkg_map field on ctx, 6 call sites updated, 20 new tests. Enables real IMPORTS edges for JS/TS monorepo package imports.

Findings

Reviewer flagged two "critical" issues that are false positives:

  • [resolved] pass_pkgmap.c:305 — cbm_ht_get_key() was flagged as missing, but it exists at hash_table.h:49 / hash_table.c:173. Build confirms zero link errors. The duplicate package handling pattern is correct.
  • [resolved] pass_pkgmap.c:294-307 — Memory management concern depends on above function existing, which it does.

Remaining valid observations (all consistent with existing codebase patterns):

  • [nit] pass_pkgmap.c:283 — No NULL check after strdup for pkg_dir. Matches existing pattern (pipeline code doesn't check strdup returns — OOM at this point is unrecoverable).
  • [nit] pass_pkgmap.c:370 — Fixed-size 1024/2048 buffers for paths. Same pattern as all other pipeline pass files.
  • [nit] pass_pkgmap.c:318 — Stack max 512 for directory traversal. Extreme repos might hit this, but 512 levels is well beyond practical monorepo depth.

CI Status

No CI on fork branch. Local: 2761 tests pass (20 new + 2741 existing), 0 regressions. Behavioral verification confirms IMPORTS edges now appear for cross-package imports.

Verdict

APPROVE — Both critical findings are false positives (function exists in foundation layer). Implementation follows architect's design, matches existing codebase patterns, and is comprehensively tested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

ES/TS module specifiers produce zero IMPORTS edges — pipeline resolves by name only

1 participant

@dLo999