Skip to content

fix(agent): recover from hallucinated file paths - #994

Open
xwil1 wants to merge 2 commits into
alibaba:mainfrom
xwil1:fix/issue-989-file-read-recovery
Open

fix(agent): recover from hallucinated file paths#994
xwil1 wants to merge 2 commits into
alibaba:mainfrom
xwil1:fix/issue-989-file-read-recovery

Conversation

@xwil1

@xwil1 xwil1 commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Summary

Addresses the soft-guard failure mode described in #989.

  • validate file_read paths against the reviewed Git ref before git show
  • classify missing-at-ref paths as deterministic structural rejections
  • after three consecutive rejected reads, use the existing file_find provider to return current-ref candidates to the same conversation
  • refund at most three invalid-only rounds so the agent has bounded room to return to the review mainline
  • preserve the tool schema and append-only conversation history, so provider prefix caching is not invalidated
  • preserve Unicode and space-containing paths in file_find by using raw NUL-delimited Git output

Root cause and behavior

file_read previously passed a model-supplied path directly to git show. A hallucinated or deleted path produced an opaque Git failure while still consuming the main-task tool budget. A persistent model could spend the rest of the review retrying nonexistent paths and leave the item partial without reviewing the changed file.

This change detects that deterministic condition before git show and, after a bounded threshold, gives the model concrete paths from the reviewed ref or tells it to return to the current diff. It does not auto-select a candidate and does not report completion on the model's behalf. A model that keeps ignoring the recovery result still ends with StopMaxRounds/partial.

The implementation also incorporates the initial automated review feedback:

  • failed candidate searches are not cached, so a later recovery batch can retry them
  • equivalent model paths such as ./src/main.go, src//main.go, and backslash-separated paths are normalized before an exact file match
  • successful candidate results remain cacheable because the reviewed ref is immutable during the run

This PR intentionally implements the smaller soft-guard direction from #989. Restricting reads to diff files or adding a review --include allowlist remains separate work.

Test footprint

The PR adds 420 lines across three focused test files:

  • internal/llmloop/path_recovery_test.go: 328 added lines
  • internal/tool/file_find_test.go: 42 added lines
  • internal/tool/file_read_test.go: 50 added lines

The main loop regression covers three rejected reads followed by a valid read, a review comment, and task_done. Additional cases cover invented, deleted, and moved paths; successful-read reset; bounded refunds; retry after a transient candidate-search error; Unicode and space-containing filenames; and equivalent non-canonical paths.

The full PR is 8 files with 704 additions and 12 deletions. A duplicate 325-line command-level test was removed during review; the retained tests exercise the same production runner and tool implementations without duplicating the harness.

Validation

  • make check
  • make test
  • make coverage (91.6%, threshold 90%)
  • go test -race ./internal/tool -run 'TestRead_GitShow_NormalizesEquivalentPaths|TestReadLines_GitShow_MissingPathIsStructuralError' -count=10
  • go test -race ./internal/llmloop -run 'TestRecoverInvalidPathsDoesNotCacheFinderErrors|TestRunPerFile_(RecoversCandidatesAfterRepeatedMissingPath|RecoversInventedDeletedAndRenamedPaths|SuccessfulReadResetsMissingPathBatch|MissingPathRefundIsBounded)' -count=10

@github-actions

github-actions Bot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

🔍 OpenCodeReview found 5 issue(s) in this PR.

  • ✅ Successfully posted inline: 5 comment(s)

Comment thread internal/llmloop/path_recovery.go
Comment thread internal/llmloop/path_recovery.go
Comment thread internal/llmloop/path_recovery.go
Comment thread internal/llmloop/path_recovery.go
Comment thread internal/tool/filereader.go
@xwil1
xwil1 force-pushed the fix/issue-989-file-read-recovery branch from a15326b to 16e238f Compare August 18, 2026 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant