Skip to content

Commit 784bdf7

Browse files
committed
refactor: migrate raw SQL into repository pattern (Phase 3.3)
Split src/db/repository.js into src/db/repository/ directory with 10 domain files: nodes, edges, build-stmts, complexity, cfg, dataflow, cochange, embeddings, graph-read, and barrel index. Extracted ~120 inline db.prepare() calls from 14 consumer modules into shared repository functions, eliminating duplication of common patterns like getNodeId (5 files), bulkNodeIdsByFile (3 files), callee/caller joins (6+ call sites), and file purge cascades. Net reduction: -437 lines across the codebase. All 1573 tests pass. Impact: 73 functions changed, 112 affected
1 parent 6d0f34e commit 784bdf7

26 files changed

Lines changed: 1078 additions & 695 deletions

docs/roadmap/ROADMAP.md

Lines changed: 38 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -562,11 +562,11 @@ Plus updated enums on existing tools (edge_kinds, symbol kinds).
562562
563563
**Context:** Phases 2.5 and 2.7 added 38 modules and grew the codebase from 5K to 26,277 lines without introducing shared abstractions. The dual-function anti-pattern was replicated across 19 modules. Three independent AST analysis engines (complexity, CFG, dataflow) totaling 4,801 lines share the same fundamental pattern but no infrastructure. Raw SQL is scattered across 25+ modules touching 13 tables. The priority ordering has been revised based on actual growth patterns -- the new #1 priority is the unified AST analysis framework.
564564

565-
### 3.1 -- Unified AST Analysis Framework ★ Critical 🔄
565+
### 3.1 -- Unified AST Analysis Framework ★ Critical
566566

567567
Unify the independent AST analysis engines (complexity, CFG, dataflow) plus AST node storage into a shared visitor framework. These four modules independently implement the same pattern: per-language rules map → AST walk → collect data → write to DB → query → format.
568568

