Make turn diff tracker multi-env aware#26433
Conversation
6a70fc8 to
320d4a4
Compare
320d4a4 to
776fcab
Compare
…iff-shared-multi-env
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e0d32deb6d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review this |
|
Codex Review: Didn't find any major issues. Breezy! ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
jif-oai
left a comment
There was a problem hiding this comment.
First 2 comments are the real blockers IMO. The last one is just an opinion
|
|
||
| async fn turn_diff_display_roots(turn_context: &TurnContext) -> Vec<(String, PathBuf)> { | ||
| let mut display_roots = Vec::new(); | ||
| for turn_environment in &turn_context.environments.turn_environments { |
There was a problem hiding this comment.
Can we resolve the display root lazily for the environment when its first delta is tracked? Otherwise this feels a bit racy/latency if we have slow secondary remote
Why
Turn diffs were tracked as one flat set of absolute paths. In multi-environment turns, local and remote environments can report the same path while representing different filesystems, so a single path key can collapse distinct changes or attribute them to the wrong environment.
The environment name is NOT included in the generated unified diff. This can come later.