fix(agent): list deleted files under their own path in the change list

[Trainingcqy]

Description

The default plan and main task prompts both carry an <other_changed_files> block that lists the other changed files in one place. Deleted files that pass the static filters are listed there too, as the comment on fileDecision.retained states: "they carry no new content to review but still belong to the change-file list the prompts show."

Each entry in the block is produced by formatDiffEntry, which prints d.NewPath. A deleted file's NewPath is the sentinel /dev/null and its real path is only in OldPath, so every deleted file is listed as the same /dev/null. Below is an excerpt from the main task request when ocr review --commit HEAD runs on a commit that deletes helper.go and other.go and modifies main.go:

<other_changed_files>
DELETED   /dev/null (+0/-3)
DELETED   /dev/null (+0/-3)
</other_changed_files>

The model learns only that two files were deleted, with no way to tell which two. In this example main.go still calls Helper(), yet nothing in the prompt lets the model connect that call to the deleted helper.go.

Fix: formatDiffEntry now prints effectivePath(d), a helper the package already has. The block in the plan and main task prompts is built from the same value, so the fix applies to both. After the fix, the same request contains:

<other_changed_files>
DELETED   helper.go (+0/-3)
DELETED   other.go (+0/-3)
</other_changed_files>

The file list used for grouping is also produced by formatDiffEntry, but it contains no deleted files, so it is unaffected.

The deletion fixture in TestBuildChangeFilesExceptGroup was written as NewPath: "removed.go", a shape the parser never produces, so the test did not catch this problem. It now uses the shape the parser actually produces.

Out of scope: file_read_diff still cannot read a deleted file's diff. The model can now see these file names, and a query for one returns "diff not found", the same result as querying /dev/null before the fix.

Type of Change

• Bug fix (non-breaking change that fixes an issue)
• New feature (non-breaking change that adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Refactoring (no functional changes)
• Documentation update
• CI / Build / Tooling

How Has This Been Tested?

• With the updated test kept and the production change reverted, TestBuildChangeFilesExceptGroup fails:

  agent_test.go:952: output missing "DELETED   removed.go (+0/-30)"
  agent_test.go:959: a deletion must be listed under its own path, not the /dev/null sentinel; got:
      ADDED   helper.go (+12/-0)
      DELETED   /dev/null (+0/-30)
      RENAMED   renamed.go (+5/-5)
  

• End to end: I built binaries from main and from this branch and ran ocr review --commit HEAD against a local stub that records requests and is compatible with the OpenAI API. The two excerpts above come from the main task requests it recorded. The change in this example is small, so the plan phase was skipped.

• make test passes locally

• Manual testing (describe below)

Checklist

• My code follows the project's coding style (go fmt, go vet)
• I have performed a self-review of my code
• I have added tests that prove my fix is effective or my feature works
• New and existing unit tests pass locally with my changes
• I have updated the documentation accordingly (if applicable)
• I have signed the CLA
• I did not use AI/LLM to create this PR, or I disclosed the tool/model below and reviewed its output; I did not attribute commits to AI and will answer maintainer questions and review comments myself without AI/LLM.

AI/LLM disclosure, Claude Code (Claude Opus 5.5) was used to investigate the failure, prepare the changes, and draft this PR.

[github-actions]

✅ OpenCodeReview: Review complete: 0 finding(s) across 1 selected item(s).

[lizhengfeng101]

Thanks, the fix looks good to me.

One optional follow-up: now that the change list shows DELETED helper.go, the model may call file_read_diff on helper.go to see what was removed. That lookup still fails because injectDiffMap (internal/agent/agent.go:617) skips the /dev/null sentinel and keys the map by NewPath:

if d.NewPath != "/dev/null" {
	m[d.NewPath] = d.Diff
}

Keying it by effectivePath(d) instead would make the deleted file's diff readable too. Happy to take this here or in a separate PR, whichever you prefer.

[Trainingcqy]

Thanks for the suggestion. I opened #1698 separately to key the map by effectivePath(d) as you suggested, and this PR stays as it is.