569-
**Completed:** Phases 1-7 implemented a pluggable visitor framework with a shared DFS walker (`walkWithVisitors`), an analysis engine orchestrator (`runAnalyses`), and three visitors (complexity, dataflow, AST-store) that share a single tree traversal per file. `builder.js` collapsed from 4 sequential `buildXxx` blocks into one `runAnalyses` call.
569+
**Completed:** All 4 analyses (complexity, CFG, dataflow, AST-store) now run in a single DFS walk via `walkWithVisitors`. The CFG visitor rewrite ([#392](https://github.com/optave/codegraph/pull/392)) eliminated the Mode A/B split, replaced the 813-line `buildFunctionCFG` with a node-level visitor, and derives cyclomatic complexity directly from CFG structure (`E - N + 2`). `cfg.js` reduced from 1,242 → 518 lines.
570570

571571
```
572572
src/
@@ -577,6 +577,7 @@ src/
577577
visitor-utils.js # Shared helpers (functionName, extractParams, etc.)
578578
visitors/
579579
complexity-visitor.js # Cognitive/cyclomatic/nesting + Halstead
580+
cfg-visitor.js # Basic-block + edge construction via DFS hooks
580581
ast-store-visitor.js # new/throw/await/string/regex extraction
581582
dataflow-visitor.js # Scope stack + define-use chains
582583
shared.js # findFunctionNode, rule factories, ext mapping
@@ -591,76 +592,71 @@ src/
591592
-`builder.js` → single `runAnalyses` call replaces 4 sequential blocks + WASM pre-parse
592593
- ✅ Extracted pure computations to `metrics.js` (Halstead derived math, LOC, MI)
593594
- ✅ Extracted shared helpers to `visitor-utils.js` (from dataflow.js)
594-
- 🔲 **CFG visitor rewrite** (see below)
595-
596-
**Remaining: CFG visitor rewrite.** `buildFunctionCFG` (813 lines) uses a statement-level traversal (`getStatements` + `processStatement` with `loopStack`, `labelMap`, `blockIndex`) that is fundamentally incompatible with the node-level DFS used by `walkWithVisitors`. This is why the engine runs CFG as a separate Mode B pass — the only analysis that can't participate in the shared single-DFS walk.
597-
598-
Rewrite the CFG algorithm as a node-level visitor that builds basic blocks and edges incrementally via `enterNode`/`exitNode` hooks, tracking block boundaries at branch/loop/return nodes the same way the complexity visitor tracks nesting. This eliminates the last redundant tree traversal during build and lets CFG share the exact same DFS pass as complexity, dataflow, and AST extraction. The statement-level `getStatements` helper and per-language `CFG_RULES.statementTypes` can be replaced by detecting block-terminating node types in `enterNode`. Also simplifies `engine.js` by removing the Mode A/B split and WASM pre-parse special-casing for CFG.
599-
600-
**Remaining: Derive cyclomatic complexity from CFG.** Once CFG participates in the unified walk, cyclomatic complexity can be derived directly from CFG edge/block counts (`edges - nodes + 2`) rather than independently computed by the complexity visitor. This creates a single source of truth for control flow metrics and eliminates redundant computation. Can also be done as a simpler SQL-only approach against stored `cfg_blocks`/`cfg_edges` tables (see backlog ID 45).
595+
- ✅ CFG visitor rewrite — node-level DFS visitor replaces statement-level `buildFunctionCFG`, Mode A/B split eliminated ([#392](https://github.com/optave/codegraph/pull/392))
596+
- ✅ Cyclomatic complexity derived from CFG (`E - N + 2`) — single source of truth for control flow metrics ([#392](https://github.com/optave/codegraph/pull/392))
601597

602598
**Affected files:** `src/complexity.js`, `src/cfg.js`, `src/dataflow.js`, `src/ast.js` → split into `src/ast-analysis/`
603599

604-
### 3.2 -- Command/Query Separation ★ Critical 🔄
605-
606-
> **v3.1.1 progress:** CLI display wrappers for all query commands extracted to `queries-cli.js` (866 lines, 15 functions). Shared `result-formatter.js` (`outputResult()` for JSON/NDJSON) and `test-filter.js` created. `queries.js` reduced from 3,395 → 2,490 lines — all `*Data()` functions remain, CLI formatting fully separated ([#373](https://github.com/optave/codegraph/pull/373)).
607-
608-
-`queries.js` CLI wrappers → `queries-cli.js` (15 functions)
609-
- ✅ Shared `result-formatter.js` (`outputResult` for JSON/NDJSON dispatch)
610-
- ✅ Shared `test-filter.js` (`isTestFile` predicate)
611-
- 🔲 Extract CLI wrappers from remaining modules (audit, batch, check, cochange, communities, complexity, cfg, dataflow, flow, manifesto, owners, structure, triage, branch-compare)
612-
- 🔲 Introduce `CommandRunner` shared lifecycle
613-
- 🔲 Per-command `src/commands/` directory structure
600+
### 3.2 -- Command/Query Separation ★ Critical ✅
614601

615-
Eliminate the `*Data()` / `*()` dual-function pattern replicated across 19 modules. Every analysis module (queries, audit, batch, check, cochange, communities, complexity, cfg, dataflow, ast, flow, manifesto, owners, structure, triage, branch-compare, viewer) currently implements both data extraction AND CLI formatting.
616-
617-
Introduce a shared `CommandRunner` that handles the open-DB -> validate -> execute -> format -> paginate -> output lifecycle. Each command only implements unique query + analysis logic. Formatting is always separate and pluggable (CLI text, JSON, NDJSON, Mermaid, DOT).
602+
CLI display wrappers extracted from all 19 analysis modules into dedicated `src/commands/` files. Shared infrastructure (`result-formatter.js`, `test-filter.js`) moved to `src/infrastructure/`. `*Data()` functions remain in original modules — MCP dynamic imports unchanged. ~1,059 lines of CLI formatting code separated from analysis logic ([#373](https://github.com/optave/codegraph/pull/373), [#393](https://github.com/optave/codegraph/pull/393)).
618603

619604
```
620605
src/
621-
commands/ # One file per command
622-
query.js # { execute(args, ctx) -> data, format(data, opts) -> string }
623-
impact.js
624-
audit.js
625-
check.js
626-
cfg.js
627-
dataflow.js
628-
...
606+
commands/ # One file per command (16 files)
607+
audit.js, batch.js, cfg.js, check.js, cochange.js, communities.js,
608+
complexity.js, dataflow.js, flow.js, branch-compare.js, manifesto.js,
609+
owners.js, sequence.js, structure.js, triage.js, query.js (barrel re-export)
629610
630611
infrastructure/
631-
command-runner.js # Shared lifecycle
632-
result-formatter.js # Shared formatting: table, JSON, NDJSON, Mermaid, DOT
612+
result-formatter.js # Shared formatting: JSON, NDJSON dispatch
633613
test-filter.js # Shared --no-tests / isTestFile logic
634614
```
635615

616+
-`queries.js` CLI wrappers → `queries-cli.js` (15 functions)
617+
- ✅ Shared `result-formatter.js` (`outputResult` for JSON/NDJSON dispatch)
618+
- ✅ Shared `test-filter.js` (`isTestFile` predicate)
619+
- ✅ CLI wrappers extracted from remaining 15 modules into `src/commands/` ([#393](https://github.com/optave/codegraph/pull/393))
620+
- ✅ Per-command `src/commands/` directory structure ([#393](https://github.com/optave/codegraph/pull/393))
621+
-`src/infrastructure/` directory for shared utilities ([#393](https://github.com/optave/codegraph/pull/393))
622+
- ⏭️ `CommandRunner` shared lifecycle — deferred (command files vary too much for a single pattern today)
623+
636624
**Affected files:** All 19 modules with dual-function pattern, `src/cli.js`, `src/mcp.js`
637625

638-
### 3.3 -- Repository Pattern for Data Access ★ Critical 🔄
626+
### 3.3 -- Repository Pattern for Data Access ★ Critical
639627

640628
> **v3.1.1 progress:** `src/db/` directory created with `repository.js` (134 lines), `query-builder.js` (280 lines), and `migrations.js` (312 lines). All db usage across the codebase wrapped in try/finally for reliable `db.close()` ([#371](https://github.com/optave/codegraph/pull/371), [#384](https://github.com/optave/codegraph/pull/384), [#383](https://github.com/optave/codegraph/pull/383)).
629+
>
630+
> **v3.1.2 progress:** `repository.js` split into `src/db/repository/` directory with 10 domain files (nodes, edges, build-stmts, complexity, cfg, dataflow, cochange, embeddings, graph-read, barrel). Raw SQL migrated from 14 src/ modules into repository layer. `connection.js` already complete (89 lines handling open/close/WAL/pragma/locks/readonly).
641631
642632
-`src/db/` directory structure created
643-
-`repository.js`initial Repository class
633+
-`repository/`domain-split repository (nodes, edges, build-stmts, complexity, cfg, dataflow, cochange, embeddings, graph-read)
644634
-`query-builder.js` — lightweight SQL builder (280 lines)
645635
-`migrations.js` — schema migrations extracted (312 lines)
636+
-`connection.js` — connection setup (open, WAL mode, pragma tuning, readonly, locks)
646637
- ✅ All db usage wrapped in try/finally for reliable `db.close()`
647-
- 🔲 Migrate remaining raw SQL from 25+ modules into Repository
648-
- 🔲 `connection.js` — extract connection setup (open, WAL mode, pragma tuning)
649-
650-
Consolidate all SQL into a single `Repository` class. Currently SQL is scattered across 25+ modules that each independently open the DB and write raw SQL inline across 13 tables.
638+
- ✅ Migrate remaining raw SQL from 14 modules into Repository
651639

652640
```
653641
src/
654642
db/
655643
connection.js # Open, WAL mode, pragma tuning
656644
migrations.js # Schema versions (currently 13 migrations)
657-
repository.js # ALL data access methods across all 13 tables
658645
query-builder.js # Lightweight SQL builder for common filtered queries
646+
repository/
647+
index.js # Barrel re-export
648+
nodes.js # Node lookups: getNodeId, findFileNodes, bulkNodeIdsByFile, etc.
649+
edges.js # Edge queries: findCallees, findCallers, import/hierarchy edges
650+
build-stmts.js # Cascade purge: purgeFileData, purgeFilesData
651+
complexity.js # function_complexity table reads
652+
cfg.js # cfg_blocks/cfg_edges reads + deletes
653+
dataflow.js # dataflow table checks
654+
cochange.js # co_changes/co_change_meta reads
655+
embeddings.js # embeddings/embedding_meta reads
656+
graph-read.js # Cross-table reads for export/communities
659657
```
660658

661-
Add a query builder for the common pattern "find nodes WHERE kind IN (...) AND file NOT LIKE '%test%' ORDER BY ... LIMIT ? OFFSET ?". Not an ORM -- a thin SQL builder that eliminates string construction across 25 modules.
662-
663-
**Affected files:** `src/db.js` -> split into `src/db/`, SQL extracted from all modules
659+
**Affected files:** `src/db.js` barrel updated, raw SQL extracted from `queries.js`, `builder.js`, `watcher.js`, `structure.js`, `complexity.js`, `cfg.js`, `dataflow.js`, `ast.js`, `ast-analysis/engine.js`, `embedder.js`, `sequence.js`, `communities.js`
664660

665661
### 3.4 -- Decompose queries.js (3,395 Lines) 🔄
666662

src/ast-analysis/engine.js

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
import path from 'node:path';
2121
import { performance } from 'node:perf_hooks';
22+
import { bulkNodeIdsByFile } from '../db.js';
2223
import { debug } from '../logger.js';
2324
import { computeLOCMetrics, computeMaintainabilityIndex } from './metrics.js';
2425
import {
@@ -115,11 +116,6 @@ export async function runAnalyses(db, fileSymbols, rootDir, opts, engineOpts) {
115116
// engine output). This eliminates ~3 redundant tree traversals per file.
116117
const t0walk = performance.now();
117118

118-
// Pre-load node ID map for AST parent resolution
119-
const bulkGetNodeIds = doAst
120-
? db.prepare('SELECT id, name, kind, line FROM nodes WHERE file = ?')
121-
: null;
122-
123119
for (const [relPath, symbols] of fileSymbols) {
124120
if (!symbols._tree) continue; // No WASM tree — native path handles it
125121

@@ -140,10 +136,8 @@ export async function runAnalyses(db, fileSymbols, rootDir, opts, engineOpts) {
140136
let astVisitor = null;
141137
if (doAst && astTypeMap && WALK_EXTENSIONS.has(ext) && !symbols.astNodes?.length) {
142138
const nodeIdMap = new Map();
143-
if (bulkGetNodeIds) {
144-
for (const row of bulkGetNodeIds.all(relPath)) {
145-
nodeIdMap.set(`${row.name}|${row.kind}|${row.line}`, row.id);
146-
}
139+
for (const row of bulkNodeIdsByFile(db, relPath)) {
140+
nodeIdMap.set(`${row.name}|${row.kind}|${row.line}`, row.id);
147141
}
148142
astVisitor = createAstStoreVisitor(astTypeMap, defs, relPath, nodeIdMap);
149143
visitors.push(astVisitor);

src/ast.js

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { AST_TYPE_MAPS } from './ast-analysis/rules/index.js';
1111
import { buildExtensionSet } from './ast-analysis/shared.js';
1212
import { walkWithVisitors } from './ast-analysis/visitor.js';
1313
import { createAstStoreVisitor } from './ast-analysis/visitors/ast-store-visitor.js';
14-
import { openReadonlyOrFail } from './db.js';
14+
import { bulkNodeIdsByFile, openReadonlyOrFail } from './db.js';
1515
import { debug } from './logger.js';
1616
import { paginateResult } from './paginate.js';
1717

@@ -77,9 +77,6 @@ export async function buildAstNodes(db, fileSymbols, _rootDir, _engineOpts) {
7777
return;
7878
}
7979

80-
// Bulk-fetch all node IDs per file (replaces per-def getNodeId calls)
81-
const bulkGetNodeIds = db.prepare('SELECT id, name, kind, line FROM nodes WHERE file = ?');
82-
8380
const tx = db.transaction((rows) => {
8481
for (const r of rows) {
8582
insertStmt.run(r.file, r.line, r.kind, r.name, r.text, r.receiver, r.parentNodeId);
@@ -93,7 +90,7 @@ export async function buildAstNodes(db, fileSymbols, _rootDir, _engineOpts) {
9390

9491
// Pre-load all node IDs for this file into a map (read-only, fast)
9592
const nodeIdMap = new Map();
96-
for (const row of bulkGetNodeIds.all(relPath)) {
93+
for (const row of bulkNodeIdsByFile(db, relPath)) {
9794
nodeIdMap.set(`${row.name}|${row.kind}|${row.line}`, row.id);
9895
}
9996

0 commit comments

Comments
 (0)