Skip to content

Commit 9baf93e

Browse files
committed
Allow viewing files beyond the current patch.
1 parent 8a4472a commit 9baf93e

6 files changed

Lines changed: 435 additions & 20 deletions

File tree

electron/codex.cjs

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
const { spawn } = require('node:child_process');
2+
const { existsSync, promises: fs } = require('node:fs');
3+
const { tmpdir } = require('node:os');
4+
const { join } = require('node:path');
5+
6+
const CODEX_TIMEOUT_MS = 45_000;
7+
const CODEX_MODEL = 'gpt-5.3-codex-spark';
8+
const CODEX_REASONING_EFFORT = 'high';
9+
10+
const getCodexCommand = () => {
11+
if (process.env.CODIFF_CODEX_PATH) {
12+
return process.env.CODIFF_CODEX_PATH;
13+
}
14+
15+
for (const path of ['/opt/homebrew/bin/codex', '/usr/local/bin/codex']) {
16+
if (existsSync(path)) {
17+
return path;
18+
}
19+
}
20+
21+
return 'codex';
22+
};
23+
24+
const oneLine = (value, fallback = '') =>
25+
(typeof value === 'string' ? value : fallback).replace(/\s+/g, ' ').trim();
26+
27+
const truncate = (value, maxLength) => {
28+
if (value.length <= maxLength) {
29+
return value;
30+
}
31+
32+
return `${value.slice(0, maxLength)}\n...[truncated]`;
33+
};
34+
35+
const cleanText = (value, fallback = '') =>
36+
oneLine(value, fallback).replace(/\s*\.{3}\[truncated]$/i, '');
37+
38+
const normalizeEnum = (value, allowed, fallback) => (allowed.has(value) ? value : fallback);
39+
40+
const parseJSONMessage = (message) => {
41+
try {
42+
return JSON.parse(message);
43+
} catch {
44+
const match = message.match(/\{[\s\S]*\}/);
45+
if (!match) {
46+
throw new Error('Codex did not return JSON.');
47+
}
48+
49+
return JSON.parse(match[0]);
50+
}
51+
};
52+
53+
const runCodex = async (
54+
repoRoot,
55+
prompt,
56+
schema,
57+
outputName = 'codex-output.json',
58+
timeoutMessage = 'Codex timed out.',
59+
) => {
60+
const directory = await fs.mkdtemp(join(tmpdir(), 'codiff-codex-'));
61+
const outputPath = join(directory, outputName);
62+
const schemaPath = join(directory, 'schema.json');
63+
await fs.writeFile(schemaPath, JSON.stringify(schema), 'utf8');
64+
65+
return await new Promise((resolve, reject) => {
66+
let stderr = '';
67+
let stdinError = null;
68+
let stdout = '';
69+
let finished = false;
70+
71+
const child = spawn(
72+
getCodexCommand(),
73+
[
74+
'exec',
75+
'-m',
76+
CODEX_MODEL,
77+
'-c',
78+
`model_reasoning_effort="${CODEX_REASONING_EFFORT}"`,
79+
'--cd',
80+
repoRoot,
81+
'--sandbox',
82+
'read-only',
83+
'--ephemeral',
84+
'--ignore-rules',
85+
'--color',
86+
'never',
87+
'--output-schema',
88+
schemaPath,
89+
'--output-last-message',
90+
outputPath,
91+
'-',
92+
],
93+
{
94+
env: process.env,
95+
stdio: ['pipe', 'pipe', 'pipe'],
96+
},
97+
);
98+
99+
const timer = setTimeout(() => {
100+
if (!finished) {
101+
finished = true;
102+
child.kill('SIGTERM');
103+
reject(new Error(timeoutMessage));
104+
}
105+
}, CODEX_TIMEOUT_MS);
106+
107+
child.stdout.on('data', (chunk) => {
108+
stdout += chunk.toString();
109+
});
110+
child.stderr.on('data', (chunk) => {
111+
stderr += chunk.toString();
112+
});
113+
child.stdin.on('error', (error) => {
114+
stdinError = error;
115+
});
116+
child.on('error', (error) => {
117+
finished = true;
118+
clearTimeout(timer);
119+
reject(error);
120+
});
121+
child.on('close', async (code) => {
122+
if (finished) {
123+
return;
124+
}
125+
126+
finished = true;
127+
clearTimeout(timer);
128+
129+
if (code !== 0) {
130+
reject(
131+
new Error(
132+
oneLine(stderr || stdout || stdinError?.message, `Codex exited with code ${code}.`),
133+
),
134+
);
135+
return;
136+
}
137+
138+
try {
139+
const message = await fs.readFile(outputPath, 'utf8');
140+
resolve(message);
141+
} catch {
142+
resolve(stdout);
143+
}
144+
});
145+
146+
child.stdin.end(prompt, () => {});
147+
}).finally(() => fs.rm(directory, { force: true, recursive: true }).catch(() => {}));
148+
};
149+
150+
module.exports = {
151+
cleanText,
152+
normalizeEnum,
153+
oneLine,
154+
parseJSONMessage,
155+
runCodex,
156+
truncate,
157+
};

