fix(bookmarks): play the item of the bookmark

AGENTS.md

@@ -148,6 +148,8 @@ Styles from theme config. Format helpers (timestamps, durations). `RunProgram` r
 - Each launch makes a new mpv handle, which plays at the speed 1.0. `handlePlayerReady` sets the speed of the model in the new player.
 - The listening time (`timeListened`) is the time of the clock between two position ticks of a player that plays (`trackListeningTime`). It is not the change of the position: at the speed 2.0, the position moves two seconds in each second. A player that is paused or idle does not play. mpv reports no pause while it waits for data, so a position tick also reads the mpv property `core-idle` (`GetIdle`, `PositionMsg.Idle`). A test moves the clock with `Model.now`.
 - pine finds the end of a track through `errors.Is(err, player.ErrClosed)` on a position tick. spruce uses `PlayerEndMsg`, and a tick with `ErrClosed` can arrive before it. Do not match the text of a player error.
+- Enter on a bookmark (`handleSeekToBookmark`) seeks the player only if the book or the episode of the bookmark plays. If not, it stops the playback and starts that book or that episode at the time of the bookmark.
+- A bookmark of ABS has no episode ID. `handleAddBookmark` puts the episode title and `episodeTitleSeparator` before the title of a podcast bookmark, and `bookmarkEpisode` finds the episode from that title. The bookmarks on a server have that format, so do not change it.
 
 ## Testing
 

internal/app/bookmarks.go

