Ground explicit file edits in real file contents#34
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08452d0fe8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| var ( | ||
| fileEditVerbPattern = regexp.MustCompile(`(?i)\b(modify|edit|update|rewrite|replace|append|prepend|insert|remove|delete|change)\b`) | ||
| pathTokenPattern = regexp.MustCompile(`(?:^|[\s"'` + "`" + `])((?:~|\.{1,2}|/)[^\s"'` + "`" + `,;:()]+|(?:\.[A-Za-z0-9_-]+)|(?:[A-Za-z0-9._-]+\.[A-Za-z0-9._-]+))`) |
There was a problem hiding this comment.
The file-target regex treats any dotted token as a path, so prompts like “update Node to 20.11.1” or “update to v1.2” will be classified as explicit file edits and trigger read_file repair/refusal even when no file path was requested. Because needsFileGrounding runs on every command/script response, this can block valid non-file workflows that include common version strings.
Useful? React with 👍 / 👎.
| if err := json.Unmarshal(call.Arguments, &args); err != nil || strings.TrimSpace(args.Path) == "" { | ||
| continue | ||
| } | ||
| for _, target := range targets { | ||
| if targetMatchesReadPath(target, args.Path, cwd, home) { |
There was a problem hiding this comment.
A target is considered grounded solely from read_file call arguments, without checking whether the tool actually returned file contents. If read_file is called with an unreadable/missing path (permission error, bad resolution, etc.), the grounding check still passes and the model may emit a mutating script despite never seeing real file contents, which breaks the fail-closed guarantee described in this change.
Useful? React with 👍 / 👎.
1 participant
Non-technical summary
This fixes a reliability gap in
intent's natural-language mode for file edits. When a user names a real file and asksintentto change it, the model can no longer skip straight to an ungrounded snippet that never touches the file.intentnow forces that workflow to read the target file first or fail closed.That matters now because issue #22 is a direct break in the product promise: a request to modify a known file should yield an executable edit grounded in the file's actual contents, not pseudocode.
Technical summary
read_fileon the named target before returning a command or scriptread_filetool call; if it still does not, return a refusal instead of surfacing the ungrounded editinternal/model/prompt.goso explicit file edits requireread_file(path) FIRSTinternal/engine/engine_test.gofor both the repair path and the fail-closed path, and updatedocs/SPEC.mdto document the contractPATH="/opt/homebrew/bin:$PATH" go test ./internal/engine ./internal/model ./internal/cli,PATH="/opt/homebrew/bin:$PATH" go test ./...,PATH="/opt/homebrew/bin:$PATH" go vet ./...,PATH="/opt/homebrew/bin:$PATH" make buildAdditional notes
Trade-off: the detector is intentionally narrow and keys off explicit edit verbs plus file-like targets. It improves the real bug path without trying to infer every possible mutation request shape in one increment.
Deferred: broader grounding around implicit targets, multi-file rewrite flows, or richer path resolution heuristics remains separate work.
Remaining gap: this ensures explicit file-edit requests must read the named file before proposing a mutation, but it does not redesign the wider prompt/tooling strategy for all mutation tasks.