fix(serialize): partition overlapping highlight spans into disjoint segments

internal/serialize/spans.go

@@ -213,6 +213,8 @@ func BuildHighlightSpans(fileBytes []byte, actions []Action, side string, extraS
 			lineMerged = append(lineMerged, curr)
 		}
 
+		lineMerged = partitionLineSpans(lineMerged, side)
+
 		slices.SortFunc(lineMerged, func(a, b internalSpan) int {
 			return cmp.Or(
 				cmp.Compare(a.startCol, b.startCol),
@@ -377,6 +379,125 @@ func nodeLen(a *Action, side string) int {
 	return 0
 }
 
+// spanASTLength returns the AST byte range for a span on the given side.
+// Falls back to column width when no AST node is attached so spans can still be compared.
+func spanASTLength(sp internalSpan, side string) int {
+	if n := nodeLen(sp.actRef, side); n > 0 {
+		return n
+	}
+	return sp.endCol - sp.startCol
+}
+
+// parseSpanActionPriority assigns tiebreaker priorities when two spans have identical AST lengths.
+// Higher values win: move_update (5) > update (4) > move (3) > insert (2) > delete (1).
+func parseSpanActionPriority(act string) int {
+	switch act {
+	case "move_update":
+		return 5
+	case "update":
+		return 4
+	case "move":
+		return 3
+	case "insert":
+		return 2
+	case "delete":
+		return 1
+	default:
+		return 0
+	}
+}
+
+// partitionLineSpans resolves cross-action overlaps for a single line's spans.
+func partitionLineSpans(spans []internalSpan, side string) []internalSpan {
+	if len(spans) <= 1 {
+		return spans
+	}
+
+	var hasOverlap bool
+checkOverlap:
+	for i := range len(spans) {
+		for j := i + 1; j < len(spans); j++ {
+			if spans[i].startCol < spans[j].endCol && spans[j].startCol < spans[i].endCol {
+				hasOverlap = true
+				break checkOverlap
+			}
+		}
+	}
+	if !hasOverlap {
+		return spans
+	}
+
+	boundaries := make(map[int]struct{})
+	for _, sp := range spans {
+		boundaries[sp.startCol] = struct{}{}
+		boundaries[sp.endCol] = struct{}{}
+	}
+	sortedBounds := slices.Sorted(maps.Keys(boundaries))
+
+	var segments []internalSpan
+	for i := range len(sortedBounds) - 1 {
+		segStart := sortedBounds[i]
+		segEnd := sortedBounds[i+1]
+		if segStart >= segEnd {
+			continue
+		}
+
+		var (
+			winner  *internalSpan
+			hasMove bool
+		)
+		for j := range spans {
+			sp := &spans[j]
+			if sp.startCol <= segStart && sp.endCol >= segEnd {
+				if sp.action == "move" {
+					hasMove = true
+				}
+				if winner == nil {
+					winner = sp
+					continue
+				}
+				wAstLen := spanASTLength(*winner, side)
+				sAstLen := spanASTLength(*sp, side)
+				candWidth := sp.endCol - sp.startCol
+				wCandWidth := winner.endCol - winner.startCol
+				if sAstLen < wAstLen ||
+					(sAstLen == wAstLen && candWidth < wCandWidth) ||
+					(sAstLen == wAstLen && candWidth == wCandWidth && parseSpanActionPriority(sp.action) > parseSpanActionPriority(winner.action)) {
+					winner = sp
+				}
+			}
+		}
+		if winner != nil {
+			action := winner.action
+			if action == "update" && hasMove {
+				action = "move_update"
+			}
+			segments = append(segments, internalSpan{
+				startCol: segStart,
+				endCol:   segEnd,
+				action:   action,
+				actRef:   winner.actRef,
+			})
+		}
+	}
+
+	if len(segments) == 0 {
+		return spans
+	}
+	var coalesced []internalSpan
+	cur := segments[0]
+	for i := 1; i < len(segments); i++ {
+		if segments[i].action == cur.action && segments[i].startCol == cur.endCol {
+			cur.endCol = segments[i].endCol
+		} else {
+			coalesced = append(coalesced, cur)
+			cur = segments[i]
+		}
+	}
+	coalesced = append(coalesced, cur)
+	return coalesced
+}
+
 // absorbSyntacticDelimiters expands a span to cover adjacent member connectors
 // (., ->, ::) or sequence commas so punctuation doesn't get left unhighlighted.
 func absorbSyntacticDelimiters(fileBytes []byte, startByte, endByte uint32, parent *NodeRef) (uint32, uint32) {

internal/serialize/spans_test.go

@@ -90,6 +90,8 @@ func TestBuildHighlightSpansInnerSpanPreservation(t *testing.T) {
 	fileBytes := []byte("def hello_world():\n")
 	// Container delete covers 0..18 (astLen=18)
 	// Inner delete covers 4..15 (astLen=11)
+	// Partitioner: inner wins [4,15), container gets [0,4) and [15,18).
+	// Same action coalesces → 1 span covering entire range.
 	actions := []Action{
 		{
 			Action: "delete",
@@ -102,11 +104,15 @@ func TestBuildHighlightSpansInnerSpanPreservation(t *testing.T) {
 	}
 
 	leftSpans := BuildHighlightSpans(fileBytes, actions, "left")
-	if len(leftSpans) != 2 {
-		t.Fatalf("expected 2 delete spans (outer and inner preserved), got %d", len(leftSpans))
+	if len(leftSpans) != 1 {
+		t.Fatalf("expected 1 coalesced delete span (same action merges), got %d: %+v", len(leftSpans), leftSpans)
+	}
+	if leftSpans[0].StartCol != 0 || leftSpans[0].EndCol != 18 {
+		t.Errorf("expected coalesced span [0,18), got [%d,%d)", leftSpans[0].StartCol, leftSpans[0].EndCol)
 	}
 
-	// Also test when inner starts at same start col (e.g. 0..10 and 0..18)
+	// Also test when inner starts at same start col (e.g. 0..3 and 0..18)
+	// Partitioner: inner wins [0,3), outer wins [3,18), same action coalesces → 1 span
 	actionsCoaligned := []Action{
 		{
 			Action: "delete",
@@ -119,8 +125,11 @@ func TestBuildHighlightSpansInnerSpanPreservation(t *testing.T) {
 	}
 
 	coalignedSpans := BuildHighlightSpans(fileBytes, actionsCoaligned, "left")
-	if len(coalignedSpans) != 2 {
-		t.Fatalf("expected 2 delete spans for coaligned start (inner 0..3 and outer 0..18), got %d", len(coalignedSpans))
+	if len(coalignedSpans) != 1 {
+		t.Fatalf("expected 1 coalesced delete span for coaligned start, got %d: %+v", len(coalignedSpans), coalignedSpans)
+	}
+	if coalignedSpans[0].StartCol != 0 || coalignedSpans[0].EndCol != 18 {
+		t.Errorf("expected coalesced span [0,18), got [%d,%d)", coalignedSpans[0].StartCol, coalignedSpans[0].EndCol)
 	}
 }
 
@@ -447,3 +456,117 @@ func TestNestedMoveActionsKeepsOutermost(t *testing.T) {
 		t.Fatal("expected outermost move span, got none")
 	}
 }
+
+func TestPartitionDisjointSpans_MoveDeleteOverlap(t *testing.T) {
+	fileBytes := []byte("abcdef\n")
+	// move covers [0,6) astLen=6, delete covers [0,6) astLen=6
+	// Same astLen → tiebreak by priority: move(3) > delete(2) → move wins entire range
+	actions := []Action{
+		{Action: "move", Node: &NodeRef{Type: "stmt", StartByte: 0, EndByte: 6}},
+		{Action: "delete", Node: &NodeRef{Type: "stmt", StartByte: 0, EndByte: 6}},
+	}
+	spans := BuildHighlightSpans(fileBytes, actions, "left")
+	if len(spans) != 1 {
+		t.Fatalf("expected 1 span after partition, got %d: %+v", len(spans), spans)
+	}
+	if spans[0].Action != "move" {
+		t.Errorf("expected move to win tiebreak, got %s", spans[0].Action)
+	}
+}
+
+func TestPartitionDisjointSpans_CoextensiveDuplicates(t *testing.T) {
+	fileBytes := []byte("hello world\n")
+	// Two identical delete spans on same bytes → inner (same astLen) wins, coalesces to 1
+	actions := []Action{
+		{Action: "delete", Node: &NodeRef{Type: "a", StartByte: 0, EndByte: 11}},
+		{Action: "delete", Node: &NodeRef{Type: "b", StartByte: 0, EndByte: 11}},
+	}
+	spans := BuildHighlightSpans(fileBytes, actions, "left")
+	if len(spans) != 1 {
+		t.Fatalf("expected 1 coalesced span for coextensive duplicates, got %d: %+v", len(spans), spans)
+	}
+}
+
+func TestPartitionDisjointSpans_UpdateInsideMove(t *testing.T) {
+	fileBytes := []byte("abcdef\n")
+	// An update inside a move promotes to move_update, flanked by the outer move segments.
+	// [0,2) move, [2,4) move_update, [4,6) move.
+	actions := []Action{
+		{Action: "move", Node: &NodeRef{Type: "stmt", StartByte: 0, EndByte: 6}},
+		{Action: "update", Node: &NodeRef{Type: "id", StartByte: 2, EndByte: 4}},
+	}
+	spans := BuildHighlightSpans(fileBytes, actions, "left")
+	if len(spans) != 3 {
+		t.Fatalf("expected 3 spans (move, move_update, move), got %d: %+v", len(spans), spans)
+	}
+	// Find the middle span
+	for _, s := range spans {
+		if s.StartCol == 2 && s.EndCol == 4 {
+			if s.Action != "move_update" {
+				t.Errorf("expected move_update for update inside move, got %s", s.Action)
+			}
+			return
+		}
+	}
+	t.Error("expected move_update span at [2,4)")
+}
+
+func TestPartitionDisjointSpans_ThreeWayNesting(t *testing.T) {
+	fileBytes := []byte("0123456789abcdef\n")
+	// Three-way conflict: delete [0,16), move [0,16), and nested update [2,5).
+	// Move beats delete on tiebreak; update beats move on AST specificity and promotes to move_update.
+	actions := []Action{
+		{Action: "delete", Node: &NodeRef{Type: "stmt", StartByte: 0, EndByte: 16}},
+		{Action: "move", Node: &NodeRef{Type: "stmt", StartByte: 0, EndByte: 16}},
+		{Action: "update", Node: &NodeRef{Type: "id", StartByte: 2, EndByte: 5}},
+	}
+	spans := BuildHighlightSpans(fileBytes, actions, "left")
+	if len(spans) != 3 {
+		t.Fatalf("expected 3 disjoint spans, got %d: %+v", len(spans), spans)
+	}
+	// Verify update segment is promoted to moveUpdate (inside original move)
+	for _, s := range spans {
+		if s.StartCol == 2 && s.EndCol == 5 {
+			if s.Action != "move_update" {
+				t.Errorf("expected moveUpdate at [2,5) (inside move), got %s", s.Action)
+			}
+			return
+		}
+	}
+	t.Error("expected moveUpdate span at [2,5)")
+}
+
+func TestPartitionDisjointSpans_NilActionRefFallback(t *testing.T) {
+	fileBytes := []byte("abcdef\n")
+	// move with ActionRef (astLen=6) vs nil-ActionRef delete (fallback=span width=6)
+	// Same astLen → tiebreak: move(3) > delete(2) → move wins
+	actions := []Action{
+		{Action: "move", Node: &NodeRef{Type: "stmt", StartByte: 0, EndByte: 6}},
+		{Action: "delete", Node: nil}, // nil Node → ActionRef will be nil on the span
+	}
+	spans := BuildHighlightSpans(fileBytes, actions, "left")
+	// The nil-Node delete won't produce a span at all (addSpan requires non-nil Node for delete)
+	// So only the move span survives
+	if len(spans) != 1 {
+		t.Fatalf("expected 1 span (nil-Node delete produces nothing), got %d: %+v", len(spans), spans)
+	}
+	if spans[0].Action != "move" {
+		t.Errorf("expected move, got %s", spans[0].Action)
+	}
+}
+
+func TestPartitionDisjointSpans_DisjointSpansUnchanged(t *testing.T) {
+	fileBytes := []byte("hello world\n")
+	// Two non-overlapping spans → partitioner should not change them
+	actions := []Action{
+		{Action: "delete", Node: &NodeRef{Type: "a", StartByte: 0, EndByte: 5}},
+		{Action: "insert", Node: &NodeRef{Type: "b", StartByte: 6, EndByte: 11}},
+	}
+	spans := BuildHighlightSpans(fileBytes, actions, "left")
+	if len(spans) != 1 {
+		t.Fatalf("expected 1 delete span (insert not on left), got %d: %+v", len(spans), spans)
+	}
+	if spans[0].StartCol != 0 || spans[0].EndCol != 5 || spans[0].Action != "delete" {
+		t.Errorf("expected delete [0,5), got %+v", spans[0])
+	}
+}

tests/integration/pipeline_test.go

@@ -334,6 +334,51 @@ func TestPipeline(t *testing.T) {
 	}
 }
 
+// TestNoOverlappingSpans verifies that highlight spans produced by BuildHighlightSpans
+// are pairwise disjoint on every line: no two spans on the same line share any byte column.
+func TestNoOverlappingSpans(t *testing.T) {
+	fixtures := allFixtures(t)
+	if len(fixtures) == 0 {
+		t.Fatal("no fixtures found in testdata/")
+	}
+
+	for _, name := range fixtures {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+			f := loadFixture(t, name)
+			result := runPipeline(t, f)
+
+			var uiEnv serialize.Envelope
+			if err := json.Unmarshal(result.UIJSON, &uiEnv); err != nil {
+				t.Fatalf("UI output is not valid JSON: %v", err)
+			}
+
+			checkDisjoint := func(t *testing.T, spans []serialize.HighlightSpan, side string) {
+				t.Helper()
+				byLine := make(map[int][]serialize.HighlightSpan)
+				for _, s := range spans {
+					byLine[s.Line] = append(byLine[s.Line], s)
+				}
+				for line, ls := range byLine {
+					for i := 0; i < len(ls); i++ {
+						for j := i + 1; j < len(ls); j++ {
+							a, b := ls[i], ls[j]
+							if a.StartCol < b.EndCol && b.StartCol < a.EndCol {
+								t.Errorf("%s line %d: overlapping spans [%d,%d) %s and [%d,%d) %s",
+									side, line, a.StartCol, a.EndCol, a.Action,
+									b.StartCol, b.EndCol, b.Action)
+							}
+						}
+					}
+				}
+			}
+
+			checkDisjoint(t, uiEnv.LeftHighlights, "left")
+			checkDisjoint(t, uiEnv.RightHighlights, "right")
+		})
+	}
+}
+
 // TestPipelineIdenticalFiles checks that diffing a file against itself returns zero actions.
 func TestPipelineIdenticalFiles(t *testing.T) {
 	fixtures := allFixtures(t)