@@ -3,6 +3,7 @@ package app
 import (
 	"context"
 	"fmt"
+	"strings"
 	"time"
 
 	"github.com/Thelost77/pine/internal/abs"
@@ -37,7 +38,7 @@ func (m Model) handleAddBookmark(msg detail.AddBookmarkCmd) (Model, tea.Cmd) {
 	currentTime := m.player.Position
 	title := fmt.Sprintf("Bookmark at %s", ui.FormatTimestamp(currentTime))
 	if m.episodeID != "" && m.player.Title != "" {
-		title = fmt.Sprintf("%s — %s", m.player.Title, title)
+		title = m.player.Title + episodeTitleSeparator + title
 	}
 	logger.Info("creating bookmark", "itemID", itemID, "time", currentTime, "title", title)
 	existing := m.detail.Bookmarks()
@@ -58,16 +59,73 @@ func (m Model) handleAddBookmark(msg detail.AddBookmarkCmd) (Model, tea.Cmd) {
 	}
 }
 
-// handleSeekToBookmark seeks the player to a bookmark's timestamp (book-global).
-func (m Model) handleSeekToBookmark(msg detail.SeekToBookmarkCmd) (Model, tea.Cmd) {
-	if !m.isPlaying() {
-		if m.client == nil || msg.Item.ID == "" {
-			return m, nil
+// episodeTitleSeparator is between the episode title and the bookmark title
+// in the title of a bookmark of a podcast episode.
+const episodeTitleSeparator = " — "
+
+// bookmarkEpisode finds the episode of a podcast bookmark. A bookmark of ABS
+// has no episode ID, so handleAddBookmark puts the episode title before the
+// bookmark title. If more than one episode title is before the bookmark title,
+// the longest is the episode.
+func bookmarkEpisode(title string, episodes []abs.PodcastEpisode) (abs.PodcastEpisode, bool) {
+	var episode abs.PodcastEpisode
+	found := false
+	for _, ep := range episodes {
+		if ep.Title == "" || !strings.HasPrefix(title, ep.Title+episodeTitleSeparator) {
+			continue
 		}
-		m.setSeriesContext(msg.Item)
-		return m, m.startPlaybackAtBookPositionCmd(msg.Item, msg.Time)
+		if !found || len(ep.Title) > len(episode.Title) {
+			episode, found = ep, true
+		}
+	}
+	return episode, found
+}
+
+// handleSeekToBookmark moves the playback to a bookmark's timestamp
+// (book-global). If the book or the episode of the bookmark plays, it seeks
+// the player. If not, it starts the book or the episode at that timestamp.
+func (m Model) handleSeekToBookmark(msg detail.SeekToBookmarkCmd) (Model, tea.Cmd) {
+	isPodcast := msg.Item.MediaType == "podcast"
+	var episode abs.PodcastEpisode
+	hasEpisode := false
+	if isPodcast {
+		episode, hasEpisode = bookmarkEpisode(msg.Title, m.detail.Episodes())
+	}
+
+	// The title of a podcast bookmark can give no episode, for example after
+	// an edit of the title. Then a bookmark of the podcast that plays seeks
+	// the episode that plays.
+	if m.isPlaying() && m.itemID == msg.Item.ID && (!hasEpisode || m.episodeID == episode.ID) {
+		return m.seekToBookGlobalPosition(msg.Time)
+	}
+	if m.client == nil || msg.Item.ID == "" {
+		return m, nil
+	}
+	if isPodcast && !hasEpisode {
+		logger.Warn("bookmark seek skipped: episode not found", "itemID", msg.Item.ID, "title", msg.Title)
+		cmd := m.err.SetError(fmt.Errorf("cannot find the episode of this bookmark"))
+		m.propagateSize()
+		return m, cmd
+	}
+
+	// Different item or episode playing → stop it first
+	var closeCmd, stopCmd tea.Cmd
+	if m.isPlaying() {
+		logger.Info("switching playback", "from", m.itemID+"/"+m.episodeID, "to", msg.Item.ID+"/"+episode.ID)
+		m, closeCmd, stopCmd = m.stopPlaybackCmds()
+	}
+
+	m.setSeriesContext(msg.Item)
+	var startCmd tea.Cmd
+	if hasEpisode {
+		startCmd = m.startEpisodePlaybackAtPositionCmd(msg.Item, episode, msg.Time)
+	} else {
+		startCmd = m.startPlaybackAtBookPositionCmd(msg.Item, msg.Time)
+	}
+	if stopCmd != nil {
+		return m, tea.Batch(stopCmd, closeThen(closeCmd, startCmd))
 	}
-	return m.seekToBookGlobalPosition(msg.Time)
+	return m, startCmd
 }
 
 // handleDeleteBookmark deletes a bookmark and removes it from the local list.

internal/app/bookmarks_test.go

@@ -288,3 +288,32 @@ func TestHandleSeekToBookmarkStartsPlaybackWhenStopped(t *testing.T) {
 		t.Errorf("expected no progress patch before starting playback, got %v", progressBody)
 	}
 }
+
+func TestBookmarkEpisode(t *testing.T) {
+	episodes := []abs.PodcastEpisode{
+		{ID: "ep-1", Title: "Pilot"},
+		{ID: "ep-2", Title: "Pilot — Part 2"},
+		{ID: "ep-3", Title: ""},
+	}
+
+	tests := []struct {
+		name   string
+		title  string
+		wantID string
+		wantOK bool
+	}{
+		{"title of handleAddBookmark", "Pilot — Bookmark at 2:00", "ep-1", true},
+		{"longest episode title", "Pilot — Part 2 — Bookmark at 2:00", "ep-2", true},
+		{"no episode title", "Bookmark at 2:00", "", false},
+		{"episode title with no separator", "Pilot", "", false},
+		{"empty episode title", " — Bookmark at 2:00", "", false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			episode, ok := bookmarkEpisode(tt.title, episodes)
+			if ok != tt.wantOK || episode.ID != tt.wantID {
+				t.Errorf("bookmarkEpisode(%q) = %q, %v, want %q, %v", tt.title, episode.ID, ok, tt.wantID, tt.wantOK)
+			}
+		})
+	}
+}

internal/app/e2e_test.go

@@ -576,6 +576,223 @@ func TestE2E_BookmarkEnterStartsPlaybackWhenStopped(t *testing.T) {
 	}
 }
 
+func TestE2E_BookmarkEnterSwitchesToItemOfBookmark(t *testing.T) {
+	log := &apiLog{}
+	state := &e2eServerState{bookmarks: make(map[string][]abs.Bookmark)}
+	state.bookmarks["item-002"] = []abs.Bookmark{
+		testBookmark("In the other book", 300.0),
+	}
+	srv := newFullMockABSServer(log, state)
+	defer srv.Close()
+
+	mp := &mockPlayer{position: 50, duration: 3600}
+	m := newE2EModelAuthenticated(srv, mp)
+	m = e2eSetSize(m, 120, 40)
+
+	// Set up: item-001 plays
+	m.sessionID = "sess-other"
+	m.itemID = "item-001"
+	m.player.Playing = true
+	m.player.Position = 50.0
+	m.player.Duration = 3600.0
+
+	// Open the detail screen of item-002 and select its bookmark
+	item := testLibraryItem("item-002", "1984")
+	res, cmd := m.Update(home.NavigateDetailMsg{Item: item})
+	m = res.(Model)
+	assertScreen(t, m, ScreenDetail)
+	m = feedCmdChain(m, cmd, 5)
+
+	m, _ = e2ePressSpecial(m, tea.KeyTab)
+	if !m.detail.FocusBookmarks() {
+		t.Fatal("expected bookmark focus after Tab")
+	}
+
+	m, cmd = e2ePressSpecial(m, tea.KeyEnter)
+	m, cmd = feedCmd(m, cmd)
+	m = feedCmdChain(m, cmd, 2)
+
+	assertAPICallMade(t, log, "POST", "/api/session/sess-other/close")
+	assertAPICallMade(t, log, "POST", "/api/items/item-002/play")
+	if m.itemID != "item-002" {
+		t.Fatalf("itemID = %q, want item-002", m.itemID)
+	}
+	if m.sessionID != "sess-e2e" {
+		t.Fatalf("sessionID = %q, want sess-e2e", m.sessionID)
+	}
+	if m.player.Position != 300.0 {
+		t.Fatalf("player position = %f, want 300.0", m.player.Position)
+	}
+	if !mp.launched {
+		t.Fatal("expected player launch after bookmark enter")
+	}
+}
+
+func TestE2E_PodcastBookmarkEnterSwitchesToEpisodeOfBookmark(t *testing.T) {
+	log := &apiLog{}
+	state := &e2eServerState{bookmarks: make(map[string][]abs.Bookmark)}
+	state.bookmarks["pod-001"] = []abs.Bookmark{
+		testBookmark("Episode 2 - Deep Dive — Bookmark at 5:00", 300.0),
+	}
+	srv := newFullMockABSServer(log, state)
+	defer srv.Close()
+
+	mp := &mockPlayer{position: 50, duration: 3600}
+	m := newE2EModelAuthenticated(srv, mp)
+	m = e2eSetSize(m, 120, 40)
+
+	// Set up: a book plays
+	m.sessionID = "sess-other"
+	m.itemID = "item-001"
+	m.player.Playing = true
+	m.player.Position = 50.0
+	m.player.Duration = 3600.0
+
+	// Open the detail screen of the podcast and select its bookmark
+	item := testPodcastItem("pod-001", "Tech Talk Daily")
+	res, cmd := m.Update(home.NavigateDetailMsg{Item: item})
+	m = res.(Model)
+	assertScreen(t, m, ScreenDetail)
+	m = feedCmdChain(m, cmd, 5)
+
+	m, _ = e2ePressSpecial(m, tea.KeyTab)
+	if !m.detail.FocusBookmarks() {
+		t.Fatal("expected bookmark focus after Tab")
+	}
+
+	m, cmd = e2ePressSpecial(m, tea.KeyEnter)
+	m, cmd = feedCmd(m, cmd)
+	m = feedCmdChain(m, cmd, 2)
+
+	assertAPICallMade(t, log, "POST", "/api/session/sess-other/close")
+	assertAPICallMade(t, log, "POST", "/api/items/pod-001/play/pod-001-ep-002")
+	if m.itemID != "pod-001" || m.episodeID != "pod-001-ep-002" {
+		t.Fatalf("playing %q/%q, want pod-001/pod-001-ep-002", m.itemID, m.episodeID)
+	}
+	if m.player.Position != 300.0 {
+		t.Fatalf("player position = %f, want 300.0", m.player.Position)
+	}
+	if !mp.launched {
+		t.Fatal("expected player launch after bookmark enter")
+	}
+}
+
+func TestE2E_PodcastBookmarkEnterWithSamePodcastPlaying(t *testing.T) {
+	log := &apiLog{}
+	state := &e2eServerState{bookmarks: make(map[string][]abs.Bookmark)}
+	state.bookmarks["pod-001"] = []abs.Bookmark{
+		testBookmark("Episode 1 - Pilot — Bookmark at 2:00", 120.0),
+		testBookmark("Episode 2 - Deep Dive — Bookmark at 5:00", 300.0),
+	}
+	srv := newFullMockABSServer(log, state)
+	defer srv.Close()
+
+	mp := &mockPlayer{position: 50, duration: testEpisodeDuration}
+	m := newE2EModelAuthenticated(srv, mp)
+	m = e2eSetSize(m, 120, 40)
+
+	// Set up: episode 1 of the podcast plays
+	m.sessionID = "sess-ep-one"
+	m.itemID = "pod-001"
+	m.episodeID = "pod-001-ep-001"
+	m.player.Playing = true
+	m.player.Position = 50.0
+	m.player.Duration = testEpisodeDuration
+
+	item := testPodcastItem("pod-001", "Tech Talk Daily")
+	res, cmd := m.Update(home.NavigateDetailMsg{Item: item})
+	m = res.(Model)
+	assertScreen(t, m, ScreenDetail)
+	m = feedCmdChain(m, cmd, 5)
+
+	m, _ = e2ePressSpecial(m, tea.KeyTab)
+	if !m.detail.FocusBookmarks() {
+		t.Fatal("expected bookmark focus after Tab")
+	}
+
+	// --- Bookmark of the episode that plays: seek ---
+	m, cmd = e2ePressSpecial(m, tea.KeyEnter)
+	m, cmd = feedCmd(m, cmd)
+	m = feedCmdChain(m, cmd, 2)
+
+	assertNoAPICall(t, log, "POST", "/api/items/pod-001/play")
+	if m.sessionID != "sess-ep-one" {
+		t.Fatalf("sessionID = %q, want sess-ep-one", m.sessionID)
+	}
+	mp.mu.Lock()
+	pos := mp.position
+	mp.mu.Unlock()
+	if pos != 120.0 {
+		t.Fatalf("mock player position = %f, want 120.0 (bookmark time)", pos)
+	}
+
+	// --- Bookmark of a different episode: start that episode ---
+	m, _ = e2ePressKey(m, 'j')
+	m, cmd = e2ePressSpecial(m, tea.KeyEnter)
+	m, cmd = feedCmd(m, cmd)
+	m = feedCmdChain(m, cmd, 2)
+
+	assertAPICallMade(t, log, "POST", "/api/session/sess-ep-one/close")
+	assertAPICallMade(t, log, "POST", "/api/items/pod-001/play/pod-001-ep-002")
+	if m.itemID != "pod-001" || m.episodeID != "pod-001-ep-002" {
+		t.Fatalf("playing %q/%q, want pod-001/pod-001-ep-002", m.itemID, m.episodeID)
+	}
+	if m.player.Position != 300.0 {
+		t.Fatalf("player position = %f, want 300.0", m.player.Position)
+	}
+}
+
+func TestE2E_PodcastBookmarkEnterWithUnknownEpisodeKeepsPlayback(t *testing.T) {
+	log := &apiLog{}
+	state := &e2eServerState{bookmarks: make(map[string][]abs.Bookmark)}
+	state.bookmarks["pod-001"] = []abs.Bookmark{
+		testBookmark("A title with no episode", 300.0),
+	}
+	srv := newFullMockABSServer(log, state)
+	defer srv.Close()
+
+	mp := &mockPlayer{position: 50, duration: 3600}
+	m := newE2EModelAuthenticated(srv, mp)
+	m = e2eSetSize(m, 120, 40)
+
+	// Set up: a book plays
+	m.sessionID = "sess-other"
+	m.itemID = "item-001"
+	m.player.Playing = true
+	m.player.Position = 50.0
+	m.player.Duration = 3600.0
+
+	item := testPodcastItem("pod-001", "Tech Talk Daily")
+	res, cmd := m.Update(home.NavigateDetailMsg{Item: item})
+	m = res.(Model)
+	assertScreen(t, m, ScreenDetail)
+	m = feedCmdChain(m, cmd, 5)
+
+	m, _ = e2ePressSpecial(m, tea.KeyTab)
+	if !m.detail.FocusBookmarks() {
+		t.Fatal("expected bookmark focus after Tab")
+	}
+
+	m, cmd = e2ePressSpecial(m, tea.KeyEnter)
+	m, cmd = feedCmd(m, cmd)
+	m = feedCmdChain(m, cmd, 2)
+
+	assertNoAPICall(t, log, "POST", "/api/session/sess-other/close")
+	assertNoAPICall(t, log, "POST", "/api/items/pod-001/play")
+	if m.sessionID != "sess-other" || m.itemID != "item-001" {
+		t.Fatalf("playing %q in session %q, want item-001 in sess-other", m.itemID, m.sessionID)
+	}
+	mp.mu.Lock()
+	pos := mp.position
+	mp.mu.Unlock()
+	if pos != 50.0 {
+		t.Fatalf("mock player position = %f, want 50.0 (no seek)", pos)
+	}
+	if !m.err.HasError() {
+		t.Fatal("expected an error for a bookmark with no known episode")
+	}
+}
+
 func TestE2E_BookmarkTitleEdit(t *testing.T) {
 	log := &apiLog{}
 	state := &e2eServerState{bookmarks: make(map[string][]abs.Bookmark)}