Skip to content

Commit 5aac6b0

Browse files
authored
feat(config): restore path aliases with TS 6.x-compatible subpath imports (#672)
* perf(db): generic query execution on NativeDatabase (6.16) Add queryAll/queryGet methods to NativeDatabase for parameterized SELECT execution via rusqlite, returning rows as serde_json::Value objects. This enables NodeQuery and arbitrary SQL to dispatch through the native engine without porting the SQL builder to Rust. - NativeDatabase: queryAll, queryGet, validateSchemaVersion methods - NodeQuery: all()/get() accept optional nativeDb for native dispatch - connection.ts: closeDbPair/closeDbPairDeferred lifecycle helpers - finalize.ts/pipeline.ts: use unified close helpers - detect-changes.ts, build-structure.ts: 3 starter straggler migrations - Comprehensive db.prepare() audit (194 calls, tiered migration plan) - 42 new tests (parity, query execution, version check; skip-guarded) * feat(config): restore path aliases with TS 6.x-compatible subpath imports Replace deprecated baseUrl/paths with Node.js subpath imports (package.json imports field) + customConditions in tsconfig.json. Uses @codegraph/source condition for compile-time resolution to src/, default condition for runtime resolution to dist/. - Add imports field with conditional mappings for all 11 src/ subdirectories + #types - Add customConditions: ["@codegraph/source"] to tsconfig.json - Add resolve.conditions to vitest.config.ts for Vite resolver - Update verify-imports.ts to resolve #-prefixed specifiers - Migrate 3 deeply nested files as proof-of-concept Closes #668 * fix(db): restore hasTable probe to check table existence not row count (#672) The nativeDb migration changed the file_hashes probe from unconditionally setting hasTable=true on successful query to only setting it when the query returns a row. This caused an empty file_hashes table (exists but no rows) to be treated as a full build instead of incremental, breaking the detect-changes test. * fix(db): address review feedback on query safety and docs (#672) - Document row_to_json BLOB/error-to-null contract in Rust doc comment - Clarify queryAll/queryGet doc comments re: SELECT-only intent - Add validateNativeParams runtime check before dispatching to nativeDb
1 parent 37ec95f commit 5aac6b0

21 files changed

Lines changed: 895 additions & 64 deletions

crates/codegraph-core/src/native_db.rs

Lines changed: 158 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
//! Any changes there MUST be reflected here (and vice-versa).
99
1010
use napi_derive::napi;
11-
use rusqlite::{params, Connection, OpenFlags};
11+
use rusqlite::{params, types::ValueRef, Connection, OpenFlags};
1212
use send_wrapper::SendWrapper;
1313

1414
use crate::ast_db::{self, FileAstBatch};
@@ -549,6 +549,104 @@ impl NativeDatabase {
549549
Ok(())
550550
}
551551

552+
// ── Phase 6.16: Generic query execution & version validation ────────
553+
554+
/// Execute a parameterized query and return all rows as JSON objects.
555+
/// Each row is a `{ column_name: value, ... }` object.
556+
/// Params are positional (`?1, ?2, ...`) and accept string, number, or null.
557+
///
558+
/// **Note**: Designed for SELECT statements. Passing DML/DDL will not error
559+
/// at the Rust layer but is not an intended use — all current callers pass
560+
/// SELECT-only SQL generated by `NodeQuery.build()`.
561+
#[napi]
562+
pub fn query_all(
563+
&self,
564+
sql: String,
565+
params: Vec<serde_json::Value>,
566+
) -> napi::Result<Vec<serde_json::Value>> {
567+
let conn = self.conn()?;
568+
let rusqlite_params = json_to_rusqlite_params(&params)?;
569+
let param_refs: Vec<&dyn rusqlite::types::ToSql> =
570+
rusqlite_params.iter().map(|v| v as &dyn rusqlite::types::ToSql).collect();
571+
572+
let mut stmt = conn
573+
.prepare(&sql)
574+
.map_err(|e| napi::Error::from_reason(format!("queryAll prepare failed: {e}")))?;
575+
576+
let col_count = stmt.column_count();
577+
let col_names: Vec<String> = (0..col_count)
578+
.map(|i| stmt.column_name(i).unwrap_or("?").to_owned())
579+
.collect();
580+
581+
let rows = stmt
582+
.query_map(param_refs.as_slice(), |row| {
583+
Ok(row_to_json(row, col_count, &col_names))
584+
})
585+
.map_err(|e| napi::Error::from_reason(format!("queryAll query failed: {e}")))?;
586+
587+
let mut result = Vec::new();
588+
for row in rows {
589+
let val =
590+
row.map_err(|e| napi::Error::from_reason(format!("queryAll row failed: {e}")))?;
591+
result.push(val);
592+
}
593+
Ok(result)
594+
}
595+
596+
/// Execute a parameterized query and return the first row, or null.
597+
/// See `query_all` for parameter and contract details.
598+
#[napi]
599+
pub fn query_get(
600+
&self,
601+
sql: String,
602+
params: Vec<serde_json::Value>,
603+
) -> napi::Result<Option<serde_json::Value>> {
604+
let conn = self.conn()?;
605+
let rusqlite_params = json_to_rusqlite_params(&params)?;
606+
let param_refs: Vec<&dyn rusqlite::types::ToSql> =
607+
rusqlite_params.iter().map(|v| v as &dyn rusqlite::types::ToSql).collect();
608+
609+
let mut stmt = conn
610+
.prepare(&sql)
611+
.map_err(|e| napi::Error::from_reason(format!("queryGet prepare failed: {e}")))?;
612+
613+
let col_count = stmt.column_count();
614+
let col_names: Vec<String> = (0..col_count)
615+
.map(|i| stmt.column_name(i).unwrap_or("?").to_owned())
616+
.collect();
617+
618+
let mut query_rows = stmt
619+
.query(param_refs.as_slice())
620+
.map_err(|e| napi::Error::from_reason(format!("queryGet query failed: {e}")))?;
621+
622+
match query_rows.next() {
623+
Ok(Some(row)) => Ok(Some(row_to_json(row, col_count, &col_names))),
624+
Ok(None) => Ok(None),
625+
Err(e) => Err(napi::Error::from_reason(format!(
626+
"queryGet row failed: {e}"
627+
))),
628+
}
629+
}
630+
631+
/// Validate that the DB's codegraph_version matches the expected version.
632+
/// Returns `true` if versions match or no version is stored.
633+
/// Prints a warning to stderr on mismatch.
634+
#[napi]
635+
pub fn validate_schema_version(&self, expected_version: String) -> napi::Result<bool> {
636+
let stored = self.get_build_meta("codegraph_version".to_string())?;
637+
match stored {
638+
None => Ok(true),
639+
Some(ref v) if v == &expected_version => Ok(true),
640+
Some(v) => {
641+
eprintln!(
642+
"[codegraph] DB was built with v{v}, running v{expected_version}. \
643+
Consider: codegraph build --no-incremental"
644+
);
645+
Ok(false)
646+
}
647+
}
648+
}
649+
552650
// ── Phase 6.15: Build pipeline write operations ─────────────────────
553651

554652
/// Bulk-insert nodes, children, containment edges, exports, and file hashes.
@@ -698,3 +796,62 @@ fn has_column(conn: &Connection, table: &str, column: &str) -> bool {
698796
Err(_) => false,
699797
}
700798
}
799+
800+
/// Convert a JSON param array to rusqlite-compatible values.
801+
fn json_to_rusqlite_params(
802+
params: &[serde_json::Value],
803+
) -> napi::Result<Vec<rusqlite::types::Value>> {
804+
params
805+
.iter()
806+
.enumerate()
807+
.map(|(i, v)| match v {
808+
serde_json::Value::Null => Ok(rusqlite::types::Value::Null),
809+
serde_json::Value::Number(n) => {
810+
if let Some(int) = n.as_i64() {
811+
Ok(rusqlite::types::Value::Integer(int))
812+
} else if let Some(float) = n.as_f64() {
813+
Ok(rusqlite::types::Value::Real(float))
814+
} else {
815+
Err(napi::Error::from_reason(format!(
816+
"param[{i}]: unsupported number {n}"
817+
)))
818+
}
819+
}
820+
serde_json::Value::String(s) => Ok(rusqlite::types::Value::Text(s.clone())),
821+
other => Err(napi::Error::from_reason(format!(
822+
"param[{i}]: unsupported type {}",
823+
other
824+
))),
825+
})
826+
.collect()
827+
}
828+
829+
/// Convert a rusqlite row to a serde_json::Value object.
830+
///
831+
/// **Contract**: Only Integer, Real, Text, and Null column types are supported.
832+
/// BLOB columns are mapped to `null` because the current codegraph schema has no
833+
/// BLOB columns and the generic query path is not designed for binary data.
834+
/// Cell-level read errors are also mapped to `null` to avoid partial-row failures.
835+
fn row_to_json(
836+
row: &rusqlite::Row<'_>,
837+
col_count: usize,
838+
col_names: &[String],
839+
) -> serde_json::Value {
840+
let mut map = serde_json::Map::with_capacity(col_count);
841+
for i in 0..col_count {
842+
let val = match row.get_ref(i) {
843+
Ok(ValueRef::Integer(n)) => serde_json::json!(n),
844+
Ok(ValueRef::Real(f)) => serde_json::json!(f),
845+
Ok(ValueRef::Text(s)) => {
846+
serde_json::Value::String(String::from_utf8_lossy(s).into_owned())
847+
}
848+
Ok(ValueRef::Null) => serde_json::Value::Null,
849+
// BLOB: no codegraph schema columns use BLOB; map to null (see contract above)
850+
Ok(ValueRef::Blob(_)) => serde_json::Value::Null,
851+
// Cell read error: map to null to avoid partial-row failures
852+
Err(_) => serde_json::Value::Null,
853+
};
854+
map.insert(col_names[i].clone(), val);
855+
}
856+
serde_json::Value::Object(map)
857+
}

