fix: dogfood fixes 9.1–9.4 — version warning, barrel exports, quieter tsconfig, Set compat#634
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 resolves four concrete dogfooding issues found during the v3.4.0 release cycle, covering a DB version mismatch warning, barrel-file re-export edge tracing, a noisy tsconfig parse warning, and a backwards-compat shim for the
Confidence Score: 5/5Safe to merge — all three previously-flagged critical concerns are confirmed resolved and no new issues were found. The redundant build_meta query per call (now _versionWarned is set unconditionally), the unintentional mutation of SUPPORTED_EXTENSIONS (now wraps a fresh Set), and the non-atomic barrel edge deletion (now inside the enclosing transaction) are all fixed in this head commit. The four targeted dogfooding fixes are clean, narrowly scoped, and well-guarded with error handling and test-reset hooks. No new bugs or regressions are introduced. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant CLI
participant openReadonlyOrFail
participant BuildMeta as build_meta table
participant buildEdges
CLI->>openReadonlyOrFail: first query call
openReadonlyOrFail->>BuildMeta: SELECT value WHERE key is codegraph_version
BuildMeta-->>openReadonlyOrFail: stored build version
openReadonlyOrFail->>openReadonlyOrFail: compare vs package.json version
alt versions differ
openReadonlyOrFail->>CLI: warn once, suggest no-incremental rebuild
end
openReadonlyOrFail->>openReadonlyOrFail: set warned flag = true
openReadonlyOrFail-->>CLI: readonly db handle
CLI->>openReadonlyOrFail: subsequent query calls
openReadonlyOrFail->>openReadonlyOrFail: warned flag is true, skip check
openReadonlyOrFail-->>CLI: readonly db handle
Note over buildEdges: Incremental build barrel edge rebuild (atomic)
buildEdges->>buildEdges: begin transaction
buildEdges->>buildEdges: DELETE all edges for barrel-only files
buildEdges->>buildEdges: buildImportEdges (reexports only per barrel)
buildEdges->>buildEdges: buildCallEdges
buildEdges->>buildEdges: batchInsertEdges
buildEdges->>buildEdges: commit or rollback atomically
Reviews (3): Last reviewed commit: "fix: move barrel edge deletion into buil..." | 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.
There was a problem hiding this comment.
Fixed in 74453fd. Moved the deleteOutgoingEdges call from resolveImports (where it committed immediately outside any transaction) into the buildEdges transaction in build-edges.ts. The deletion now runs inside the same db.transaction() that re-creates the reexport edges, so if buildEdges throws or the process crashes, both the delete and insert are rolled back atomically — no silent edge loss.
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