From 4c87aae1fc7356c5a4b279ef2f082fbc3d18c8a3 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Sat, 28 Mar 2026 21:23:16 -0600 Subject: [PATCH 1/4] 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) --- crates/codegraph-core/src/native_db.rs | 147 +++++++++- docs/migration/db-prepare-audit.md | 127 +++++++++ docs/roadmap/ROADMAP.md | 17 +- src/db/connection.ts | 37 ++- src/db/index.ts | 4 +- src/db/query-builder.ts | 24 +- src/db/repository/nodes.ts | 10 +- src/domain/graph/builder/pipeline.ts | 12 +- .../graph/builder/stages/build-structure.ts | 8 +- .../graph/builder/stages/detect-changes.ts | 21 +- src/domain/graph/builder/stages/finalize.ts | 29 +- src/types.ts | 8 + tests/unit/native-db-query.test.ts | 117 ++++++++ tests/unit/native-db-version-check.test.ts | 42 +++ .../unit/query-builder-native-parity.test.ts | 250 ++++++++++++++++++ 15 files changed, 799 insertions(+), 54 deletions(-) create mode 100644 docs/migration/db-prepare-audit.md create mode 100644 tests/unit/native-db-query.test.ts create mode 100644 tests/unit/native-db-version-check.test.ts create mode 100644 tests/unit/query-builder-native-parity.test.ts diff --git a/crates/codegraph-core/src/native_db.rs b/crates/codegraph-core/src/native_db.rs index df7fe69b..aa81d4ef 100644 --- a/crates/codegraph-core/src/native_db.rs +++ b/crates/codegraph-core/src/native_db.rs @@ -8,7 +8,7 @@ //! Any changes there MUST be reflected here (and vice-versa). use napi_derive::napi; -use rusqlite::{params, Connection, OpenFlags}; +use rusqlite::{params, types::ValueRef, Connection, OpenFlags}; use send_wrapper::SendWrapper; use crate::ast_db::{self, FileAstBatch}; @@ -549,6 +549,99 @@ impl NativeDatabase { Ok(()) } + // ── Phase 6.16: Generic query execution & version validation ──────── + + /// Execute a parameterized SELECT and return all rows as JSON objects. + /// Each row is a `{ column_name: value, ... }` object. + /// Params are positional (`?1, ?2, ...`) and accept string, number, or null. + #[napi] + pub fn query_all( + &self, + sql: String, + params: Vec, + ) -> napi::Result> { + let conn = self.conn()?; + let rusqlite_params = json_to_rusqlite_params(¶ms)?; + let param_refs: Vec<&dyn rusqlite::types::ToSql> = + rusqlite_params.iter().map(|v| v as &dyn rusqlite::types::ToSql).collect(); + + let mut stmt = conn + .prepare(&sql) + .map_err(|e| napi::Error::from_reason(format!("queryAll prepare failed: {e}")))?; + + let col_count = stmt.column_count(); + let col_names: Vec = (0..col_count) + .map(|i| stmt.column_name(i).unwrap_or("?").to_owned()) + .collect(); + + let rows = stmt + .query_map(param_refs.as_slice(), |row| { + Ok(row_to_json(row, col_count, &col_names)) + }) + .map_err(|e| napi::Error::from_reason(format!("queryAll query failed: {e}")))?; + + let mut result = Vec::new(); + for row in rows { + let val = + row.map_err(|e| napi::Error::from_reason(format!("queryAll row failed: {e}")))?; + result.push(val); + } + Ok(result) + } + + /// Execute a parameterized SELECT and return the first row, or null. + #[napi] + pub fn query_get( + &self, + sql: String, + params: Vec, + ) -> napi::Result> { + let conn = self.conn()?; + let rusqlite_params = json_to_rusqlite_params(¶ms)?; + let param_refs: Vec<&dyn rusqlite::types::ToSql> = + rusqlite_params.iter().map(|v| v as &dyn rusqlite::types::ToSql).collect(); + + let mut stmt = conn + .prepare(&sql) + .map_err(|e| napi::Error::from_reason(format!("queryGet prepare failed: {e}")))?; + + let col_count = stmt.column_count(); + let col_names: Vec = (0..col_count) + .map(|i| stmt.column_name(i).unwrap_or("?").to_owned()) + .collect(); + + let mut query_rows = stmt + .query(param_refs.as_slice()) + .map_err(|e| napi::Error::from_reason(format!("queryGet query failed: {e}")))?; + + match query_rows.next() { + Ok(Some(row)) => Ok(Some(row_to_json(row, col_count, &col_names))), + Ok(None) => Ok(None), + Err(e) => Err(napi::Error::from_reason(format!( + "queryGet row failed: {e}" + ))), + } + } + + /// Validate that the DB's codegraph_version matches the expected version. + /// Returns `true` if versions match or no version is stored. + /// Prints a warning to stderr on mismatch. + #[napi] + pub fn validate_schema_version(&self, expected_version: String) -> napi::Result { + let stored = self.get_build_meta("codegraph_version".to_string())?; + match stored { + None => Ok(true), + Some(ref v) if v == &expected_version => Ok(true), + Some(v) => { + eprintln!( + "[codegraph] DB was built with v{v}, running v{expected_version}. \ + Consider: codegraph build --no-incremental" + ); + Ok(false) + } + } + } + // ── Phase 6.15: Build pipeline write operations ───────────────────── /// Bulk-insert nodes, children, containment edges, exports, and file hashes. @@ -698,3 +791,55 @@ fn has_column(conn: &Connection, table: &str, column: &str) -> bool { Err(_) => false, } } + +/// Convert a JSON param array to rusqlite-compatible values. +fn json_to_rusqlite_params( + params: &[serde_json::Value], +) -> napi::Result> { + params + .iter() + .enumerate() + .map(|(i, v)| match v { + serde_json::Value::Null => Ok(rusqlite::types::Value::Null), + serde_json::Value::Number(n) => { + if let Some(int) = n.as_i64() { + Ok(rusqlite::types::Value::Integer(int)) + } else if let Some(float) = n.as_f64() { + Ok(rusqlite::types::Value::Real(float)) + } else { + Err(napi::Error::from_reason(format!( + "param[{i}]: unsupported number {n}" + ))) + } + } + serde_json::Value::String(s) => Ok(rusqlite::types::Value::Text(s.clone())), + other => Err(napi::Error::from_reason(format!( + "param[{i}]: unsupported type {}", + other + ))), + }) + .collect() +} + +/// Convert a rusqlite row to a serde_json::Value object. +fn row_to_json( + row: &rusqlite::Row<'_>, + col_count: usize, + col_names: &[String], +) -> serde_json::Value { + let mut map = serde_json::Map::with_capacity(col_count); + for i in 0..col_count { + let val = match row.get_ref(i) { + Ok(ValueRef::Integer(n)) => serde_json::json!(n), + Ok(ValueRef::Real(f)) => serde_json::json!(f), + Ok(ValueRef::Text(s)) => { + serde_json::Value::String(String::from_utf8_lossy(s).into_owned()) + } + Ok(ValueRef::Null) => serde_json::Value::Null, + Ok(ValueRef::Blob(_)) => serde_json::Value::Null, + Err(_) => serde_json::Value::Null, + }; + map.insert(col_names[i].clone(), val); + } + serde_json::Value::Object(map) +} diff --git a/docs/migration/db-prepare-audit.md b/docs/migration/db-prepare-audit.md new file mode 100644 index 00000000..bfdb990d --- /dev/null +++ b/docs/migration/db-prepare-audit.md @@ -0,0 +1,127 @@ +# `db.prepare()` Migration Audit + +> **Phase 6.16** — Audit of all direct `better-sqlite3` `.prepare()` calls. +> Goal: every call routes through either `Repository` or `NativeDatabase` methods. + +## Summary + +| Tier | Layer | Files | Calls | Status | +|------|-------|-------|-------|--------| +| 0 | DB infrastructure | 4 | 7 | Done (repository + migrations) | +| 0 | Starter migrations | 2 | 3 | Done (6.16 PR) | +| 1 | Build pipeline | 7 | 52 | Next — ctx.nativeDb available | +| 2 | Domain analysis | 8 | 29 | Requires NativeDatabase in read path | +| 3 | Features | 14 | 94 | Requires NativeDatabase in read path | +| 3 | Shared utilities | 3 | 9 | Requires NativeDatabase in read path | +| — | **Total** | **43** | **194** | — | + +## Tier 0 — Already Abstracted + +These are either inside the Repository pattern or in schema migration code. + +| File | Calls | Notes | +|------|-------|-------| +| `db/repository/build-stmts.ts` | 3 | Repository layer | +| `db/repository/cfg.ts` | 1 | Repository layer | +| `db/migrations.ts` | 3 | Schema DDL — keep as-is | + +## Tier 0 — Starter Migrations (6.16 PR) + +Converted to `nativeDb` dispatch in the 6.16 PR: + +| File | Calls | What | +|------|-------|------| +| `domain/graph/builder/stages/detect-changes.ts` | 2 | file_hashes probe + full read | +| `domain/graph/builder/stages/build-structure.ts` | 1 | file node count | + +## Tier 1 — Build Pipeline (ctx.nativeDb available) + +These run during the build pipeline where `ctx.nativeDb` is already open. +Migrate using the same `ctx.nativeDb ? nativeDb.queryAll/queryGet(...) : db.prepare(...)` pattern. + +| File | Calls | What | +|------|-------|------| +| `domain/graph/builder/stages/build-structure.ts` | 10 | dir metrics, role UPDATEs, line counts | +| `domain/graph/builder/stages/detect-changes.ts` | 7 | journal queries, mtime checks, CFG count | +| `domain/graph/builder/incremental.ts` | 6 | incremental rebuild queries | +| `domain/graph/builder/stages/build-edges.ts` | 5 | edge dedup, containment edges | +| `domain/graph/builder/stages/finalize.ts` | 5 | build metadata, embedding count | +| `domain/graph/builder/stages/resolve-imports.ts` | 4 | import resolution lookups | +| `domain/graph/builder/stages/insert-nodes.ts` | 3 | node insertion (JS fallback path) | +| `domain/graph/builder/stages/collect-files.ts` | 2 | file collection queries | +| `domain/graph/builder/helpers.ts` | 2 | utility queries | +| `domain/graph/watcher.ts` | 9 | watch mode incremental | + +## Tier 2 — Domain Analysis (query-time, read-only) + +These run in the query pipeline which currently uses `openReadonlyOrFail()` (better-sqlite3 only). +Migrating these requires adding NativeDatabase to the read path. + +| File | Calls | What | +|------|-------|------| +| `domain/analysis/module-map.ts` | 20 | Module map queries (heaviest file) | +| `domain/analysis/symbol-lookup.ts` | 2 | Symbol search | +| `domain/analysis/dependencies.ts` | 2 | Dependency queries | +| `domain/analysis/diff-impact.ts` | 1 | Diff impact analysis | +| `domain/analysis/exports.ts` | 1 | Export analysis | +| `domain/analysis/fn-impact.ts` | 1 | Function impact | +| `domain/analysis/roles.ts` | 1 | Role queries | +| `domain/search/generator.ts` | 4 | Embedding generation | +| `domain/search/stores/fts5.ts` | 1 | FTS5 search | +| `domain/search/search/keyword.ts` | 1 | Keyword search | +| `domain/search/search/prepare.ts` | 1 | Search preparation | + +## Tier 3 — Features Layer (query-time, read-only) + +Same dependency as Tier 2 — requires NativeDatabase in the read path. + +| File | Calls | What | +|------|-------|------| +| `features/structure.ts` | 21 | Structure analysis (heaviest) | +| `features/export.ts` | 13 | Graph export | +| `features/dataflow.ts` | 10 | Dataflow analysis | +| `features/structure-query.ts` | 9 | Structure queries | +| `features/audit.ts` | 7 | Audit command | +| `features/cochange.ts` | 6 | Co-change analysis | +| `features/branch-compare.ts` | 4 | Branch comparison | +| `features/check.ts` | 3 | CI check predicates | +| `features/owners.ts` | 3 | CODEOWNERS integration | +| `features/cfg.ts` | 2 | Control flow graph | +| `features/ast.ts` | 2 | AST queries | +| `features/manifesto.ts` | 2 | Rule engine | +| `features/sequence.ts` | 2 | Sequence diagrams | +| `features/complexity.ts` | 1 | Complexity metrics | +| `features/boundaries.ts` | 1 | Architecture boundaries | +| `features/shared/find-nodes.ts` | 1 | Shared node finder | + +## Tier 3 — Shared Utilities + +| File | Calls | What | +|------|-------|------| +| `shared/generators.ts` | 4 | Generator utilities | +| `shared/hierarchy.ts` | 4 | Hierarchy traversal | +| `shared/normalize.ts` | 1 | Normalization helpers | + +## Migration Recipe + +### For Tier 1 (build pipeline): +```typescript +// Before: +const row = db.prepare('SELECT ...').get(...args); + +// After: +const sql = 'SELECT ...'; +const row = ctx.nativeDb + ? ctx.nativeDb.queryGet(sql, [...args]) + : db.prepare(sql).get(...args); +``` + +### For Tiers 2-3 (query pipeline): +Requires adding a `nativeDb` parameter to query-path functions, or opening +a NativeDatabase in `openReadonlyOrFail()`. This is phase 6.17+ work. + +## Decision Log + +- **`iterate()` stays on better-sqlite3**: rusqlite can't stream across FFI. Only used by `iterateFunctionNodes` — bounded row counts. +- **Migrations stay as-is**: Schema DDL runs once, no performance concern. +- **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. diff --git a/docs/roadmap/ROADMAP.md b/docs/roadmap/ROADMAP.md index 532ad537..42ee537a 100644 --- a/docs/roadmap/ROADMAP.md +++ b/docs/roadmap/ROADMAP.md @@ -1322,16 +1322,17 @@ Structure building is unchanged — at 22ms it's already fast. ### 6.16 -- Dynamic SQL & Edge Cases -**Not started.** Handle the remaining non-trivial DB patterns that don't map cleanly to fixed Repository methods. +**Done.** Generic parameterized query execution on NativeDatabase, connection lifecycle helpers, version validation, and `db.prepare()` audit. -**Plan:** -- **`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 -- **`openReadonlyOrFail` version-check logic:** Port the schema-version validation that runs on read-only DB opens -- **Advisory lock mechanism:** Keep in JS (filesystem-based, not SQLite) — ensure `NativeDatabase.close()` integrates with the existing lock lifecycle -- **`closeDbDeferred` / WAL checkpoint deferral:** Keep deferred-close logic in JS, call `NativeDatabase.close()` when ready -- **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 +**Delivered:** +- **`NativeDatabase.queryAll` / `queryGet`:** Generic parameterized SELECT execution via rusqlite, returning rows as JSON objects. Uses `serde_json::Value` for dynamic column support +- **`NodeQuery` native dispatch:** `all()` and `get()` accept optional `nativeDb` parameter for rusqlite execution. Combinatorial parity test suite covers all filter/JOIN/ORDER BY combinations +- **`NativeDatabase.validateSchemaVersion`:** Schema version check for future read-path callers +- **`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` +- **Starter straggler migrations:** 3 build-pipeline reads in `detect-changes.ts` and `build-structure.ts` dispatch through `nativeDb` when available +- **`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) -**Affected files:** `crates/codegraph-core/src/native_db.rs`, `src/db/connection.ts`, `src/db/query-builder.ts`, `src/db/repository/sqlite-repository.ts` +**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` ### 6.17 -- Cleanup & better-sqlite3 Isolation diff --git a/src/db/connection.ts b/src/db/connection.ts index c504887e..650223f1 100644 --- a/src/db/connection.ts +++ b/src/db/connection.ts @@ -5,7 +5,7 @@ import { fileURLToPath } from 'node:url'; import Database from 'better-sqlite3'; import { debug, warn } from '../infrastructure/logger.js'; import { DbError } from '../shared/errors.js'; -import type { BetterSqlite3Database } from '../types.js'; +import type { BetterSqlite3Database, NativeDatabase } from '../types.js'; import { Repository } from './repository/base.js'; import { SqliteRepository } from './repository/sqlite-repository.js'; @@ -208,6 +208,41 @@ export function closeDbDeferred(db: LockedDatabase): void { }); } +// ── Paired close helpers (Phase 6.16) ────────────────────────────────── +// When both a NativeDatabase and better-sqlite3 handle are open on the same +// DB file, these helpers ensure NativeDatabase is closed first (fast, ~1ms) +// before the better-sqlite3 close (which forces a WAL checkpoint, ~250ms). + +/** A better-sqlite3 handle optionally paired with a NativeDatabase. */ +export interface LockedDatabasePair { + db: LockedDatabase; + nativeDb?: NativeDatabase; +} + +/** Close both handles: NativeDatabase first (fast), then better-sqlite3 (releases lock). */ +export function closeDbPair(pair: LockedDatabasePair): void { + if (pair.nativeDb) { + try { + pair.nativeDb.close(); + } catch { + /* ignore */ + } + } + closeDb(pair.db); +} + +/** Close NativeDatabase immediately, defer better-sqlite3 WAL checkpoint. */ +export function closeDbPairDeferred(pair: LockedDatabasePair): void { + if (pair.nativeDb) { + try { + pair.nativeDb.close(); + } catch { + /* ignore */ + } + } + closeDbDeferred(pair.db); +} + export function findDbPath(customPath?: string): string { if (customPath) return path.resolve(customPath); const rawCeiling = findRepoRoot(); diff --git a/src/db/index.ts b/src/db/index.ts index d850252f..7129ca4a 100644 --- a/src/db/index.ts +++ b/src/db/index.ts @@ -1,9 +1,11 @@ // Barrel re-export — keeps all existing `import { ... } from '…/db/index.js'` working. -export type { LockedDatabase } from './connection.js'; +export type { LockedDatabase, LockedDatabasePair } from './connection.js'; export { closeDb, closeDbDeferred, + closeDbPair, + closeDbPairDeferred, findDbPath, findRepoRoot, flushDeferredClose, diff --git a/src/db/query-builder.ts b/src/db/query-builder.ts index 66fe5cd9..0fadc1af 100644 --- a/src/db/query-builder.ts +++ b/src/db/query-builder.ts @@ -1,6 +1,6 @@ import { DbError } from '../shared/errors.js'; import { DEAD_ROLE_PREFIX, EVERY_EDGE_KIND } from '../shared/kinds.js'; -import type { BetterSqlite3Database } from '../types.js'; +import type { BetterSqlite3Database, NativeDatabase } from '../types.js'; // ─── Validation Helpers ───────────────────────────────────────────── @@ -314,15 +314,29 @@ export class NodeQuery { return { sql, params }; } - /** Execute and return all rows. */ - all>(db: BetterSqlite3Database): TRow[] { + /** Execute and return all rows. When `nativeDb` is provided, dispatches through rusqlite. */ + all>( + db: BetterSqlite3Database, + nativeDb?: NativeDatabase, + ): TRow[] { const { sql, params } = this.build(); + if (nativeDb) { + return nativeDb.queryAll(sql, params as Array) as TRow[]; + } return db.prepare(sql).all(...params) as TRow[]; } - /** Execute and return first row. */ - get>(db: BetterSqlite3Database): TRow | undefined { + /** Execute and return first row. When `nativeDb` is provided, dispatches through rusqlite. */ + get>( + db: BetterSqlite3Database, + nativeDb?: NativeDatabase, + ): TRow | undefined { const { sql, params } = this.build(); + if (nativeDb) { + return (nativeDb.queryGet(sql, params as Array) ?? undefined) as + | TRow + | undefined; + } return db.prepare(sql).get(...params) as TRow | undefined; } diff --git a/src/db/repository/nodes.ts b/src/db/repository/nodes.ts index da5d0a13..e611d2c4 100644 --- a/src/db/repository/nodes.ts +++ b/src/db/repository/nodes.ts @@ -4,6 +4,7 @@ import type { BetterSqlite3Database, ChildNodeRow, ListFunctionOpts, + NativeDatabase, NodeIdRow, NodeRow, NodeRowWithFanIn, @@ -24,6 +25,7 @@ export function findNodesWithFanIn( db: BetterSqlite3Database, namePattern: string, opts: QueryOpts = {}, + nativeDb?: NativeDatabase, ): NodeRowWithFanIn[] { const q = new NodeQuery() .select('n.*, COALESCE(fi.cnt, 0) AS fan_in') @@ -37,7 +39,7 @@ export function findNodesWithFanIn( q.fileFilter(opts.file); } - return q.all(db); + return q.all(db, nativeDb); } /** @@ -46,6 +48,7 @@ export function findNodesWithFanIn( export function findNodesForTriage( db: BetterSqlite3Database, opts: TriageQueryOpts = {}, + nativeDb?: NativeDatabase, ): TriageNodeRow[] { if (opts.kind && !(EVERY_SYMBOL_KIND as readonly string[]).includes(opts.kind)) { throw new ConfigError( @@ -77,7 +80,7 @@ export function findNodesForTriage( .roleFilter(opts.role) .orderBy('n.file, n.line'); - return q.all(db); + return q.all(db, nativeDb); } /** @@ -99,8 +102,9 @@ function _functionNodeQuery(opts: ListFunctionOpts = {}): InstanceType { // Gate: ≤5 changed files AND significantly more existing files (>20) to // avoid triggering on small test fixtures where directory metrics matter. const existingFileCount = !isFullBuild - ? (db.prepare("SELECT COUNT(*) as c FROM nodes WHERE kind = 'file'").get() as { c: number }).c + ? ( + (ctx.nativeDb + ? ctx.nativeDb.queryGet("SELECT COUNT(*) as c FROM nodes WHERE kind = 'file'", []) + : db.prepare("SELECT COUNT(*) as c FROM nodes WHERE kind = 'file'").get()) as { + c: number; + } + ).c : 0; const useSmallIncrementalFastPath = !isFullBuild && diff --git a/src/domain/graph/builder/stages/detect-changes.ts b/src/domain/graph/builder/stages/detect-changes.ts index 045340ba..5636ee03 100644 --- a/src/domain/graph/builder/stages/detect-changes.ts +++ b/src/domain/graph/builder/stages/detect-changes.ts @@ -10,7 +10,7 @@ import path from 'node:path'; import { closeDb } from '../../../../db/index.js'; import { debug, info } from '../../../../infrastructure/logger.js'; import { normalizePath } from '../../../../shared/constants.js'; -import type { BetterSqlite3Database, ExtractorOutput } from '../../../../types.js'; +import type { BetterSqlite3Database, ExtractorOutput, NativeDatabase } from '../../../../types.js'; import { parseFilesAuto } from '../../../parser.js'; import { readJournal, writeJournalHeader } from '../../journal.js'; import type { PipelineContext } from '../context.js'; @@ -58,11 +58,14 @@ function getChangedFiles( db: BetterSqlite3Database, allFiles: string[], rootDir: string, + nativeDb?: NativeDatabase, ): ChangeResult { let hasTable = false; try { - db.prepare('SELECT 1 FROM file_hashes LIMIT 1').get(); - hasTable = true; + const probe = nativeDb + ? nativeDb.queryGet('SELECT 1 FROM file_hashes LIMIT 1', []) + : db.prepare('SELECT 1 FROM file_hashes LIMIT 1').get(); + if (probe) hasTable = true; } catch { /* table doesn't exist */ } @@ -75,11 +78,11 @@ function getChangedFiles( }; } - const existing = new Map( - (db.prepare('SELECT file, hash, mtime, size FROM file_hashes').all() as FileHashRow[]).map( - (r) => [r.file, r], - ), - ); + const sql = 'SELECT file, hash, mtime, size FROM file_hashes'; + const rows = nativeDb + ? (nativeDb.queryAll(sql, []) as unknown as FileHashRow[]) + : (db.prepare(sql).all() as FileHashRow[]); + const existing = new Map(rows.map((r) => [r.file, r])); const removed = detectRemovedFiles(existing, allFiles, rootDir); const journalResult = tryJournalTier(db, existing, rootDir, removed); @@ -421,7 +424,7 @@ export async function detectChanges(ctx: PipelineContext): Promise { } const increResult = incremental && !forceFullRebuild - ? getChangedFiles(db, allFiles, rootDir) + ? getChangedFiles(db, allFiles, rootDir, ctx.nativeDb) : { changed: allFiles.map((f): ChangedFile => ({ file: f })), removed: [] as string[], diff --git a/src/domain/graph/builder/stages/finalize.ts b/src/domain/graph/builder/stages/finalize.ts index 763f9c96..8ceac2eb 100644 --- a/src/domain/graph/builder/stages/finalize.ts +++ b/src/domain/graph/builder/stages/finalize.ts @@ -6,7 +6,12 @@ import { tmpdir } from 'node:os'; import path from 'node:path'; import { performance } from 'node:perf_hooks'; -import { closeDb, closeDbDeferred, getBuildMeta, setBuildMeta } from '../../../../db/index.js'; +import { + closeDbPair, + closeDbPairDeferred, + getBuildMeta, + setBuildMeta, +} from '../../../../db/index.js'; import { debug, info, warn } from '../../../../infrastructure/logger.js'; import { CODEGRAPH_VERSION } from '../../../../shared/version.js'; import { writeJournalHeader } from '../../journal.js'; @@ -183,24 +188,16 @@ export async function finalize(ctx: PipelineContext): Promise { // separately via timing.closeDbMs when available. ctx.timing.finalizeMs = performance.now() - t0; - // Close NativeDatabase before better-sqlite3 (Phase 6.13) - if (ctx.nativeDb) { - try { - ctx.nativeDb.close(); - } catch { - /* ignore */ - } - } - - // For small incremental builds, defer db.close() to the next event loop tick. - // The WAL checkpoint in db.close() costs ~250ms on Windows NTFS due to fsync. - // Deferring lets buildGraph() return immediately; the checkpoint runs after. - // Skip for temp directories (tests) — they rmSync immediately after build. + // Close NativeDatabase (fast, ~1ms) then better-sqlite3 (WAL checkpoint). + // For small incremental builds, defer the expensive WAL checkpoint to the + // next event loop tick. Skip for temp directories (tests) — they rmSync + // immediately after build. + const pair = { db, nativeDb: ctx.nativeDb }; const isTempDir = path.resolve(rootDir).startsWith(path.resolve(tmpdir())); if (!isFullBuild && allSymbols.size <= 5 && !isTempDir) { - closeDbDeferred(db); + closeDbPairDeferred(pair); } else { - closeDb(db); + closeDbPair(pair); } // Write journal header after successful build diff --git a/src/types.ts b/src/types.ts index b6d8f83e..8eb8bda2 100644 --- a/src/types.ts +++ b/src/types.ts @@ -1978,6 +1978,14 @@ export interface NativeDatabase { leaf: number; } | null; purgeFilesData(files: string[], purgeHashes?: boolean): void; + + // ── Generic query execution & version validation (6.16) ───────────── + /** Execute a parameterized SELECT and return all rows as objects. */ + queryAll(sql: string, params: Array): Record[]; + /** Execute a parameterized SELECT and return the first row, or null. */ + queryGet(sql: string, params: Array): Record | null; + /** Validate DB codegraph_version matches expected. Warns on mismatch. */ + validateSchemaVersion(expectedVersion: string): boolean; } // ════════════════════════════════════════════════════════════════════════ diff --git a/tests/unit/native-db-query.test.ts b/tests/unit/native-db-query.test.ts new file mode 100644 index 00000000..6af51076 --- /dev/null +++ b/tests/unit/native-db-query.test.ts @@ -0,0 +1,117 @@ +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { getNative, isNativeAvailable } from '../../src/infrastructure/native.js'; +import type { NativeDatabase } from '../../src/types.js'; + +const hasNativeDb = + isNativeAvailable() && typeof getNative().NativeDatabase?.prototype?.queryAll === 'function'; + +describe.skipIf(!hasNativeDb)('NativeDatabase.queryAll / queryGet', () => { + let nativeDb: NativeDatabase; + let dbPath: string; + + beforeEach(() => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-native-query-')); + dbPath = path.join(tmpDir, 'test.db'); + const NativeDB = getNative().NativeDatabase; + nativeDb = NativeDB.openReadWrite(dbPath); + nativeDb.initSchema(); + + // Seed test data + nativeDb.exec(` + INSERT INTO nodes (name, kind, file, line, role) VALUES ('foo', 'function', 'src/foo.js', 1, 'core'); + INSERT INTO nodes (name, kind, file, line, role) VALUES ('bar', 'method', 'src/bar.js', 10, 'utility'); + INSERT INTO nodes (name, kind, file, line, role) VALUES ('Baz', 'class', 'src/baz.js', 20, 'entry'); + `); + }); + + afterEach(() => { + nativeDb.close(); + fs.rmSync(path.dirname(dbPath), { recursive: true, force: true }); + }); + + it('returns all rows with correct column names', () => { + const rows = nativeDb.queryAll('SELECT name, kind FROM nodes ORDER BY name', []); + expect(rows).toHaveLength(3); + expect(rows[0]).toEqual({ name: 'Baz', kind: 'class' }); + expect(rows[1]).toEqual({ name: 'bar', kind: 'method' }); + expect(rows[2]).toEqual({ name: 'foo', kind: 'function' }); + }); + + it('returns empty array for no matches', () => { + const rows = nativeDb.queryAll("SELECT * FROM nodes WHERE name = 'nope'", []); + expect(rows).toEqual([]); + }); + + it('handles string parameters', () => { + const rows = nativeDb.queryAll('SELECT name FROM nodes WHERE kind = ?', ['function']); + expect(rows).toHaveLength(1); + expect(rows[0]).toEqual({ name: 'foo' }); + }); + + it('handles number parameters', () => { + const rows = nativeDb.queryAll('SELECT name FROM nodes WHERE line > ?', [5]); + expect(rows).toHaveLength(2); + }); + + it('handles null parameters', () => { + nativeDb.exec( + "INSERT INTO nodes (name, kind, file, line, role) VALUES ('orphan', 'function', 'x.js', 1, NULL)", + ); + const rows = nativeDb.queryAll('SELECT name FROM nodes WHERE role IS NULL', []); + expect(rows.some((r) => r.name === 'orphan')).toBe(true); + }); + + it('handles multiple parameters', () => { + const rows = nativeDb.queryAll('SELECT name FROM nodes WHERE kind = ? AND line >= ?', [ + 'method', + 10, + ]); + expect(rows).toEqual([{ name: 'bar' }]); + }); + + it('returns null columns as null', () => { + nativeDb.exec( + "INSERT INTO nodes (name, kind, file, line, role) VALUES ('nul', 'function', 'x.js', 1, NULL)", + ); + const rows = nativeDb.queryAll("SELECT role FROM nodes WHERE name = 'nul'", []); + expect(rows[0]!.role).toBeNull(); + }); + + it('handles integer and real column types', () => { + const row = nativeDb.queryGet('SELECT 42 AS int_val, 3.14 AS real_val', []); + expect(row).toBeDefined(); + expect(row!.int_val).toBe(42); + expect(row!.real_val).toBeCloseTo(3.14); + }); + + // -- queryGet -- + + it('queryGet returns first row', () => { + const row = nativeDb.queryGet('SELECT name FROM nodes ORDER BY name LIMIT 1', []); + expect(row).toEqual({ name: 'Baz' }); + }); + + it('queryGet returns null for no matches', () => { + const row = nativeDb.queryGet("SELECT * FROM nodes WHERE name = 'nope'", []); + expect(row).toBeNull(); + }); + + it('queryGet with parameters', () => { + const row = nativeDb.queryGet('SELECT name, line FROM nodes WHERE kind = ?', ['class']); + expect(row).toEqual({ name: 'Baz', line: 20 }); + }); + + // -- Error handling -- + + it('throws on invalid SQL', () => { + expect(() => nativeDb.queryAll('SELECT * FROM nonexistent_table', [])).toThrow(); + }); + + it('throws on closed database', () => { + nativeDb.close(); + expect(() => nativeDb.queryAll('SELECT 1', [])).toThrow(/closed/i); + }); +}); diff --git a/tests/unit/native-db-version-check.test.ts b/tests/unit/native-db-version-check.test.ts new file mode 100644 index 00000000..65fd403d --- /dev/null +++ b/tests/unit/native-db-version-check.test.ts @@ -0,0 +1,42 @@ +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { getNative, isNativeAvailable } from '../../src/infrastructure/native.js'; +import type { NativeDatabase } from '../../src/types.js'; + +const hasNativeDb = + isNativeAvailable() && + typeof getNative().NativeDatabase?.prototype?.validateSchemaVersion === 'function'; + +describe.skipIf(!hasNativeDb)('NativeDatabase.validateSchemaVersion', () => { + let nativeDb: NativeDatabase; + let dbPath: string; + + beforeEach(() => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-native-version-')); + dbPath = path.join(tmpDir, 'test.db'); + const NativeDB = getNative().NativeDatabase; + nativeDb = NativeDB.openReadWrite(dbPath); + nativeDb.initSchema(); + }); + + afterEach(() => { + nativeDb.close(); + fs.rmSync(path.dirname(dbPath), { recursive: true, force: true }); + }); + + it('returns true when no version is stored', () => { + expect(nativeDb.validateSchemaVersion('1.0.0')).toBe(true); + }); + + it('returns true when versions match', () => { + nativeDb.setBuildMeta([{ key: 'codegraph_version', value: '3.4.0' }]); + expect(nativeDb.validateSchemaVersion('3.4.0')).toBe(true); + }); + + it('returns false and warns when versions mismatch', () => { + nativeDb.setBuildMeta([{ key: 'codegraph_version', value: '3.3.0' }]); + expect(nativeDb.validateSchemaVersion('3.4.0')).toBe(false); + }); +}); diff --git a/tests/unit/query-builder-native-parity.test.ts b/tests/unit/query-builder-native-parity.test.ts new file mode 100644 index 00000000..518322d4 --- /dev/null +++ b/tests/unit/query-builder-native-parity.test.ts @@ -0,0 +1,250 @@ +/** + * NodeQuery parity test: verifies that queryAll/queryGet through NativeDatabase + * produces identical results to better-sqlite3 for all filter combinations. + */ + +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import Database from 'better-sqlite3'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { initSchema } from '../../src/db/migrations.js'; +import { NodeQuery } from '../../src/db/query-builder.js'; +import { getNative, isNativeAvailable } from '../../src/infrastructure/native.js'; +import type { BetterSqlite3Database, NativeDatabase } from '../../src/types.js'; + +const hasNativeDb = + isNativeAvailable() && typeof getNative().NativeDatabase?.prototype?.queryAll === 'function'; + +/** + * Normalize row values for comparison — SQLite integer types may differ between + * better-sqlite3 (BigInt for large values) and rusqlite (always i64 → number). + */ +function normalizeRows(rows: Record[]): Record[] { + return rows.map((row) => { + const normalized: Record = {}; + for (const [key, value] of Object.entries(row)) { + normalized[key] = typeof value === 'bigint' ? Number(value) : value; + } + return normalized; + }); +} + +/** Seed identical data into both databases. */ +const SEED_SQL = ` + INSERT INTO nodes (name, kind, file, line, role, exported) VALUES ('foo', 'function', 'src/core/foo.js', 1, 'core', 1); + INSERT INTO nodes (name, kind, file, line, role, exported) VALUES ('bar', 'method', 'src/core/bar.js', 10, 'utility', 1); + INSERT INTO nodes (name, kind, file, line, role, exported) VALUES ('Baz', 'class', 'src/baz.js', 20, 'entry', 0); + INSERT INTO nodes (name, kind, file, line, role, exported) VALUES ('testHelper', 'function', 'src/foo.test.js', 1, 'dead', 0); + INSERT INTO nodes (name, kind, file, line, role, exported) VALUES ('specHelper', 'function', 'src/bar.spec.js', 1, 'dead-leaf', 0); + INSERT INTO nodes (name, kind, file, line, role, exported) VALUES ('__test__util', 'function', 'src/__tests__/util.js', 5, NULL, 0); +`; + +/** Seed edges and complexity for JOIN tests. */ +const SEED_EDGES_SQL = ` + INSERT INTO edges (source_id, target_id, kind, confidence, dynamic) + SELECT b.id, f.id, 'calls', 1.0, 0 + FROM nodes b, nodes f WHERE b.name = 'bar' AND f.name = 'foo'; + INSERT INTO edges (source_id, target_id, kind, confidence, dynamic) + SELECT bz.id, f.id, 'calls', 1.0, 0 + FROM nodes bz, nodes f WHERE bz.name = 'Baz' AND f.name = 'foo'; +`; + +const SEED_COMPLEXITY_SQL = ` + INSERT INTO function_complexity (node_id, cognitive, cyclomatic, max_nesting, halstead_volume, halstead_difficulty, halstead_effort, loc, maintainability_index) + SELECT id, 5, 3, 2, 100, 10, 1000, 15, 80 FROM nodes WHERE name = 'foo'; + INSERT INTO function_complexity (node_id, cognitive, cyclomatic, max_nesting, halstead_volume, halstead_difficulty, halstead_effort, loc, maintainability_index) + SELECT id, 12, 8, 4, 250, 20, 5000, 40, 55 FROM nodes WHERE name = 'bar'; +`; + +describe.skipIf(!hasNativeDb)('NodeQuery native parity', () => { + let db: BetterSqlite3Database; + let nativeDb: NativeDatabase; + let tmpDir: string; + + beforeEach(() => { + // better-sqlite3 in-memory + db = new Database(':memory:') as unknown as BetterSqlite3Database; + initSchema(db); + db.exec(SEED_SQL); + db.exec(SEED_EDGES_SQL); + db.exec(SEED_COMPLEXITY_SQL); + + // NativeDatabase on temp file + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-parity-')); + const dbPath = path.join(tmpDir, 'test.db'); + const NativeDB = getNative().NativeDatabase; + nativeDb = NativeDB.openReadWrite(dbPath); + nativeDb.initSchema(); + nativeDb.exec(SEED_SQL); + nativeDb.exec(SEED_EDGES_SQL); + nativeDb.exec(SEED_COMPLEXITY_SQL); + }); + + afterEach(() => { + db.close(); + nativeDb.close(); + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + /** Run a NodeQuery through both engines and assert identical results. */ + function assertParity(q: InstanceType): void { + const jsRows = normalizeRows(q.all(db) as Record[]); + const nativeRows = normalizeRows(q.all(db, nativeDb) as Record[]); + expect(nativeRows).toEqual(jsRows); + } + + /** Run queryGet parity. */ + function assertGetParity(q: InstanceType): void { + const jsRow = q.get(db) as Record | undefined; + const nativeRow = q.get(db, nativeDb) as Record | undefined; + const jsNormalized = jsRow ? normalizeRows([jsRow])[0] : undefined; + const nativeNormalized = nativeRow ? normalizeRows([nativeRow])[0] : undefined; + expect(nativeNormalized).toEqual(jsNormalized); + } + + // ── Basic filters ──────────────────────────────────────────────────── + + it('no filters', () => { + assertParity(new NodeQuery()); + }); + + it('kinds: single', () => { + assertParity(new NodeQuery().kinds(['function'])); + }); + + it('kinds: multiple', () => { + assertParity(new NodeQuery().kinds(['function', 'method', 'class'])); + }); + + it('kindFilter: exact', () => { + assertParity(new NodeQuery().kindFilter('class')); + }); + + it('fileFilter: single string', () => { + assertParity(new NodeQuery().fileFilter('core')); + }); + + it('fileFilter: array', () => { + assertParity(new NodeQuery().fileFilter(['foo', 'bar'])); + }); + + it('fileFilter: with LIKE wildcards', () => { + assertParity(new NodeQuery().fileFilter('__test__')); + }); + + it('nameLike: basic', () => { + assertParity(new NodeQuery().nameLike('ba')); + }); + + it('nameLike: with underscore (LIKE wildcard)', () => { + assertParity(new NodeQuery().nameLike('_oo')); + }); + + it('roleFilter: exact', () => { + assertParity(new NodeQuery().roleFilter('core')); + }); + + it('roleFilter: dead prefix match', () => { + assertParity(new NodeQuery().roleFilter('dead')); + }); + + it('excludeTests: true', () => { + assertParity(new NodeQuery().excludeTests(true)); + }); + + it('excludeTests: false', () => { + assertParity(new NodeQuery().excludeTests(false)); + }); + + // ── ORDER BY & LIMIT ──────────────────────────────────────────────── + + it('orderBy: single column', () => { + assertParity(new NodeQuery().orderBy('n.name')); + }); + + it('orderBy: multiple columns with direction', () => { + assertParity(new NodeQuery().orderBy('n.file ASC, n.line DESC')); + }); + + it('limit', () => { + assertParity(new NodeQuery().orderBy('n.name').limit(2)); + }); + + // ── Custom SELECT ─────────────────────────────────────────────────── + + it('custom select', () => { + assertParity(new NodeQuery().select('n.name, n.kind, n.file')); + }); + + // ── JOINs ─────────────────────────────────────────────────────────── + + it('withFanIn', () => { + assertParity( + new NodeQuery().select('n.name, COALESCE(fi.cnt, 0) AS fan_in').withFanIn().orderBy('n.name'), + ); + }); + + it('withFanOut', () => { + assertParity( + new NodeQuery() + .select('n.name, COALESCE(fo.cnt, 0) AS fan_out') + .withFanOut() + .orderBy('n.name'), + ); + }); + + it('withComplexity', () => { + assertParity( + new NodeQuery() + .select('n.name, COALESCE(fc.cognitive, 0) AS cog') + .withComplexity() + .orderBy('n.name'), + ); + }); + + // ── Combined filters ──────────────────────────────────────────────── + + it('kinds + fileFilter + excludeTests', () => { + assertParity( + new NodeQuery().kinds(['function', 'method']).fileFilter('core').excludeTests(true), + ); + }); + + it('kinds + roleFilter + orderBy + limit', () => { + assertParity( + new NodeQuery().kinds(['function']).roleFilter('core').orderBy('n.name DESC').limit(5), + ); + }); + + it('full triage query (all JOINs + filters)', () => { + assertParity( + new NodeQuery() + .select( + `n.id, n.name, n.kind, n.file, n.line, + COALESCE(fi.cnt, 0) AS fan_in, + COALESCE(fc.cognitive, 0) AS cognitive`, + ) + .kinds(['function', 'method', 'class']) + .withFanIn() + .withComplexity() + .excludeTests(true) + .fileFilter('core') + .orderBy('n.file, n.line'), + ); + }); + + it('raw where clause', () => { + assertParity(new NodeQuery().where('n.line > ?', 5)); + }); + + // ── queryGet parity ───────────────────────────────────────────────── + + it('queryGet: first row', () => { + assertGetParity(new NodeQuery().orderBy('n.name').limit(1)); + }); + + it('queryGet: no match', () => { + assertGetParity(new NodeQuery().where("n.name = 'nonexistent'")); + }); +}); From 2b755f03e6b9d3c144511557864d8bba3709ce7e Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Sun, 29 Mar 2026 00:45:00 -0600 Subject: [PATCH 2/4] 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 --- package.json | 14 ++++++ scripts/verify-imports.ts | 44 ++++++++++++++++++- .../graph/builder/stages/build-edges.ts | 8 ++-- .../graph/builder/stages/build-structure.ts | 8 ++-- .../graph/builder/stages/collect-files.ts | 4 +- tsconfig.json | 1 + vitest.config.ts | 3 ++ 7 files changed, 71 insertions(+), 11 deletions(-) diff --git a/package.json b/package.json index 0efd4cec..d0353659 100644 --- a/package.json +++ b/package.json @@ -4,6 +4,20 @@ "description": "Local code graph CLI — parse codebases with tree-sitter, build dependency graphs, query them", "type": "module", "main": "dist/index.js", + "imports": { + "#shared/*": { "@codegraph/source": "./src/shared/*", "default": "./dist/shared/*" }, + "#infrastructure/*": { "@codegraph/source": "./src/infrastructure/*", "default": "./dist/infrastructure/*" }, + "#db/*": { "@codegraph/source": "./src/db/*", "default": "./dist/db/*" }, + "#domain/*": { "@codegraph/source": "./src/domain/*", "default": "./dist/domain/*" }, + "#features/*": { "@codegraph/source": "./src/features/*", "default": "./dist/features/*" }, + "#presentation/*": { "@codegraph/source": "./src/presentation/*", "default": "./dist/presentation/*" }, + "#graph/*": { "@codegraph/source": "./src/graph/*", "default": "./dist/graph/*" }, + "#mcp/*": { "@codegraph/source": "./src/mcp/*", "default": "./dist/mcp/*" }, + "#ast-analysis/*": { "@codegraph/source": "./src/ast-analysis/*", "default": "./dist/ast-analysis/*" }, + "#extractors/*": { "@codegraph/source": "./src/extractors/*", "default": "./dist/extractors/*" }, + "#cli/*": { "@codegraph/source": "./src/cli/*", "default": "./dist/cli/*" }, + "#types": { "@codegraph/source": "./src/types.ts", "default": "./dist/types.js" } + }, "exports": { ".": { "import": "./dist/index.js", diff --git a/scripts/verify-imports.ts b/scripts/verify-imports.ts index c97924d6..eac306b6 100644 --- a/scripts/verify-imports.ts +++ b/scripts/verify-imports.ts @@ -17,7 +17,37 @@ import { fileURLToPath } from 'node:url'; const __filename = fileURLToPath(import.meta.url); const __dirname = dirname(__filename); -const srcDir = resolve(__dirname, '..', 'src'); +const projectRoot = resolve(__dirname, '..'); +const srcDir = resolve(projectRoot, 'src'); + +// ── load package.json subpath imports ─────────────────────────────────── +const pkg = JSON.parse(readFileSync(resolve(projectRoot, 'package.json'), 'utf8')); +const subpathImports: Record = pkg.imports || {}; + +/** Resolve a #-prefixed subpath import to an absolute file path (source condition). */ +function resolveSubpathImport(specifier: string): string | null { + for (const [pattern, mapping] of Object.entries(subpathImports)) { + const target = + typeof mapping === 'string' + ? mapping + : (mapping as Record)['@codegraph/source'] ?? + (mapping as Record).default; + if (!target) continue; + + // Exact match (e.g., "#types") + if (pattern === specifier) return resolve(projectRoot, target); + + // Wildcard match (e.g., "#shared/*") + if (pattern.includes('*')) { + const prefix = pattern.slice(0, pattern.indexOf('*')); + if (specifier.startsWith(prefix)) { + const rest = specifier.slice(prefix.length); + return resolve(projectRoot, target.replace('*', rest)); + } + } + } + return null; +} // ── collect source files ──────────────────────────────────────────────── function walk(dir) { @@ -102,6 +132,18 @@ function extractDynamicImports(filePath) { // ── resolve a specifier to a file on disk ─────────────────────────────── function resolveSpecifier(specifier, fromFile) { + // Handle #-prefixed subpath imports via package.json "imports" field + if (specifier.startsWith('#')) { + const resolved = resolveSubpathImport(specifier); + if (!resolved) return specifier; // no matching pattern — broken + if (existsSync(resolved) && statSync(resolved).isFile()) return null; + if (resolved.endsWith('.js')) { + const tsTarget = resolved.replace(/\.js$/, '.ts'); + if (existsSync(tsTarget) && statSync(tsTarget).isFile()) return null; + } + return specifier; // mapped but file missing — broken + } + // Skip bare specifiers (packages): 'node:*', '@scope/pkg', 'pkg' if (!specifier.startsWith('.') && !specifier.startsWith('/')) return null; diff --git a/src/domain/graph/builder/stages/build-edges.ts b/src/domain/graph/builder/stages/build-edges.ts index 8aafcb91..f5fc1794 100644 --- a/src/domain/graph/builder/stages/build-edges.ts +++ b/src/domain/graph/builder/stages/build-edges.ts @@ -6,9 +6,9 @@ */ import path from 'node:path'; import { performance } from 'node:perf_hooks'; -import { getNodeId } from '../../../../db/index.js'; -import { debug } from '../../../../infrastructure/logger.js'; -import { loadNative } from '../../../../infrastructure/native.js'; +import { getNodeId } from '#db/index.js'; +import { debug } from '#infrastructure/logger.js'; +import { loadNative } from '#infrastructure/native.js'; import type { BetterSqlite3Database, Call, @@ -18,7 +18,7 @@ import type { NativeAddon, NodeRow, TypeMapEntry, -} from '../../../../types.js'; +} from '#types'; import { computeConfidence } from '../../resolve.js'; import type { PipelineContext } from '../context.js'; import { BUILTIN_RECEIVERS, batchInsertEdges } from '../helpers.js'; diff --git a/src/domain/graph/builder/stages/build-structure.ts b/src/domain/graph/builder/stages/build-structure.ts index 646cd17f..ab8ec295 100644 --- a/src/domain/graph/builder/stages/build-structure.ts +++ b/src/domain/graph/builder/stages/build-structure.ts @@ -5,10 +5,10 @@ */ import path from 'node:path'; import { performance } from 'node:perf_hooks'; -import { debug } from '../../../../infrastructure/logger.js'; -import { loadNative } from '../../../../infrastructure/native.js'; -import { normalizePath } from '../../../../shared/constants.js'; -import type { ExtractorOutput } from '../../../../types.js'; +import { debug } from '#infrastructure/logger.js'; +import { loadNative } from '#infrastructure/native.js'; +import { normalizePath } from '#shared/constants.js'; +import type { ExtractorOutput } from '#types'; import type { PipelineContext } from '../context.js'; import { readFileSafe } from '../helpers.js'; diff --git a/src/domain/graph/builder/stages/collect-files.ts b/src/domain/graph/builder/stages/collect-files.ts index aaa658b5..64567ceb 100644 --- a/src/domain/graph/builder/stages/collect-files.ts +++ b/src/domain/graph/builder/stages/collect-files.ts @@ -7,8 +7,8 @@ */ import fs from 'node:fs'; import path from 'node:path'; -import { debug, info } from '../../../../infrastructure/logger.js'; -import { normalizePath } from '../../../../shared/constants.js'; +import { debug, info } from '#infrastructure/logger.js'; +import { normalizePath } from '#shared/constants.js'; import { readJournal } from '../../journal.js'; import type { PipelineContext } from '../context.js'; import { collectFiles as collectFilesUtil } from '../helpers.js'; diff --git a/tsconfig.json b/tsconfig.json index ebe45473..9b4cb5d4 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -8,6 +8,7 @@ /* Modules */ "module": "nodenext", "moduleResolution": "nodenext", + "customConditions": ["@codegraph/source"], "rootDir": "./src", /* Emit */ diff --git a/vitest.config.ts b/vitest.config.ts index 76d7f5e0..0eede49a 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -6,6 +6,9 @@ const supportsStripTypes = major > 22 || (major === 22 && minor >= 6); const stripFlag = major >= 23 ? '--strip-types' : '--experimental-strip-types'; export default defineConfig({ + resolve: { + conditions: ['@codegraph/source'], + }, test: { globals: true, testTimeout: 30000, From 314b1d79a1ba39ddd862f93af9a52a789ec6e2d7 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Sun, 29 Mar 2026 00:53:47 -0600 Subject: [PATCH 3/4] 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. --- src/domain/graph/builder/stages/detect-changes.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/domain/graph/builder/stages/detect-changes.ts b/src/domain/graph/builder/stages/detect-changes.ts index 5636ee03..ad5d83d7 100644 --- a/src/domain/graph/builder/stages/detect-changes.ts +++ b/src/domain/graph/builder/stages/detect-changes.ts @@ -62,10 +62,13 @@ function getChangedFiles( ): ChangeResult { let hasTable = false; try { - const probe = nativeDb - ? nativeDb.queryGet('SELECT 1 FROM file_hashes LIMIT 1', []) - : db.prepare('SELECT 1 FROM file_hashes LIMIT 1').get(); - if (probe) hasTable = true; + if (nativeDb) { + nativeDb.queryGet('SELECT 1 FROM file_hashes LIMIT 1', []); + } else { + db.prepare('SELECT 1 FROM file_hashes LIMIT 1').get(); + } + // Query succeeded → table exists (result may be undefined if table is empty) + hasTable = true; } catch { /* table doesn't exist */ } From d3aea40b9cc97eafc8cbf19bc99776eb4ad84bb6 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Sun, 29 Mar 2026 00:54:09 -0600 Subject: [PATCH 4/4] 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 --- crates/codegraph-core/src/native_db.rs | 16 ++++++++++++++-- src/db/query-builder.ts | 15 +++++++++++++-- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/crates/codegraph-core/src/native_db.rs b/crates/codegraph-core/src/native_db.rs index 7e6f3265..a01cb1b3 100644 --- a/crates/codegraph-core/src/native_db.rs +++ b/crates/codegraph-core/src/native_db.rs @@ -551,9 +551,13 @@ impl NativeDatabase { // ── Phase 6.16: Generic query execution & version validation ──────── - /// Execute a parameterized SELECT and return all rows as JSON objects. + /// Execute a parameterized query and return all rows as JSON objects. /// Each row is a `{ column_name: value, ... }` object. /// Params are positional (`?1, ?2, ...`) and accept string, number, or null. + /// + /// **Note**: Designed for SELECT statements. Passing DML/DDL will not error + /// at the Rust layer but is not an intended use — all current callers pass + /// SELECT-only SQL generated by `NodeQuery.build()`. #[napi] pub fn query_all( &self, @@ -589,7 +593,8 @@ impl NativeDatabase { Ok(result) } - /// Execute a parameterized SELECT and return the first row, or null. + /// Execute a parameterized query and return the first row, or null. + /// See `query_all` for parameter and contract details. #[napi] pub fn query_get( &self, @@ -822,6 +827,11 @@ fn json_to_rusqlite_params( } /// Convert a rusqlite row to a serde_json::Value object. +/// +/// **Contract**: Only Integer, Real, Text, and Null column types are supported. +/// BLOB columns are mapped to `null` because the current codegraph schema has no +/// BLOB columns and the generic query path is not designed for binary data. +/// Cell-level read errors are also mapped to `null` to avoid partial-row failures. fn row_to_json( row: &rusqlite::Row<'_>, col_count: usize, @@ -836,7 +846,9 @@ fn row_to_json( serde_json::Value::String(String::from_utf8_lossy(s).into_owned()) } Ok(ValueRef::Null) => serde_json::Value::Null, + // BLOB: no codegraph schema columns use BLOB; map to null (see contract above) Ok(ValueRef::Blob(_)) => serde_json::Value::Null, + // Cell read error: map to null to avoid partial-row failures Err(_) => serde_json::Value::Null, }; map.insert(col_names[i].clone(), val); diff --git a/src/db/query-builder.ts b/src/db/query-builder.ts index 0fadc1af..4cef1885 100644 --- a/src/db/query-builder.ts +++ b/src/db/query-builder.ts @@ -66,6 +66,17 @@ function validateEdgeKind(edgeKind: string): void { } } +/** Runtime-validate that every param is string, number, or null before sending to nativeDb. */ +function validateNativeParams(params: (string | number)[]): Array { + for (let i = 0; i < params.length; i++) { + const p = params[i]; + if (p !== null && typeof p !== 'string' && typeof p !== 'number') { + throw new DbError(`NodeQuery param[${i}] has unsupported type: ${typeof p}`); + } + } + return params as Array; +} + // ─── LIKE Escaping ────────────────────────────────────────────────── /** Escape LIKE wildcards in a literal string segment. */ @@ -321,7 +332,7 @@ export class NodeQuery { ): TRow[] { const { sql, params } = this.build(); if (nativeDb) { - return nativeDb.queryAll(sql, params as Array) as TRow[]; + return nativeDb.queryAll(sql, validateNativeParams(params)) as TRow[]; } return db.prepare(sql).all(...params) as TRow[]; } @@ -333,7 +344,7 @@ export class NodeQuery { ): TRow | undefined { const { sql, params } = this.build(); if (nativeDb) { - return (nativeDb.queryGet(sql, params as Array) ?? undefined) as + return (nativeDb.queryGet(sql, validateNativeParams(params)) ?? undefined) as | TRow | undefined; }