docs/migration/db-prepare-audit.md

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
# `db.prepare()` Migration Audit
2+
3+
> **Phase 6.16** — Audit of all direct `better-sqlite3` `.prepare()` calls.
4+
> Goal: every call routes through either `Repository` or `NativeDatabase` methods.
5+
6+
## Summary
7+
8+
| Tier | Layer | Files | Calls | Status |
9+
|------|-------|-------|-------|--------|
10+
| 0 | DB infrastructure | 4 | 7 | Done (repository + migrations) |
11+
| 0 | Starter migrations | 2 | 3 | Done (6.16 PR) |
12+
| 1 | Build pipeline | 7 | 52 | Next — ctx.nativeDb available |
13+
| 2 | Domain analysis | 8 | 29 | Requires NativeDatabase in read path |
14+
| 3 | Features | 14 | 94 | Requires NativeDatabase in read path |
15+
| 3 | Shared utilities | 3 | 9 | Requires NativeDatabase in read path |
16+
|| **Total** | **43** | **194** ||
17+
18+
## Tier 0 — Already Abstracted
19+
20+
These are either inside the Repository pattern or in schema migration code.
21+
22+
| File | Calls | Notes |
23+
|------|-------|-------|
24+
| `db/repository/build-stmts.ts` | 3 | Repository layer |
25+
| `db/repository/cfg.ts` | 1 | Repository layer |
26+
| `db/migrations.ts` | 3 | Schema DDL — keep as-is |
27+
28+
## Tier 0 — Starter Migrations (6.16 PR)
29+
30+
Converted to `nativeDb` dispatch in the 6.16 PR:
31+
32+
| File | Calls | What |
33+
|------|-------|------|
34+
| `domain/graph/builder/stages/detect-changes.ts` | 2 | file_hashes probe + full read |
35+
| `domain/graph/builder/stages/build-structure.ts` | 1 | file node count |
36+
37+
## Tier 1 — Build Pipeline (ctx.nativeDb available)
38+
39+
These run during the build pipeline where `ctx.nativeDb` is already open.
40+
Migrate using the same `ctx.nativeDb ? nativeDb.queryAll/queryGet(...) : db.prepare(...)` pattern.
41+
42+
| File | Calls | What |
43+
|------|-------|------|
44+
| `domain/graph/builder/stages/build-structure.ts` | 10 | dir metrics, role UPDATEs, line counts |
45+
| `domain/graph/builder/stages/detect-changes.ts` | 7 | journal queries, mtime checks, CFG count |
46+
| `domain/graph/builder/incremental.ts` | 6 | incremental rebuild queries |
47+
| `domain/graph/builder/stages/build-edges.ts` | 5 | edge dedup, containment edges |
48+
| `domain/graph/builder/stages/finalize.ts` | 5 | build metadata, embedding count |
49+
| `domain/graph/builder/stages/resolve-imports.ts` | 4 | import resolution lookups |
50+
| `domain/graph/builder/stages/insert-nodes.ts` | 3 | node insertion (JS fallback path) |
51+
| `domain/graph/builder/stages/collect-files.ts` | 2 | file collection queries |
52+
| `domain/graph/builder/helpers.ts` | 2 | utility queries |
53+
| `domain/graph/watcher.ts` | 9 | watch mode incremental |
54+
55+
## Tier 2 — Domain Analysis (query-time, read-only)
56+
57+
These run in the query pipeline which currently uses `openReadonlyOrFail()` (better-sqlite3 only).
58+
Migrating these requires adding NativeDatabase to the read path.
59+
60+
| File | Calls | What |
61+
|------|-------|------|
62+
| `domain/analysis/module-map.ts` | 20 | Module map queries (heaviest file) |
63+
| `domain/analysis/symbol-lookup.ts` | 2 | Symbol search |
64+
| `domain/analysis/dependencies.ts` | 2 | Dependency queries |
65+
| `domain/analysis/diff-impact.ts` | 1 | Diff impact analysis |
66+
| `domain/analysis/exports.ts` | 1 | Export analysis |
67+
| `domain/analysis/fn-impact.ts` | 1 | Function impact |
68+
| `domain/analysis/roles.ts` | 1 | Role queries |
69+
| `domain/search/generator.ts` | 4 | Embedding generation |
70+
| `domain/search/stores/fts5.ts` | 1 | FTS5 search |
71+
| `domain/search/search/keyword.ts` | 1 | Keyword search |
72+
| `domain/search/search/prepare.ts` | 1 | Search preparation |
73+
74+
## Tier 3 — Features Layer (query-time, read-only)
75+
76+
Same dependency as Tier 2 — requires NativeDatabase in the read path.
77+
78+
| File | Calls | What |
79+
|------|-------|------|
80+
| `features/structure.ts` | 21 | Structure analysis (heaviest) |
81+
| `features/export.ts` | 13 | Graph export |
82+
| `features/dataflow.ts` | 10 | Dataflow analysis |
83+
| `features/structure-query.ts` | 9 | Structure queries |
84+
| `features/audit.ts` | 7 | Audit command |
85+
| `features/cochange.ts` | 6 | Co-change analysis |
86+
| `features/branch-compare.ts` | 4 | Branch comparison |
87+
| `features/check.ts` | 3 | CI check predicates |
88+
| `features/owners.ts` | 3 | CODEOWNERS integration |
89+
| `features/cfg.ts` | 2 | Control flow graph |
90+
| `features/ast.ts` | 2 | AST queries |
91+
| `features/manifesto.ts` | 2 | Rule engine |
92+
| `features/sequence.ts` | 2 | Sequence diagrams |
93+
| `features/complexity.ts` | 1 | Complexity metrics |
94+
| `features/boundaries.ts` | 1 | Architecture boundaries |
95+
| `features/shared/find-nodes.ts` | 1 | Shared node finder |
96+
97+
## Tier 3 — Shared Utilities
98+
99+
| File | Calls | What |
100+
|------|-------|------|
101+
| `shared/generators.ts` | 4 | Generator utilities |
102+
| `shared/hierarchy.ts` | 4 | Hierarchy traversal |
103+
| `shared/normalize.ts` | 1 | Normalization helpers |
104+
105+
## Migration Recipe
106+
107+
### For Tier 1 (build pipeline):
108+
```typescript
109+
// Before:
110+
const row = db.prepare('SELECT ...').get(...args);
111+
112+
// After:
113+
const sql = 'SELECT ...';
114+
const row = ctx.nativeDb
115+
? ctx.nativeDb.queryGet(sql, [...args])
116+
: db.prepare(sql).get(...args);
117+
```
118+
119+
### For Tiers 2-3 (query pipeline):
120+
Requires adding a `nativeDb` parameter to query-path functions, or opening
121+
a NativeDatabase in `openReadonlyOrFail()`. This is phase 6.17+ work.
122+
123+
## Decision Log
124+
125+
- **`iterate()` stays on better-sqlite3**: rusqlite can't stream across FFI. Only used by `iterateFunctionNodes` — bounded row counts.
126+
- **Migrations stay as-is**: Schema DDL runs once, no performance concern.
127+
- **Features/analysis layers blocked on read-path NativeDatabase**: These only have a better-sqlite3 handle via `openReadonlyOrFail()`. Adding NativeDatabase to the read path is a phase 6.17 prerequisite.

docs/roadmap/ROADMAP.md

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1322,16 +1322,17 @@ Structure building is unchanged — at 22ms it's already fast.
13221322

13231323
### 6.16 -- Dynamic SQL & Edge Cases
13241324

1325-
**Not started.** Handle the remaining non-trivial DB patterns that don't map cleanly to fixed Repository methods.
1325+
**Done.** Generic parameterized query execution on NativeDatabase, connection lifecycle helpers, version validation, and `db.prepare()` audit.
13261326

1327-
**Plan:**
1328-
- **`NodeQuery` builder edge cases:** Ensure the Rust-side replica handles all filter combinations, JOIN paths, ORDER BY variations, and LIMIT/OFFSET correctly — fuzz-test with random filter combinations against the JS builder
1329-
- **`openReadonlyOrFail` version-check logic:** Port the schema-version validation that runs on read-only DB opens
1330-
- **Advisory lock mechanism:** Keep in JS (filesystem-based, not SQLite) — ensure `NativeDatabase.close()` integrates with the existing lock lifecycle
1331-
- **`closeDbDeferred` / WAL checkpoint deferral:** Keep deferred-close logic in JS, call `NativeDatabase.close()` when ready
1332-
- **Raw `db.prepare()` stragglers:** Audit all 383 callers of `.prepare()` and ensure every one routes through either `Repository` or `NativeDatabase` methods — no direct better-sqlite3 usage on the native path
1327+
**Delivered:**
1328+
- **`NativeDatabase.queryAll` / `queryGet`:** Generic parameterized SELECT execution via rusqlite, returning rows as JSON objects. Uses `serde_json::Value` for dynamic column support
1329+
- **`NodeQuery` native dispatch:** `all()` and `get()` accept optional `nativeDb` parameter for rusqlite execution. Combinatorial parity test suite covers all filter/JOIN/ORDER BY combinations
1330+
- **`NativeDatabase.validateSchemaVersion`:** Schema version check for future read-path callers
1331+
- **`closeDbPair` / `closeDbPairDeferred`:** Unified connection lifecycle helpers — close NativeDatabase first (fast), then better-sqlite3 (WAL checkpoint). Replaces manual close sequences in `finalize.ts` and `pipeline.ts`
1332+
- **Starter straggler migrations:** 3 build-pipeline reads in `detect-changes.ts` and `build-structure.ts` dispatch through `nativeDb` when available
1333+
- **`db.prepare()` audit:** 194 calls across 43 files documented in `docs/migration/db-prepare-audit.md` with tiered migration path (Tier 0 done, Tier 1 build pipeline next, Tiers 2-3 blocked on read-path NativeDatabase)
13331334

