Skip to content

Commit 5842265

Browse files
committed
Change docs writer strategy from git branches to candidate changes directories
1 parent 9d4554a commit 5842265

2 files changed

Lines changed: 142 additions & 65 deletions

File tree

src/__tests__/docs-writer.e2e.test.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import { describe, expect, it } from 'bun:test'
2020
import {
2121
cleanupDraftedDocsChange,
2222
cleanupPlannedDocsTaskResult,
23-
materializeDocsChangeFromPatch,
23+
materializeDocsChange,
2424
planDocsChangesForTask,
2525
} from '../docs-writer'
2626

@@ -142,7 +142,7 @@ describe('docs writer e2e', () => {
142142
const repoDir = createTestRepo()
143143
let completed = false
144144
let plannedResult: Awaited<ReturnType<typeof planDocsChangesForTask>> | null = null
145-
let materializedDraft: ReturnType<typeof materializeDocsChangeFromPatch> | null = null
145+
let materializedDraft: ReturnType<typeof materializeDocsChange> | null = null
146146

147147
try {
148148
console.log(`Docs writer test repo: ${repoDir}`)
@@ -178,15 +178,16 @@ describe('docs writer e2e', () => {
178178

179179
expect(accepted.accepted).toBe(true)
180180
expect(accepted.overfit).toBe(false)
181-
expect(accepted.branchName).toBeString()
182-
expect(accepted.commitSha).toBeString()
183-
expect(accepted.patchText).toContain('docs/')
181+
expect(accepted.fileChanges).toBeDefined()
182+
expect(accepted.fileChanges?.length ?? 0).toBeGreaterThan(0)
183+
const touchedDocsPath = accepted.fileChanges?.some((c) => c.path.startsWith('docs/'))
184+
expect(touchedDocsPath).toBe(true)
184185
expect(accepted.diffText).toContain('APP_MODE')
185186

186-
materializedDraft = materializeDocsChangeFromPatch(repoDir, accepted.patchText || '')
187+
materializedDraft = materializeDocsChange(repoDir, accepted.fileChanges || [])
187188
expect(materializedDraft).toBeDefined()
188189
if (!materializedDraft) {
189-
throw new Error('failed to materialize accepted docs patch')
190+
throw new Error('failed to materialize accepted docs change')
190191
}
191192
expect(materializedDraft.diffText).toContain('APP_CACHE_DIR')
192193

@@ -200,8 +201,7 @@ describe('docs writer e2e', () => {
200201

201202
expect(rejected.accepted).toBe(false)
202203
expect(rejected.overfit || rejected.reason.toLowerCase().includes('overfit')).toBe(true)
203-
expect(rejected.branchName).toBeUndefined()
204-
expect(rejected.commitSha).toBeUndefined()
204+
expect(rejected.fileChanges).toBeUndefined()
205205

206206
const status = execSync('git status --short', {
207207
cwd: repoDir,

src/docs-writer.ts

Lines changed: 133 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { execFileSync, execSync } from 'child_process'
1+
import { execSync } from 'child_process'
22
import fs from 'fs'
33
import os from 'os'
44
import path from 'path'
@@ -31,16 +31,20 @@ export interface DraftedDocsChange {
3131
diffText: string
3232
}
3333

34+
export interface FileChange {
35+
path: string
36+
content?: string
37+
delete?: boolean
38+
}
39+
3440
export interface PlannedDocsChange {
3541
text: string
3642
priority: number
3743
source: SuggestionSource
3844
accepted: boolean
3945
reason: string
4046
overfit: boolean
41-
branchName?: string
42-
commitSha?: string
43-
patchText?: string
47+
fileChanges?: FileChange[]
4448
diffText?: string
4549
}
4650

@@ -51,6 +55,16 @@ export interface PlannedDocsTaskResult {
5155
candidates: PlannedDocsChange[]
5256
}
5357

58+
const ALLOWED_DOCS_ROOTS = ['docs/', 'AGENTS.md', 'CLAUDE.md']
59+
60+
function isAllowedDocsPath(relPath: string): boolean {
61+
const normalized = path.posix.normalize(relPath.replace(/\\/g, '/'))
62+
if (normalized.startsWith('..') || path.isAbsolute(normalized)) return false
63+
return ALLOWED_DOCS_ROOTS.some(
64+
(root) => normalized === root || normalized.startsWith(root),
65+
)
66+
}
67+
5468
export const CODING_AGENT_SUGGESTIONS_FILE = 'evalbuff-coding-suggestions.json'
5569
const DOCS_WRITER_PLAN_FILE = 'evalbuff-doc-changes-plan.json'
5670
export const DEFAULT_DOC_SUGGESTION_PRIORITY_FLOOR = 40
@@ -68,8 +82,8 @@ const DocsWriterPlanEntrySchema = z.object({
6882
accepted: z.boolean(),
6983
reason: z.string(),
7084
overfit: z.boolean().default(false),
71-
branchName: z.string().optional(),
72-
commitSha: z.string().optional(),
85+
scratchDir: z.string().optional(),
86+
deletedPaths: z.array(z.string()).default([]),
7387
})
7488

7589
const DocsWriterPlanSchema = z.object({
@@ -287,20 +301,25 @@ You must reject a suggestion instead of editing docs when ANY of the following i
287301
288302
## Implementation workflow
289303
304+
You will write each accepted candidate's changes into a per-candidate SCRATCH
305+
DIRECTORY. Do NOT modify the real docs/, AGENTS.md, or CLAUDE.md at any point.
306+
Do NOT run any git commands.
307+
290308
1. Read the current docs first.
291309
2. Immediately create \`${DOCS_WRITER_PLAN_FILE}\` in the repo root with one entry per suggestion. Start with every entry marked \`accepted: false\` and a placeholder \`reason\`. Update this file as you make decisions. Do not wait until the end to create it.
292310
3. Evaluate every suggestion and decide whether it should be accepted.
293-
4. For each accepted suggestion:
294-
- Run \`git checkout --quiet ${baseCommit}\`
295-
- Run \`git checkout -B evalbuff-doc-change-N\`
296-
- Implement exactly one independent docs change.
297-
- Keep it general, reusable, and not overfit.
298-
- Run \`git add docs AGENTS.md CLAUDE.md\`
299-
- Run \`git commit -m "evalbuff: doc change N"\`
300-
- Record the branch name and commit SHA in \`${DOCS_WRITER_PLAN_FILE}\`
301-
- Run \`git checkout --quiet ${baseCommit}\` before moving to the next suggestion so branches stay independent.
302-
5. For each rejected suggestion, make no docs changes and record the rejection reason in \`${DOCS_WRITER_PLAN_FILE}\`.
303-
6. Before finishing, ensure HEAD is back at \`${baseCommit}\`.
311+
4. For each ACCEPTED suggestion N (N starts at 1):
312+
- Work inside \`evalbuff-candidates/candidate-N/\`. Mirror the real docs tree there.
313+
- To MODIFY an existing docs file, first copy it into the scratch dir, then edit the copy. Example for \`docs/patterns/foo.md\`:
314+
* \`mkdir -p evalbuff-candidates/candidate-N/docs/patterns\`
315+
* \`cp docs/patterns/foo.md evalbuff-candidates/candidate-N/docs/patterns/foo.md\`
316+
* edit \`evalbuff-candidates/candidate-N/docs/patterns/foo.md\`
317+
- To MODIFY \`AGENTS.md\` or \`CLAUDE.md\`, copy to \`evalbuff-candidates/candidate-N/AGENTS.md\` (or \`CLAUDE.md\`) and edit the copy.
318+
- To CREATE a new file, write it directly at \`evalbuff-candidates/candidate-N/<real-path>\`.
319+
- To DELETE a file, do NOT delete the real file. Add its path to the candidate's \`deletedPaths\` list in \`${DOCS_WRITER_PLAN_FILE}\`.
320+
- Record the scratch dir in the plan entry as \`scratchDir: "evalbuff-candidates/candidate-N"\`.
321+
- Keep the change general, reusable, and not overfit.
322+
5. For each REJECTED suggestion, make no changes and record the rejection reason in \`${DOCS_WRITER_PLAN_FILE}\`.
304323
305324
## Required output shape
306325
@@ -314,25 +333,29 @@ You must reject a suggestion instead of editing docs when ANY of the following i
314333
"accepted": true,
315334
"reason": "Why this is broadly useful and not overfit",
316335
"overfit": false,
317-
"branchName": "evalbuff-doc-change-1",
318-
"commitSha": "abc123"
336+
"scratchDir": "evalbuff-candidates/candidate-1",
337+
"deletedPaths": []
319338
},
320339
{
321340
"text": "another suggestion",
322341
"priority": 20,
323342
"source": "agent",
324343
"accepted": false,
325344
"reason": "Rejected because this is overfit to one task",
326-
"overfit": true
345+
"overfit": true,
346+
"deletedPaths": []
327347
}
328348
]
329349
}
330350
\`\`\`
331351
332352
Rules:
333-
- ONLY modify docs/, AGENTS.md, or CLAUDE.md.
353+
- ONLY write files under \`evalbuff-candidates/candidate-N/docs/\`, \`evalbuff-candidates/candidate-N/AGENTS.md\`, or \`evalbuff-candidates/candidate-N/CLAUDE.md\`.
354+
- Do NOT modify the real docs/, AGENTS.md, or CLAUDE.md.
334355
- Do NOT modify source code.
335-
- Every accepted branch must stand on its own when diffed against \`${baseCommit}\`.
356+
- Do NOT run git commands.
357+
- Mirror real file paths EXACTLY under the scratch dir. The relative layout inside \`evalbuff-candidates/candidate-N/\` must match where the files live in the repo.
358+
- Each candidate's scratch dir must stand on its own. Do not share files between candidates — if two candidates both touch \`AGENTS.md\`, each candidate copies it into its own scratch dir independently.
336359
- Keep AGENTS.md changes limited to doc-index maintenance or factual corrections.
337360
- Verify referenced helpers, scripts, file paths, and symbols against the codebase before documenting them.
338361
- Do not document aspirational behavior.
@@ -358,29 +381,28 @@ Rules:
358381
const candidates: PlannedDocsChange[] = []
359382
for (const entry of parsed.data.candidates) {
360383
const planned: PlannedDocsChange = {
361-
...entry,
384+
text: entry.text,
385+
priority: entry.priority,
386+
source: entry.source,
387+
accepted: entry.accepted,
388+
reason: entry.reason,
389+
overfit: entry.overfit,
362390
}
363391

364-
if (entry.accepted && entry.branchName) {
365-
try {
366-
const patchText = execFileSync(
367-
'git',
368-
['diff', '--binary', `${baseCommit}..${entry.branchName}`, '--', 'docs', 'AGENTS.md', 'CLAUDE.md'],
369-
{ cwd: repoDir, encoding: 'utf-8' },
370-
)
371-
execFileSync('git', ['checkout', '--quiet', entry.branchName], { cwd: repoDir, stdio: 'ignore' })
372-
const before = getDocsSnapshot(repoPath)
373-
const after = getDocsSnapshot(repoDir)
374-
const diffText = computeDocsDiffText(before, after)
375-
execFileSync('git', ['checkout', '--quiet', baseCommit], { cwd: repoDir, stdio: 'ignore' })
376-
planned.patchText = patchText
377-
planned.diffText = diffText
378-
} catch {
392+
if (entry.accepted) {
393+
const fileChanges = buildFileChangesFromScratch(
394+
repoDir,
395+
entry.scratchDir,
396+
entry.deletedPaths,
397+
)
398+
if (fileChanges.length === 0) {
379399
planned.accepted = false
380-
planned.reason = `Rejected because the committed docs change could not be extracted: ${planned.reason}`
381-
planned.overfit = planned.overfit || false
382-
delete planned.branchName
383-
delete planned.commitSha
400+
planned.reason = `Rejected because the docs writer produced no scratch-dir files for this candidate: ${planned.reason}`
401+
} else {
402+
planned.fileChanges = fileChanges
403+
const before = getDocsSnapshot(repoPath)
404+
const after = applyFileChangesToSnapshot(before, fileChanges)
405+
planned.diffText = computeDocsDiffText(before, after)
384406
}
385407
}
386408

@@ -403,9 +425,63 @@ export function cleanupPlannedDocsTaskResult(result: PlannedDocsTaskResult): voi
403425
}
404426
}
405427

406-
export function materializeDocsChangeFromPatch(
428+
function buildFileChangesFromScratch(
429+
repoDir: string,
430+
scratchDir: string | undefined,
431+
deletedPaths: string[],
432+
): FileChange[] {
433+
const changes: FileChange[] = []
434+
const seen = new Set<string>()
435+
436+
if (scratchDir) {
437+
const scratchRoot = path.join(repoDir, scratchDir)
438+
if (fs.existsSync(scratchRoot) && fs.statSync(scratchRoot).isDirectory()) {
439+
const walk = (dir: string, prefix: string): void => {
440+
for (const entry of fs.readdirSync(dir, { withFileTypes: true })) {
441+
const fullPath = path.join(dir, entry.name)
442+
const rel = prefix ? path.posix.join(prefix, entry.name) : entry.name
443+
if (entry.isDirectory()) {
444+
walk(fullPath, rel)
445+
} else if (entry.isFile()) {
446+
if (!isAllowedDocsPath(rel)) continue
447+
const content = fs.readFileSync(fullPath, 'utf-8')
448+
changes.push({ path: rel, content })
449+
seen.add(rel)
450+
}
451+
}
452+
}
453+
walk(scratchRoot, '')
454+
}
455+
}
456+
457+
for (const p of deletedPaths) {
458+
const normalized = path.posix.normalize(p.replace(/\\/g, '/'))
459+
if (!isAllowedDocsPath(normalized) || seen.has(normalized)) continue
460+
changes.push({ path: normalized, delete: true })
461+
seen.add(normalized)
462+
}
463+
464+
return changes
465+
}
466+
467+
function applyFileChangesToSnapshot(
468+
before: Record<string, string>,
469+
fileChanges: FileChange[],
470+
): Record<string, string> {
471+
const after: Record<string, string> = { ...before }
472+
for (const change of fileChanges) {
473+
if (change.delete) {
474+
delete after[change.path]
475+
} else if (change.content !== undefined) {
476+
after[change.path] = change.content
477+
}
478+
}
479+
return after
480+
}
481+
482+
export function materializeDocsChange(
407483
repoPath: string,
408-
patchText: string,
484+
fileChanges: FileChange[],
409485
): DraftedDocsChange | null {
410486
const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'evalbuff-docs-materialized-'))
411487
const repoDir = path.join(tempDir, 'repo')
@@ -421,19 +497,20 @@ export function materializeDocsChangeFromPatch(
421497
copyDocsIntoRepo(repoPath, repoDir)
422498

423499
const before = getDocsSnapshot(repoDir)
424-
const patchPath = path.join(tempDir, 'docs-change.patch')
425-
fs.writeFileSync(patchPath, patchText.endsWith('\n') ? patchText : patchText + '\n')
426500

427-
try {
428-
execFileSync('git', ['apply', '--whitespace=nowarn', '--allow-empty', patchPath], {
429-
cwd: repoDir,
430-
stdio: 'ignore',
431-
})
432-
} catch {
433-
execFileSync('git', ['apply', '--3way', '--whitespace=nowarn', patchPath], {
434-
cwd: repoDir,
435-
stdio: 'ignore',
436-
})
501+
for (const change of fileChanges) {
502+
if (!isAllowedDocsPath(change.path)) continue
503+
const fullPath = path.join(repoDir, change.path)
504+
if (change.delete) {
505+
try {
506+
fs.rmSync(fullPath, { force: true })
507+
} catch {
508+
// ignore
509+
}
510+
} else if (change.content !== undefined) {
511+
fs.mkdirSync(path.dirname(fullPath), { recursive: true })
512+
fs.writeFileSync(fullPath, change.content)
513+
}
437514
}
438515

439516
const after = getDocsSnapshot(repoDir)

0 commit comments

Comments
 (0)