477 lines
35 KiB
HTML
477 lines
35 KiB
HTML
<!DOCTYPE HTML>
|
||
<html lang="en" class="navy" dir="ltr">
|
||
<head>
|
||
<!-- Book generated using mdBook -->
|
||
<meta charset="UTF-8">
|
||
<title>Player Facade Enforcement - JellyTau Documentation</title>
|
||
|
||
|
||
<!-- Custom HTML head -->
|
||
|
||
<meta name="description" content="Requirements, traceability, and architecture for the JellyTau Jellyfin client.">
|
||
<meta name="viewport" content="width=device-width, initial-scale=1">
|
||
<meta name="theme-color" content="#ffffff">
|
||
|
||
<link rel="icon" href="../favicon.svg">
|
||
<link rel="shortcut icon" href="../favicon.png">
|
||
<link rel="stylesheet" href="../css/variables.css">
|
||
<link rel="stylesheet" href="../css/general.css">
|
||
<link rel="stylesheet" href="../css/chrome.css">
|
||
<link rel="stylesheet" href="../css/print.css" media="print">
|
||
|
||
<!-- Fonts -->
|
||
<link rel="stylesheet" href="../FontAwesome/css/font-awesome.css">
|
||
<link rel="stylesheet" href="../fonts/fonts.css">
|
||
|
||
<!-- Highlight.js Stylesheets -->
|
||
<link rel="stylesheet" href="../highlight.css">
|
||
<link rel="stylesheet" href="../tomorrow-night.css">
|
||
<link rel="stylesheet" href="../ayu-highlight.css">
|
||
|
||
<!-- Custom theme stylesheets -->
|
||
|
||
</head>
|
||
<body class="sidebar-visible no-js">
|
||
<div id="body-container">
|
||
<!-- Provide site root to javascript -->
|
||
<script>
|
||
var path_to_root = "../";
|
||
var default_theme = window.matchMedia("(prefers-color-scheme: dark)").matches ? "navy" : "navy";
|
||
</script>
|
||
|
||
<!-- Work around some values being stored in localStorage wrapped in quotes -->
|
||
<script>
|
||
try {
|
||
var theme = localStorage.getItem('mdbook-theme');
|
||
var sidebar = localStorage.getItem('mdbook-sidebar');
|
||
|
||
if (theme.startsWith('"') && theme.endsWith('"')) {
|
||
localStorage.setItem('mdbook-theme', theme.slice(1, theme.length - 1));
|
||
}
|
||
|
||
if (sidebar.startsWith('"') && sidebar.endsWith('"')) {
|
||
localStorage.setItem('mdbook-sidebar', sidebar.slice(1, sidebar.length - 1));
|
||
}
|
||
} catch (e) { }
|
||
</script>
|
||
|
||
<!-- Set the theme before any content is loaded, prevents flash -->
|
||
<script>
|
||
var theme;
|
||
try { theme = localStorage.getItem('mdbook-theme'); } catch(e) { }
|
||
if (theme === null || theme === undefined) { theme = default_theme; }
|
||
var html = document.querySelector('html');
|
||
html.classList.remove('navy')
|
||
html.classList.add(theme);
|
||
var body = document.querySelector('body');
|
||
body.classList.remove('no-js')
|
||
body.classList.add('js');
|
||
</script>
|
||
|
||
<input type="checkbox" id="sidebar-toggle-anchor" class="hidden">
|
||
|
||
<!-- Hide / unhide sidebar before it is displayed -->
|
||
<script>
|
||
var body = document.querySelector('body');
|
||
var sidebar = null;
|
||
var sidebar_toggle = document.getElementById("sidebar-toggle-anchor");
|
||
if (document.body.clientWidth >= 1080) {
|
||
try { sidebar = localStorage.getItem('mdbook-sidebar'); } catch(e) { }
|
||
sidebar = sidebar || 'visible';
|
||
} else {
|
||
sidebar = 'hidden';
|
||
}
|
||
sidebar_toggle.checked = sidebar === 'visible';
|
||
body.classList.remove('sidebar-visible');
|
||
body.classList.add("sidebar-" + sidebar);
|
||
</script>
|
||
|
||
<nav id="sidebar" class="sidebar" aria-label="Table of contents">
|
||
<div class="sidebar-scrollbox">
|
||
<ol class="chapter"><li class="chapter-item expanded affix "><a href="../index.html">Introduction</a></li><li class="chapter-item expanded affix "><li class="part-title">Requirements & Traceability</li><li class="chapter-item expanded "><a href="../requirements.html"><strong aria-hidden="true">1.</strong> Requirements Specification</a></li><li class="chapter-item expanded "><a href="../traceability.html"><strong aria-hidden="true">2.</strong> Traceability Matrix</a></li><li class="chapter-item expanded "><a href="../traceability-ci.html"><strong aria-hidden="true">3.</strong> Traceability CI</a></li><li class="chapter-item expanded "><a href="../traces-quick-ref.html"><strong aria-hidden="true">4.</strong> Traces Quick Reference</a></li><li class="chapter-item expanded affix "><li class="part-title">Architecture</li><li class="chapter-item expanded "><a href="../architecture/index.html"><strong aria-hidden="true">5.</strong> Overview</a><a class="toggle"><div>❱</div></a></li><li><ol class="section"><li class="chapter-item "><a href="../architecture/01-rust-backend.html"><strong aria-hidden="true">5.1.</strong> Rust Backend</a></li><li class="chapter-item "><a href="../architecture/02-svelte-frontend.html"><strong aria-hidden="true">5.2.</strong> Svelte Frontend</a></li><li class="chapter-item "><a href="../architecture/03-data-flow.html"><strong aria-hidden="true">5.3.</strong> Data Flow</a></li><li class="chapter-item "><a href="../architecture/04-type-sync-and-threading.html"><strong aria-hidden="true">5.4.</strong> Type Sync & Threading</a></li><li class="chapter-item "><a href="../architecture/05-platform-backends.html"><strong aria-hidden="true">5.5.</strong> Platform Backends</a></li><li class="chapter-item "><a href="../architecture/06-downloads-and-offline.html"><strong aria-hidden="true">5.6.</strong> Downloads & Offline</a></li><li class="chapter-item "><a href="../architecture/07-connectivity.html"><strong aria-hidden="true">5.7.</strong> Connectivity</a></li><li class="chapter-item "><a href="../architecture/08-database-design.html"><strong aria-hidden="true">5.8.</strong> Database Design</a></li><li class="chapter-item "><a href="../architecture/09-security.html"><strong aria-hidden="true">5.9.</strong> Security</a></li></ol></li><li class="chapter-item expanded "><li class="part-title">UX</li><li class="chapter-item expanded "><a href="../ux-flows.html"><strong aria-hidden="true">6.</strong> UX Flows</a></li><li class="chapter-item expanded affix "><li class="part-title">Specs — Pending Work</li><li class="chapter-item expanded "><a href="../specs/index.html"><strong aria-hidden="true">7.</strong> Specs Index</a></li><li class="chapter-item expanded "><a href="../specs/SPEC-TEMPLATE.html"><strong aria-hidden="true">8.</strong> Spec Template</a></li><li class="chapter-item expanded "><a href="../specs/SPEC-REVIEW-CHECKLIST.html"><strong aria-hidden="true">9.</strong> Spec Review Checklist</a></li><li class="chapter-item expanded "><a href="../specs/playback-backend-unification.html"><strong aria-hidden="true">10.</strong> Playback Backend Unification</a></li><li class="chapter-item expanded "><a href="../specs/linux-native-video-spike.html"><strong aria-hidden="true">11.</strong> Linux Native Video Spike</a></li><li class="chapter-item expanded "><a href="../specs/backend-owned-stream-selection.html"><strong aria-hidden="true">12.</strong> Backend-Owned Stream Selection</a></li><li class="chapter-item expanded "><a href="../specs/player-facade-enforcement.html" class="active"><strong aria-hidden="true">13.</strong> Player Facade Enforcement</a></li><li class="chapter-item expanded "><a href="../specs/windows-native-audio-backend.html"><strong aria-hidden="true">14.</strong> Windows Native Audio Backend</a></li><li class="chapter-item expanded "><a href="../specs/libmpv2-migration.html"><strong aria-hidden="true">15.</strong> libmpv2 Migration</a></li><li class="chapter-item expanded "><a href="../specs/read-through-media-cache.html"><strong aria-hidden="true">16.</strong> Read-Through Media Cache</a></li><li class="chapter-item expanded "><a href="../specs/scoped-search.html"><strong aria-hidden="true">17.</strong> Scoped Search</a></li><li class="chapter-item expanded "><a href="../specs/scoped-search-boundary.html"><strong aria-hidden="true">18.</strong> Scoped Search Boundary</a></li><li class="chapter-item expanded "><a href="../specs/scoped-search-boundary-implementation.html"><strong aria-hidden="true">19.</strong> Scoped Search Boundary — Implementation</a></li><li class="chapter-item expanded "><a href="../specs/frontend-domain-model.html"><strong aria-hidden="true">20.</strong> Frontend Domain Model</a></li><li class="chapter-item expanded "><a href="../specs/build-provenance.html"><strong aria-hidden="true">21.</strong> Build Provenance</a></li><li class="chapter-item expanded affix "><li class="part-title">Build & Release</li><li class="chapter-item expanded "><a href="../build/build-release.html"><strong aria-hidden="true">22.</strong> Build & Release</a></li><li class="chapter-item expanded "><a href="../release-checklist.html"><strong aria-hidden="true">23.</strong> Release Checklist</a></li><li class="chapter-item expanded "><a href="../build/build-desktop-packages.html"><strong aria-hidden="true">24.</strong> Desktop Packaging</a></li><li class="chapter-item expanded "><a href="../build/build-windows.html"><strong aria-hidden="true">25.</strong> Windows Build</a></li><li class="chapter-item expanded "><a href="../defect-windows.html"><strong aria-hidden="true">26.</strong> Defect Windows</a></li><li class="chapter-item expanded "><a href="../build/docker.html"><strong aria-hidden="true">27.</strong> Docker</a></li><li class="chapter-item expanded "><a href="../build/build-builder-image.html"><strong aria-hidden="true">28.</strong> Builder Image</a></li><li class="chapter-item expanded affix "><li class="spacer"></li><li class="chapter-item expanded affix "><a href="../api-redirect.html">Rust API Reference (rustdoc)</a></li></ol>
|
||
</div>
|
||
<div id="sidebar-resize-handle" class="sidebar-resize-handle">
|
||
<div class="sidebar-resize-indicator"></div>
|
||
</div>
|
||
</nav>
|
||
|
||
<!-- Track and set sidebar scroll position -->
|
||
<script>
|
||
var sidebarScrollbox = document.querySelector('#sidebar .sidebar-scrollbox');
|
||
sidebarScrollbox.addEventListener('click', function(e) {
|
||
if (e.target.tagName === 'A') {
|
||
sessionStorage.setItem('sidebar-scroll', sidebarScrollbox.scrollTop);
|
||
}
|
||
}, { passive: true });
|
||
var sidebarScrollTop = sessionStorage.getItem('sidebar-scroll');
|
||
sessionStorage.removeItem('sidebar-scroll');
|
||
if (sidebarScrollTop) {
|
||
// preserve sidebar scroll position when navigating via links within sidebar
|
||
sidebarScrollbox.scrollTop = sidebarScrollTop;
|
||
} else {
|
||
// scroll sidebar to current active section when navigating via "next/previous chapter" buttons
|
||
var activeSection = document.querySelector('#sidebar .active');
|
||
if (activeSection) {
|
||
activeSection.scrollIntoView({ block: 'center' });
|
||
}
|
||
}
|
||
</script>
|
||
|
||
<div id="page-wrapper" class="page-wrapper">
|
||
|
||
<div class="page">
|
||
<div id="menu-bar-hover-placeholder"></div>
|
||
<div id="menu-bar" class="menu-bar sticky">
|
||
<div class="left-buttons">
|
||
<label id="sidebar-toggle" class="icon-button" for="sidebar-toggle-anchor" title="Toggle Table of Contents" aria-label="Toggle Table of Contents" aria-controls="sidebar">
|
||
<i class="fa fa-bars"></i>
|
||
</label>
|
||
<button id="theme-toggle" class="icon-button" type="button" title="Change theme" aria-label="Change theme" aria-haspopup="true" aria-expanded="false" aria-controls="theme-list">
|
||
<i class="fa fa-paint-brush"></i>
|
||
</button>
|
||
<ul id="theme-list" class="theme-popup" aria-label="Themes" role="menu">
|
||
<li role="none"><button role="menuitem" class="theme" id="light">Light</button></li>
|
||
<li role="none"><button role="menuitem" class="theme" id="rust">Rust</button></li>
|
||
<li role="none"><button role="menuitem" class="theme" id="coal">Coal</button></li>
|
||
<li role="none"><button role="menuitem" class="theme" id="navy">Navy</button></li>
|
||
<li role="none"><button role="menuitem" class="theme" id="ayu">Ayu</button></li>
|
||
</ul>
|
||
<button id="search-toggle" class="icon-button" type="button" title="Search. (Shortkey: s)" aria-label="Toggle Searchbar" aria-expanded="false" aria-keyshortcuts="S" aria-controls="searchbar">
|
||
<i class="fa fa-search"></i>
|
||
</button>
|
||
</div>
|
||
|
||
<h1 class="menu-title">JellyTau Documentation</h1>
|
||
|
||
<div class="right-buttons">
|
||
<a href="../print.html" title="Print this book" aria-label="Print this book">
|
||
<i id="print-button" class="fa fa-print"></i>
|
||
</a>
|
||
<a href="https://gitea.tourolle.paris/dtourolle/jellytau" title="Git repository" aria-label="Git repository">
|
||
<i id="git-repository-button" class="fa fa-github"></i>
|
||
</a>
|
||
<a href="https://gitea.tourolle.paris/dtourolle/jellytau/_edit/master/docs/../docs/specs/player-facade-enforcement.md" title="Suggest an edit" aria-label="Suggest an edit">
|
||
<i id="git-edit-button" class="fa fa-edit"></i>
|
||
</a>
|
||
|
||
</div>
|
||
</div>
|
||
|
||
<div id="search-wrapper" class="hidden">
|
||
<form id="searchbar-outer" class="searchbar-outer">
|
||
<input type="search" id="searchbar" name="searchbar" placeholder="Search this book ..." aria-controls="searchresults-outer" aria-describedby="searchresults-header">
|
||
</form>
|
||
<div id="searchresults-outer" class="searchresults-outer hidden">
|
||
<div id="searchresults-header" class="searchresults-header"></div>
|
||
<ul id="searchresults">
|
||
</ul>
|
||
</div>
|
||
</div>
|
||
|
||
<!-- Apply ARIA attributes after the sidebar and the sidebar toggle button are added to the DOM -->
|
||
<script>
|
||
document.getElementById('sidebar-toggle').setAttribute('aria-expanded', sidebar === 'visible');
|
||
document.getElementById('sidebar').setAttribute('aria-hidden', sidebar !== 'visible');
|
||
Array.from(document.querySelectorAll('#sidebar a')).forEach(function(link) {
|
||
link.setAttribute('tabIndex', sidebar === 'visible' ? 0 : -1);
|
||
});
|
||
</script>
|
||
|
||
<div id="content" class="content">
|
||
<main>
|
||
<h1 id="spec-enforce-the-unified-player-boundary"><a class="header" href="#spec-enforce-the-unified-player-boundary">Spec: Enforce the unified player boundary</a></h1>
|
||
<p><strong>Status:</strong> Proposed — not started. The count below has not improved: ~60
|
||
<code>commands.player*</code> call sites still live outside <code>src/lib/player/</code>, and no lint
|
||
rule enforces the boundary. This remains the one stated design principle with no
|
||
automated check.
|
||
<strong>Requirements:</strong> ⚠️ the suggested id <strong>DR-095 has since been allocated</strong> to seek
|
||
clamping — allocate a fresh id (DR-215 or later) on implementation. Relates to
|
||
UR-005 and the unified-player-boundary
|
||
principle in CLAUDE.md and <a href="../architecture/02-svelte-frontend.html">02-svelte-frontend.md</a>
|
||
<strong>UX spec:</strong> n/a — refactor, no user-visible change.
|
||
<strong>Supersedes / revises:</strong> n/a</p>
|
||
<h2 id="summary"><a class="header" href="#summary">Summary</a></h2>
|
||
<p>The stated principle is that UI controls playback <strong>only</strong> through
|
||
<code>playerController</code> (<a href="../../src/lib/player/index.ts">src/lib/player/index.ts</a>),
|
||
never by calling <code>commands.player*</code> directly. There are <strong>52 direct call sites
|
||
outside</strong> that facade. This spec routes the genuine playback-control calls
|
||
through the facade, narrows the principle's wording so it stops forbidding
|
||
things it never meant to forbid, and adds the lint rule that keeps it true —
|
||
because this rule is the one design principle in the audit with <strong>no automated
|
||
check at all</strong>, and it is also the one that drifted furthest.</p>
|
||
<h2 id="motivation"><a class="header" href="#motivation">Motivation</a></h2>
|
||
<p>Direct <code>commands.player*</code> usage outside <code>src/lib/player/</code>, by file:</p>
|
||
<div class="table-wrapper"><table><thead><tr><th>File</th><th>Sites</th></tr></thead><tbody>
|
||
<tr><td><a href="../../src/lib/stores/queue.ts">queue.ts</a></td><td>10</td></tr>
|
||
<tr><td><a href="../../src/routes/player/%5Bid%5D/+page.svelte">player/[id]/+page.svelte</a></td><td>9</td></tr>
|
||
<tr><td><a href="../../src/lib/components/player/VideoPlayer.svelte">VideoPlayer.svelte</a></td><td>8</td></tr>
|
||
<tr><td><a href="../../src/routes/settings/+page.svelte">settings/+page.svelte</a></td><td>5</td></tr>
|
||
<tr><td><a href="../../src/lib/stores/sleepTimer.ts">sleepTimer.ts</a> / <a href="../../src/lib/stores/auth.ts">auth.ts</a> / <a href="../../src/lib/api/autoplay.ts">autoplay.ts</a></td><td>4 each</td></tr>
|
||
<tr><td><a href="../../src/lib/services/preload.ts">preload.ts</a></td><td>3</td></tr>
|
||
<tr><td><a href="../../src/routes/library/%5Bid%5D/+page.svelte">library/[id]</a>, <a href="../../src/lib/services/playerEvents.ts">playerEvents.ts</a>, <a href="../../src/lib/stores/playbackMode.ts">playbackMode.ts</a></td><td>1–2 each</td></tr>
|
||
</tbody></table>
|
||
</div>
|
||
<p>These are <strong>not</strong> equivalent violations, and treating them as one number is why
|
||
the rule has been easy to ignore. Three distinct groups:</p>
|
||
<p><strong>(a) Genuine violations — playback control with a facade method that already
|
||
exists.</strong> <code>playerStop</code> ×6, <code>playerPlayTracks</code> ×4, <code>playerSeek</code> ×2,
|
||
<code>playerPlayAlbumTrack</code> ×2, <code>playerNext</code>, <code>playerPrevious</code>, <code>playerSkipTo</code>,
|
||
<code>playerToggleShuffle</code>, <code>playerCycleRepeat</code>, <code>playerRemoveFromQueue</code>,
|
||
<code>playerMoveInQueue</code>, <code>playerAddTrackById</code>, <code>playerAddTracksByIds</code>,
|
||
<code>playerSetSubtitleTrack</code>, <code>playerPlayItem</code>. The facade exposes <code>stop()</code>,
|
||
<code>seek()</code>, <code>next()</code>, <code>previous()</code>, <code>skipTo()</code>, <code>toggleShuffle()</code>,
|
||
<code>cycleRepeat()</code>, <code>removeFromQueue()</code>, <code>moveInQueue()</code>, <code>addTrackById()</code>,
|
||
<code>addTracksByIds()</code>, <code>setSubtitleTrack()</code>, <code>playTracks()</code>, <code>playAlbumTrack()</code>,
|
||
<code>playItem()</code> — every one of these has a facade equivalent that is simply not
|
||
being called. <code>queue.ts</code> is the starkest case: it imports <code>commands</code> directly
|
||
and re-implements ten methods the facade already provides.</p>
|
||
<p><strong>(b) Playback control with no facade method.</strong> <code>playerPlayQueue</code>,
|
||
<code>playerGetQueue</code>, <code>playerGetStatus</code>, <code>playerEnterBackgroundAudio</code>,
|
||
<code>playerExitBackgroundAudio</code>, <code>playerSetSleepTimer</code>, <code>playerCancelSleepTimer</code>,
|
||
<code>playerPlayNextEpisode</code>, <code>playerCancelAutoplayCountdown</code>. In scope for the
|
||
principle, but currently <em>impossible</em> to comply with — the facade has no surface
|
||
for them. A rule that cannot be followed is not being broken so much as it is
|
||
unfinished.</p>
|
||
<p><strong>(c) Not playback control.</strong> <code>playerConfigureJellyfin</code> ×3,
|
||
<code>playerDisableJellyfin</code>, <code>playerGet/SetAudioSettings</code>,
|
||
<code>playerGet/SetVideoSettings</code>, <code>playerGetEqPresets</code>,
|
||
<code>playerGet/SetAutoplaySettings</code>, <code>playerGet/SetCacheConfig</code>,
|
||
<code>playerPreloadUpcoming</code>. These are configuration and lifecycle calls that happen
|
||
to live under the <code>player_</code> command prefix. The principle is about <em>who is
|
||
authoritative for playback state</em> — settings CRUD isn't that.</p>
|
||
<p>The audit's read: the rule as written is violated 52 times, which makes real
|
||
drift indistinguishable from acceptable usage, and that ambiguity is what lets
|
||
group (a) persist. Note also that the principle <strong>is</strong> well-honoured where it
|
||
matters most — the read side is clean, with UI reading state exclusively from
|
||
the facade's re-exported stores. The write side is what drifted.</p>
|
||
<h2 id="layer-assignment"><a class="header" href="#layer-assignment">Layer assignment</a></h2>
|
||
<p>Frontend-internal refactor. No domain logic moves and nothing new crosses IPC —
|
||
the same Rust commands are called, through one module instead of many.</p>
|
||
<div class="table-wrapper"><table><thead><tr><th>Logic / responsibility</th><th>Layer</th><th>Why it belongs there</th></tr></thead><tbody>
|
||
<tr><td>Playback command dispatch (adapter routing: native vs HTML5)</td><td>Frontend — <code>src/lib/player/</code> <strong>only</strong></td><td>Presentation-layer plumbing, but must be centralised: the facade picks between the native backend and the HTML5 <code><video></code> adapter. A caller bypassing it silently skips that routing.</td></tr>
|
||
<tr><td>Playback <em>authority</em> (position, pause, rate, track changes)</td><td><strong>Rust / the player</strong></td><td>Unchanged. The player is authoritative; UI is a consumer. This spec does not touch that direction.</td></tr>
|
||
<tr><td>Queue mutation commands</td><td>Frontend facade → Rust</td><td>Rust owns queue state; the facade is the single call path to it.</td></tr>
|
||
<tr><td>Player settings CRUD (EQ, video, autoplay, cache)</td><td>Frontend, <strong>outside</strong> the facade</td><td>Configuration, not playback control — read/written on a settings page with no adapter routing. Explicitly carved out below.</td></tr>
|
||
<tr><td>Backend→frontend event handling</td><td><code>playerEvents.ts</code></td><td>Already correct. It is the facade's own plumbing, not a bypassing consumer.</td></tr>
|
||
</tbody></table>
|
||
</div>
|
||
<p>No Jellyfin taxonomy is involved, so no boundary-leak risk.</p>
|
||
<h2 id="design"><a class="header" href="#design">Design</a></h2>
|
||
<h3 id="1-narrow-the-principle-to-what-it-actually-means"><a class="header" href="#1-narrow-the-principle-to-what-it-actually-means">1. Narrow the principle to what it actually means</a></h3>
|
||
<p>Amend CLAUDE.md and <a href="../architecture/02-svelte-frontend.html">02-svelte-frontend.md</a>:</p>
|
||
<blockquote>
|
||
<p><strong>Unified player boundary.</strong> UI controls <strong>playback</strong> — transport, queue
|
||
mutation, track selection, playback initiation — <em>only</em> through
|
||
<code>playerController</code>. Player <strong>configuration</strong> commands (<code>player_*_settings</code>,
|
||
<code>player_configure_jellyfin</code>, <code>player_*_cache_config</code>, <code>player_preload_upcoming</code>)
|
||
are ordinary IPC and may be called directly from settings surfaces.</p>
|
||
</blockquote>
|
||
<p>This is a clarification, not a relaxation: it makes group (c) explicitly fine so
|
||
that a violation count means something. A rule with 52 nominal violations, most
|
||
of them acceptable, provides no signal.</p>
|
||
<h3 id="2-fill-the-facade-gaps-group-b"><a class="header" href="#2-fill-the-facade-gaps-group-b">2. Fill the facade gaps (group b)</a></h3>
|
||
<p>Add to <code>playerController</code>, each a thin pass-through preserving current
|
||
behaviour:</p>
|
||
<pre><code class="language-ts">playQueue, getQueue, getStatus,
|
||
enterBackgroundAudio, exitBackgroundAudio,
|
||
setSleepTimer, cancelSleepTimer,
|
||
playNextEpisode, cancelAutoplayCountdown,
|
||
</code></pre>
|
||
<p>Do this <strong>first</strong> — group (a) cannot be fully migrated while callers still need
|
||
a direct import for a neighbouring call, and a file that imports <code>commands</code> for
|
||
one reason will keep using it for others.</p>
|
||
<h3 id="3-migrate-group-a"><a class="header" href="#3-migrate-group-a">3. Migrate group (a)</a></h3>
|
||
<p>Mechanical: replace <code>commands.playerX(...)</code> with <code>playerController.x(...)</code>.
|
||
Highest-value first: <code>queue.ts</code> (10 sites, all direct facade equivalents), then
|
||
<code>player/[id]/+page.svelte</code>, <code>VideoPlayer.svelte</code>, <code>sleepTimer.ts</code>,
|
||
<code>playbackMode.ts</code>, <code>library/[id]/+page.svelte</code>.</p>
|
||
<p>Two sites need care rather than substitution:</p>
|
||
<ul>
|
||
<li><strong><code>playerEvents.ts</code></strong> (<code>playerOnPlaybackEnded</code>, <code>playerStop</code> in the error
|
||
path). This module <em>is</em> the facade's event plumbing — the counterpart to
|
||
<code>index.ts</code>, inside the boundary conceptually though not by directory. Treat
|
||
<code>src/lib/services/playerEvents.ts</code> as <strong>inside</strong> the boundary and exempt it,
|
||
rather than making it call the facade that calls back into it. Record this in
|
||
the lint config with the reason.</li>
|
||
<li><strong><code>VideoPlayer.svelte</code></strong> — registers its own adapter via <code>setActiveAdapter</code>.
|
||
Its <code>playerStop</code>/<code>playerPlayItem</code> calls interact with adapter lifecycle, and
|
||
CLAUDE.md's gotcha ("no lifecycle calls after an <code>await</code> in <code>onMount</code>") applies.
|
||
Migrate this file <strong>last and on its own</strong>, so an Android seek regression is
|
||
bisectable to one commit.</li>
|
||
</ul>
|
||
<h3 id="4-add-the-lint-rule-the-part-that-makes-it-stick"><a class="header" href="#4-add-the-lint-rule-the-part-that-makes-it-stick">4. Add the lint rule (the part that makes it stick)</a></h3>
|
||
<p>The audit's finding was that principles with working checks held up and
|
||
principles without them drifted. This principle has no check. Add
|
||
<code>scripts/check-player-boundary.sh</code>, wired as <code>bun run check:player-boundary</code> and
|
||
into <code>test-all.sh</code>:</p>
|
||
<pre><code class="language-sh"># Playback-control commands that MUST go through the facade.
|
||
CONTROL='player(Play|Pause|Toggle|Stop|Seek|Next|Previous|SkipTo|ToggleShuffle|CycleRepeat|RemoveFromQueue|MoveInQueue|SetVolume|ToggleMute|SetSubtitleTrack|SeekVideo|SwitchAudioTrack|PlayTracks|PlayAlbumTrack|PlayItem|PlayQueue|AddTrackById|AddTracksByIds|GetQueue|GetStatus|EnterBackgroundAudio|ExitBackgroundAudio|SetSleepTimer|CancelSleepTimer|PlayNextEpisode|CancelAutoplayCountdown|OnPlaybackEnded)'
|
||
|
||
# Inside the boundary: the facade and its event plumbing.
|
||
EXEMPT='^src/lib/player/|^src/lib/services/playerEvents\.ts$'
|
||
</code></pre>
|
||
<p>Flag <code>commands.$CONTROL</code> in non-test <code>src/</code> files outside <code>EXEMPT</code>. Config
|
||
commands are deliberately absent from the list, matching §1 — so the check
|
||
encodes the narrowed rule rather than the aspirational one.</p>
|
||
<p>An ESLint <code>no-restricted-syntax</code> rule would give better editor feedback, but the
|
||
project has no ESLint config; a shell check matches the existing
|
||
<code>check:boundary</code> precedent and adds no dependency.</p>
|
||
<h2 id="out-of-scope"><a class="header" href="#out-of-scope">Out of scope</a></h2>
|
||
<ul>
|
||
<li>Changing playback <em>behaviour</em> — pure refactor.</li>
|
||
<li>The one-directional state principle (audited clean; UI reads from facade
|
||
stores only).</li>
|
||
<li>Moving settings CRUD behind the facade (§1 explicitly carves it out).</li>
|
||
<li>Introducing ESLint.</li>
|
||
<li>Refactoring <code>VideoPlayer.svelte</code>'s 2079 lines generally, beyond its facade
|
||
call sites.</li>
|
||
<li>The <code>commands.player*</code> calls <strong>inside</strong> <code>src/lib/player/</code> — that is the
|
||
facade doing its job.</li>
|
||
</ul>
|
||
<h2 id="acceptance-criteria"><a class="header" href="#acceptance-criteria">Acceptance criteria</a></h2>
|
||
<ul>
|
||
<li><input disabled="" type="checkbox"/>
|
||
<code>playerController</code> exposes the group-(b) methods listed in §2.</li>
|
||
<li><input disabled="" type="checkbox"/>
|
||
<code>grep -rn "commands\.player" src/ --include='*.ts' --include='*.svelte' | grep -v '^src/lib/player/' | grep -v 'playerEvents\.ts' | grep -v '\.test\.' | grep -v bindings.ts</code>
|
||
returns <strong>only</strong> configuration commands per §1 — no transport, queue, or
|
||
playback-initiation call.</li>
|
||
<li><input disabled="" type="checkbox"/>
|
||
<code>queue.ts</code> no longer imports <code>commands</code> from bindings.</li>
|
||
<li><input disabled="" type="checkbox"/>
|
||
<code>bun run check:player-boundary</code> exists, is wired into <code>test-all.sh</code>, and
|
||
passes.</li>
|
||
<li><input disabled="" type="checkbox"/>
|
||
The check <strong>fails</strong> when a <code>commands.playerStop()</code> is added to a non-exempt
|
||
file — verify explicitly, as with the other gates in this batch.</li>
|
||
<li><input disabled="" type="checkbox"/>
|
||
The check does <strong>not</strong> fail on <code>commands.playerSetAudioSettings()</code> in
|
||
<code>settings/+page.svelte</code> (the §1 carve-out works).</li>
|
||
<li><input disabled="" type="checkbox"/>
|
||
CLAUDE.md and <code>02-svelte-frontend.md</code> carry the narrowed wording, including
|
||
the config carve-out and the <code>playerEvents.ts</code> exemption with its reason.</li>
|
||
<li><input disabled="" type="checkbox"/>
|
||
<strong>No behavioural change</strong>: audio and video playback, queue reorder,
|
||
shuffle/repeat, sleep timer, background audio, and autoplay all behave as
|
||
before on <strong>both Linux and Android</strong>.</li>
|
||
<li><input disabled="" type="checkbox"/>
|
||
Android seek and <code>onMount</code> lifecycle still correct after the
|
||
<code>VideoPlayer.svelte</code> migration (the known-fragile path).</li>
|
||
<li><input disabled="" type="checkbox"/>
|
||
<code>bun run check</code> and <code>bun run test</code> pass.</li>
|
||
<li><input disabled="" type="checkbox"/>
|
||
<code>bun run check:boundary</code> passes.</li>
|
||
<li><input disabled="" type="checkbox"/>
|
||
Changed code carries <code>// TRACES:</code> comments.</li>
|
||
<li><input disabled="" type="checkbox"/>
|
||
No Rust change, so no <code>bindings.ts</code> regeneration.</li>
|
||
</ul>
|
||
<h2 id="testing"><a class="header" href="#testing">Testing</a></h2>
|
||
<p><strong>Frontend</strong> (<code>bun run test</code>):</p>
|
||
<ul>
|
||
<li>Extend the existing facade tests to cover each new group-(b) method: it
|
||
forwards to the right command with the right arguments, and routes to the
|
||
active adapter where applicable.</li>
|
||
<li><code>queue.ts</code> tests: assert calls land on <code>playerController</code>, not <code>commands</code>. Mock
|
||
the facade — a test that mocks <code>commands</code> would pass either way and guard
|
||
nothing.</li>
|
||
<li>Keep <code>tauriIntegration.test.ts</code> and the other IPC param-naming tests green;
|
||
they cover the camelCase rule this refactor must not disturb.</li>
|
||
</ul>
|
||
<p><strong>Manual</strong> (no automated coverage for these paths):</p>
|
||
<ul>
|
||
<li>Linux: play/pause/seek/next/prev, queue reorder, shuffle, repeat, sleep timer,
|
||
transcoded video (HLS), background audio enter/exit.</li>
|
||
<li>Android: the same, plus lockscreen/MediaSession controls, and <strong>seek after
|
||
entering the player</strong> — the specific regression CLAUDE.md warns about.</li>
|
||
</ul>
|
||
<p>Because this is a pure refactor, the strongest signal is that no test <em>changes
|
||
expectation</em>. A test needing its assertions rewritten means behaviour moved —
|
||
investigate rather than update it.</p>
|
||
<h2 id="traces"><a class="header" href="#traces">TRACES</a></h2>
|
||
<p>Allocate in <code>requirements.md</code>:</p>
|
||
<ul>
|
||
<li><strong>DR-095</strong> — "UI playback control is routed exclusively through the
|
||
<code>playerController</code> facade (<code>src/lib/player/</code>), with <code>playerEvents.ts</code> inside
|
||
the boundary as its event plumbing and player <em>configuration</em> commands
|
||
explicitly outside it; enforced by <code>scripts/check-player-boundary.sh</code>."
|
||
Category: Player. Traces to UR-005. Status: Done on merge.</li>
|
||
</ul>
|
||
<pre><code class="language-typescript">// src/lib/player/index.ts
|
||
// TRACES: UR-005 | DR-095
|
||
</code></pre>
|
||
<p>New facade tests take <code>@req-test: UT-089</code> onward (next free UT is <strong>UT-089</strong>;
|
||
coordinate if landing alongside the sibling specs, which draw from the same
|
||
pool).</p>
|
||
<h2 id="notes-for-the-implementer"><a class="header" href="#notes-for-the-implementer">Notes for the implementer</a></h2>
|
||
<ul>
|
||
<li>A parallel Claude session may be active in this repo — <code>git diff</code> before
|
||
"repairing" unexpected changes (CLAUDE.md §Gotchas).</li>
|
||
<li><strong>Order matters</strong>: §2 (fill gaps) → §3 (migrate, <code>VideoPlayer.svelte</code> last and
|
||
alone) → §4 (add the check). Adding the check first turns <code>master</code> red.</li>
|
||
<li>🔴 <strong><code>VideoPlayer.svelte</code></strong>: no lifecycle calls after an <code>await</code> in <code>onMount</code> —
|
||
it flips to HTML5 mode and breaks Android seek. Do not let a mechanical
|
||
substitution introduce an <code>await</code> before a lifecycle call.</li>
|
||
<li>The facade's <code>requireHandle()</code> may throw where a raw <code>commands</code> call did not.
|
||
Check each migrated call site's error handling rather than assuming the
|
||
try/catch still covers the same cases.</li>
|
||
<li><code>playbackMode.ts</code> interacts with remote-mode routing (<code>play_on_session</code> vs
|
||
local MPV). Verify remote casting still works after migrating its
|
||
<code>playerPlayTracks</code> call.</li>
|
||
<li>This spec is deliberately the <em>lowest</em> priority of the audit batch: it is the
|
||
largest diff and the only one carrying real regression risk, while the
|
||
traceability gate is a few lines and restores a dead safety net.</li>
|
||
</ul>
|
||
|
||
</main>
|
||
|
||
<nav class="nav-wrapper" aria-label="Page navigation">
|
||
<!-- Mobile navigation buttons -->
|
||
<a rel="prev" href="../specs/backend-owned-stream-selection.html" class="mobile-nav-chapters previous" title="Previous chapter" aria-label="Previous chapter" aria-keyshortcuts="Left">
|
||
<i class="fa fa-angle-left"></i>
|
||
</a>
|
||
|
||
<a rel="next prefetch" href="../specs/windows-native-audio-backend.html" class="mobile-nav-chapters next" title="Next chapter" aria-label="Next chapter" aria-keyshortcuts="Right">
|
||
<i class="fa fa-angle-right"></i>
|
||
</a>
|
||
|
||
<div style="clear: both"></div>
|
||
</nav>
|
||
</div>
|
||
</div>
|
||
|
||
<nav class="nav-wide-wrapper" aria-label="Page navigation">
|
||
<a rel="prev" href="../specs/backend-owned-stream-selection.html" class="nav-chapters previous" title="Previous chapter" aria-label="Previous chapter" aria-keyshortcuts="Left">
|
||
<i class="fa fa-angle-left"></i>
|
||
</a>
|
||
|
||
<a rel="next prefetch" href="../specs/windows-native-audio-backend.html" class="nav-chapters next" title="Next chapter" aria-label="Next chapter" aria-keyshortcuts="Right">
|
||
<i class="fa fa-angle-right"></i>
|
||
</a>
|
||
</nav>
|
||
|
||
</div>
|
||
|
||
|
||
|
||
|
||
<script>
|
||
window.playground_copyable = true;
|
||
</script>
|
||
|
||
|
||
<script src="../elasticlunr.min.js"></script>
|
||
<script src="../mark.min.js"></script>
|
||
<script src="../searcher.js"></script>
|
||
|
||
<script src="../clipboard.min.js"></script>
|
||
<script src="../highlight.js"></script>
|
||
<script src="../book.js"></script>
|
||
|
||
<!-- Custom JS scripts -->
|
||
|
||
|
||
</div>
|
||
</body>
|
||
</html>
|