electron/review-assist.cjs

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
const { parseJSONMessage, runCodex, truncate } = require('./codex.cjs');
2+
3+
const MAX_PATCH_CHARS = 24_000;
4+
const MAX_OTHER_FILES = 40;
5+
6+
const reviewAssistantSchema = {
7+
additionalProperties: false,
8+
properties: {
9+
reply: { type: 'string' },
10+
version: { const: 1, type: 'number' },
11+
},
12+
required: ['version', 'reply'],
13+
type: 'object',
14+
};
15+
16+
const getFileDigest = (file) => ({
17+
oldPath: file.oldPath,
18+
path: file.path,
19+
status: file.status,
20+
summaries: file.sections
21+
.map((section) => section.summary?.reason)
22+
.filter((summary) => typeof summary === 'string' && summary.trim()),
23+
});
24+
25+
const buildReviewAssistantInput = (state, request) => {
26+
const comment = request?.comment ?? {};
27+
const file = state.files.find((candidate) => candidate.path === comment.filePath);
28+
const section = file?.sections.find((candidate) => candidate.id === comment.sectionId);
29+
30+
return {
31+
comment: {
32+
body: typeof comment.body === 'string' ? comment.body : '',
33+
filePath: comment.filePath,
34+
lineNumber: comment.lineNumber,
35+
side: comment.side,
36+
},
37+
focus: file
38+
? {
39+
file: getFileDigest(file),
40+
patchExcerpt: section
41+
? truncate(section.patch || section.summary?.reason || '', MAX_PATCH_CHARS)
42+
: 'No patch context available.',
43+
section: section
44+
? {
45+
binary: section.binary,
46+
kind: section.kind,
47+
loadState: section.loadState,
48+
summary: section.summary?.reason,
49+
}
50+
: null,
51+
}
52+
: null,
53+
nearbyFiles: state.files
54+
.filter((candidate) => candidate.path !== comment.filePath)
55+
.slice(0, MAX_OTHER_FILES)
56+
.map(getFileDigest),
57+
root: state.root,
58+
source: state.source,
59+
walkthroughNote: request?.walkthroughNote ?? null,
60+
};
61+
};
62+
63+
const buildReviewAssistantPrompt = (state, request) => `You are Codex inside Codiff.
64+
65+
A human reviewer wrote a rough inline review note and clicked Ask Codex.
66+
Reply as a concise assistant in the same inline conversation.
67+
Use only the repository change digest below; do not inspect the repository or run shell commands.
68+
If there is walkthrough context, use it as review orientation, not as proof.
69+
You are the code-review expert in this conversation, so explain the change directly.
70+
71+
Your job:
72+
- Turn vague unease into coherent, actionable review feedback.
73+
- If the note asks "why", explain why this change is needed based on the diff.
74+
- If useful, suggest a clearer review comment the human could use.
75+
- Prefer questions and concrete risks over accusations.
76+
- Do not hedge. Avoid words and phrases like "appears", "seems", "might", "likely", "probably", "I think", "I suspect", and "the intent".
77+
- Say "This change introduces...", "This change moves...", or "This is needed because..." instead of "This change appears to...".
78+
- If the diff does not provide enough evidence, state the concrete uncertainty after the explanation.
79+
- Do not say the change is correct unless the diff proves it.
80+
- Do not invent bugs, unstated requirements, or files outside the digest.
81+
- Keep the reply under 180 words.
82+
- Markdown is allowed.
83+
84+
Repository change digest:
85+
${JSON.stringify(buildReviewAssistantInput(state, request), null, 2)}
86+
`;
87+
88+
const cleanReply = (value, fallback = '') =>
89+
(typeof value === 'string' ? value : fallback).replace(/\n{3,}/g, '\n\n').trim();
90+
91+
const normalizeReviewAssistantReply = (input) => ({
92+
reply: cleanReply(input?.reply, 'Codex could not produce a useful reply.'),
93+
version: 1,
94+
});
95+
96+
const readReviewAssistantReply = async (state, request) => {
97+
try {
98+
const response = await runCodex(
99+
state.root,
100+
buildReviewAssistantPrompt(state, request),
101+
reviewAssistantSchema,
102+
'review-assistant.json',
103+
'Codex review reply timed out.',
104+
);
105+
const parsed = parseJSONMessage(response);
106+
107+
return {
108+
reply: normalizeReviewAssistantReply(parsed).reply,
109+
status: 'ready',
110+
};
111+
} catch (error) {
112+
if (error && error.code === 'ENOENT') {
113+
return {
114+
reason:
115+
'Codex is not installed locally. Install and use Codex, then ask from this comment again.',
116+
status: 'unavailable',
117+
};
118+
}
119+
120+
return {
121+
reason: error instanceof Error ? error.message : String(error),
122+
status: 'unavailable',
123+
};
124+
}
125+
};
126+
127+
module.exports = {
128+
buildReviewAssistantInput,
129+
buildReviewAssistantPrompt,
130+
normalizeReviewAssistantReply,
131+
readReviewAssistantReply,
132+
reviewAssistantSchema,
133+
};

electron/review-assist.test.ts

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
import { createRequire } from 'node:module';
2+
import { expect, test } from 'vite-plus/test';
3+
4+
const require = createRequire(import.meta.url);
5+
const { buildReviewAssistantInput, normalizeReviewAssistantReply } =
6+
require('./review-assist.cjs') as {
7+
buildReviewAssistantInput: (
8+
state: unknown,
9+
request: unknown,
10+
) => {
11+
comment: {
12+
body: string;
13+
filePath: string;
14+
lineNumber: number;
15+
side: string;
16+
};
17+
focus: {
18+
patchExcerpt: string;
19+
} | null;
20+
walkthroughNote: unknown;
21+
};
22+
normalizeReviewAssistantReply: (input: unknown) => {
23+
reply: string;
24+
version: 1;
25+
};
26+
};
27+
28+
test('builds focused inline review assistant context', () => {
29+
const input = buildReviewAssistantInput(
30+
{
31+
files: [
32+
{
33+
path: 'src/state.ts',
34+
sections: [
35+
{
36+
binary: false,
37+
id: 'src/state.ts:unstaged',
38+
kind: 'unstaged',
39+
patch: '+const duplicated = true;',
40+
summary: {
41+
reason: 'State handling changed.',
42+
},
43+
},
44+
],
45+
status: 'modified',
46+
},
47+
],
48+
root: '/repo',
49+
source: {
50+
type: 'working-tree',
51+
},
52+
},
53+
{
54+
comment: {
55+
body: 'this feels risky, why is this needed?',
56+
filePath: 'src/state.ts',
57+
lineNumber: 1,
58+
sectionId: 'src/state.ts:unstaged',
59+
side: 'additions',
60+
},
61+
walkthroughNote: {
62+
action: 'review',
63+
context: 'Check whether state stays synchronized.',
64+
groupReason: 'Shared state first.',
65+
groupTitle: 'Review carefully',
66+
impact: 'wide',
67+
reason: 'State contract affects multiple paths.',
68+
},
69+
},
70+
);
71+
72+
expect(input.comment.body).toBe('this feels risky, why is this needed?');
73+
expect(input.focus?.patchExcerpt).toContain('+const duplicated = true;');
74+
expect(input.walkthroughNote).toMatchObject({
75+
context: 'Check whether state stays synchronized.',
76+
});
77+
});
78+
79+
test('normalizes review assistant markdown without flattening it', () => {
80+
expect(
81+
normalizeReviewAssistantReply({
82+
reply: 'Likely reason:\n\n- Keep state local\n\nSuggested comment: **Why here?**',
83+
version: 1,
84+
}),
85+
).toEqual({
86+
reply: 'Likely reason:\n\n- Keep state local\n\nSuggested comment: **Why here?**',
87+
version: 1,
88+
});
89+
});

0 commit comments

Comments
 (0)