1334-
**Affected files:** `crates/codegraph-core/src/native_db.rs`, `src/db/connection.ts`, `src/db/query-builder.ts`, `src/db/repository/sqlite-repository.ts`
1335+
**Affected files:** `crates/codegraph-core/src/native_db.rs`, `src/db/connection.ts`, `src/db/query-builder.ts`, `src/db/repository/nodes.ts`, `src/types.ts`, `src/domain/graph/builder/stages/finalize.ts`, `src/domain/graph/builder/pipeline.ts`, `src/domain/graph/builder/stages/detect-changes.ts`, `src/domain/graph/builder/stages/build-structure.ts`
13351336

13361337
### 6.17 -- Cleanup & better-sqlite3 Isolation
13371338

package.json

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,20 @@
44
"description": "Local code graph CLI — parse codebases with tree-sitter, build dependency graphs, query them",
55
"type": "module",
66
"main": "dist/index.js",
7+
"imports": {
8+
"#shared/*": { "@codegraph/source": "./src/shared/*", "default": "./dist/shared/*" },
9+
"#infrastructure/*": { "@codegraph/source": "./src/infrastructure/*", "default": "./dist/infrastructure/*" },
10+
"#db/*": { "@codegraph/source": "./src/db/*", "default": "./dist/db/*" },
11+
"#domain/*": { "@codegraph/source": "./src/domain/*", "default": "./dist/domain/*" },
12+
"#features/*": { "@codegraph/source": "./src/features/*", "default": "./dist/features/*" },
13+
"#presentation/*": { "@codegraph/source": "./src/presentation/*", "default": "./dist/presentation/*" },
14+
"#graph/*": { "@codegraph/source": "./src/graph/*", "default": "./dist/graph/*" },
15+
"#mcp/*": { "@codegraph/source": "./src/mcp/*", "default": "./dist/mcp/*" },
16+
"#ast-analysis/*": { "@codegraph/source": "./src/ast-analysis/*", "default": "./dist/ast-analysis/*" },
17+
"#extractors/*": { "@codegraph/source": "./src/extractors/*", "default": "./dist/extractors/*" },
18+
"#cli/*": { "@codegraph/source": "./src/cli/*", "default": "./dist/cli/*" },
19+
"#types": { "@codegraph/source": "./src/types.ts", "default": "./dist/types.js" }
20+
},
721
"exports": {
822
".": {
923
"import": "./dist/index.js",

0 commit comments

Comments
 (0)