fix: dogfood fixes 9.1–9.4 — version warning, barrel exports, quieter tsconfig, Set compat#634
fix: dogfood fixes 9.1–9.4 — version warning, barrel exports, quieter tsconfig, Set compat#634carlos-alm wants to merge 3 commits intomainfrom
Conversation
|
Claude finished @carlos-alm's task —— View job PR Review AnalysisTodo List
Phase 1: Root Cause Analysis ✅Problems Being Solved:
Root Cause Assessment: All fixes address real user pain points without masking underlying issues. The approaches are sound. Phase 2: Configuration Change Detection ✅No HIGH-RISK changes detected:
Appropriate warning level change: tsconfig.json parse failure demoted from Phase 3: Test Integrity ✅No test integrity issues:
Phase 4: Code Quality ✅Implementation Quality: 9.1 DB Version Check (src/db/connection.ts:224-241):
9.2 Barrel Export Tracing (src/domain/graph/builder/stages/build-edges.ts:111-112):
9.3 Quieter tsconfig (src/domain/graph/builder/helpers.ts:152):
9.4 Set Compatibility (src/shared/constants.ts):
Potential Issues: None found. All implementations are defensive, well-structured, and maintain backward compatibility where possible. Phase 5: Backlog Compliance ✅Zero-dep: ✅ No new runtime dependencies added Final Assessment |
Greptile SummaryThis PR addresses four dogfooding regressions from the v3.4.0 report: a DB version mismatch warning, barrel file re-export edge tracing, quieter tsconfig parse noise, and backward-compatible Key changes:
Confidence Score: 4/5Safe to merge for normal usage; one P1 transactional gap in the incremental barrel cleanup path can cause silent edge loss on mid-build failures. Three of the four fixes are clean and the previously flagged issues have been addressed. The remaining concern is the non-transactional deletion in resolve-imports.ts: if the pipeline fails after deleteOutgoingEdges.run() commits but before buildEdges inserts the replacement edges, the barrel file's reexports edges are permanently absent from the DB and the file won't re-appear in barrelCandidates on subsequent incremental runs. This is a real but narrow failure window (requires a mid-build crash), and users can recover with --no-incremental. Score reflects convergence from prior review rounds. src/domain/graph/builder/stages/resolve-imports.ts — the deleteOutgoingEdges call at line 61 should ideally be moved inside the buildEdges transaction to be atomic with edge re-creation. Important Files Changed
Sequence DiagramsequenceDiagram
participant Build as buildGraph()
participant RI as resolveImports()
participant DB as SQLite DB
participant BE as buildEdges()
Build->>RI: await resolveImports(ctx)
RI->>DB: SELECT barrel candidates (reexports edges)
loop each barrel candidate not in fileSymbols
RI->>RI: parseFilesAuto(absPath)
RI->>DB: DELETE outgoing edges (outside tx)
RI->>RI: fileSymbols.set / barrelOnlyFiles.add
end
RI-->>Build: ctx updated
Build->>BE: await buildEdges(ctx)
BE->>DB: BEGIN TRANSACTION
loop each file in fileSymbols
alt isBarrelOnly && !imp.reexport
BE->>BE: skip (regular import)
else
BE->>DB: insert reexports / imports edge
end
end
BE->>DB: COMMIT
BE-->>Build: done
note over RI,DB: If buildEdges fails, the DELETE is already committed but INSERT is rolled back
Reviews (2): Last reviewed commit: "fix: address review feedback — version c..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (!_versionWarned) { | ||
| try { | ||
| const row = db | ||
| .prepare<{ value: string }>('SELECT value FROM build_meta WHERE key = ?') | ||
| .get('codegraph_version'); | ||
| const buildVersion = row?.value; | ||
| const currentVersion = getPackageVersion(); | ||
| if (buildVersion && currentVersion && buildVersion !== currentVersion) { | ||
| warn( | ||
| `DB was built with codegraph v${buildVersion}, running v${currentVersion}. Consider: codegraph build --no-incremental`, | ||
| ); | ||
| _versionWarned = true; | ||
| } | ||
| } catch { | ||
| // build_meta table may not exist in older DBs — silently ignore | ||
| } | ||
| } |
There was a problem hiding this comment.
_versionWarned is only set to true when a mismatch is detected, so every subsequent call to openReadonlyOrFail() with matching versions will still execute the prepare(...).get(...) round-trip against build_meta. In normal usage (versions match, multiple queries in one session), this adds a redundant DB query per call. The guard comment says "Warn once per process" which is accurate, but the check also runs repeatedly.
Moving _versionWarned = true outside the mismatch conditional would prevent the repeated query:
| if (!_versionWarned) { | |
| try { | |
| const row = db | |
| .prepare<{ value: string }>('SELECT value FROM build_meta WHERE key = ?') | |
| .get('codegraph_version'); | |
| const buildVersion = row?.value; | |
| const currentVersion = getPackageVersion(); | |
| if (buildVersion && currentVersion && buildVersion !== currentVersion) { | |
| warn( | |
| `DB was built with codegraph v${buildVersion}, running v${currentVersion}. Consider: codegraph build --no-incremental`, | |
| ); | |
| _versionWarned = true; | |
| } | |
| } catch { | |
| // build_meta table may not exist in older DBs — silently ignore | |
| } | |
| } | |
| // Warn once if the DB was built with a different codegraph version | |
| if (!_versionWarned) { | |
| try { | |
| const row = db | |
| .prepare<{ value: string }>('SELECT value FROM build_meta WHERE key = ?') | |
| .get('codegraph_version'); | |
| const buildVersion = row?.value; | |
| const currentVersion = getPackageVersion(); | |
| if (buildVersion && currentVersion && buildVersion !== currentVersion) { | |
| warn( | |
| `DB was built with codegraph v${buildVersion}, running v${currentVersion}. Consider: codegraph build --no-incremental`, | |
| ); | |
| } | |
| } catch { | |
| // build_meta table may not exist in older DBs — silently ignore | |
| } | |
| _versionWarned = true; | |
| } |
There was a problem hiding this comment.
Fixed in b60fbb7. Moved _versionWarned = true outside the mismatch conditional so the flag is set unconditionally after the first check, eliminating the redundant build_meta query on every subsequent call when versions match.
src/shared/constants.ts
Outdated
| ); | ||
|
|
||
| export { SUPPORTED_EXTENSIONS as EXTENSIONS }; | ||
| export const EXTENSIONS: ArrayCompatSet<string> = withArrayCompat(SUPPORTED_EXTENSIONS); |
There was a problem hiding this comment.
withArrayCompat mutates SUPPORTED_EXTENSIONS in place
withArrayCompat(SUPPORTED_EXTENSIONS) casts the original Set to ArrayCompatSet and directly assigns .toArray onto it, mutating the module-level SUPPORTED_EXTENSIONS export from domain/parser.js. Any code that imports SUPPORTED_EXTENSIONS directly will now see the toArray property on it as a side effect of loading constants.ts. While adding a property to a Set won't interfere with .has() or iteration, it's an unintentional mutation of a sibling module's export.
Wrapping in a fresh Set keeps the original untouched:
| export const EXTENSIONS: ArrayCompatSet<string> = withArrayCompat(SUPPORTED_EXTENSIONS); | |
| export const EXTENSIONS: ArrayCompatSet<string> = withArrayCompat(new Set(SUPPORTED_EXTENSIONS)); |
There was a problem hiding this comment.
Fixed in b60fbb7. EXTENSIONS is now wrapped in new Set(SUPPORTED_EXTENSIONS) so withArrayCompat mutates the copy rather than the original SUPPORTED_EXTENSIONS export from the parser module.
… compat 9.1 — Warn on graph load when DB was built with a different codegraph version. The check runs once per process in openReadonlyOrFail() and suggests `build --no-incremental`. 9.2 — Barrel-only files now emit reexport edges during build. Previously the entire file was skipped in buildImportEdges; now only non-reexport imports are skipped, so `codegraph exports` can follow re-export chains. 9.3 — Demote "Failed to parse tsconfig.json" from warn to debug level so it no longer clutters every build output. 9.4 — Document EXTENSIONS/IGNORE_DIRS Array→Set breaking change in CHANGELOG. Add .toArray() convenience method and export ArrayCompatSet type for consumers migrating from the pre-3.4 array API.
…ge duplication (#634) - Move _versionWarned flag outside mismatch conditional to avoid redundant build_meta queries when versions match. - Wrap SUPPORTED_EXTENSIONS in new Set() to avoid mutating the sibling module's export. - Delete outgoing edges for barrel-only files before re-adding them to fileSymbols during incremental builds, preventing duplicate reexport edges.
carlos-alm
force-pushed
the
feat/dogfood-fixes-9.1-9.4
branch
from
62852b8 to
b60fbb7
Compare
|
Both P2 items addressed in b60fbb7:
|
| const deleteOutgoingEdges = db.prepare( | ||
| 'DELETE FROM edges WHERE source_id IN (SELECT id FROM nodes WHERE file = ?)', | ||
| ); | ||
| for (const { file: relPath } of barrelCandidates) { | ||
| if (fileSymbols.has(relPath)) continue; | ||
| const absPath = path.join(rootDir, relPath); | ||
| try { | ||
| const symbols = await parseFilesAuto([absPath], rootDir, engineOpts); | ||
| const fileSym = symbols.get(relPath); | ||
| if (fileSym) { | ||
| deleteOutgoingEdges.run(relPath); | ||
| fileSymbols.set(relPath, fileSym); |
There was a problem hiding this comment.
deleteOutgoingEdges.run(relPath) commits immediately to the DB (outside any transaction), but the corresponding reexports edges aren't re-created until buildEdges runs its own db.transaction() later. If the process crashes or buildEdges throws between those two points, the barrel file's outgoing edges are gone from the DB — but because the reexports rows no longer exist, the file won't appear in barrelCandidates on the next incremental run either. The file silently disappears from the barrel graph until the user explicitly runs --no-incremental.
The cleanest fix is to push the deletion into the buildEdges transaction so delete and insert are atomic:
// In buildImportEdges, before inserting reexport edges for a barrel-only file:
if (isBarrelOnly) {
db.prepare(
'DELETE FROM edges WHERE source_id IN (SELECT id FROM nodes WHERE file = ?)',
).run(relPath);
}If keeping the deletion in resolveImports is preferred, wrapping the loop body in a savepoint or a db.transaction() call would achieve the same atomicity guarantee.
1 participant
Summary
Addresses four dogfooding issues from the v3.4.0 report:
openReadonlyOrFail()now checksbuild_meta.codegraph_versionagainst the running version and warns (once per process) with a suggestion tobuild --no-incrementalbuildImportEdges, so noreexportsedges were created. Now only non-reexport imports are skipped, lettingcodegraph exportsfollow re-export chains through barrel fileswarntodebugso it only appears with--verbose.toArray()convenience method and exportedArrayCompatSettype for consumers migrating from the pre-3.4 array APITest plan
codegraph build && codegraph exports src/index.ts— should show re-exported symbols instead of "No exported symbols found"build_meta.codegraph_versionin DB to a different value, run any query — should see version mismatch warning oncecodegraph buildwithout--verbose— no tsconfig.json warningimport { EXTENSIONS } from '@optave/codegraph'; EXTENSIONS.toArray()— returns string array