Skip to content

Commit 7bf2143

Browse files
jongioCopilot
andcommitted
fix: make sortGroupsByLatest take precedence over sortGroupsByLabel in pivot mode
When sorting by 'updated' in pivot/grouped mode, sortGroupsByLatest was called to reorder groups by recency, but sortGroupsByLabel always ran unconditionally afterward, overwriting the recency-based ordering with alphabetical label ordering. Changed to an else branch so the two sorts are mutually exclusive: updated-sort uses recency, all others use label. Fixed in both the main load path and the deep-search path. Added regression tests verifying the correct group ordering behavior. Co-authored-by: Copilot <[email protected]>
1 parent 2946c6f commit 7bf2143

2 files changed

Lines changed: 72 additions & 3 deletions

File tree

internal/tui/model.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3125,11 +3125,13 @@ func (m Model) loadSessionsCmd() tea.Cmd {
31253125
return dataErrorMsg{err: err}
31263126
}
31273127
// When sorting by updated time, reorder groups so the most
3128-
// recently active folder appears first.
3128+
// recently active group appears first; otherwise sort groups
3129+
// alphabetically by their pivot label.
31293130
if sortOpts.Field == data.SortByUpdated {
31303131
sortGroupsByLatest(groups, sortOpts.Order)
3132+
} else {
3133+
sortGroupsByLabel(groups, pivotOrd)
31313134
}
3132-
sortGroupsByLabel(groups, pivotOrd)
31333135
return groupsLoadedMsg{groups: groups}
31343136
}
31353137
sessions, err := store.ListSessions(filter, sortOpts, limit)
@@ -3547,8 +3549,9 @@ func (m Model) deepSearchCmd(version int) tea.Cmd {
35473549
}
35483550
if sortOpts.Field == data.SortByUpdated {
35493551
sortGroupsByLatest(groups, sortOpts.Order)
3552+
} else {
3553+
sortGroupsByLabel(groups, pivotOrd)
35503554
}
3551-
sortGroupsByLabel(groups, pivotOrd)
35523555
return deepSearchResultMsg{version: version, groups: groups}
35533556
}
35543557
sessions, err := store.ListSessions(filter, sortOpts, limit)

internal/tui/model_helpers_test.go

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,72 @@ func TestSortGroupsByLatest_SingleGroup(t *testing.T) {
241241
}
242242
}
243243

244+
// ---------------------------------------------------------------------------
245+
// Group sort ordering: sortGroupsByLatest must not be overridden by label sort
246+
// ---------------------------------------------------------------------------
247+
248+
// TestSortGroupsByLatest_NotOverriddenByLabel is a regression test ensuring
249+
// that when sort field is SortByUpdated, groups are ordered by recency, NOT
250+
// alphabetically by label. Before the fix, sortGroupsByLabel always ran
251+
// after sortGroupsByLatest, overwriting the recency-based ordering.
252+
func TestSortGroupsByLatest_NotOverriddenByLabel(t *testing.T) {
253+
// Groups whose labels are alphabetical (A < B < Z) but whose sessions
254+
// have recency in the opposite order (Z=newest, A=oldest).
255+
groups := []data.SessionGroup{
256+
{Label: "A-folder", Sessions: []data.Session{{LastActiveAt: "2024-01-01T00:00:00Z"}}},
257+
{Label: "B-folder", Sessions: []data.Session{{LastActiveAt: "2024-01-02T00:00:00Z"}}},
258+
{Label: "Z-folder", Sessions: []data.Session{{LastActiveAt: "2024-01-03T00:00:00Z"}}},
259+
}
260+
261+
// Simulate the fixed loadSessionsCmd logic: SortByUpdated → sortGroupsByLatest only.
262+
sortOpts := data.SortOptions{Field: data.SortByUpdated, Order: data.Descending}
263+
pivotOrd := data.Ascending
264+
265+
if sortOpts.Field == data.SortByUpdated {
266+
sortGroupsByLatest(groups, sortOpts.Order)
267+
} else {
268+
sortGroupsByLabel(groups, pivotOrd)
269+
}
270+
271+
// Expect recency order: Z-folder (newest) first, A-folder (oldest) last.
272+
if groups[0].Label != "Z-folder" {
273+
t.Errorf("expected first group 'Z-folder' (newest), got %q", groups[0].Label)
274+
}
275+
if groups[2].Label != "A-folder" {
276+
t.Errorf("expected last group 'A-folder' (oldest), got %q", groups[2].Label)
277+
}
278+
}
279+
280+
// TestSortGroupsByLabel_UsedWhenNotSortByUpdated verifies that when sorting
281+
// by a non-updated field, groups are sorted alphabetically by label.
282+
func TestSortGroupsByLabel_UsedWhenNotSortByUpdated(t *testing.T) {
283+
groups := []data.SessionGroup{
284+
{Label: "Z-folder", Sessions: []data.Session{{LastActiveAt: "2024-01-03T00:00:00Z"}}},
285+
{Label: "A-folder", Sessions: []data.Session{{LastActiveAt: "2024-01-01T00:00:00Z"}}},
286+
{Label: "M-folder", Sessions: []data.Session{{LastActiveAt: "2024-01-02T00:00:00Z"}}},
287+
}
288+
289+
sortOpts := data.SortOptions{Field: data.SortByName, Order: data.Descending}
290+
pivotOrd := data.Ascending
291+
292+
if sortOpts.Field == data.SortByUpdated {
293+
sortGroupsByLatest(groups, sortOpts.Order)
294+
} else {
295+
sortGroupsByLabel(groups, pivotOrd)
296+
}
297+
298+
// Expect alphabetical ascending: A, M, Z.
299+
if groups[0].Label != "A-folder" {
300+
t.Errorf("expected first group 'A-folder', got %q", groups[0].Label)
301+
}
302+
if groups[1].Label != "M-folder" {
303+
t.Errorf("expected second group 'M-folder', got %q", groups[1].Label)
304+
}
305+
if groups[2].Label != "Z-folder" {
306+
t.Errorf("expected third group 'Z-folder', got %q", groups[2].Label)
307+
}
308+
}
309+
244310
// ---------------------------------------------------------------------------
245311
// resolveTheme
246312
// ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)