From 24ea6bb12d8c62bb6283cc8f5e9c9ead0b233c41 Mon Sep 17 00:00:00 2001 From: Jon Hadfield Date: Mon, 21 Sep 2026 18:52:12 +0100 Subject: [PATCH] Keep the most recently updated note in each set of duplicates Deleting strictly by the duplicate_of marker removed the copy, which is the newer note. Anyone who duplicated a note and then carried on editing the copy would have lost that work. Group each note with the copies made from it, following duplicate_of, so a copy of a copy lands in the same set. Keep the most recently updated note in the set and delete the rest. On identical timestamps prefer the original, then the lower UUID, so the choice is stable. The listing now shows the last-updated date of every note, and which one is being kept. Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 2 +- cmd/sncli/duplicates.go | 48 ++++--- internal/sncli/duplicates.go | 203 ++++++++++++++++++++++-------- internal/sncli/duplicates_test.go | 167 +++++++++++++++++------- 4 files changed, 303 insertions(+), 117 deletions(-) diff --git a/README.md b/README.md index a37cd10..caeaa6f 100644 --- a/README.md +++ b/README.md @@ -224,7 +224,7 @@ sn get notes --title "Meeting Notes" --rich # Edit a note in $EDITOR sn edit note --title "Meeting Notes" -# See which notes the app marked as copies, then delete them +# Review duplicate notes, then delete all but the newest of each set sn delete duplicates --dry-run sn delete duplicates diff --git a/cmd/sncli/duplicates.go b/cmd/sncli/duplicates.go index 117a4cd..0c39766 100644 --- a/cmd/sncli/duplicates.go +++ b/cmd/sncli/duplicates.go @@ -10,9 +10,9 @@ import ( "github.com/urfave/cli/v2" ) -// maxDuplicatesListed caps how many duplicates are listed before the rest are -// summarised, so a large account does not fill the terminal. -const maxDuplicatesListed = 20 +// maxDuplicateGroupsListed caps how many sets of duplicates are listed before +// the rest are summarised, so a large account does not fill the terminal. +const maxDuplicateGroupsListed = 20 func processDeleteDuplicates(c *cli.Context, opts configOptsOutput) error { sess, _, err := cache.GetSession(common.NewHTTPClient(), opts.useSession, opts.sessKey, opts.server, opts.debug) @@ -35,21 +35,23 @@ func processDeleteDuplicates(c *cli.Context, opts configOptsOutput) error { return fmt.Errorf("failed to find duplicate notes: %w", err) } - writeDuplicatesKept(c, found.KeptOriginalMissing) + writeDuplicatesKept(c, found.KeptNoOriginal) - if len(found.Deleted) == 0 { + toDelete := found.Deleted() + + if len(toDelete) == 0 { _, _ = fmt.Fprintln(c.App.Writer, "no duplicate notes found") return nil } - writeDuplicatesFound(c, found.Deleted, dryRun) + writeDuplicateGroups(c, found.Groups, len(toDelete), dryRun) if dryRun { return nil } - if !c.Bool("yes") && !confirmDeleteDuplicates(c, len(found.Deleted)) { + if !c.Bool("yes") && !confirmDeleteDuplicates(c, len(toDelete)) { return nil } @@ -60,41 +62,47 @@ func processDeleteDuplicates(c *cli.Context, opts configOptsOutput) error { return fmt.Errorf("failed to delete duplicate notes: %w", err) } - _, _ = fmt.Fprintln(c.App.Writer, color.Green.Sprintf("%d duplicate notes deleted", len(out.Deleted))) + _, _ = fmt.Fprintln(c.App.Writer, color.Green.Sprintf("%d duplicate notes deleted", len(out.Deleted()))) return nil } -func writeDuplicatesFound(c *cli.Context, dups []sncli.DuplicateNote, dryRun bool) { - heading := fmt.Sprintf("%d duplicate notes to delete:", len(dups)) +func writeDuplicateGroups(c *cli.Context, groups []sncli.DuplicateGroup, numToDelete int, dryRun bool) { + heading := fmt.Sprintf("%d notes to delete, in %d sets of duplicates:", numToDelete, len(groups)) if dryRun { - heading = fmt.Sprintf("%d duplicate notes would be deleted:", len(dups)) + heading = fmt.Sprintf("%d notes would be deleted, in %d sets of duplicates:", numToDelete, len(groups)) } _, _ = fmt.Fprintln(c.App.Writer, heading) - for x, dup := range dups { - if x == maxDuplicatesListed { - _, _ = fmt.Fprintln(c.App.Writer, color.Gray.Sprintf(" ... and %d more", len(dups)-maxDuplicatesListed)) + for x, group := range groups { + if x == maxDuplicateGroupsListed { + _, _ = fmt.Fprintln(c.App.Writer, color.Gray.Sprintf(" ... and %d more sets", len(groups)-maxDuplicateGroupsListed)) break } - _, _ = fmt.Fprintf(c.App.Writer, " %s %s\n", dup.UUID, dup.Title) + _, _ = fmt.Fprintf(c.App.Writer, " keep %s %s %s\n", + group.Keep.UpdatedDate(), group.Keep.UUID, group.Keep.Title) + + for _, dup := range group.Delete { + _, _ = fmt.Fprintf(c.App.Writer, " %s %s %s %s\n", + color.Red.Sprint("delete"), dup.UpdatedDate(), dup.UUID, dup.Title) + } } } -func writeDuplicatesKept(c *cli.Context, kept []sncli.DuplicateNote) { +func writeDuplicatesKept(c *cli.Context, kept []sncli.NoteSummary) { if len(kept) == 0 { return } _, _ = fmt.Fprintln(c.App.Writer, color.Yellow.Sprintf( - "%d duplicates kept, as the note each was copied from is no longer in the account:", len(kept))) + "%d copies kept, as the note each was made from is no longer in the account:", len(kept))) for x, dup := range kept { - if x == maxDuplicatesListed { - _, _ = fmt.Fprintln(c.App.Writer, color.Gray.Sprintf(" ... and %d more", len(kept)-maxDuplicatesListed)) + if x == maxDuplicateGroupsListed { + _, _ = fmt.Fprintln(c.App.Writer, color.Gray.Sprintf(" ... and %d more", len(kept)-maxDuplicateGroupsListed)) break } @@ -104,7 +112,7 @@ func writeDuplicatesKept(c *cli.Context, kept []sncli.DuplicateNote) { } func confirmDeleteDuplicates(c *cli.Context, num int) bool { - _, _ = fmt.Fprintf(c.App.Writer, "delete %d duplicate notes? ", num) + _, _ = fmt.Fprintf(c.App.Writer, "delete %d notes? ", num) var input string diff --git a/internal/sncli/duplicates.go b/internal/sncli/duplicates.go index 86eedc2..094d67e 100644 --- a/internal/sncli/duplicates.go +++ b/internal/sncli/duplicates.go @@ -2,61 +2,156 @@ package sncli import ( "fmt" + "sort" + "time" "github.com/jonhadfield/gosn-v2/cache" - "github.com/jonhadfield/gosn-v2/common" "github.com/jonhadfield/gosn-v2/items" ) -// DuplicateNote is a note that Standard Notes marked as a copy of another, -// by setting duplicate_of when the note was duplicated. -type DuplicateNote struct { - UUID string - // Title of the duplicate itself +// NoteSummary identifies a note in duplicate output. +type NoteSummary struct { + UUID string Title string - // OriginalUUID is the note this one was copied from - OriginalUUID string - // OriginalPresent reports whether that original is still in the account. - // When it is not, deleting the duplicate would lose the content, so the - // duplicate is kept. - OriginalPresent bool + // UpdatedAt is the note's last-updated timestamp, as the API reports it. + UpdatedAt int64 + // UpdatedAtText is the same time as text, for display. + UpdatedAtText string + // Marked reports whether the note carries duplicate_of, meaning Standard + // Notes created it by duplicating another note. + Marked bool } -// FindDuplicateNotes returns the notes marked as duplicates, and whether the -// note each was copied from still exists. -func FindDuplicateNotes(in items.Items) []DuplicateNote { - live := make(map[string]struct{}) +// UpdatedDate returns the note's last-updated time as a date and time for +// display, or the raw value if it cannot be parsed. +func (n NoteSummary) UpdatedDate() string { + t, err := time.Parse(timeLayout, n.UpdatedAtText) + if err != nil { + return n.UpdatedAtText + } + + return t.Format("2006-01-02 15:04") +} + +// DuplicateGroup is a note and the copies made from it, linked by +// duplicate_of. The most recently updated note is kept and the rest deleted, +// so that editing a copy after duplicating it does not lose that work. +type DuplicateGroup struct { + Keep NoteSummary + Delete []NoteSummary +} + +func summariseNote(note *items.Note) NoteSummary { + return NoteSummary{ + UUID: note.GetUUID(), + Title: note.Content.GetTitle(), + UpdatedAt: note.GetUpdatedAtTimestamp(), + UpdatedAtText: note.GetUpdatedAt(), + Marked: note.GetDuplicateOf() != "", + } +} + +// newerThan reports whether a should be kept in preference to b. The most +// recently updated wins; on a tie an original is preferred over a copy, and +// then the lower UUID, so the choice is stable. +func newerThan(a, b NoteSummary) bool { + switch { + case a.UpdatedAt != b.UpdatedAt: + return a.UpdatedAt > b.UpdatedAt + case a.Marked != b.Marked: + return !a.Marked + default: + return a.UUID < b.UUID + } +} + +// FindDuplicateGroups groups notes with the copies made from them and reports +// which to keep. It also returns copies whose original is no longer in the +// account: those are the only remaining copy of their content, so they are +// left alone. +func FindDuplicateGroups(in items.Items) (groups []DuplicateGroup, keptNoOriginal []NoteSummary) { + live := make(map[string]*items.Note) for _, item := range in { - if item.GetContentType() == common.SNItemTypeNote && !item.IsDeleted() { - live[item.GetUUID()] = struct{}{} + if note, isNote := item.(*items.Note); isNote && !note.IsDeleted() { + live[note.GetUUID()] = note } } - var duplicates []DuplicateNote + // union-find over the live notes, joining each copy to the note it was + // made from, so that a copy of a copy ends up in one group + parent := make(map[string]string, len(live)) - for _, item := range in { - note, isNote := item.(*items.Note) - if !isNote || note.IsDeleted() || note.GetDuplicateOf() == "" { - continue + var find func(string) string + + find = func(u string) string { + if parent[u] != u { + parent[u] = find(parent[u]) } + return parent[u] + } + + for uuid := range live { + parent[uuid] = uuid + } + + for uuid, note := range live { original := note.GetDuplicateOf() - _, originalPresent := live[original] + if original == "" { + continue + } + + if _, ok := live[original]; !ok { + keptNoOriginal = append(keptNoOriginal, summariseNote(note)) + + continue + } - duplicates = append(duplicates, DuplicateNote{ - UUID: note.GetUUID(), - Title: note.Content.GetTitle(), - OriginalUUID: original, - OriginalPresent: originalPresent, - }) + parent[find(uuid)] = find(original) } - return duplicates + members := make(map[string][]NoteSummary) + + for uuid, note := range live { + root := find(uuid) + members[root] = append(members[root], summariseNote(note)) + } + + for _, group := range members { + if len(group) < 2 { + continue + } + + keep := group[0] + + for _, candidate := range group[1:] { + if newerThan(candidate, keep) { + keep = candidate + } + } + + var toDelete []NoteSummary + + for _, candidate := range group { + if candidate.UUID != keep.UUID { + toDelete = append(toDelete, candidate) + } + } + + sort.Slice(toDelete, func(i, j int) bool { return toDelete[i].UUID < toDelete[j].UUID }) + + groups = append(groups, DuplicateGroup{Keep: keep, Delete: toDelete}) + } + + sort.Slice(groups, func(i, j int) bool { return groups[i].Keep.UUID < groups[j].Keep.UUID }) + sort.Slice(keptNoOriginal, func(i, j int) bool { return keptNoOriginal[i].UUID < keptNoOriginal[j].UUID }) + + return groups, keptNoOriginal } -// DeleteDuplicateNotesConfig deletes notes marked as duplicates of another -// note. +// DeleteDuplicateNotesConfig deletes the superseded notes in each set of +// duplicates. type DeleteDuplicateNotesConfig struct { Session *cache.Session // DryRun reports what would be deleted without deleting anything. @@ -66,15 +161,27 @@ type DeleteDuplicateNotesConfig struct { // DeleteDuplicateNotesOutput describes what was, or would be, deleted. type DeleteDuplicateNotesOutput struct { - // Deleted holds the duplicates removed, or, for a dry run, those that - // would be removed. - Deleted []DuplicateNote - // KeptOriginalMissing holds duplicates left alone because the note they - // were copied from is no longer in the account. - KeptOriginalMissing []DuplicateNote + // Groups holds each set of duplicates, with the note kept and those + // deleted. + Groups []DuplicateGroup + // KeptNoOriginal holds copies left alone because the note they were made + // from is no longer in the account. + KeptNoOriginal []NoteSummary +} + +// Deleted returns every note deleted, across all groups. +func (o DeleteDuplicateNotesOutput) Deleted() []NoteSummary { + var all []NoteSummary + + for _, group := range o.Groups { + all = append(all, group.Delete...) + } + + return all } -// Run finds duplicates and, unless this is a dry run, deletes them. +// Run finds duplicates and, unless this is a dry run, deletes all but the most +// recently updated note in each set. func (i *DeleteDuplicateNotesConfig) Run() (DeleteDuplicateNotesOutput, error) { var out DeleteDuplicateNotesOutput @@ -103,17 +210,11 @@ func (i *DeleteDuplicateNotesConfig) Run() (DeleteDuplicateNotesOutput, error) { } } - for _, dup := range FindDuplicateNotes(all) { - if dup.OriginalPresent { - out.Deleted = append(out.Deleted, dup) - - continue - } + out.Groups, out.KeptNoOriginal = FindDuplicateGroups(all) - out.KeptOriginalMissing = append(out.KeptOriginalMissing, dup) - } + toDelete := out.Deleted() - if i.DryRun || len(out.Deleted) == 0 { + if i.DryRun || len(toDelete) == 0 { _ = i.Session.CacheDB.Close() return out, nil @@ -121,8 +222,8 @@ func (i *DeleteDuplicateNotesConfig) Run() (DeleteDuplicateNotesOutput, error) { var notesToDelete items.Notes - for _, dup := range out.Deleted { - note := byUUID[dup.UUID] + for _, summary := range toDelete { + note := byUUID[summary.UUID] if note == nil || note.GetContent() == nil { continue } diff --git a/internal/sncli/duplicates_test.go b/internal/sncli/duplicates_test.go index f4a648e..b859f90 100644 --- a/internal/sncli/duplicates_test.go +++ b/internal/sncli/duplicates_test.go @@ -8,13 +8,14 @@ import ( "github.com/stretchr/testify/require" ) -func testNote(uuid, title, duplicateOf string, deleted bool) *items.Note { +func testNote(uuid, title, duplicateOf string, updatedAt int64, deleted bool) *items.Note { note := &items.Note{ ItemCommon: items.ItemCommon{ - UUID: uuid, - ContentType: common.SNItemTypeNote, - DuplicateOf: duplicateOf, - Deleted: deleted, + UUID: uuid, + ContentType: common.SNItemTypeNote, + DuplicateOf: duplicateOf, + UpdatedAtTimestamp: updatedAt, + Deleted: deleted, }, } note.Content.SetTitle(title) @@ -22,78 +23,154 @@ func testNote(uuid, title, duplicateOf string, deleted bool) *items.Note { return note } -func TestFindDuplicateNotesMarkedOnly(t *testing.T) { +func TestFindDuplicateGroupsKeepsNewerCopy(t *testing.T) { + // the copy was edited after it was made, so it holds the latest work in := items.Items{ - testNote("original", "Shopping", "", false), - testNote("copy", "Shopping copy", "original", false), - testNote("unrelated", "Ideas", "", false), + testNote("original", "Shopping", "", 100, false), + testNote("copy", "Shopping copy", "original", 200, false), } - dups := FindDuplicateNotes(in) + groups, kept := FindDuplicateGroups(in) - require.Len(t, dups, 1) - require.Equal(t, "copy", dups[0].UUID) - require.Equal(t, "Shopping copy", dups[0].Title) - require.Equal(t, "original", dups[0].OriginalUUID) - require.True(t, dups[0].OriginalPresent) + require.Empty(t, kept) + require.Len(t, groups, 1) + require.Equal(t, "copy", groups[0].Keep.UUID) + require.Len(t, groups[0].Delete, 1) + require.Equal(t, "original", groups[0].Delete[0].UUID) } -// A duplicate whose original is gone is the only copy of that content left, -// so it must not be treated as safe to delete. -func TestFindDuplicateNotesOriginalMissing(t *testing.T) { +func TestFindDuplicateGroupsKeepsNewerOriginal(t *testing.T) { + // the usual case: the copy was never touched after being made in := items.Items{ - testNote("copy", "Shopping copy", "gone", false), + testNote("original", "Shopping", "", 300, false), + testNote("copy", "Shopping copy", "original", 200, false), } - dups := FindDuplicateNotes(in) + groups, _ := FindDuplicateGroups(in) - require.Len(t, dups, 1) - require.False(t, dups[0].OriginalPresent) + require.Len(t, groups, 1) + require.Equal(t, "original", groups[0].Keep.UUID) + require.Equal(t, "copy", groups[0].Delete[0].UUID) } -func TestFindDuplicateNotesOriginalDeleted(t *testing.T) { +func TestFindDuplicateGroupsTiePrefersOriginal(t *testing.T) { + // identical timestamps: keep the note that is not marked as a copy in := items.Items{ - testNote("original", "Shopping", "", true), - testNote("copy", "Shopping copy", "original", false), + testNote("original", "Shopping", "", 100, false), + testNote("copy", "Shopping copy", "original", 100, false), } - dups := FindDuplicateNotes(in) + groups, _ := FindDuplicateGroups(in) - require.Len(t, dups, 1) - require.False(t, dups[0].OriginalPresent, "a deleted original should not count as present") + require.Len(t, groups, 1) + require.Equal(t, "original", groups[0].Keep.UUID) } -func TestFindDuplicateNotesSkipsDeletedDuplicates(t *testing.T) { +// A copy whose original is gone is the only note holding that content, so it +// must not be deleted. +func TestFindDuplicateGroupsOriginalMissing(t *testing.T) { in := items.Items{ - testNote("original", "Shopping", "", false), - testNote("copy", "Shopping copy", "original", true), + testNote("copy", "Shopping copy", "gone", 100, false), } - require.Empty(t, FindDuplicateNotes(in)) + groups, kept := FindDuplicateGroups(in) + + require.Empty(t, groups) + require.Len(t, kept, 1) + require.Equal(t, "copy", kept[0].UUID) } -func TestFindDuplicateNotesNone(t *testing.T) { +func TestFindDuplicateGroupsOriginalDeleted(t *testing.T) { in := items.Items{ - testNote("a", "One", "", false), - testNote("b", "Two", "", false), + testNote("original", "Shopping", "", 100, true), + testNote("copy", "Shopping copy", "original", 200, false), } - require.Empty(t, FindDuplicateNotes(in)) + groups, kept := FindDuplicateGroups(in) + + require.Empty(t, groups, "a deleted original is not a live duplicate") + require.Len(t, kept, 1) } -func TestFindDuplicateNotesChain(t *testing.T) { - // a copy of a copy: both are duplicates, and both originals are present +func TestFindDuplicateGroupsSkipsDeletedCopies(t *testing.T) { in := items.Items{ - testNote("original", "Shopping", "", false), - testNote("copy", "Shopping copy", "original", false), - testNote("copy2", "Shopping copy 2", "copy", false), + testNote("original", "Shopping", "", 100, false), + testNote("copy", "Shopping copy", "original", 200, true), } - dups := FindDuplicateNotes(in) + groups, kept := FindDuplicateGroups(in) - require.Len(t, dups, 2) + require.Empty(t, groups) + require.Empty(t, kept) +} - for _, dup := range dups { - require.True(t, dup.OriginalPresent) +func TestFindDuplicateGroupsNone(t *testing.T) { + in := items.Items{ + testNote("a", "One", "", 100, false), + testNote("b", "Two", "", 200, false), } + + groups, kept := FindDuplicateGroups(in) + + require.Empty(t, groups) + require.Empty(t, kept) +} + +// A copy of a copy belongs to one set, so only the newest of the three is +// kept. +func TestFindDuplicateGroupsChain(t *testing.T) { + in := items.Items{ + testNote("original", "Shopping", "", 100, false), + testNote("copy", "Shopping copy", "original", 400, false), + testNote("copy2", "Shopping copy 2", "copy", 200, false), + } + + groups, kept := FindDuplicateGroups(in) + + require.Empty(t, kept) + require.Len(t, groups, 1) + require.Equal(t, "copy", groups[0].Keep.UUID) + require.Len(t, groups[0].Delete, 2) + require.Equal(t, []string{"copy2", "original"}, + []string{groups[0].Delete[0].UUID, groups[0].Delete[1].UUID}) +} + +// Two unrelated sets of duplicates stay separate. +func TestFindDuplicateGroupsSeparateSets(t *testing.T) { + in := items.Items{ + testNote("a", "One", "", 100, false), + testNote("a-copy", "One copy", "a", 200, false), + testNote("b", "Two", "", 300, false), + testNote("b-copy", "Two copy", "b", 100, false), + } + + groups, _ := FindDuplicateGroups(in) + + require.Len(t, groups, 2) + + keep := map[string]string{} + for _, g := range groups { + keep[g.Keep.UUID] = g.Delete[0].UUID + } + + require.Equal(t, map[string]string{"a-copy": "a", "b": "b-copy"}, keep) +} + +func TestDeleteDuplicateNotesOutputDeleted(t *testing.T) { + out := DeleteDuplicateNotesOutput{ + Groups: []DuplicateGroup{ + {Keep: NoteSummary{UUID: "a"}, Delete: []NoteSummary{{UUID: "a-copy"}}}, + {Keep: NoteSummary{UUID: "b"}, Delete: []NoteSummary{{UUID: "b-copy"}, {UUID: "b-copy2"}}}, + }, + } + + require.Len(t, out.Deleted(), 3) +} + +func TestNoteSummaryUpdatedDate(t *testing.T) { + require.Equal(t, "2026-09-21 14:30", + NoteSummary{UpdatedAtText: "2026-09-21T14:30:00.000Z"}.UpdatedDate()) + + // an unparseable value is shown as it came from the API + require.Equal(t, "not a time", NoteSummary{UpdatedAtText: "not a time"}.UpdatedDate()) }