From 53244c099ace3c3e9b703355ca5594fd945864a3 Mon Sep 17 00:00:00 2001 From: k1LoW Date: Tue, 7 Jul 2026 14:34:18 +0900 Subject: [PATCH] fix: preserve frozen slides when slide count changes generateActions resolved freeze by overwriting before[i] with after[i] after slide-count padding, on the assumption that index i in before corresponds to index i in after. Inserting or deleting slides before a frozen slide breaks that assumption. The overwrite corrupted the padded slides and their internal new/delete marks, so the Hungarian mapping ended up updating or deleting the very slide that should have been preserved. Instead, resolve each frozen slide's counterpart before padding, using an order-preserving alignment where non-frozen slides act as anchors through content similarity and frozen slides are matched by relative order between those anchors. Frozen slides cannot be matched by content because their actual content may have diverged from the markdown source, which is often the reason for freezing in the first place. The counterpart's current content is then substituted into after, so the similarity-based pipeline sees the pair as identical and generates no update or delete actions for it while still allowing moves. The "freeze slide moved" test expectation changes from [A, C, C] to [A, C, B] because the frozen slide's actual content is now preserved and moved instead of being overwritten by its neighbor. Fix #479 --- action.go | 29 ++++-- action_test.go | 241 ++++++++++++++++++++++++++++++++++++++++++++++++- freeze.go | 110 ++++++++++++++++++++++ freeze_test.go | 134 +++++++++++++++++++++++++++ 4 files changed, 505 insertions(+), 9 deletions(-) create mode 100644 freeze.go create mode 100644 freeze_test.go diff --git a/action.go b/action.go index 32ebb72..6232d8a 100644 --- a/action.go +++ b/action.go @@ -53,19 +53,34 @@ func generateActions(before, after Slides) (_ []*action, err error) { beforeCopy := copySlides(before) afterCopy := copySlides(after) + // Resolve which before slide each frozen after slide corresponds to, and + // substitute the counterpart's current content into after. The + // similarity-based pipeline below then sees the pair as identical and + // generates no update/delete actions for it, regardless of slide count + // changes (https://github.com/k1LoW/deck/issues/479). + frozenPairs := resolveFrozenSlides(beforeCopy, afterCopy) + for i, afterSlide := range afterCopy { + if !afterSlide.Freeze { + continue + } + if beforeIdx, ok := frozenPairs[i]; ok { + preserved := copySlide(beforeCopy[beforeIdx]) + preserved.Freeze = true + afterCopy[i] = preserved + } else { + // A frozen slide without a counterpart is created from its + // markdown content. Freeze must be cleared because it would + // suppress content application on append. + afterSlide.Freeze = false + } + } + // Adjust slide count adjustedBefore, adjustedAfter, err := adjustSlideCount(beforeCopy, afterCopy) if err != nil { return nil, fmt.Errorf("failed to adjust slide count: %w", err) } - // Prevent actions from being generated from indexes that should be frozen. - for i, afterSlide := range adjustedAfter { - if afterSlide.Freeze { - adjustedBefore[i] = copySlide(afterSlide) - } - } - // Map slides algorithm mapping, err := mapSlides(adjustedBefore, adjustedAfter) if err != nil { diff --git a/action_test.go b/action_test.go index 3a1f62e..fdf20cd 100644 --- a/action_test.go +++ b/action_test.go @@ -1659,8 +1659,8 @@ var tests = []struct { }, { Layout: "title", - Titles: []string{"Slide C"}, - TitleBodies: toBodies([]string{"Slide C"}), + Titles: []string{"Slide B"}, + TitleBodies: toBodies([]string{"Slide B"}), }, }, }, @@ -1699,6 +1699,243 @@ var tests = []struct { }, }, }, + { + name: "freeze slide with insert before frozen slide", + before: Slides{ + { + Layout: "title", + Titles: []string{"Slide 1"}, + TitleBodies: toBodies([]string{"Slide 1"}), + }, + { + Layout: "title", + Titles: []string{"Slide 2"}, + TitleBodies: toBodies([]string{"Slide 2"}), + }, + }, + after: Slides{ + { + Layout: "title", + Titles: []string{"Slide 1"}, + TitleBodies: toBodies([]string{"Slide 1"}), + }, + { + Layout: "title", + Titles: []string{"Slide NEW"}, + TitleBodies: toBodies([]string{"Slide NEW"}), + }, + { + Layout: "title", + Titles: []string{"Slide 2"}, + TitleBodies: toBodies([]string{"Slide 2"}), + Freeze: true, + }, + }, + want: Slides{ + { + Layout: "title", + Titles: []string{"Slide 1"}, + TitleBodies: toBodies([]string{"Slide 1"}), + }, + { + Layout: "title", + Titles: []string{"Slide NEW"}, + TitleBodies: toBodies([]string{"Slide NEW"}), + }, + { + Layout: "title", + Titles: []string{"Slide 2"}, + TitleBodies: toBodies([]string{"Slide 2"}), + }, + }, + }, + { + name: "freeze slide with delete before frozen slide", + before: Slides{ + { + Layout: "title", + Titles: []string{"Slide 1"}, + TitleBodies: toBodies([]string{"Slide 1"}), + }, + { + Layout: "title", + Titles: []string{"Slide 2"}, + TitleBodies: toBodies([]string{"Slide 2"}), + }, + { + Layout: "title", + Titles: []string{"Slide 3"}, + TitleBodies: toBodies([]string{"Slide 3"}), + }, + }, + after: Slides{ + { + Layout: "title", + Titles: []string{"Slide 1"}, + TitleBodies: toBodies([]string{"Slide 1"}), + }, + { + Layout: "title", + Titles: []string{"Slide 3"}, + TitleBodies: toBodies([]string{"Slide 3"}), + Freeze: true, + }, + }, + want: Slides{ + { + Layout: "title", + Titles: []string{"Slide 1"}, + TitleBodies: toBodies([]string{"Slide 1"}), + }, + { + Layout: "title", + Titles: []string{"Slide 3"}, + TitleBodies: toBodies([]string{"Slide 3"}), + }, + }, + }, + { + // The actual content of the frozen slide has diverged from its + // markdown source (the typical reason for freezing). The diverged + // content must survive an insertion before the frozen slide. + name: "freeze diverged slide with insert before frozen slide", + before: Slides{ + { + Layout: "title", + Titles: []string{"Slide 1"}, + TitleBodies: toBodies([]string{"Slide 1"}), + }, + { + Layout: "title", + Titles: []string{"Slide 2 edited manually"}, + TitleBodies: toBodies([]string{"Slide 2 edited manually"}), + }, + }, + after: Slides{ + { + Layout: "title", + Titles: []string{"Slide 1"}, + TitleBodies: toBodies([]string{"Slide 1"}), + }, + { + Layout: "title", + Titles: []string{"Slide NEW"}, + TitleBodies: toBodies([]string{"Slide NEW"}), + }, + { + Layout: "title", + Titles: []string{"Slide 2"}, + TitleBodies: toBodies([]string{"Slide 2"}), + Freeze: true, + }, + }, + want: Slides{ + { + Layout: "title", + Titles: []string{"Slide 1"}, + TitleBodies: toBodies([]string{"Slide 1"}), + }, + { + Layout: "title", + Titles: []string{"Slide NEW"}, + TitleBodies: toBodies([]string{"Slide NEW"}), + }, + { + Layout: "title", + Titles: []string{"Slide 2 edited manually"}, + TitleBodies: toBodies([]string{"Slide 2 edited manually"}), + }, + }, + }, + { + name: "freeze all slides with insert between", + before: Slides{ + { + Layout: "title", + Titles: []string{"Slide P"}, + TitleBodies: toBodies([]string{"Slide P"}), + }, + { + Layout: "title", + Titles: []string{"Slide Q"}, + TitleBodies: toBodies([]string{"Slide Q"}), + }, + }, + after: Slides{ + { + Layout: "title", + Titles: []string{"Slide P"}, + TitleBodies: toBodies([]string{"Slide P"}), + Freeze: true, + }, + { + Layout: "title", + Titles: []string{"Slide NEW"}, + TitleBodies: toBodies([]string{"Slide NEW"}), + }, + { + Layout: "title", + Titles: []string{"Slide Q"}, + TitleBodies: toBodies([]string{"Slide Q"}), + Freeze: true, + }, + }, + want: Slides{ + { + Layout: "title", + Titles: []string{"Slide P"}, + TitleBodies: toBodies([]string{"Slide P"}), + }, + { + Layout: "title", + Titles: []string{"Slide NEW"}, + TitleBodies: toBodies([]string{"Slide NEW"}), + }, + { + Layout: "title", + Titles: []string{"Slide Q"}, + TitleBodies: toBodies([]string{"Slide Q"}), + }, + }, + }, + { + // A frozen slide that has no counterpart in the presentation is + // created from its markdown content. Freezing protects it from the + // next apply onward. + name: "freeze new slide without counterpart", + before: Slides{ + { + Layout: "title", + Titles: []string{"Slide 1"}, + TitleBodies: toBodies([]string{"Slide 1"}), + }, + }, + after: Slides{ + { + Layout: "title", + Titles: []string{"Slide 1"}, + TitleBodies: toBodies([]string{"Slide 1"}), + }, + { + Layout: "title", + Titles: []string{"Slide F"}, + TitleBodies: toBodies([]string{"Slide F"}), + Freeze: true, + }, + }, + want: Slides{ + { + Layout: "title", + Titles: []string{"Slide 1"}, + TitleBodies: toBodies([]string{"Slide 1"}), + }, + { + Layout: "title", + Titles: []string{"Slide F"}, + TitleBodies: toBodies([]string{"Slide F"}), + }, + }, + }, } func TestGenerateActions(t *testing.T) { diff --git a/freeze.go b/freeze.go new file mode 100644 index 0000000..70b01d8 --- /dev/null +++ b/freeze.go @@ -0,0 +1,110 @@ +// freeze.go contains the logic that resolves which existing (before) slide +// each frozen (after) slide corresponds to. +package deck + +import "slices" + +// frozenMatchFloor is the minimum alignment score granted to a frozen after +// slide against any before slide. Frozen slides are expected to have diverged +// from their markdown source (that is often why they are frozen), so content +// similarity alone cannot identify their counterpart. The floor lets the +// order-preserving alignment adopt a counterpart by relative position, while +// staying below genuine content matches (e.g. layout+title scores 130) so +// that non-frozen slides win their anchors first. +const frozenMatchFloor = 100 + +// resolveFrozenSlides determines which before slide each frozen after slide +// corresponds to. It returns a map with after index as key and before index +// as value. Frozen after slides without a counterpart are absent from the map. +// +// Frozen slides cannot be matched by content similarity because their actual +// content may have diverged from the markdown source. Instead, non-frozen +// slides act as anchors through their content similarity, and frozen slides +// are matched by relative order between those anchors using an +// order-preserving alignment (Needleman-Wunsch style). A second pass matches +// the remaining frozen slides against unmatched before slides by similarity, +// which covers frozen slides moved across anchors. +func resolveFrozenSlides(before, after Slides) map[int]int { + resolved := make(map[int]int) + if len(before) == 0 || !slices.ContainsFunc(after, func(s *Slide) bool { return s.Freeze }) { + return resolved + } + + m := len(before) + n := len(after) + // dp[i][j] is the best alignment score between before[:i] and after[:j]. + // Gaps (unmatched slides on either side) carry no penalty. + dp := make([][]int, m+1) + for i := range dp { + dp[i] = make([]int, n+1) + } + for i := 1; i <= m; i++ { + for j := 1; j <= n; j++ { + dp[i][j] = max( + dp[i-1][j-1]+frozenAlignScore(before[i-1], after[j-1]), + dp[i-1][j], + dp[i][j-1], + ) + } + } + + // Backtrack, preferring matches over gaps so that frozen slides adopt a + // counterpart whenever doing so does not lower the total score. + matchedBefore := make([]bool, m) + i, j := m, n + for i > 0 && j > 0 { + score := frozenAlignScore(before[i-1], after[j-1]) + switch { + case score > 0 && dp[i][j] == dp[i-1][j-1]+score: + if after[j-1].Freeze { + resolved[j-1] = i - 1 + } + matchedBefore[i-1] = true + i-- + j-- + case dp[i][j] == dp[i-1][j]: + i-- + default: + j-- + } + } + + // Second pass: frozen slides that the order-preserving alignment could + // not match (e.g. frozen slides moved across anchors) adopt the most + // similar unmatched before slide, as long as the content still resembles + // the markdown source. + for j, afterSlide := range after { + if !afterSlide.Freeze { + continue + } + if _, ok := resolved[j]; ok { + continue + } + bestIdx := -1 + bestScore := 0 + for i, beforeSlide := range before { + if matchedBefore[i] { + continue + } + if score := getSimilarity(beforeSlide, afterSlide); score > bestScore { + bestIdx = i + bestScore = score + } + } + if bestIdx >= 0 && bestScore >= frozenMatchFloor { + resolved[j] = bestIdx + matchedBefore[bestIdx] = true + } + } + + return resolved +} + +// frozenAlignScore is the alignment score used by resolveFrozenSlides. +func frozenAlignScore(beforeSlide, afterSlide *Slide) int { + score := getSimilarity(beforeSlide, afterSlide) + if afterSlide.Freeze { + return max(score, frozenMatchFloor) + } + return score +} diff --git a/freeze_test.go b/freeze_test.go new file mode 100644 index 0000000..7fa6bcd --- /dev/null +++ b/freeze_test.go @@ -0,0 +1,134 @@ +package deck + +import ( + "testing" + + "github.com/google/go-cmp/cmp" +) + +func TestResolveFrozenSlides(t *testing.T) { + tests := []struct { + name string + before Slides + after Slides + want map[int]int + }{ + { + name: "empty slides", + before: Slides{}, + after: Slides{}, + want: map[int]int{}, + }, + { + name: "no frozen slides", + before: Slides{ + {Layout: "title", Titles: []string{"Slide 1"}}, + }, + after: Slides{ + {Layout: "title", Titles: []string{"Slide 1"}}, + }, + want: map[int]int{}, + }, + { + name: "frozen slide with empty before", + before: Slides{}, + after: Slides{ + {Layout: "title", Titles: []string{"Slide 1"}, Freeze: true}, + }, + want: map[int]int{}, + }, + { + name: "insert before frozen slide", + before: Slides{ + {Layout: "title", Titles: []string{"Slide 1"}}, + {Layout: "title", Titles: []string{"Slide 2"}}, + }, + after: Slides{ + {Layout: "title", Titles: []string{"Slide 1"}}, + {Layout: "title", Titles: []string{"Slide NEW"}}, + {Layout: "title", Titles: []string{"Slide 2"}, Freeze: true}, + }, + want: map[int]int{2: 1}, + }, + { + name: "delete before frozen slide", + before: Slides{ + {Layout: "title", Titles: []string{"Slide 1"}}, + {Layout: "title", Titles: []string{"Slide 2"}}, + {Layout: "title", Titles: []string{"Slide 3"}}, + }, + after: Slides{ + {Layout: "title", Titles: []string{"Slide 1"}}, + {Layout: "title", Titles: []string{"Slide 3"}, Freeze: true}, + }, + want: map[int]int{1: 2}, + }, + { + // The frozen slide's actual content has diverged from its + // markdown source. It must still adopt a counterpart by + // relative order between anchors. + name: "diverged frozen slide adopts counterpart by order", + before: Slides{ + {Layout: "title", Titles: []string{"Slide 1"}}, + {Layout: "title", Titles: []string{"Slide 2 edited manually"}}, + }, + after: Slides{ + {Layout: "title", Titles: []string{"Slide 1"}}, + {Layout: "title", Titles: []string{"Slide 2"}, Freeze: true}, + }, + want: map[int]int{1: 1}, + }, + { + // The frozen slide crosses the anchor "Slide C" in the desired + // order. The order-preserving alignment cannot express this + // move, so the second pass matches it by similarity. + name: "frozen slide moved across anchors", + before: Slides{ + {Layout: "title", Titles: []string{"Slide A"}}, + {Layout: "title", Titles: []string{"Slide B"}}, + {Layout: "title", Titles: []string{"Slide C"}}, + }, + after: Slides{ + {Layout: "title", Titles: []string{"Slide A"}}, + {Layout: "title", Titles: []string{"Slide C"}}, + {Layout: "title", Titles: []string{"Slide B"}, Freeze: true}, + }, + want: map[int]int{2: 1}, + }, + { + // Every before slide is claimed by a non-frozen anchor, so the + // frozen slide has no counterpart and is absent from the result. + name: "frozen slide without counterpart", + before: Slides{ + {Layout: "title", Titles: []string{"Slide 1"}}, + }, + after: Slides{ + {Layout: "title", Titles: []string{"Slide 1"}}, + {Layout: "title", Titles: []string{"Slide F"}, Freeze: true}, + }, + want: map[int]int{}, + }, + { + name: "all slides frozen with insert between", + before: Slides{ + {Layout: "title", Titles: []string{"Slide P"}}, + {Layout: "title", Titles: []string{"Slide Q"}}, + }, + after: Slides{ + {Layout: "title", Titles: []string{"Slide P"}, Freeze: true}, + {Layout: "title", Titles: []string{"Slide NEW"}}, + {Layout: "title", Titles: []string{"Slide Q"}, Freeze: true}, + }, + want: map[int]int{0: 0, 2: 1}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := resolveFrozenSlides(tt.before, tt.after) + if diff := cmp.Diff(got, tt.want); diff != "" { + t.Error(diff) + } + }) + } +}