Skip to content

Commit 33b200c

Browse files
committed
Fixes.
1 parent 4a44e08 commit 33b200c

14 files changed

Lines changed: 221 additions & 63 deletions

bin/walkthrough-guide.md

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,9 @@ choose.
3939
- **`support[]`** — changed hunks that should stay off the main path. Use it for generated files,
4040
lockfiles, snapshots, docs-only changes, and repeated mechanical edits unless they are essential
4141
to review. Codiff adds any omitted live-diff hunks to support. Generated-like files are one
42-
synthetic hunk per changed section; never split them, use `reason: "Generated files"` for
43-
generated-only support items so they render together, but keep behavior-relevant snapshots or
44-
artifacts on the main path. Codiff also recognizes `linguist-generated` and `gitlab-generated`
45-
attributes from `.gitattributes`.
42+
synthetic hunk per changed section; never split them. Codiff groups generated-only support items
43+
automatically, but keep behavior-relevant snapshots or artifacts on the main path. Codiff also
44+
recognizes `linguist-generated` and `gitlab-generated` attributes from `.gitattributes`.
4645
- **`changeType?` / `commitNote?`** — optional commit composer metadata for committable
4746
walkthroughs.
4847
- **`commit?`** — for working-tree walkthroughs, include `title` and `body` when there is enough

core/__tests__/ReviewCodeView-scroll.test.tsx

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,25 @@ test('generated files are collapsed by default and can be explicitly expanded pe
172172
}
173173
});
174174

