Skip to content

Commit 9ef9d59

Browse files
authored
fix: emit real unified diff for remote workspace pending changes (#429) (#430)
* fix: emit real unified diff for remote workspace pending changes RemoteWorkspaceBackend::build_full_file_diff() was not a unified diff. It dumped the entire old content as '-' lines and the entire new content as '+' lines regardless of how small the actual change was. Agents that called workspace_git_diff to verify a surgical workspace_edit saw their whole file marked for removal and re-addition, and consistently misinterpreted this as 'whole-file line-ending weather' — concluding the edit had nuked the file even though the actual write to GitHub was correct. Real example: the world-of-wordpress agent hit this pattern three cycles in a row on the Observatory page in run 26050485830. A one-line <h2> heading change showed up as '@@ -1,250 +1,278 @@' followed by every line of the file removed then re-added, and the agent reverted its (correct) edit to avoid committing a noisy rewrite. This change replaces the fake whole-file diff with a real unified-diff generator: - Common-prefix/suffix trimming on the line arrays, so the O(ND) core only runs on the actually-different middle window. Typical 'surgical edit in large file' cases finish in O(N) instead of O(N^2). - Myers' O(ND) diff algorithm with V-array trace, walked backwards to reconstruct an ordered edit script. - Hunk grouping with three lines of context on each side, exactly the format git diff --no-color produces. - Proper @@ -start,count +start,count @@ markers, including the 0,0 edge cases for pure addition and pure deletion. Verified on seven representative cases: - two-line surgical (replaces phantom whole-file replace with single context line + one -/+ pair) - identical content (header only, no hunks) - pure addition (@@ -0,0 +1,N @@ matching git) - pure deletion (@@ -1,N +0,0 @@ matching git) - middle change in 10-line file (correct hunk with 3-line context) - two separate hunks (correctly split into two @@ blocks) - observatory-like 250-line file with one change at line 125 (7-line hunk instead of 528-line phantom diff) Existing smoke test passes unchanged (the -return 'old';/+return 'new'; assertions still match real unified diff output). Two new assertions lock in the new behavior: a real hunk header is emitted, and unchanged lines appear as space-prefixed context rather than as -/+ pairs. Closes #429. AI assistance: Yes - Claude Code (Sonnet 4.5) diagnosed the agent 'line-ending weather' pattern from the world-creator transcript, traced it to build_full_file_diff, implemented Myers' algorithm in PHP, and verified the output against representative cases including the Observatory-sized file. Chris confirmed the upstream-fix-first approach over agent-side workarounds. * fix: narrow strpos result before substr_replace in edit_file Pre-existing phpstan finding surfaced by CI's --changed-since scoping when the diff generator overhaul touched this same file. $count > 0 at line 357 guarantees strpos cannot return false, but phpstan can't see that across the early-return. Make it explicit so static analysis is clean. No behavior change — the false branch falls back to the original content (which is unreachable anyway given the prior check).
1 parent 3bc3032 commit 9ef9d59

2 files changed

Lines changed: 346 additions & 14 deletions

File tree

inc/Workspace/RemoteWorkspaceBackend.php

Lines changed: 344 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -365,9 +365,15 @@ public function edit_file( string $handle, string $path, string $old_string, str
365365
return new \WP_Error( 'multiple_matches', sprintf( 'Found %d matches for old_string.', $count ), array( 'status' => 400 ) );
366366
}
367367

368-
$new_content = $replace_all
369-
? str_replace( $old_string, $new_string, $content )
370-
: substr_replace( $content, $new_string, strpos( $content, $old_string ), strlen( $old_string ) );
368+
if ( $replace_all ) {
369+
$new_content = str_replace( $old_string, $new_string, $content );
370+
} else {
371+
$offset = strpos( $content, $old_string );
372+
// $count > 0 above guarantees strpos cannot return false here.
373+
$new_content = false === $offset
374+
? $content
375+
: substr_replace( $content, $new_string, $offset, strlen( $old_string ) );
376+
}
371377

372378
$write = $this->write_file( $handle, $path, $new_content );
373379
if ( is_wp_error( $write ) ) {
@@ -454,7 +460,7 @@ public function git_diff( string $handle, ?string $from = null, ?string $to = nu
454460
$old_content = (string) ( $current['files'][0]['content'] ?? '' );
455461
}
456462

457-
$diff .= $this->build_full_file_diff( $changed_path, $old_content, (string) $new_content );
463+
$diff .= $this->build_unified_file_diff( $changed_path, $old_content, (string) $new_content );
458464
}
459465

460466
return array(
@@ -660,22 +666,58 @@ private function should_retry_default_ref( array|\WP_Error $file ): bool {
660666
return false;
661667
}
662668

663-
private function build_full_file_diff( string $path, string $old_content, string $new_content ): string {
669+
/**
670+
* Build a unified diff for a single file's pending change.
671+
*
672+
* Uses Myers' algorithm to find a minimal edit script, then groups adjacent
673+
* edits into hunks with surrounding context (default 3 lines). Output matches
674+
* the format produced by `git diff --no-color`, so consumers that scan the
675+
* diff for `-foo` / `+bar` lines see actual changed lines rather than a fake
676+
* whole-file replace.
677+
*
678+
* Previously this method emitted every old line as `-` followed by every new
679+
* line as `+`, regardless of how small the actual change was. That misled
680+
* agents into thinking surgical edits had rewritten the entire file.
681+
*
682+
* @see https://github.com/Extra-Chill/data-machine-code/issues/429
683+
*/
684+
private function build_unified_file_diff( string $path, string $old_content, string $new_content, int $context_lines = 3 ): string {
685+
$header = 'diff --git a/' . $path . ' b/' . $path . "\n";
686+
$header .= '--- a/' . $path . "\n";
687+
$header .= '+++ b/' . $path . "\n";
688+
689+
if ( $old_content === $new_content ) {
690+
return $header;
691+
}
692+
664693
$old_lines = $this->diff_lines( $old_content );
665694
$new_lines = $this->diff_lines( $new_content );
666695

667-
$diff = 'diff --git a/' . $path . ' b/' . $path . "\n";
668-
$diff .= '--- a/' . $path . "\n";
669-
$diff .= '+++ b/' . $path . "\n";
670-
$diff .= sprintf( '@@ -1,%d +1,%d @@', count( $old_lines ), count( $new_lines ) ) . "\n";
671-
foreach ( $old_lines as $line ) {
672-
$diff .= '-' . $line . "\n";
696+
$ops = $this->myers_diff( $old_lines, $new_lines );
697+
if ( empty( $ops ) ) {
698+
return $header;
699+
}
700+
701+
$hunks = $this->group_diff_hunks( $ops, $context_lines );
702+
if ( empty( $hunks ) ) {
703+
return $header;
673704
}
674-
foreach ( $new_lines as $line ) {
675-
$diff .= '+' . $line . "\n";
705+
706+
$body = '';
707+
foreach ( $hunks as $hunk ) {
708+
$body .= sprintf(
709+
"@@ -%d,%d +%d,%d @@\n",
710+
$hunk['old_start'],
711+
$hunk['old_count'],
712+
$hunk['new_start'],
713+
$hunk['new_count']
714+
);
715+
foreach ( $hunk['lines'] as $line ) {
716+
$body .= $line . "\n";
717+
}
676718
}
677719

678-
return $diff;
720+
return $header . $body;
679721
}
680722

681723
/** @return array<int,string> */
@@ -687,6 +729,294 @@ private function diff_lines( string $content ): array {
687729
return explode( "\n", rtrim( $content, "\n" ) );
688730
}
689731

732+
/**
733+
* Myers' diff algorithm producing an edit script over two arrays of lines.
734+
*
735+
* Returns an ordered list of operations:
736+
* ['op' => '=', 'line' => string] (unchanged)
737+
* ['op' => '-', 'line' => string] (removed from old)
738+
* ['op' => '+', 'line' => string] (added in new)
739+
*
740+
* Trims common prefix/suffix first so the O(ND) core only runs on the
741+
* actually-different middle window — typical "surgical edit in large file"
742+
* cases finish in O(N) instead of O(N^2).
743+
*
744+
* @param array<int,string> $a Old lines.
745+
* @param array<int,string> $b New lines.
746+
* @return array<int,array{op:string,line:string}>
747+
*/
748+
private function myers_diff( array $a, array $b ): array {
749+
$ops = array();
750+
751+
$prefix = 0;
752+
$a_len = count( $a );
753+
$b_len = count( $b );
754+
$min = min( $a_len, $b_len );
755+
while ( $prefix < $min && $a[ $prefix ] === $b[ $prefix ] ) {
756+
$ops[] = array(
757+
'op' => '=',
758+
'line' => $a[ $prefix ],
759+
);
760+
++$prefix;
761+
}
762+
763+
$suffix = 0;
764+
$max_suffix = $min - $prefix;
765+
while ( $suffix < $max_suffix && $a[ $a_len - 1 - $suffix ] === $b[ $b_len - 1 - $suffix ] ) {
766+
++$suffix;
767+
}
768+
769+
$middle_a = array_slice( $a, $prefix, $a_len - $prefix - $suffix );
770+
$middle_b = array_slice( $b, $prefix, $b_len - $prefix - $suffix );
771+
772+
foreach ( $this->myers_middle_diff( $middle_a, $middle_b ) as $op ) {
773+
$ops[] = $op;
774+
}
775+
776+
for ( $i = $a_len - $suffix; $i < $a_len; $i++ ) {
777+
$ops[] = array(
778+
'op' => '=',
779+
'line' => $a[ $i ],
780+
);
781+
}
782+
783+
return $ops;
784+
}
785+
786+
/**
787+
* Core Myers algorithm over a (presumably small) middle window.
788+
*
789+
* Implements Eugene Myers' O(ND) algorithm, recording trace V-arrays at each
790+
* D-step and walking back to reconstruct the edit script. Falls back to a
791+
* simple "remove-all then add-all" emission if the middle is degenerate
792+
* (one side empty), which is both faster and produces the same result.
793+
*
794+
* @param array<int,string> $a
795+
* @param array<int,string> $b
796+
* @return array<int,array{op:string,line:string}>
797+
*/
798+
private function myers_middle_diff( array $a, array $b ): array {
799+
$n = count( $a );
800+
$m = count( $b );
801+
802+
if ( 0 === $n && 0 === $m ) {
803+
return array();
804+
}
805+
if ( 0 === $n ) {
806+
$ops = array();
807+
foreach ( $b as $line ) {
808+
$ops[] = array(
809+
'op' => '+',
810+
'line' => $line,
811+
);
812+
}
813+
return $ops;
814+
}
815+
if ( 0 === $m ) {
816+
$ops = array();
817+
foreach ( $a as $line ) {
818+
$ops[] = array(
819+
'op' => '-',
820+
'line' => $line,
821+
);
822+
}
823+
return $ops;
824+
}
825+
826+
$max = $n + $m;
827+
$offset = $max;
828+
$trace = array();
829+
$v = array_fill( 0, 2 * $max + 1, 0 );
830+
831+
for ( $d = 0; $d <= $max; $d++ ) {
832+
for ( $k = -$d; $k <= $d; $k += 2 ) {
833+
if ( -$d === $k || ( $d !== $k && $v[ $k - 1 + $offset ] < $v[ $k + 1 + $offset ] ) ) {
834+
$x = $v[ $k + 1 + $offset ];
835+
} else {
836+
$x = $v[ $k - 1 + $offset ] + 1;
837+
}
838+
$y = $x - $k;
839+
while ( $x < $n && $y < $m && $a[ $x ] === $b[ $y ] ) {
840+
++$x;
841+
++$y;
842+
}
843+
$v[ $k + $offset ] = $x;
844+
if ( $x >= $n && $y >= $m ) {
845+
$trace[] = $v;
846+
return $this->myers_backtrack( $trace, $a, $b, $d, $offset );
847+
}
848+
}
849+
$trace[] = $v;
850+
}
851+
852+
return array();
853+
}
854+
855+
/**
856+
* Walk the recorded Myers trace backwards to build the ordered edit script.
857+
*
858+
* @param array<int,array<int,int>> $trace
859+
* @param array<int,string> $a
860+
* @param array<int,string> $b
861+
* @return array<int,array{op:string,line:string}>
862+
*/
863+
private function myers_backtrack( array $trace, array $a, array $b, int $d, int $offset ): array {
864+
$ops = array();
865+
$x = count( $a );
866+
$y = count( $b );
867+
868+
for ( ; $d > 0; $d-- ) {
869+
$v = $trace[ $d - 1 ];
870+
$k = $x - $y;
871+
if ( -$d === $k || ( $d !== $k && $v[ $k - 1 + $offset ] < $v[ $k + 1 + $offset ] ) ) {
872+
$prev_k = $k + 1;
873+
} else {
874+
$prev_k = $k - 1;
875+
}
876+
$prev_x = $v[ $prev_k + $offset ];
877+
$prev_y = $prev_x - $prev_k;
878+
879+
while ( $x > $prev_x && $y > $prev_y ) {
880+
$ops[] = array(
881+
'op' => '=',
882+
'line' => $a[ $x - 1 ],
883+
);
884+
--$x;
885+
--$y;
886+
}
887+
if ( $x === $prev_x ) {
888+
$ops[] = array(
889+
'op' => '+',
890+
'line' => $b[ $y - 1 ],
891+
);
892+
} else {
893+
$ops[] = array(
894+
'op' => '-',
895+
'line' => $a[ $x - 1 ],
896+
);
897+
}
898+
$x = $prev_x;
899+
$y = $prev_y;
900+
}
901+
while ( $x > 0 && $y > 0 ) {
902+
$ops[] = array(
903+
'op' => '=',
904+
'line' => $a[ $x - 1 ],
905+
);
906+
--$x;
907+
--$y;
908+
}
909+
while ( $x > 0 ) {
910+
$ops[] = array(
911+
'op' => '-',
912+
'line' => $a[ $x - 1 ],
913+
);
914+
--$x;
915+
}
916+
while ( $y > 0 ) {
917+
$ops[] = array(
918+
'op' => '+',
919+
'line' => $b[ $y - 1 ],
920+
);
921+
--$y;
922+
}
923+
924+
return array_reverse( $ops );
925+
}
926+
927+
/**
928+
* Group consecutive non-context edit operations into unified-diff hunks.
929+
*
930+
* Each hunk has up to `$context_lines` of unchanged context on each side.
931+
* Returns one entry per hunk with `old_start`, `old_count`, `new_start`,
932+
* `new_count`, and a list of `+line` / `-line` / ` line` strings ready to
933+
* emit between `@@` markers.
934+
*
935+
* @param array<int,array{op:string,line:string}> $ops
936+
* @return array<int,array{old_start:int,old_count:int,new_start:int,new_count:int,lines:array<int,string>}>
937+
*/
938+
private function group_diff_hunks( array $ops, int $context_lines ): array {
939+
$hunks = array();
940+
$count = count( $ops );
941+
942+
$old_line = 1;
943+
$new_line = 1;
944+
$i = 0;
945+
946+
while ( $i < $count ) {
947+
if ( '=' === $ops[ $i ]['op'] ) {
948+
++$old_line;
949+
++$new_line;
950+
++$i;
951+
continue;
952+
}
953+
954+
$context_before = min( $context_lines, $i );
955+
$hunk_start_i = $i - $context_before;
956+
$hunk_old_start = $old_line - $context_before;
957+
$hunk_new_start = $new_line - $context_before;
958+
959+
$lines = array();
960+
$old_count = 0;
961+
$new_count = 0;
962+
for ( $j = $hunk_start_i; $j < $i; $j++ ) {
963+
$lines[] = ' ' . $ops[ $j ]['line'];
964+
++$old_count;
965+
++$new_count;
966+
}
967+
968+
$tail_eq = 0;
969+
while ( $i < $count ) {
970+
$op = $ops[ $i ]['op'];
971+
if ( '=' === $op ) {
972+
++$tail_eq;
973+
if ( $tail_eq > 2 * $context_lines ) {
974+
--$tail_eq;
975+
break;
976+
}
977+
$lines[] = ' ' . $ops[ $i ]['line'];
978+
++$old_count;
979+
++$new_count;
980+
++$old_line;
981+
++$new_line;
982+
++$i;
983+
continue;
984+
}
985+
$tail_eq = 0;
986+
if ( '-' === $op ) {
987+
$lines[] = '-' . $ops[ $i ]['line'];
988+
++$old_count;
989+
++$old_line;
990+
} else {
991+
$lines[] = '+' . $ops[ $i ]['line'];
992+
++$new_count;
993+
++$new_line;
994+
}
995+
++$i;
996+
}
997+
998+
$keep_tail = min( $context_lines, $tail_eq );
999+
$drop_tail = $tail_eq - $keep_tail;
1000+
if ( $drop_tail > 0 ) {
1001+
$lines = array_slice( $lines, 0, count( $lines ) - $drop_tail );
1002+
$old_count -= $drop_tail;
1003+
$new_count -= $drop_tail;
1004+
$old_line -= $drop_tail;
1005+
$new_line -= $drop_tail;
1006+
}
1007+
1008+
$hunks[] = array(
1009+
'old_start' => 0 === $old_count ? max( 0, $hunk_old_start - 1 ) : $hunk_old_start,
1010+
'old_count' => $old_count,
1011+
'new_start' => 0 === $new_count ? max( 0, $hunk_new_start - 1 ) : $hunk_new_start,
1012+
'new_count' => $new_count,
1013+
'lines' => $lines,
1014+
);
1015+
}
1016+
1017+
return $hunks;
1018+
}
1019+
6901020
/**
6911021
* Compatibility no-op: commit already wrote to the remote branch.
6921022
*

0 commit comments

Comments
 (0)