mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-29 06:32:28 +00:00
* fix(cli): make --no-stats actually omit volatile counts (#1477) Closes #1477. The `--no-stats` flag on `gitnexus analyze` was advertised as "Omit volatile file/symbol counts from AGENTS.md and CLAUDE.md" but had no effect: every reindex still rewrote the markdown with fresh count phrases, producing chore-commit churn on every run — the exact problem the flag was added to solve in #704. Root cause is commander.js negation-flag semantics. `.option( '--no-stats', ...)` registers the option under the accessor `stats` (boolean, default `true`; `false` when the flag is passed), NOT `noStats`. The two action-handler reads in `analyze.ts` (lines 414 and 500 pre-fix) read `options?.noStats`, which is always `undefined`, so the `noStats` payload always reached `runFullAnalysis` / `generateAIContextFiles` as `undefined`/falsy and the count branch in the template always fired. Fixed by replacing `options?.noStats` with `options?.stats === false` at both reads. The strict `=== false` check (rather than `!options?.stats`) means absent options or absent `.stats` field fall through as no-stats=false, preserving the documented default-on behaviour. Also updated the `AnalyzeOptions` interface to declare `stats?: boolean` (matching commander's actual output) with a JSDoc explaining the negation, since the prior `noStats?: boolean` shape was a static-type misrepresentation of what commander provides at runtime. Internal call sites that re-pack `{ noStats: ... }` for downstream consumers (`run-analyze.ts`, `ai-context.ts`) keep their existing field name — those interfaces are not commander- shaped, so `noStats` is the correct name there. ## Regression tests Two new unit tests in `test/unit/ai-context.test.ts`: * `omits volatile counts when noStats option is set (#1477)` — asserts the count parenthetical is absent from both CLAUDE.md and AGENTS.md when `noStats: true` is passed. * `preserves volatile counts when noStats is not set (default)` — documents the default-on path so a future refactor can't silently flip the default. Both call `generateAIContextFiles` directly with distinctive numbers that would unmistakably leak through if the omit branch is broken. ## Manual verification * `vitest run test/unit/ai-context.test.ts` → 13/13 pass (11 prior + 2 new). * Verified before-fix behaviour by checking out main, running `npx gitnexus analyze --no-stats` against an indexed repo, and observing the count phrase still present. Re-running on the fix branch with the same flag strips the phrase as documented. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(autofix): apply prettier + eslint fixes via /autofix command * fix(cli): resolve merge conflict markers in analyze.ts (PR #1478) Remove leftover conflict hunks from main merge; keep commander stats shape (stats?: boolean), wire noStats: options?.stats === false into runFullAnalysis and generateAIContextFiles, and retain indexOnly / skipSkills / skipAgentsMd wiring from main. Co-authored-by: Cursor <cursoragent@cursor.com> * test(cli): cover analyzeCommand → runFullAnalysis noStats bridge (#1477) Assert commander-shaped options.stats maps to the internal noStats payload (including explicit true/false and skipAgentsMd combination) so the CLI bridge cannot regress without failing tests. Co-authored-by: Cursor <cursoragent@cursor.com> * test(cli): cover AGENTS.md default stats + skills noStats bridge (#1478) - Assert volatile stats phrase in both CLAUDE.md and AGENTS.md when noStats is omitted - Add bridge test for --skills regeneration path with stats:false → generateAIContextFiles noStats - Note shared noStats expression beside skills-path call; stub process.exit for full analyze path Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
parent
0502e350b4
commit
b1925ec829
3 changed files with 213 additions and 4 deletions
|
|
@ -117,8 +117,18 @@ export interface AnalyzeOptions {
|
|||
verbose?: boolean;
|
||||
/** Skip AGENTS.md and CLAUDE.md gitnexus block updates. */
|
||||
skipAgentsMd?: boolean;
|
||||
/** Omit volatile symbol/relationship counts from AGENTS.md and CLAUDE.md. */
|
||||
noStats?: boolean;
|
||||
/**
|
||||
* Stats inclusion in AGENTS.md and CLAUDE.md.
|
||||
*
|
||||
* Commander.js represents `--no-stats` as `stats: boolean` (default
|
||||
* `true`; `false` when the user passes `--no-stats`), NOT as
|
||||
* `noStats: boolean`. Reading the negated form would always be
|
||||
* `undefined` and the flag would silently no-op (#1477). Consumers
|
||||
* that want "did the user request --no-stats?" should compare with
|
||||
* `=== false` to distinguish the explicit-off case from the
|
||||
* default-on case.
|
||||
*/
|
||||
stats?: boolean;
|
||||
/** Skip installing standard GitNexus skill files to .claude/skills/gitnexus/. */
|
||||
skipSkills?: boolean;
|
||||
/** Pure index mode: skip all file injection (AGENTS.md, CLAUDE.md, skills). */
|
||||
|
|
@ -449,7 +459,12 @@ export const analyzeCommand = async (inputPath?: string, options?: AnalyzeOption
|
|||
skipGit: options?.skipGit,
|
||||
skipAgentsMd,
|
||||
skipSkills,
|
||||
noStats: options?.noStats,
|
||||
// commander.js `.option('--no-stats', …)` registers the flag as
|
||||
// `options.stats` (boolean, default true; `false` when the user
|
||||
// passed --no-stats). Reading `options?.noStats` here returns
|
||||
// undefined every time, so the flag was a no-op on the markdown
|
||||
// rewrite path before this fix. See #1477.
|
||||
noStats: options?.stats === false,
|
||||
registryName: options?.name,
|
||||
// Registry-collision bypass — its own CLI flag, intentionally NOT
|
||||
// overloading --force. A user who hits the collision guard should
|
||||
|
|
@ -537,7 +552,13 @@ export const analyzeCommand = async (inputPath?: string, options?: AnalyzeOption
|
|||
processes: s.processes,
|
||||
},
|
||||
skillResult.skills,
|
||||
{ skipAgentsMd, skipSkills, noStats: options?.noStats },
|
||||
{
|
||||
skipAgentsMd,
|
||||
skipSkills,
|
||||
// Mirror runFullAnalysis `noStats` bridge (#1477) — same expression;
|
||||
// exercised on the `--skills` path by analyze-no-stats-bridge.test.ts.
|
||||
noStats: options?.stats === false,
|
||||
},
|
||||
);
|
||||
}
|
||||
} catch {
|
||||
|
|
|
|||
|
|
@ -45,6 +45,54 @@ describe('generateAIContextFiles', () => {
|
|||
expect(content).toContain('TestProject');
|
||||
});
|
||||
|
||||
it('omits volatile counts when noStats option is set (#1477)', async () => {
|
||||
// Distinct subdir per case so we can assert on a clean slate.
|
||||
const subDir = await fs.mkdtemp(path.join(os.tmpdir(), 'gn-no-stats-test-'));
|
||||
const subStorage = path.join(subDir, '.gitnexus');
|
||||
await fs.mkdir(subStorage, { recursive: true });
|
||||
try {
|
||||
// Stats values picked to be unmistakable if they leak through.
|
||||
const stats = { nodes: 12345, edges: 67890, processes: 99 };
|
||||
await generateAIContextFiles(subDir, subStorage, 'NoStatsProject', stats, undefined, {
|
||||
noStats: true,
|
||||
});
|
||||
|
||||
for (const f of ['CLAUDE.md', 'AGENTS.md']) {
|
||||
const content = await fs.readFile(path.join(subDir, f), 'utf-8');
|
||||
expect(content).toContain('NoStatsProject');
|
||||
// The "(N symbols, N relationships, N execution flows)"
|
||||
// phrase MUST NOT appear when noStats=true.
|
||||
expect(content).not.toMatch(
|
||||
/\(\d+\s+symbols,\s+\d+\s+relationships,\s+\d+\s+execution flows\)/,
|
||||
);
|
||||
// And the distinctive numbers must not leak via any other path.
|
||||
expect(content).not.toContain('12345');
|
||||
expect(content).not.toContain('67890');
|
||||
}
|
||||
} finally {
|
||||
await fs.rm(subDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it('preserves volatile counts when noStats is not set (default)', async () => {
|
||||
const subDir = await fs.mkdtemp(path.join(os.tmpdir(), 'gn-with-stats-test-'));
|
||||
const subStorage = path.join(subDir, '.gitnexus');
|
||||
await fs.mkdir(subStorage, { recursive: true });
|
||||
try {
|
||||
const stats = { nodes: 12345, edges: 67890, processes: 99 };
|
||||
await generateAIContextFiles(subDir, subStorage, 'WithStatsProject', stats);
|
||||
for (const f of ['CLAUDE.md', 'AGENTS.md']) {
|
||||
const content = await fs.readFile(path.join(subDir, f), 'utf-8');
|
||||
expect(content).toContain('WithStatsProject');
|
||||
expect(content).toMatch(
|
||||
/\(12345\s+symbols,\s+67890\s+relationships,\s+99\s+execution flows\)/,
|
||||
);
|
||||
}
|
||||
} finally {
|
||||
await fs.rm(subDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it('keeps the load-bearing repo-specific sections in the CLAUDE.md block (#856)', async () => {
|
||||
// The trimmed block must still contain everything that is genuinely
|
||||
// unique per repo or load-bearing for the agent: the freshness warning,
|
||||
|
|
|
|||
140
gitnexus/test/unit/analyze-no-stats-bridge.test.ts
Normal file
140
gitnexus/test/unit/analyze-no-stats-bridge.test.ts
Normal file
|
|
@ -0,0 +1,140 @@
|
|||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
const { runFullAnalysisMock, generateAIContextFilesMock, generateSkillFilesMock } = vi.hoisted(
|
||||
() => {
|
||||
const runFullAnalysisMock = vi.fn();
|
||||
const generateAIContextFilesMock = vi.fn(async () => ({ files: [] as string[] }));
|
||||
const generateSkillFilesMock = vi.fn(async () => ({
|
||||
skills: [{ name: 'c', label: 'Community', symbolCount: 1, fileCount: 1 }],
|
||||
outputPath: '/repo/.claude/skills/generated',
|
||||
}));
|
||||
return { runFullAnalysisMock, generateAIContextFilesMock, generateSkillFilesMock };
|
||||
},
|
||||
);
|
||||
|
||||
vi.mock('../../src/core/run-analyze.js', () => ({
|
||||
runFullAnalysis: runFullAnalysisMock,
|
||||
}));
|
||||
|
||||
vi.mock('../../src/cli/ai-context.js', () => ({
|
||||
generateAIContextFiles: generateAIContextFilesMock,
|
||||
}));
|
||||
|
||||
vi.mock('../../src/cli/skill-gen.js', () => ({
|
||||
generateSkillFiles: generateSkillFilesMock,
|
||||
}));
|
||||
|
||||
vi.mock('../../src/core/lbug/lbug-adapter.js', () => ({
|
||||
closeLbug: vi.fn(async () => undefined),
|
||||
}));
|
||||
|
||||
vi.mock('../../src/storage/repo-manager.js', () => ({
|
||||
getStoragePaths: vi.fn(() => ({ storagePath: '.gitnexus', lbugPath: '.gitnexus/lbug' })),
|
||||
getGlobalRegistryPath: vi.fn(() => 'registry.json'),
|
||||
RegistryNameCollisionError: class RegistryNameCollisionError extends Error {},
|
||||
AnalysisNotFinalizedError: class AnalysisNotFinalizedError extends Error {},
|
||||
assertAnalysisFinalized: vi.fn(async () => undefined),
|
||||
}));
|
||||
|
||||
vi.mock('../../src/storage/git.js', () => ({
|
||||
getGitRoot: vi.fn(() => '/repo'),
|
||||
hasGitDir: vi.fn(() => true),
|
||||
}));
|
||||
|
||||
vi.mock('../../src/core/ingestion/utils/max-file-size.js', () => ({
|
||||
getMaxFileSizeBannerMessage: vi.fn(() => null),
|
||||
}));
|
||||
|
||||
describe('analyzeCommand commander → runFullAnalysis noStats bridge (#1477)', () => {
|
||||
beforeEach(() => {
|
||||
vi.resetModules();
|
||||
runFullAnalysisMock.mockReset();
|
||||
runFullAnalysisMock.mockResolvedValue({
|
||||
repoName: 'repo',
|
||||
repoPath: '/repo',
|
||||
stats: {},
|
||||
alreadyUpToDate: true,
|
||||
});
|
||||
generateAIContextFilesMock.mockReset();
|
||||
generateAIContextFilesMock.mockResolvedValue({ files: [] });
|
||||
generateSkillFilesMock.mockReset();
|
||||
generateSkillFilesMock.mockResolvedValue({
|
||||
skills: [{ name: 'c', label: 'Community', symbolCount: 1, fileCount: 1 }],
|
||||
outputPath: '/repo/.claude/skills/generated',
|
||||
});
|
||||
process.exitCode = undefined;
|
||||
process.env.NODE_OPTIONS = `${process.env.NODE_OPTIONS ?? ''} --max-old-space-size=8192`.trim();
|
||||
});
|
||||
|
||||
it('maps commander-shaped stats:false to noStats:true (equivalent to --no-stats)', async () => {
|
||||
const { analyzeCommand } = await import('../../src/cli/analyze.js');
|
||||
|
||||
await analyzeCommand(undefined, { stats: false });
|
||||
|
||||
expect(runFullAnalysisMock).toHaveBeenCalledTimes(1);
|
||||
const opts = runFullAnalysisMock.mock.calls[0][1];
|
||||
expect(opts.noStats).toBe(true);
|
||||
});
|
||||
|
||||
it('maps omitted stats to noStats:false (default-on preserved)', async () => {
|
||||
const { analyzeCommand } = await import('../../src/cli/analyze.js');
|
||||
|
||||
await analyzeCommand(undefined, {});
|
||||
|
||||
const opts = runFullAnalysisMock.mock.calls[0][1];
|
||||
expect(opts.noStats).toBe(false);
|
||||
});
|
||||
|
||||
it('maps explicit stats:true to noStats:false', async () => {
|
||||
const { analyzeCommand } = await import('../../src/cli/analyze.js');
|
||||
|
||||
await analyzeCommand(undefined, { stats: true });
|
||||
|
||||
const opts = runFullAnalysisMock.mock.calls[0][1];
|
||||
expect(opts.noStats).toBe(false);
|
||||
});
|
||||
|
||||
it('still maps stats:false to noStats:true when skipAgentsMd is set', async () => {
|
||||
const { analyzeCommand } = await import('../../src/cli/analyze.js');
|
||||
|
||||
await analyzeCommand(undefined, { stats: false, skipAgentsMd: true });
|
||||
|
||||
const opts = runFullAnalysisMock.mock.calls[0][1];
|
||||
expect(opts.noStats).toBe(true);
|
||||
expect(opts.skipAgentsMd).toBe(true);
|
||||
});
|
||||
|
||||
it('passes stats:false as noStats to generateAIContextFiles on the --skills regeneration path (#1477)', async () => {
|
||||
runFullAnalysisMock.mockResolvedValueOnce({
|
||||
repoName: 'repo',
|
||||
repoPath: '/repo',
|
||||
stats: {
|
||||
files: 1,
|
||||
nodes: 10,
|
||||
edges: 20,
|
||||
communities: 0,
|
||||
processes: 5,
|
||||
},
|
||||
alreadyUpToDate: false,
|
||||
pipelineResult: { communityResult: undefined },
|
||||
});
|
||||
|
||||
const exitSpy = vi.spyOn(process, 'exit').mockImplementation(() => undefined as never);
|
||||
try {
|
||||
const { analyzeCommand } = await import('../../src/cli/analyze.js');
|
||||
|
||||
await analyzeCommand(undefined, { skills: true, stats: false });
|
||||
|
||||
expect(generateSkillFilesMock).toHaveBeenCalledTimes(1);
|
||||
expect(generateAIContextFilesMock).toHaveBeenCalledTimes(1);
|
||||
const aiCtxOpts = generateAIContextFilesMock.mock.calls[0]![5];
|
||||
expect(aiCtxOpts).toEqual({
|
||||
skipAgentsMd: undefined,
|
||||
skipSkills: undefined,
|
||||
noStats: true,
|
||||
});
|
||||
} finally {
|
||||
exitSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Reference in a new issue