175+
test('explicit generated metadata can keep generated-looking paths expanded', async () => {
176+
const file = {
177+
...createChangedFile('src/__generated__/api.ts'),
178+
generated: false,
179+
};
180+
const view = await renderReact(<ReviewCodeViewHarness files={[file]} />);
181+
182+
try {
183+
await waitFor(() => {
184+
expect(
185+
view.container.querySelector('[aria-label="Collapse file"]')?.getAttribute('aria-expanded'),
186+
).toBe('true');
187+
expect(view.container.querySelector('.codiff-generated-badge')).toBeNull();
188+
});
189+
} finally {
190+
await view.cleanup();
191+
}
192+
});
193+
175194
test('switching edited Markdown back to a diff flushes and refreshes it first', async () => {
176195
const order: Array<string> = [];
177196
const initialFile = createLoadedMarkdownFile('# Edited\n', 'plan.md:initial');

core/__tests__/git-state.test.ts

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,11 @@ type PullRequestFileContent = {
3131
};
3232

3333
type GeneratedFilesModule = {
34-
readGeneratedAttributePaths: (
34+
readGeneratedAttributeStates: (
3535
repoRoot: string,
3636
paths: ReadonlyArray<string>,
3737
source?: string,
38-
) => Promise<ReadonlySet<string>>;
38+
) => Promise<ReadonlyMap<string, boolean>>;
3939
};
4040

4141
type GitStateModule = {
@@ -125,7 +125,7 @@ type GitStateModule = {
125125

126126
const execFileAsync = promisify(execFile);
127127
const require = createRequire(import.meta.url);
128-
const { readGeneratedAttributePaths } =
128+
const { readGeneratedAttributeStates } =
129129
require('../../electron/generated-files.cjs') as GeneratedFilesModule;
130130
const {
131131
collectResolvedReviewCommentIds,
@@ -193,20 +193,22 @@ test('readRepositoryState marks generated files from gitattributes and path heur
193193
await writeRepoFile(
194194
repo,
195195
'.gitattributes',
196-
'*.gen.yaml gitlab-generated\n*.generated.txt linguist-generated\nignored.txt -linguist-generated\n',
196+
'*.gen.yaml gitlab-generated\n*.generated.txt linguist-generated\nignored.txt -linguist-generated\npnpm-lock.yaml linguist-generated=false\nsrc/__generated__/** -gitlab-generated\n',
197197
);
198198
await writeRepoFile(repo, 'service.gen.yaml', 'generated\n');
199199
await writeRepoFile(repo, 'schema.generated.txt', 'generated\n');
200200
await writeRepoFile(repo, 'ignored.txt', 'source\n');
201201
await writeRepoFile(repo, 'pnpm-lock.yaml', 'lockfileVersion: 9\n');
202+
await writeRepoFile(repo, 'src/__generated__/api.ts', 'export const api = 1;\n');
202203

203204
const state = await readRepositoryState(repo);
204205
const generatedByPath = new Map(state.files.map((file) => [file.path, file.generated]));
205206

206207
expect(generatedByPath.get('service.gen.yaml')).toBe(true);
207208
expect(generatedByPath.get('schema.generated.txt')).toBe(true);
208-
expect(generatedByPath.get('pnpm-lock.yaml')).toBe(true);
209-
expect(generatedByPath.get('ignored.txt')).toBeUndefined();
209+
expect(generatedByPath.get('pnpm-lock.yaml')).toBe(false);
210+
expect(generatedByPath.get('src/__generated__/api.ts')).toBe(false);
211+
expect(generatedByPath.get('ignored.txt')).toBe(false);
210212
});
211213
});
212214

@@ -226,12 +228,21 @@ test('commit reviews evaluate generated attributes from the reviewed commit', as
226228

227229
test('historical generated attributes fall back to a temporary index without --source', async () => {
228230
await withRepo(async (repo) => {
229-
await writeRepoFile(repo, '.gitattributes', '*.api.ts linguist-generated\n');
231+
await writeRepoFile(
232+
repo,
233+
'.gitattributes',
234+
'*.api.ts linguist-generated\npnpm-lock.yaml linguist-generated=false\n',
235+
);
230236
await writeRepoFile(repo, 'client.api.ts', 'export const client = 1;\n');
237+
await writeRepoFile(repo, 'pnpm-lock.yaml', 'lockfileVersion: 9\n');
231238
await commitAll(repo, 'generated client');
232239
const commit = (await git(repo, ['rev-parse', 'HEAD'])).trim();
233240

234-
await writeRepoFile(repo, '.gitattributes', '*.api.ts -linguist-generated\n');
241+
await writeRepoFile(
242+
repo,
243+
'.gitattributes',
244+
'*.api.ts -linguist-generated\npnpm-lock.yaml linguist-generated\n',
245+
);
235246
const wrapperDirectory = await mkdtemp(join(tmpdir(), 'codiff-old-git-'));
236247
const wrapperPath = join(wrapperDirectory, 'git');
237248
await writeFile(
@@ -255,8 +266,17 @@ exec git "$@"
255266
process.env.CODIFF_TEST_ORIGINAL_PATH = originalPath ?? '';
256267
process.env.PATH = `${wrapperDirectory}:${originalPath ?? ''}`;
257268
try {
258-
const generatedPaths = await readGeneratedAttributePaths(repo, ['client.api.ts'], commit);
259-
expect(generatedPaths).toEqual(new Set(['client.api.ts']));
269+
const generatedStates = await readGeneratedAttributeStates(
270+
repo,
271+
['client.api.ts', 'pnpm-lock.yaml'],
272+
commit,
273+
);
274+
expect(generatedStates).toEqual(
275+
new Map([
276+
['client.api.ts', true],
277+
['pnpm-lock.yaml', false],
278+
]),
279+
);
260280
} finally {
261281
if (originalPath == null) {
262282
delete process.env.PATH;

core/lib/narrative-walkthrough-diff.cjs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ const isGeneratedWalkthroughPath = (path) => {
8282

8383
/** @param {{generated?: boolean; path: string}} file */
8484
const isGeneratedWalkthroughFile = (file) =>
85-
file.generated === true || isGeneratedWalkthroughPath(file.path);
85+
file.generated ?? isGeneratedWalkthroughPath(file.path);
8686

8787
/** @param {string} path */
8888
const getGeneratedWalkthroughSummary = (path) => {

core/lib/narrative-walkthrough-diff.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ const isGeneratedWalkthroughPath = (path) => {
8282

8383
/** @param {{generated?: boolean; path: string}} file */
8484
const isGeneratedWalkthroughFile = (file) =>
85-
file.generated === true || isGeneratedWalkthroughPath(file.path);
85+
file.generated ?? isGeneratedWalkthroughPath(file.path);
8686

8787
/** @param {string} path */
8888
const getGeneratedWalkthroughSummary = (path) => {

electron/__tests__/narrative-walkthrough.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -534,6 +534,33 @@ test('repository digest collapses generated files to one synthetic hunk', () =>
534534
expect(prompt).not.toContain('"id":"h2"');
535535
});
536536

537+
test('repository digest honors generated metadata that disables path heuristics', () => {
538+
const prompt = buildNarrativeWalkthroughPrompt({
539+
branch: 'main',
540+
files: [
541+
{
542+
generated: false,
543+
path: 'pnpm-lock.yaml',
544+
sections: [
545+
{
546+
id: 'pnpm-lock.yaml:staged',
547+
kind: 'staged',
548+
patch: manyHunkPatch,
549+
},
550+
],
551+
status: 'modified',
552+
},
553+
],
554+
generatedAt: 1,
555+
root: '/repo',
556+
source: { type: 'working-tree' },
557+
});
558+
559+
expect(prompt).not.toContain('"generated":true');
560+
expect(prompt).toContain('"kind":"patch"');
561+
expect(prompt).toContain('"id":"h2"');
562+
});
563+
537564
test('repository digest exposes synthetic hunk ids for non-text sections', () => {
538565
const prompt = buildNarrativeWalkthroughPrompt({
539566
branch: 'main',

electron/generated-files.cjs

Lines changed: 46 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@ const GENERATED_ATTRIBUTES = ['linguist-generated', 'gitlab-generated'];
1212
const isGeneratedAttributeValue = (value) =>
1313
value !== 'unspecified' && value !== 'unset' && value !== 'false';
1414

15+
/** @param {string} value */
16+
const isNotGeneratedAttributeValue = (value) => value === 'unset' || value === 'false';
17+
1518
/** @param {import('../core/types.ts').ReviewSource} source */
1619
const getGeneratedAttributeSource = (source) =>
1720
source.type === 'commit'
@@ -25,17 +28,19 @@ const getGeneratedAttributeSource = (source) =>
2528
: undefined;
2629

2730
/** @param {Buffer} output */
28-
const parseGeneratedAttributePaths = (output) => {
31+
const parseGeneratedAttributeStates = (output) => {
2932
const fields = output.toString('utf8').split('\0');
30-
const generatedPaths = new Set();
33+
const generatedStates = new Map();
3134
for (let index = 0; index + 2 < fields.length; index += 3) {
3235
const path = fields[index];
3336
const value = fields[index + 2];
3437
if (path && isGeneratedAttributeValue(value)) {
35-
generatedPaths.add(path);
38+
generatedStates.set(path, true);
39+
} else if (path && isNotGeneratedAttributeValue(value) && generatedStates.get(path) !== true) {
40+
generatedStates.set(path, false);
3641
}
3742
}
38-
return generatedPaths;
43+
return generatedStates;
3944
};
4045

4146
/**
@@ -44,8 +49,8 @@ const parseGeneratedAttributePaths = (output) => {
4449
* @param {ReadonlyArray<string>} options
4550
* @param {NodeJS.ProcessEnv} [env]
4651
*/
47-
const checkGeneratedAttributePaths = async (repoRoot, paths, options, env) =>
48-
parseGeneratedAttributePaths(
52+
const checkGeneratedAttributeStates = async (repoRoot, paths, options, env) =>
53+
parseGeneratedAttributeStates(
4954
await gitBufferWithInput(
5055
repoRoot,
5156
['check-attr', ...options, '-z', '--stdin', ...GENERATED_ATTRIBUTES],
@@ -63,7 +68,7 @@ const checkGeneratedAttributePaths = async (repoRoot, paths, options, env) =>
6368
* @param {ReadonlyArray<string>} paths
6469
* @param {string} source
6570
*/
66-
const readGeneratedAttributePathsFromTree = async (repoRoot, paths, source) => {
71+
const readGeneratedAttributeStatesFromTree = async (repoRoot, paths, source) => {
6772
const temporaryDirectory = await mkdtemp(join(tmpdir(), 'codiff-generated-files-'));
6873
const env = {
6974
...process.env,
@@ -72,7 +77,7 @@ const readGeneratedAttributePathsFromTree = async (repoRoot, paths, source) => {
7277

7378
try {
7479
await gitBufferWithInput(repoRoot, ['read-tree', source], Buffer.alloc(0), { env });
75-
return await checkGeneratedAttributePaths(repoRoot, paths, ['--cached'], env);
80+
return await checkGeneratedAttributeStates(repoRoot, paths, ['--cached'], env);
7681
} finally {
7782
await rm(temporaryDirectory, { force: true, recursive: true });
7883
}
@@ -83,49 +88,60 @@ const readGeneratedAttributePathsFromTree = async (repoRoot, paths, source) => {
8388
* @param {ReadonlyArray<string>} paths
8489
* @param {string | undefined} source
8590
*/
86-
const readGeneratedAttributePaths = async (repoRoot, paths, source) => {
91+
const readGeneratedAttributeStates = async (repoRoot, paths, source) => {
8792
if (paths.length === 0) {
88-
return new Set();
93+
return new Map();
8994
}
9095

9196
try {
92-
return await checkGeneratedAttributePaths(repoRoot, paths, source ? ['--source', source] : []);
97+
return await checkGeneratedAttributeStates(repoRoot, paths, source ? ['--source', source] : []);
9398
} catch {
9499
if (source) {
95100
try {
96101
// Git before 2.40 rejects `--source`; use its historical-tree fallback.
97-
return await readGeneratedAttributePathsFromTree(repoRoot, paths, source);
102+
return await readGeneratedAttributeStatesFromTree(repoRoot, paths, source);
98103
} catch {
99104
// Ignore invalid or unavailable historical sources.
100105
}
101106
}
102-
return new Set();
107+
return new Map();
103108
}
104109
};
105110

111+
/**
112+
* @param {import('../core/types.ts').RepositoryState} state
113+
* @param {ReadonlyMap<string, boolean>} generatedAttributeStates
114+
*/
115+
const applyGeneratedAttributeStates = (state, generatedAttributeStates) => ({
116+
...state,
117+
files: state.files.map((file) => {
118+
const attributeState = generatedAttributeStates.get(file.path);
119+
if (attributeState != null) {
120+
return file.generated === attributeState ? file : { ...file, generated: attributeState };
121+
}
122+
if (file.generated != null) {
123+
return file;
124+
}
125+
return isGeneratedWalkthroughPath(file.path) ? { ...file, generated: true } : file;
126+
}),
127+
});
128+
106129
/** @param {import('../core/types.ts').RepositoryState} state */
107-
const annotateGeneratedFiles = async (state) => {
108-
const generatedAttributePaths = await readGeneratedAttributePaths(
109-
state.root,
110-
state.files.map((file) => file.path),
111-
getGeneratedAttributeSource(state.source),
112-
);
113-
return {
114-
...state,
115-
files: state.files.map((file) =>
116-
file.generated ||
117-
generatedAttributePaths.has(file.path) ||
118-
isGeneratedWalkthroughPath(file.path)
119-
? { ...file, generated: true }
120-
: file,
130+
const annotateGeneratedFiles = async (state) =>
131+
applyGeneratedAttributeStates(
132+
state,
133+
await readGeneratedAttributeStates(
134+
state.root,
135+
state.files.map((file) => file.path),
136+
getGeneratedAttributeSource(state.source),
121137
),
122-
};
123-
};
138+
);
124139

125140
module.exports = {
126141
GENERATED_ATTRIBUTES,
127142
annotateGeneratedFiles,
143+
applyGeneratedAttributeStates,
128144
getGeneratedAttributeSource,
129145
isGeneratedAttributeValue,
130-
readGeneratedAttributePaths,
146+
readGeneratedAttributeStates,
131147
};

electron/git-state.cjs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,16 @@ const readRepositoryState = async (launchPath, source = { type: 'working-tree' }
7979
eagerContents: false,
8080
showWhitespace: options.showWhitespace,
8181
});
82-
const branch = (await gitOrEmpty(state.root, ['symbolic-ref', '--short', 'HEAD'])).trim() || null;
83-
return annotateGeneratedFiles({ ...state, branch });
82+
const comparisonState =
83+
source.type === 'commit' ||
84+
source.type === 'range' ||
85+
source.type === 'branch' ||
86+
source.type === 'branch-diff';
87+
const [branch, annotatedState] = await Promise.all([
88+
gitOrEmpty(state.root, ['symbolic-ref', '--short', 'HEAD']),
89+
comparisonState ? state : annotateGeneratedFiles(state),
90+
]);
91+
return { ...annotatedState, branch: branch.trim() || null };
8492
};
8593

8694
/**

0 commit comments

Comments
 (0)