From d3e337261e620d991eee0e284ac5fbdab35b1f7e Mon Sep 17 00:00:00 2001 From: 4gray Date: Mon, 27 Jul 2026 21:40:34 +0200 Subject: [PATCH] fix(portals): stop a pin write from landing on the next movie MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two findings from the review of the previous round. A pin write is an IPC round-trip, and the user can navigate during it. The continuation then applied one film's answer to another film's controller — and because unpinning returns "nothing pinned", it would clear the pin the new movie had just loaded and its Play action would quietly stop starting from the preferred source. It now commits only while the same film is still on screen, like every other async path here. The short-title scan drops its row limit. FTS keeps its window because it ranks by relevance, so what it keeps is what matters; a scan cannot rank, so a window there silently decides which valid sources the user is allowed to see. It also bought nothing: the GLOB cannot use an index, so SQLite reads every row either way and the limit only truncated the answer. What bounds the scan is its predicate — reaching it means the whole title is one or two characters. The switch-notice type moves to the module that builds it, which also removes a circular type import between the two. Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 2 +- .../title-sources.operations.spec.ts | 25 ++++++------ .../operations/title-sources.operations.ts | 28 ++++++------- docs/architecture/vod-multi-source.md | 36 ++++++++++------- .../vod-multi-source-host-races.spec.ts | 31 ++++++++++++++ .../vod-multi-source-host.service.ts | 40 +++++++++---------- .../vod-details/vod-multi-source-notice.ts | 11 ++++- 7 files changed, 110 insertions(+), 63 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index ddc4114a0..df835cb08 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -823,7 +823,7 @@ engine` (restart required) or - Finds the same movie in the user's other imported playlists and adds a "Sources N" chip to the Xtream VOD action row (only when ≥1 alternative exists), plus a `.source-caption` line reporting where playback is coming from. The chip opens a 460px anchored CDK-overlay popover (`libs/ui/components/src/lib/vod-sources/`; not `MatMenu`, which caps its width at 280px), reused unchanged in the inline player's now-playing bar and on the playback-error screen. Both chips are handed the same `matchKind` and `vodAutoFailover` and both write the setting back. The chip counts alternative **streams**; the caption ("also found in N other playlists") counts distinct **playlists** via `alternativePlaylistCount`, because the popover groups one portal's copies under that portal. - Scope v1 is **Xtream ↔ Xtream, movies only, Electron only**. Stalker never reaches the `content` table and M3U is a JSON blob whose search forces `content_type:'live'`; both are additive later since `VodSourceCandidate.portalType` already carries all three. In the PWA every entry point is gated off by a bridge `typeof` check and the chip renders nothing. - **Metadata provenance is the core contract.** Every field is `{value, provenance}` where `api`/`probe` are facts (plain tag), `parsed` is a title-regex guess (tag prefixed `~`, warn colour), and absent renders **no tag at all** plus a `check` chip. `factualOnly()` in `vod-source-metadata.util.ts` is the only accessor allowed for ranking/failover, so guesses are structurally unable to influence a decision. `VodSourceProbeStatus` separates `fail` (contacted and refused) from `unknown` (timed out / blocked / no capability) — an unchecked source is never shown as offline. Quality is derived from pixel **width** because letterboxing crops height. -- Discovery (`DB_FIND_TITLE_SOURCES`, trigram FTS over `content_title_fts`) is lazy and returns only what the `content` table can prove; titles whose tokens are all shorter than three characters ("Up", "It") fall back to a scan, since the trigram tokenizer cannot index them at all. Both queries are row-bounded, so what fills the window matters: the current playlist is excluded **in SQL** (its own duplicate rows would otherwise crowd out every alternative), and the scan matches the token as a whole word (`' ' || LOWER(title) || ' ' GLOB '*[^a-z0-9]it[^a-z0-9]*'`) ordered by title length with a wider budget, so "Titanic" and "The Italian Job" cannot push the real "It" out. Resolution is deferred to click/pin/check because `content` stores no `container_extension` and `constructVodUrl` returns `''` without one — each alternative costs a live `get_vod_info` against the foreign playlist's credentials. +- Discovery (`DB_FIND_TITLE_SOURCES`, trigram FTS over `content_title_fts`) is lazy and returns only what the `content` table can prove; titles whose tokens are all shorter than three characters ("Up", "It") fall back to a scan, since the trigram tokenizer cannot index them at all. A source that is never read looks exactly like one that does not exist, so: the current playlist is excluded **in SQL** (its own duplicate rows would otherwise crowd out every alternative), and the scan matches the token as a whole word (`' ' || LOWER(title) || ' ' GLOB '*[^a-z0-9]it[^a-z0-9]*'`) ordered by title length **with no row limit** — FTS keeps its 60-row window because it ranks by relevance, while a scan cannot rank, and the GLOB reads every row regardless so a limit would only truncate the answer. Resolution is deferred to click/pin/check because `content` stores no `container_extension` and `constructVodUrl` returns `''` without one — each alternative costs a live `get_vod_info` against the foreign playlist's credentials. - Switching = one `inlinePlayback.set({...next, startTime})`, never null-then-set, so the player and engine survive and re-seek. The carried position is read *before* the 15s persistence throttle, and `VodDetailsPlaybackService` uses a one-shot `resumeSettled` latch so a resuming engine's `timeupdate` at ~0 cannot overwrite the resume point. `handleInlineTimeUpdate` returns that verdict and the route feeds multi-source the requested `startTime` until the engine reaches it — one latch for both, or a switch during the initial seek would restart the film. - Pins are keyed portal-agnostically (`tmdb:{id}` else `title:{base}:{year}`, `vod_source_pins` table); lookups pass every alias most-trusted-first so a late TMDB id does not orphan a title-keyed pin. A pin is not decoration: the primary Play action starts from the pinned source, and it outranks everything else in failover ranking. Two identity keys: `vodMultiSourceMovieKey` (title, year, tmdbId) makes TMDB enrichment re-trigger discovery and rebuild the pin keys, while `vodMultiSourceSessionKey` (`playlistId:contentId`) decides whether that rerun is a refresh or a new session — a refresh keeps the active source, its resolved facts, the tried set, the live position and any switch in flight; only a different film resets them. - Auto-failover is `Settings.vodAutoFailover`, **opt-in and off by default**, web engines only. Each source is tried at most once per session (`triedSourceIds` only grows), so it terminates structurally, and it continues past candidates that fail to resolve rather than stopping at the first one — `switchTo` reports whether it was unresolvable (keep going) or superseded (stop), since only the former marks the candidate tried. The switch is never silent: the toast names the new playlist, offers Undo, and warns "dub may differ" only when both sides state an audio track as fact. diff --git a/apps/electron-backend/src/app/database/operations/title-sources.operations.spec.ts b/apps/electron-backend/src/app/database/operations/title-sources.operations.spec.ts index 1cc06d2d6..153dc73c4 100644 --- a/apps/electron-backend/src/app/database/operations/title-sources.operations.spec.ts +++ b/apps/electron-backend/src/app/database/operations/title-sources.operations.spec.ts @@ -71,13 +71,12 @@ describe('title-sources.operations', () => { ); }); - it('scans on a word boundary, shortest first, in a wider window', async () => { + it('scans on a word boundary, shortest first, and never truncates', async () => { // A substring scan for "It" matches "Titanic" and "The Italian - // Job"; 60 of those sorted by title can push the real "It" out of - // the window, and discovery then reports no sources at all. The - // scan therefore asks for the token as a WORD, orders by title - // length (a match is the title plus decoration: "IT (2017)"), and - // gets a wider budget than the relevance-ranked FTS path. + // Job"; capped and sorted by title, those can push the real "It" + // out of the result set and discovery reports no sources at all. + // The scan therefore asks for the token as a WORD and orders by + // title length (a match is the title plus decoration). const scan = createDbMock([]); await findTitleSources(scan.db, { title: 'It' }); const scanQuery = compiledQuery(scan.all); @@ -86,14 +85,14 @@ describe('title-sources.operations', () => { expect(scanQuery.sql).not.toContain('LIKE ?'); expect(scanQuery.params).toContain('*[^a-z0-9]it[^a-z0-9]*'); expect(scanQuery.sql).toContain('ORDER BY LENGTH(c.title)'); + // No window at all: unlike FTS this cannot rank, so a limit would + // silently decide which valid sources the user may see — and the + // GLOB reads every row regardless, so it would not even save work. + expect(scanQuery.sql).not.toContain('LIMIT'); const fts = createDbMock([]); await findTitleSources(fts.db, { title: 'Dune' }); - const [scanLimit] = scanQuery.params.slice(-1) as number[]; - const [ftsLimit] = compiledQuery(fts.all).params.slice( - -1 - ) as number[]; - expect(scanLimit).toBeGreaterThan(ftsLimit); + expect(compiledQuery(fts.all).sql).toContain('LIMIT'); }); it('does not offer a scan hit whose title merely contains the query', async () => { @@ -244,8 +243,10 @@ describe('title-sources.operations', () => { const scanQuery = compiledQuery(scan.all); expect(scanQuery.sql).toContain('cat.playlist_id <> ?'); expect(scanQuery.params).toContain('playlist-1'); + // The scan takes no window, so the cost it saves here is the rows + // read rather than the rows kept. expect(scanQuery.sql.indexOf('cat.playlist_id <> ?')).toBeLessThan( - scanQuery.sql.indexOf('LIMIT') + scanQuery.sql.indexOf('ORDER BY') ); }); diff --git a/apps/electron-backend/src/app/database/operations/title-sources.operations.ts b/apps/electron-backend/src/app/database/operations/title-sources.operations.ts index 67466b617..8bf7ca09c 100644 --- a/apps/electron-backend/src/app/database/operations/title-sources.operations.ts +++ b/apps/electron-backend/src/app/database/operations/title-sources.operations.ts @@ -22,14 +22,6 @@ import type { AppDatabase } from '../database.types'; const CANDIDATE_LIMIT = 60; -/** - * The scan path gets its own, wider window. FTS ranks by relevance, so the - * real match is near the top of its 60; the scan can only order by title, and - * its predicate is a word-boundary match rather than an equality — several - * genuinely different films can share the token ("It Follows", "Bring It On"). - */ -const SCAN_CANDIDATE_LIMIT = 200; - /** A row as it comes back from SQLite, before match confirmation. */ interface TitleSourceRow { content_id: number; @@ -82,12 +74,19 @@ function excludePlaylistClause( * handful of titles FTS structurally cannot serve. * * The predicate is a WORD-boundary match, not a substring one: `LIKE '%it%'` - * matches "Titanic" and "The Italian Job", so alphabetically earlier noise - * could fill the window and push the real "It" out of it. Padding both sides - * lets one GLOB pattern cover the start, middle and end of the title at once. - * Rows come back shortest-title-first because a matching title is the base - * plus decoration ("IT (2017) 1080p"), so the true matches sort ahead of the - * longer films that merely contain the word. + * matches "Titanic" and "The Italian Job", so noise could crowd out the real + * "It" entirely. Padding both sides lets one GLOB pattern cover the start, + * middle and end of the title at once. + * + * And it takes NO row limit, unlike the FTS path. FTS ranks by relevance, so + * a window keeps the best rows; a scan can only order by title, so a window + * silently decides which valid sources the user is allowed to see. It is also + * a false economy — the GLOB cannot use an index, so SQLite reads every row + * either way and a `LIMIT` saves only transfer. What bounds this instead is + * the predicate: reaching here means the movie's whole title is one or two + * characters, and only films carrying that exact word come back. Rows arrive + * shortest-title-first because a match is the title plus decoration + * ("IT (2017) 1080p"). * * Still a necessary-not-sufficient filter — `normalizeTitleKeys` below is what * confirms a match. Like the `LIKE` it replaces, it compares ASCII-lowercased @@ -116,7 +115,6 @@ function scanCandidateQuery(base: string, excludePlaylist: SQL) { AND ' ' || LOWER(c.title) || ' ' GLOB ${wordMatch} ${excludePlaylist} ORDER BY LENGTH(c.title), c.title - LIMIT ${SCAN_CANDIDATE_LIMIT} `; } diff --git a/docs/architecture/vod-multi-source.md b/docs/architecture/vod-multi-source.md index a57d0a345..b6a60d367 100644 --- a/docs/architecture/vod-multi-source.md +++ b/docs/architecture/vod-multi-source.md @@ -151,24 +151,32 @@ the same film. Off the screen is not, so `applyDiscoveredSources` keeps it as a row and leaves it active; a caption naming a playlist that is not streaming anything would be a lie about the one thing this feature exists to state. -## The candidate window +## What the queries are allowed to miss -Both queries take a bounded number of rows, so what fills that window decides -whether an alternative is findable at all: +A source that exists but is never read is indistinguishable, to the user, from +one that does not exist — the chip simply does not appear. So: - **The current playlist is excluded in SQL**, not afterwards. It routinely lists a film in several categories, and those rows would otherwise spend the - budget before a single other playlist was read. -- **Short titles scan on a word boundary.** The trigram tokenizer cannot index - tokens under three characters, so "Up", "It" or "Us" produce an empty `MATCH` - and fall back to a scan. A substring scan matches "Titanic" and "The Italian - Job" for "It", and enough of those sorted by title push the real film out of - the window — so the scan asks for the token as a word - (`' ' || LOWER(title) || ' ' GLOB '*[^a-z0-9]it[^a-z0-9]*'`), orders by title - length (a match is the title plus decoration: "IT (2017) 1080p"), and gets a - wider budget than the relevance-ranked FTS path. Like the `LIKE` it replaces - it compares ASCII-lowercased text, so a non-ASCII short title is no better and - no worse served than before. + FTS window before a single other playlist was read. +- **Short titles scan on a word boundary, and take no window at all.** The + trigram tokenizer cannot index tokens under three characters, so "Up", "It" + or "Us" produce an empty `MATCH` and fall back to a scan. A substring scan + matches "Titanic" and "The Italian Job" for "It", so the scan asks for the + token as a word + (`' ' || LOWER(title) || ' ' GLOB '*[^a-z0-9]it[^a-z0-9]*'`) and orders by + title length (a match is the title plus decoration: "IT (2017) 1080p"). + + The FTS path keeps its 60-row window because it ranks by relevance — the best + rows are the ones it keeps. A scan cannot rank, so a window there would + silently decide which valid sources the user may see, and it would not even + buy anything: the GLOB cannot use an index, so SQLite reads every row either + way and a `LIMIT` saves transfer, not work. What bounds the scan instead is + its predicate — reaching it means the movie's entire title is one or two + characters, and only films carrying that exact word come back. + + Like the `LIKE` it replaces it compares ASCII-lowercased text, so a non-ASCII + short title is no better and no worse served than before. Both remain necessary-not-sufficient filters: the two-tier normalized confirmation still runs afterwards, so the looser query never admits "Upgrade" diff --git a/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host-races.spec.ts b/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host-races.spec.ts index 3d71a1133..8f249a8e3 100644 --- a/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host-races.spec.ts +++ b/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host-races.spec.ts @@ -177,6 +177,37 @@ describe('VodMultiSourceHostService — stale resolutions', () => { expect(rowFor(ALT_TWO.id)?.isActive).toBe(true); }); + it('drops a pin change whose movie was navigated away from', async () => { + const pinFor = (candidate: VodSourceCandidate) => ({ + matchKey: 'title:the matrix:1999', + playlistId: candidate.playlistId, + contentId: candidate.contentId, + portalType: 'xtream', + }); + + pins.get.mockResolvedValue(pinFor(ALT_TWO)); + await loadMovie([ALT_TWO]); + expect(rowFor(ALT_TWO.id)?.isPinned).toBe(true); + + const slow = createDeferred(); + pins.clear.mockReturnValueOnce(slow.promise); + const pending = service.togglePin(ALT_TWO.id); + + // Another movie opens — with a pin of its own — before the clear + // comes back. + pins.get.mockResolvedValue(pinFor(ALT_THREE)); + await loadMovie([ALT_TWO, ALT_THREE], MOVIE_B); + + slow.resolve(true); + await pending; + + // The late unpin belongs to the film the user left. Applying it here + // would clear the pin this movie just loaded, and its Play action + // would silently stop starting from the preferred source. + expect(rowFor(ALT_THREE.id)?.isPinned).toBe(true); + expect(rowFor(ALT_TWO.id)?.isPinned).toBe(false); + }); + it('drops a probe result whose movie was navigated away from', async () => { await loadMovie([ALT_TWO]); diff --git a/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host.service.ts b/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host.service.ts index aa4100f04..b7d966f69 100644 --- a/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host.service.ts +++ b/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-host.service.ts @@ -26,7 +26,10 @@ import { runFailover, type SwitchOutcome, } from './vod-multi-source-session'; -import { buildSwitchNotice } from './vod-multi-source-notice'; +import { + buildSwitchNotice, + type VodMultiSourceSwitchNotice, +} from './vod-multi-source-notice'; import { currentSourceRow } from './vod-multi-source-current-row'; import { probeSource } from './vod-multi-source-probe'; import { @@ -60,14 +63,7 @@ export interface VodMultiSourceBindings { movie: Signal; } -export interface VodMultiSourceSwitchNotice { - playlistName: string; - resumeSeconds: number; - /** Both sides state an audio track AS FACT and those facts differ. */ - audioMayDiffer: boolean; - quality?: string; - container?: string; -} +export type { VodMultiSourceSwitchNotice }; @Injectable() export class VodMultiSourceHostService { @@ -149,10 +145,8 @@ export class VodMultiSourceHostService { /** * Wire the host's playback seam and start watching the movie on screen. * - * The effect lives here rather than in the route component because it is - * this service's own lifecycle: the router REUSES the detail component for - * detail-to-detail navigation, so a new movie must fully reset the session - * — above all the tried-source set that makes failover terminate. + * The effect lives here rather than in the route component because what it + * guards is this service's own lifecycle — see `load()`. */ bind(bindings: VodMultiSourceBindings): void { this.bindings = bindings; @@ -161,10 +155,9 @@ export class VodMultiSourceHostService { const movie = bindings.movie(); if (!movie) { // Navigating away empties the identity before the next movie's - // `load()` runs. Bumping the session here — not only in - // `load()` — closes the window in which a resolution still in - // flight for the PREVIOUS movie would pass the staleness guard - // and start its playback over the page the user is leaving. + // `load()` runs, so bumping here — not only there — closes the + // window in which a resolution still in flight for the + // PREVIOUS movie would start playing over the page being left. this.discoveryToken++; this.sessionToken++; return; @@ -279,6 +272,7 @@ export class VodMultiSourceHostService { /** Pin or unpin this source as the movie's preferred one. */ async togglePin(sourceId: string): Promise { + const session = this.sessionToken; const isPinned = this._sources().some( (source) => source.id === sourceId && source.isPinned ); @@ -289,10 +283,16 @@ export class VodMultiSourceHostService { isPinned ); - if (pinned !== undefined) { - this.controller.setPinnedSource(pinned); - this.publish(); + // The write is an IPC round-trip, and another movie can own the screen + // by the time it returns. Committing then would apply THIS film's + // answer to THAT film's controller — an unpin would clear the pin it + // just loaded, and its Play button would stop honouring it. + if (pinned === undefined || session !== this.sessionToken) { + return; } + + this.controller.setPinnedSource(pinned); + this.publish(); } /** User-triggered availability check for one row. */ diff --git a/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-notice.ts b/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-notice.ts index 30fa125f1..57697fa14 100644 --- a/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-notice.ts +++ b/libs/portal/xtream/feature/src/lib/vod-details/vod-multi-source-notice.ts @@ -1,6 +1,15 @@ import { audioDiffersFactually } from '@iptvnator/portal/shared/data-access'; import type { VodSourceCandidate } from '@iptvnator/shared/interfaces'; -import type { VodMultiSourceSwitchNotice } from './vod-multi-source-host.service'; + +/** What a switch tells the user. Lives with the code that builds it. */ +export interface VodMultiSourceSwitchNotice { + playlistName: string; + resumeSeconds: number; + /** Both sides state an audio track AS FACT and those facts differ. */ + audioMayDiffer: boolean; + quality?: string; + container?: string; +} /** * Builds what the user is told after a switch.