diff: render a supplied unified diff, and wire codex file changes to it - #5
Merged
Conversation
codex reports an edit as a ready-made unified hunk and never sends the whole file, so there was no before/after pair to build a fileDiff from. That looked like a design problem and was not one: both renderers already worked FROM unified text. Each computed it on its first line and spent the rest of the function parsing it back. So they now take the unified text and style it, and fileDiff.unifiedText is the one place that decides whether to use a supplied diff or compute one. No reconstruction, no second renderer, and both diff styles work on both backends. Behaviour-preserving for claude: the same string reaches the same styling loop, and the existing renderer tests pass unchanged. The three codex change kinds put different things in the same `diff` field, all confirmed against the live CLI: add and delete carry the file's content, update carries the hunk. Reading that wrong renders a deletion as an addition. Display paths are trimmed against the working root the agent reported on thread/started, not os.Getwd. Those are equal by convention only, and a live test rooted in a temp dir rendered the full absolute path until this was fixed — the kind of assumption that stays invisible in production.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
codex reports an edit as a ready-made unified hunk and never sends the whole file, so there was no before/after pair to build a
fileDifffrom.That looked like a design problem. It was not one: both renderers already worked from unified text.
diff.goanddiff_split.goeach calledudiff.Unified(old, new)on their first line and spent the rest of the function parsing the result back to style it. Computing it was only ever step one.So they now take the unified text and style it, and
fileDiff.unifiedText()is the single place that decides whether to use a supplied diff or compute one. No reconstruction, no second renderer, both diff styles working on both backends.Behaviour-preserving for claude
The same string reaches the same styling loop. The existing renderer tests pass unchanged, which is the check that matters here.
The three codex change kinds
All confirmed against the live CLI, because they put different things in the same
difffield:diffholdsaddnewdeleteoldupdateunifiedReading that wrong renders a deletion as an addition.
One bug a live test caught
Display paths are trimmed against the working root the agent reported on
thread/started, notos.Getwd. Those are equal by convention in production, so the difference is invisible — until a live test rooted in a temp dir rendered the full absolute path across two wrapped lines.Verified
go vet, full suite, andCATHODE_CODEX_LIVE=1driving a real edit end to end:The new test is discriminating: removing the diff path fails it.
Not verified: the TUI under
-backend codex.