diff --git a/gitnexus/src/core/ingestion/filesystem-walker.ts b/gitnexus/src/core/ingestion/filesystem-walker.ts index 4d6725e246..c30ea43216 100644 --- a/gitnexus/src/core/ingestion/filesystem-walker.ts +++ b/gitnexus/src/core/ingestion/filesystem-walker.ts @@ -74,12 +74,31 @@ export const walkRepositoryPaths = async ( if (skippedLarge > 0) { const isDefault = maxFileSizeBytes === DEFAULT_MAX_FILE_SIZE_BYTES; + const isOverrideUnset = !process.env.GITNEXUS_MAX_FILE_SIZE; const suffix = isDefault ? ', likely generated/vendored' : ''; logger.warn(` Skipped ${skippedLarge} large files (>${maxFileSizeBytes / 1024}KB${suffix})`); - if (isVerboseIngestionEnabled()) { - for (const p of skippedLargePaths) { - logger.warn(` - ${p}`); - } + + // Always show at least the first few paths so users can diagnose why + // edges are missing from a specific file (issue #1659). The full list is + // gated behind GITNEXUS_VERBOSE=1 to avoid flooding output on repos with + // many generated/vendored blobs. Sort before slicing so the preview is + // stable across runs (fs.stat callbacks race within each batch). + skippedLargePaths.sort(); + const SKIPPED_PREVIEW_CAP = 5; + const showAll = isVerboseIngestionEnabled() || skippedLargePaths.length <= SKIPPED_PREVIEW_CAP; + const preview = showAll ? skippedLargePaths : skippedLargePaths.slice(0, SKIPPED_PREVIEW_CAP); + for (const p of preview) { + logger.warn(` - ${p}`); + } + if (!showAll) { + const remaining = skippedLargePaths.length - SKIPPED_PREVIEW_CAP; + logger.warn(` ...and ${remaining} more (set GITNEXUS_VERBOSE=1 to list them all)`); + } + // Only hint about the env var when the user has not set it at all. An + // explicit GITNEXUS_MAX_FILE_SIZE=512 happens to resolve to the same + // bytes as the default but the operator clearly already knows the knob. + if (isDefault && isOverrideUnset) { + logger.warn(` Set GITNEXUS_MAX_FILE_SIZE= to include files above the default cap.`); } } diff --git a/gitnexus/test/integration/filesystem-walker.test.ts b/gitnexus/test/integration/filesystem-walker.test.ts index 2a1257ccc0..1d743a8989 100644 --- a/gitnexus/test/integration/filesystem-walker.test.ts +++ b/gitnexus/test/integration/filesystem-walker.test.ts @@ -398,5 +398,172 @@ describe('filesystem-walker', () => { expect(skipWarnings.length).toBeGreaterThan(0); expect(String(skipWarnings[0].msg ?? '')).toContain('generated/vendored'); }); + + // Regression: issue #1659. The skipped-paths list and the + // GITNEXUS_MAX_FILE_SIZE hint must appear by default, otherwise users + // see "Skipped N large files" with no actionable detail and misdiagnose + // missing IMPORTS/CALLS edges as a resolver bug. + it('lists the skipped path by default (not gated behind GITNEXUS_VERBOSE)', async () => { + await walkRepositoryPaths(sizeDir); + const pathWarnings = cap.records().filter((r) => String(r.msg ?? '').includes(BIG_FILE)); + expect(pathWarnings.length).toBeGreaterThan(0); + }); + + it('emits a GITNEXUS_MAX_FILE_SIZE hint when running with the default cap', async () => { + await walkRepositoryPaths(sizeDir); + const hint = cap + .records() + .filter((r) => String(r.msg ?? '').includes('GITNEXUS_MAX_FILE_SIZE=')); + expect(hint.length).toBe(1); + }); + + it('omits the GITNEXUS_MAX_FILE_SIZE hint when an override is active', async () => { + process.env.GITNEXUS_MAX_FILE_SIZE = '1'; + await walkRepositoryPaths(sizeDir); + const hint = cap + .records() + .filter((r) => String(r.msg ?? '').includes('GITNEXUS_MAX_FILE_SIZE=')); + expect(hint.length).toBe(0); + }); + + // Edge case from the #1661 adversarial review: setting GITNEXUS_MAX_FILE_SIZE + // to the same value as the default (512KB) used to still print the hint + // because the byte comparison resolved to equal. The hint should care + // about whether the operator set the env var, not what value they chose. + it('omits the GITNEXUS_MAX_FILE_SIZE hint when the override equals the default value', async () => { + process.env.GITNEXUS_MAX_FILE_SIZE = '512'; + await walkRepositoryPaths(sizeDir); + const hint = cap + .records() + .filter((r) => String(r.msg ?? '').includes('GITNEXUS_MAX_FILE_SIZE=')); + expect(hint.length).toBe(0); + }); + }); + + describe('large file skip preview cap (#1659)', () => { + let manyDir: string; + const ORIGINAL_ENV = process.env.GITNEXUS_MAX_FILE_SIZE; + const ORIGINAL_VERBOSE = process.env.GITNEXUS_VERBOSE; + let cap: ReturnType; + + beforeAll(async () => { + manyDir = await fs.mkdtemp(path.join(os.tmpdir(), 'gn-walker-size-many-')); + await fs.mkdir(path.join(manyDir, 'src'), { recursive: true }); + // 8 files >512KB so the preview-cap path (5) is exercised. + for (let i = 0; i < 8; i++) { + await fs.writeFile(path.join(manyDir, 'src', `big${i}.ts`), 'x'.repeat(600 * 1024)); + } + }); + + afterAll(async () => { + await fs.rm(manyDir, { recursive: true, force: true }); + }); + + beforeEach(() => { + delete process.env.GITNEXUS_MAX_FILE_SIZE; + delete process.env.GITNEXUS_VERBOSE; + _resetMaxFileSizeWarnings(); + cap = _captureLogger(); + }); + + afterEach(() => { + if (ORIGINAL_ENV === undefined) { + delete process.env.GITNEXUS_MAX_FILE_SIZE; + } else { + process.env.GITNEXUS_MAX_FILE_SIZE = ORIGINAL_ENV; + } + if (ORIGINAL_VERBOSE === undefined) { + delete process.env.GITNEXUS_VERBOSE; + } else { + process.env.GITNEXUS_VERBOSE = ORIGINAL_VERBOSE; + } + cap.restore(); + }); + + it('truncates the path list to 5 and mentions GITNEXUS_VERBOSE when over the cap', async () => { + await walkRepositoryPaths(manyDir); + const pathLines = cap.records().filter((r) => /^\s*-\s/.test(String(r.msg ?? ''))); + expect(pathLines.length).toBe(5); + const more = cap + .records() + .filter((r) => String(r.msg ?? '').includes('and 3 more (set GITNEXUS_VERBOSE=1')); + expect(more.length).toBe(1); + }); + + // Boundary check from the #1661 adversarial review: the SKIPPED_PREVIEW_CAP + // comparison is `<=`, so 5 paths should list all five without a truncation + // line and 6 paths should list exactly five plus "...and 1 more". Tested + // explicitly so a future off-by-one refactor (`<=` → `<`) fails fast. + it('lists all paths and omits the truncation line at exactly 5 skipped files', async () => { + const fiveDir = await fs.mkdtemp(path.join(os.tmpdir(), 'gn-walker-size-five-')); + try { + await fs.mkdir(path.join(fiveDir, 'src'), { recursive: true }); + for (let i = 0; i < 5; i++) { + await fs.writeFile(path.join(fiveDir, 'src', `big${i}.ts`), 'x'.repeat(600 * 1024)); + } + await walkRepositoryPaths(fiveDir); + const pathLines = cap.records().filter((r) => /^\s*-\s/.test(String(r.msg ?? ''))); + expect(pathLines.length).toBe(5); + const more = cap.records().filter((r) => String(r.msg ?? '').includes('...and ')); + expect(more.length).toBe(0); + } finally { + await fs.rm(fiveDir, { recursive: true, force: true }); + } + }); + + it('lists exactly 5 paths plus "...and 1 more" at exactly 6 skipped files', async () => { + const sixDir = await fs.mkdtemp(path.join(os.tmpdir(), 'gn-walker-size-six-')); + try { + await fs.mkdir(path.join(sixDir, 'src'), { recursive: true }); + for (let i = 0; i < 6; i++) { + await fs.writeFile(path.join(sixDir, 'src', `big${i}.ts`), 'x'.repeat(600 * 1024)); + } + await walkRepositoryPaths(sixDir); + const pathLines = cap.records().filter((r) => /^\s*-\s/.test(String(r.msg ?? ''))); + expect(pathLines.length).toBe(5); + const more = cap + .records() + .filter((r) => String(r.msg ?? '').includes('and 1 more (set GITNEXUS_VERBOSE=1')); + expect(more.length).toBe(1); + } finally { + await fs.rm(sixDir, { recursive: true, force: true }); + } + }); + + it('lists every skipped path when GITNEXUS_VERBOSE=1', async () => { + process.env.GITNEXUS_VERBOSE = '1'; + await walkRepositoryPaths(manyDir); + const pathLines = cap.records().filter((r) => /^\s*-\s/.test(String(r.msg ?? ''))); + expect(pathLines.length).toBe(8); + const more = cap.records().filter((r) => String(r.msg ?? '').includes('and ')); + expect(more.length).toBe(0); + }); + + // Issue #1659 follow-up (PR #1661 review): paths were pushed in fs.stat + // completion order, so the default preview could vary between runs on + // the same repo. The implementation sorts skippedLargePaths before + // slicing, so the listed paths come out in sorted order, which is the + // stable contract operators can rely on. + it('lists skipped paths in sorted order (deterministic preview)', async () => { + process.env.GITNEXUS_VERBOSE = '1'; + await walkRepositoryPaths(manyDir); + const pathLines = cap + .records() + .map((r) => String(r.msg ?? '')) + .filter((m) => /^\s*-\s/.test(m)) + .map((m) => m.replace(/^\s*-\s*/, '')); + expect(pathLines).toEqual([...pathLines].sort()); + // sanity-check we actually saw all 8 of the manyDir fixture + expect(pathLines).toEqual([ + 'src/big0.ts', + 'src/big1.ts', + 'src/big2.ts', + 'src/big3.ts', + 'src/big4.ts', + 'src/big5.ts', + 'src/big6.ts', + 'src/big7.ts', + ]); + }); }); });