mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-11-14 16:52:09 +08:00
fix(sync): a skipped file that now indexes re-links its importers
A file over the source-size limit, or one whose parse failed, is tracked with no nodes and its error recorded, so the imports of it fail. When an edit lets it be indexed, sync stored it as a modified file, and only an added file is looked up by what it can be imported as: an unchanged importer's import of the file itself stayed failed, and so did what a barrel passes on from it, until a full re-index. Named imports and calls bound, since they wait by name. A re-indexed file whose stored row had no nodes and recorded errors now joins addedFilePaths once the store gives it nodes, so the added-file retries and the importer and barrel walks run for it. It still counts as modified. Deciding after the store leaves a file that changes but stays over the limit, or still fails, alone, so it does not re-run those retries on every sync. Review changes: a test for the barrel shape, the one behind most of the real-repo divergence below, and the CHANGELOG entry beside the one for files saved with no symbols. Validation: 5 tests; on main's source 4 fail and the stays-over-the-limit control passes. 9 fixture edits, each compared with a fresh index of the same tree, in a later process and in the same process: main diverged in 7 of each, this in 1, a barrel growing over the limit, where only the edges' resolvedBy differs (same targets), as the PR noted. excalidraw with packages/math/src/point.ts over the limit at index time and put back: main was 1,506 edges short of a fresh index, identical here over nodes, edges, refs and files. Original PR #3022 by @inth3shadows. Fixes #3016 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
committed by
Colby Mchenry
co-authored by
Claude Opus 5.5
parent
2d98c1b69a
commit
4b0f72b7a7
@@ -66,6 +66,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
|
|||||||
- VB.NET designer code such as `New System.Drawing.Point(…)` or `Me.Size = New System.Drawing.Size(…)` is no longer linked to a project type or member that only shares the name, and a class nested inside another is matched by its bare name only from inside that class or one that inherits it.
|
- VB.NET designer code such as `New System.Drawing.Point(…)` or `Me.Size = New System.Drawing.Size(…)` is no longer linked to a project type or member that only shares the name, and a class nested inside another is matched by its bare name only from inside that class or one that inherits it.
|
||||||
- Upgrading CodeGraph while an agent session is open no longer leaves the old version's background server in charge of your project: the first session started from the new install stops it and starts a current one in its place, even while sessions opened before the upgrade are still running. That old server could no longer load the language parsers the upgrade removed, so it saved every file it re-indexed with no symbols, while new sessions could only read the index beside it without keeping it up to date. A background server from a newer install is never stopped, and sessions opened before the upgrade keep the old version until you restart them. Thanks @lipchey for the report. (#2335)
|
- Upgrading CodeGraph while an agent session is open no longer leaves the old version's background server in charge of your project: the first session started from the new install stops it and starts a current one in its place, even while sessions opened before the upgrade are still running. That old server could no longer load the language parsers the upgrade removed, so it saved every file it re-indexed with no symbols, while new sessions could only read the index beside it without keeping it up to date. A background server from a newer install is never stopped, and sessions opened before the upgrade keep the old version until you restart them. Thanks @lipchey for the report. (#2335)
|
||||||
- A file is no longer saved with no symbols when its language parser can't be loaded, which is what happened to every file a background server re-indexed after an upgrade removed its install: the file keeps what it had and is indexed again once the parser loads, and files an earlier version emptied this way are re-indexed by the next sync. A background server also exits on its own once its install is upgraded or removed, so the next session starts one from the current install. Thanks @lipchey for the report. (#2335)
|
- A file is no longer saved with no symbols when its language parser can't be loaded, which is what happened to every file a background server re-indexed after an upgrade removed its install: the file keeps what it had and is indexed again once the parser loads, and files an earlier version emptied this way are re-indexed by the next sync. A background server also exits on its own once its install is upgraded or removed, so the next session starts one from the current install. Thanks @lipchey for the report. (#2335)
|
||||||
|
- `codegraph sync` and the file watcher now link the files importing a file that was skipped, because it was over the size limit or failed to parse, once an edit lets it be indexed, as a full index does: the import of the file itself, and the names a barrel passes on from it. Before, those imports stayed unlinked until the importing files changed or the project was re-indexed. Thanks @inth3shadows for the report and the pull request. (#3016)
|
||||||
- `codegraph index` no longer stops with advice to run `codegraph daemon stop`, a command that doesn't exist, when the project's background-server lock names a process that isn't CodeGraph, such as a lock left by a crash whose process number the system has since given to another program: it now clears that stale lock, leaving the process alone, and rebuilds, and `codegraph unlock` clears it too. When a CodeGraph process that doesn't answer holds the lock, such as an older background server, the message names that process and the command that stops it, and the message about another server already serving a project now points to `codegraph daemon`. Thanks @inth3shadows for the report and @danusha2345 for the first fix. (#2770)
|
- `codegraph index` no longer stops with advice to run `codegraph daemon stop`, a command that doesn't exist, when the project's background-server lock names a process that isn't CodeGraph, such as a lock left by a crash whose process number the system has since given to another program: it now clears that stale lock, leaving the process alone, and rebuilds, and `codegraph unlock` clears it too. When a CodeGraph process that doesn't answer holds the lock, such as an older background server, the message names that process and the command that stops it, and the message about another server already serving a project now points to `codegraph daemon`. Thanks @inth3shadows for the report and @danusha2345 for the first fix. (#2770)
|
||||||
- A program that uses CodeGraph as a library and indexes chosen files with `indexFiles()` now keeps other CodeGraph processes, like a `codegraph sync` run by a git hook, out of the index until those files are stored. Before, it let go of the write lock that keeps two writers apart as soon as indexing started, so another process could write to the index at the same time.
|
- A program that uses CodeGraph as a library and indexes chosen files with `indexFiles()` now keeps other CodeGraph processes, like a `codegraph sync` run by a git hook, out of the index until those files are stored. Before, it let go of the write lock that keeps two writers apart as soon as indexing started, so another process could write to the index at the same time.
|
||||||
- When several CodeGraph processes start at the same moment after one of them crashed, like the MCP server, a git hook's `codegraph sync` and `codegraph index`, only one of them now takes over the write lock the crashed process left behind. Before, two of them could both take it, the second one deleting the first one's lock as it did, and both went on to write to the index at once. A write lock held by another user's CodeGraph, or on Windows by one running as administrator, is no longer mistaken for a crashed one's and taken over either. Thanks @inth3shadows for the report and @vedantb7 for the first fix. (#2979)
|
- When several CodeGraph processes start at the same moment after one of them crashed, like the MCP server, a git hook's `codegraph sync` and `codegraph index`, only one of them now takes over the write lock the crashed process left behind. Before, two of them could both take it, the second one deleting the first one's lock as it did, and both went on to write to the index at once. A write lock held by another user's CodeGraph, or on Windows by one running as administrator, is no longer mistaken for a crashed one's and taken over either. Thanks @inth3shadows for the report and @vedantb7 for the first fix. (#2979)
|
||||||
|
|||||||
@@ -0,0 +1,148 @@
|
|||||||
|
/**
|
||||||
|
* A file that shrinks back under the source-size limit links its importers as
|
||||||
|
* a fresh index does.
|
||||||
|
*
|
||||||
|
* Over the limit, a file is tracked with no nodes and a `size_exceeded` error,
|
||||||
|
* so `import { f1 } from './big'` has no file to land on and its refs fail.
|
||||||
|
* When an edit brings it under the limit, sync indexes it — but as a modified
|
||||||
|
* file, and only an ADDED file is looked up by what it can be imported as, so
|
||||||
|
* the failed `./big` import of an unchanged importer stayed failed: the call
|
||||||
|
* and the named import bound (they wait by name), the file-level import edge
|
||||||
|
* a full index makes did not.
|
||||||
|
*/
|
||||||
|
|
||||||
|
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
|
||||||
|
import * as fs from 'fs';
|
||||||
|
import * as os from 'os';
|
||||||
|
import * as path from 'path';
|
||||||
|
import CodeGraph from '../src/index';
|
||||||
|
import { MAX_SOURCE_FILE_SIZE_BYTES } from '../src/file-limits';
|
||||||
|
|
||||||
|
/** Every node and edge by natural key, so two indexes of the same files compare directly. */
|
||||||
|
function graphOf(cg: CodeGraph): { nodes: string[]; edges: string[] } {
|
||||||
|
const keys = new Map(
|
||||||
|
cg.getFiles()
|
||||||
|
.flatMap((file) => cg.getNodesInFile(file.path))
|
||||||
|
.map((n) => [n.id, `${n.kind} ${n.filePath}:${n.qualifiedName}:${n.startLine}`] as const)
|
||||||
|
);
|
||||||
|
const edges = cg.getOutgoingEdgesFrom([...keys.keys()])
|
||||||
|
.map((e) => `${e.kind} ${keys.get(e.source)} -> ${keys.get(e.target) ?? e.target}`);
|
||||||
|
return { nodes: [...keys.values()].sort(), edges: edges.sort() };
|
||||||
|
}
|
||||||
|
|
||||||
|
const BIG = (padding: number) =>
|
||||||
|
`export function f1(): number { return 1; }\n// ${'x'.repeat(padding)}\n`;
|
||||||
|
|
||||||
|
describe('sync links the importers of a file that shrinks under the size limit', () => {
|
||||||
|
let dir: string;
|
||||||
|
let cg: CodeGraph;
|
||||||
|
const write = (rel: string, content: string) => fs.writeFileSync(path.join(dir, rel), content);
|
||||||
|
const importsOfUser = (): string[] => {
|
||||||
|
const ids = cg.getNodesInFile('user.ts').map((n) => n.id);
|
||||||
|
return cg.getOutgoingEdgesFrom(ids)
|
||||||
|
.filter((e) => e.kind === 'imports')
|
||||||
|
.map((e) => `${cg.getNode(e.target)?.kind}:${cg.getNode(e.target)?.name}`)
|
||||||
|
.sort();
|
||||||
|
};
|
||||||
|
const matchesFullIndex = async () => {
|
||||||
|
const synced = graphOf(cg);
|
||||||
|
cg.close();
|
||||||
|
cg = await CodeGraph.recreate(dir);
|
||||||
|
await cg.indexAll();
|
||||||
|
expect(synced).toEqual(graphOf(cg));
|
||||||
|
};
|
||||||
|
|
||||||
|
beforeEach(async () => {
|
||||||
|
dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-sizecap-'));
|
||||||
|
write('big.ts', BIG(MAX_SOURCE_FILE_SIZE_BYTES + 1000));
|
||||||
|
write('user.ts', "import { f1 } from './big';\nexport const x = f1();\n");
|
||||||
|
cg = CodeGraph.initSync(dir);
|
||||||
|
await cg.indexAll();
|
||||||
|
// Over the limit: nothing in big.ts to import.
|
||||||
|
expect(importsOfUser()).toEqual([]);
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
cg?.destroy();
|
||||||
|
fs.rmSync(dir, { recursive: true, force: true });
|
||||||
|
});
|
||||||
|
|
||||||
|
it('including the import of the file itself', async () => {
|
||||||
|
write('big.ts', BIG(10));
|
||||||
|
await cg.sync();
|
||||||
|
expect(importsOfUser()).toEqual(['file:big.ts', 'function:f1']);
|
||||||
|
await matchesFullIndex();
|
||||||
|
}, 60_000);
|
||||||
|
|
||||||
|
it('and again after it grows over the limit and back', async () => {
|
||||||
|
write('big.ts', BIG(10));
|
||||||
|
await cg.sync();
|
||||||
|
write('big.ts', BIG(MAX_SOURCE_FILE_SIZE_BYTES + 2000));
|
||||||
|
await cg.sync();
|
||||||
|
expect(importsOfUser()).toEqual([]);
|
||||||
|
write('big.ts', BIG(20));
|
||||||
|
await cg.sync();
|
||||||
|
expect(importsOfUser()).toEqual(['file:big.ts', 'function:f1']);
|
||||||
|
await matchesFullIndex();
|
||||||
|
}, 60_000);
|
||||||
|
|
||||||
|
it('and the same for a file stored after its parse failed', async () => {
|
||||||
|
// Recorded as a parse failure instead of an oversize skip: still no
|
||||||
|
// nodes, so the same importers wait on it.
|
||||||
|
cg.close();
|
||||||
|
const { DatabaseSync } = require('node:sqlite') as typeof import('node:sqlite');
|
||||||
|
const db = new DatabaseSync(path.join(dir, '.codegraph', 'codegraph.db'));
|
||||||
|
db.prepare(`UPDATE files SET errors = ? WHERE path = 'big.ts'`)
|
||||||
|
.run(JSON.stringify([{ message: 'parse failed', filePath: 'big.ts', severity: 'error', code: 'parse_error' }]));
|
||||||
|
db.close();
|
||||||
|
cg = await CodeGraph.open(dir);
|
||||||
|
write('big.ts', BIG(10));
|
||||||
|
const result = await cg.sync();
|
||||||
|
expect(result.addedFilePaths).toEqual(['big.ts']);
|
||||||
|
expect(importsOfUser()).toEqual(['file:big.ts', 'function:f1']);
|
||||||
|
await matchesFullIndex();
|
||||||
|
}, 60_000);
|
||||||
|
|
||||||
|
it('and a change that keeps it over the limit is not taken for an added file', async () => {
|
||||||
|
write('big.ts', BIG(MAX_SOURCE_FILE_SIZE_BYTES + 5000));
|
||||||
|
const result = await cg.sync();
|
||||||
|
expect(result.filesModified).toBe(1);
|
||||||
|
expect(result.addedFilePaths).toBeUndefined();
|
||||||
|
expect(importsOfUser()).toEqual([]);
|
||||||
|
await matchesFullIndex();
|
||||||
|
}, 60_000);
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('sync links what a barrel passes on from a file that shrinks under the size limit', () => {
|
||||||
|
// `user.ts` imports through `lib/index.ts`, which re-exports `lib/big.ts`:
|
||||||
|
// while big.ts was over the limit, the barrel's `export *` of it failed and
|
||||||
|
// so did the names user.ts reads through the barrel.
|
||||||
|
let dir: string;
|
||||||
|
let cg: CodeGraph;
|
||||||
|
const write = (rel: string, content: string) => {
|
||||||
|
fs.mkdirSync(path.dirname(path.join(dir, rel)), { recursive: true });
|
||||||
|
fs.writeFileSync(path.join(dir, rel), content);
|
||||||
|
};
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
cg?.destroy();
|
||||||
|
fs.rmSync(dir, { recursive: true, force: true });
|
||||||
|
});
|
||||||
|
|
||||||
|
it('the barrel and the files importing through it', async () => {
|
||||||
|
dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-sizecap-barrel-'));
|
||||||
|
write('lib/index.ts', "export * from './big';\nexport * from './small';\n");
|
||||||
|
write('lib/big.ts', BIG(MAX_SOURCE_FILE_SIZE_BYTES + 1000));
|
||||||
|
write('lib/small.ts', 'export function s1(): number { return 2; }\n');
|
||||||
|
write('user.ts', "import { f1, s1 } from './lib';\nexport const x = f1() + s1();\n");
|
||||||
|
cg = CodeGraph.initSync(dir);
|
||||||
|
await cg.indexAll();
|
||||||
|
write('lib/big.ts', BIG(10));
|
||||||
|
await cg.sync();
|
||||||
|
const synced = graphOf(cg);
|
||||||
|
cg.close();
|
||||||
|
cg = await CodeGraph.recreate(dir);
|
||||||
|
await cg.indexAll();
|
||||||
|
expect(synced).toEqual(graphOf(cg));
|
||||||
|
}, 60_000);
|
||||||
|
});
|
||||||
+14
-1
@@ -149,7 +149,11 @@ export interface SyncResult {
|
|||||||
*/
|
*/
|
||||||
durationMs: number;
|
durationMs: number;
|
||||||
changedFilePaths?: string[];
|
changedFilePaths?: string[];
|
||||||
/** The part of `changedFilePaths` that was not indexed before this sync. */
|
/**
|
||||||
|
* The part of `changedFilePaths` that had nothing indexed before this sync:
|
||||||
|
* new files, and tracked ones that had no nodes (over the size limit, or a
|
||||||
|
* parse failure) and now do. The latter are still counted as modified.
|
||||||
|
*/
|
||||||
addedFilePaths?: string[];
|
addedFilePaths?: string[];
|
||||||
/** Paths not absorbed because reading or extraction failed; retain for status/retry. */
|
/** Paths not absorbed because reading or extraction failed; retain for status/retry. */
|
||||||
failedFilePaths?: string[];
|
failedFilePaths?: string[];
|
||||||
@@ -3748,6 +3752,10 @@ export class ExtractionOrchestrator {
|
|||||||
let nodesUpdated = 0;
|
let nodesUpdated = 0;
|
||||||
const changedFilePaths: string[] = [];
|
const changedFilePaths: string[] = [];
|
||||||
const addedFilePaths: string[] = [];
|
const addedFilePaths: string[] = [];
|
||||||
|
// Tracked with no nodes and the reason recorded (over the size limit, a
|
||||||
|
// parse or grammar failure): if this sync stores nodes for one, it is new
|
||||||
|
// to its importers, so it joins `addedFilePaths` once stored.
|
||||||
|
const indexedFromSkip: string[] = [];
|
||||||
const removedFilePaths: string[] = [];
|
const removedFilePaths: string[] = [];
|
||||||
// `file\0name` definition pairs for the files this sync touches, sampled
|
// `file\0name` definition pairs for the files this sync touches, sampled
|
||||||
// BEFORE their nodes are replaced/deleted. Compared against the post-store
|
// BEFORE their nodes are replaced/deleted. Compared against the post-store
|
||||||
@@ -3952,6 +3960,7 @@ export class ExtractionOrchestrator {
|
|||||||
onFileChange?.(filePath, content);
|
onFileChange?.(filePath, content);
|
||||||
filesToIndex.push(filePath);
|
filesToIndex.push(filePath);
|
||||||
changedFilePaths.push(filePath);
|
changedFilePaths.push(filePath);
|
||||||
|
if (tracked.nodeCount === 0 && (tracked.errors?.length ?? 0) > 0) indexedFromSkip.push(filePath);
|
||||||
filesModified++;
|
filesModified++;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -4001,6 +4010,10 @@ export class ExtractionOrchestrator {
|
|||||||
if (pause) await pause;
|
if (pause) await pause;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
for (const filePath of indexedFromSkip) {
|
||||||
|
if ((this.queries.getFileByPath(filePath)?.nodeCount ?? 0) > 0) addedFilePaths.push(filePath);
|
||||||
|
}
|
||||||
|
|
||||||
// Names whose definition set this sync changed: a `file\0name` pair present
|
// Names whose definition set this sync changed: a `file\0name` pair present
|
||||||
// before but not after (removed/renamed away) or after but not before
|
// before but not after (removed/renamed away) or after but not before
|
||||||
// (added). A pair on both sides is untouched as far as resolution's
|
// (added). A pair on both sides is untouched as far as resolution's
|
||||||
|
|||||||
Reference in New Issue
Block a user