docs(playback): plan accurate native diagnostics

2026-10-10 01:56:16 -08:00 · 2026-07-30 09:19:20 +02:00
1 parent ee2bb84478
commit a53a52efc4
@@ -0,0 +1,743 @@
+# Accurate Native Playback Diagnostics Implementation Plan
+
+> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
+
+**Goal:** Fix issue #1159 so unavailable HLS sources are never misreported as unsupported codecs, while retaining an explicit HTTP error status exposed by Video.js.
+
+**Architecture:** Extend the shared native-error input and normalized diagnostic with two safe structured fields: HTTP status and a bounded Video.js error type. Give explicit HTTP evidence precedence, keep independently known unsupported containers, and classify every other native code-4 failure as unknown; reuse the existing network/unknown UI copy and render `HTTP <status>` through existing diagnostic metadata and details surfaces.
+
+**Tech Stack:** Angular 21, TypeScript, Video.js 8, Jest through Nx, ngx-translate JSON catalogs, Markdown release notes.
+
+---
+
+### Task 0: Bootstrap the Nx workspace
+
+**Files:**
+- Verify only: `package.json`
+- Verify only: `pnpm-lock.yaml`
+
+- [ ] **Step 1: Install the locked dependencies**
+
+Run:
+
+```bash
+pnpm install --frozen-lockfile
+```
+
+Expected: exit 0 without changing `pnpm-lock.yaml`.
+
+- [ ] **Step 2: Verify Nx project discovery**
+
+Run:
+
+```bash
+pnpm nx show projects
+```
+
+Expected: exit 0 and output containing `ui-playback`, `web`, and `web-e2e`.
+
+### Task 1: Make native classification evidence-based
+
+**Files:**
+- Modify: `libs/ui/playback/src/lib/playback-diagnostics/playback-diagnostics.model.ts`
+- Modify: `libs/ui/playback/src/lib/playback-diagnostics/playback-diagnostics.util.ts`
+- Test: `libs/ui/playback/src/lib/playback-diagnostics/playback-diagnostics.util.spec.ts`
+
+- [ ] **Step 1: Write the failing classifier regressions**
+
+In `playback-diagnostics.util.spec.ts`, replace the MPEG-TS native code-4
+expectation with an unknown diagnosis and add the HTTP/opaque HLS cases:
+
+```typescript
+it('keeps ambiguous MPEG-TS native source failures unknown', () => {
+    const issue = classifyNativePlaybackIssue(
+        { code: 4, message: 'source not supported' },
+        createPlaybackSourceMetadata({
+            url: 'https://example.com/live/stream',
+            mimeType: 'video/mp2t',
+            player: 'videojs',
+        })
+    );
+
+    expect(issue.code).toBe(PlaybackDiagnosticCode.UnknownPlaybackError);
+    expect(issue.container).toBe('mp2t');
+    expect(issue.externalFallbackRecommended).toBe(false);
+});
+
+it('classifies an explicit Video.js HTTP failure as a network error', () => {
+    const issue = classifyNativePlaybackIssue(
+        {
+            code: 4,
+            message: 'The media could not be loaded',
+            status: 404,
+            metadata: { errorType: 'NETWORK_REQUEST_ERR' },
+        },
+        createPlaybackSourceMetadata({
+            url: 'https://example.com/missing/playlist.m3u8',
+            mimeType: 'application/x-mpegURL',
+            player: 'videojs',
+        })
+    );
+
+    expect(issue).toEqual(
+        expect.objectContaining({
+            code: PlaybackDiagnosticCode.NetworkError,
+            httpStatus: 404,
+            nativeErrorType: 'NETWORK_REQUEST_ERR',
+            externalFallbackRecommended: false,
+        })
+    );
+});
+
+it('keeps native HLS code 4 unknown when no HTTP or codec evidence exists', () => {
+    const issue = classifyNativePlaybackIssue(
+        {
+            code: 4,
+            message:
+                'The media could not be loaded, either because the server or network failed or because the format is not supported.',
+        },
+        createPlaybackSourceMetadata({
+            url: 'https://example.com/live/playlist.m3u8',
+            mimeType: 'application/x-mpegURL',
+            player: 'videojs',
+        })
+    );
+
+    expect(issue.code).toBe(PlaybackDiagnosticCode.UnknownPlaybackError);
+    expect(issue.externalFallbackRecommended).toBe(false);
+    expect(issue.httpStatus).toBeUndefined();
+});
+
+it('does not treat opaque status zero or unsafe metadata as HTTP evidence', () => {
+    const issue = classifyNativePlaybackIssue(
+        {
+            code: 4,
+            status: 0,
+            metadata: { errorType: 'request failed: token=secret value' },
+        },
+        createPlaybackSourceMetadata({
+            url: 'https://example.com/live/playlist.m3u8',
+            mimeType: 'application/x-mpegURL',
+            player: 'videojs',
+        })
+    );
+
+    expect(issue.code).toBe(PlaybackDiagnosticCode.UnknownPlaybackError);
+    expect(issue.httpStatus).toBeUndefined();
+    expect(issue.nativeErrorType).toBeUndefined();
+});
+```
+
+Keep the existing `.mkv` and `video/x-msvideo` tests unchanged so known
+unsupported containers remain covered.
+
+- [ ] **Step 2: Run the focused classifier spec to verify RED**
+
+Run:
+
+```bash
+NODE_OPTIONS=--experimental-vm-modules \
+node node_modules/jest/bin/jest.js \
+  --config jest.web-esm.workspace.ts \
+  --runTestsByPath \
+  libs/ui/playback/src/lib/playback-diagnostics/playback-diagnostics.util.spec.ts \
+  --runInBand
+```
+
+Expected: FAIL because code 4 still becomes `unsupported-codec` and the
+normalized diagnostic does not retain `httpStatus` or `nativeErrorType`.
+
+- [ ] **Step 3: Extend the diagnostic input and output contracts**
+
+In `playback-diagnostics.model.ts`, replace `NativePlaybackErrorInput` with:
+
+```typescript
+export interface NativePlaybackErrorMetadataInput {
+    readonly errorType?: unknown;
+}
+
+export interface NativePlaybackErrorInput {
+    readonly code?: number;
+    readonly message?: string;
+    readonly status?: number;
+    readonly metadata?: NativePlaybackErrorMetadataInput;
+}
+```
+
+Add these optional properties beside the existing native error properties in
+`PlaybackDiagnostic`:
+
+```typescript
+readonly httpStatus?: number;
+readonly nativeErrorType?: string;
+```
+
+- [ ] **Step 4: Implement safe evidence extraction and classification**
+
+In `playback-diagnostics.util.ts`, add:
+
+```typescript
+const MIN_HTTP_ERROR_STATUS = 400;
+const MAX_HTTP_ERROR_STATUS = 599;
+const NATIVE_ERROR_TYPE_PATTERN = /^[A-Za-z0-9._:-]{1,128}$/;
+
+function getHttpErrorStatus(
+    error: NativePlaybackErrorInput | MediaError | null | undefined
+): number | undefined {
+    if (!error || !('status' in error)) {
+        return undefined;
+    }
+
+    const status = error.status;
+    return typeof status === 'number' &&
+        Number.isInteger(status) &&
+        status >= MIN_HTTP_ERROR_STATUS &&
+        status <= MAX_HTTP_ERROR_STATUS
+        ? status
+        : undefined;
+}
+
+function getNativeErrorType(
+    error: NativePlaybackErrorInput | MediaError | null | undefined
+): string | undefined {
+    if (!error || !('metadata' in error)) {
+        return undefined;
+    }
+
+    const errorType = error.metadata?.errorType;
+    return typeof errorType === 'string' &&
+        NATIVE_ERROR_TYPE_PATTERN.test(errorType)
+        ? errorType
+        : undefined;
+}
+```
+
+At the start of `classifyNativePlaybackIssue`, extract the two values:
+
+```typescript
+const httpStatus = getHttpErrorStatus(error);
+const nativeErrorType = getNativeErrorType(error);
+```
+
+Before the existing code-2 branch, add:
+
+```typescript
+if (httpStatus !== undefined) {
+    return createPlaybackDiagnostic({
+        code: DiagnosticCode.NetworkError,
+        source: DiagnosticSource.Native,
+        metadata,
+        httpStatus,
+        nativeErrorCode,
+        nativeErrorMessage,
+        nativeErrorType,
+    });
+}
+```
+
+Pass `nativeErrorType` through every remaining native diagnostic creation.
+Change the code-4 branch to:
+
+```typescript
+if (nativeErrorCode === SOURCE_NOT_SUPPORTED_CODE) {
+    return createPlaybackDiagnostic({
+        code: isLikelyContainerIssue(metadata)
+            ? DiagnosticCode.UnsupportedContainer
+            : DiagnosticCode.UnknownPlaybackError,
+        source: DiagnosticSource.Native,
+        metadata,
+        nativeErrorCode,
+        nativeErrorMessage,
+        nativeErrorType,
+    });
+}
+```
+
+Extend `createPlaybackDiagnostic` options and return value with:
+
+```typescript
+readonly httpStatus?: number;
+readonly nativeErrorType?: string;
+```
+
+and copy both values into the resulting `PlaybackDiagnostic`.
+
+- [ ] **Step 5: Run the classifier spec to verify GREEN**
+
+Run the focused command from step 2 again.
+
+Expected: PASS, including the HLS 404, ambiguous HLS, status-zero, MPEG-TS,
+and known-container cases.
+
+- [ ] **Step 6: Commit the classifier behavior**
+
+```bash
+git add \
+  libs/ui/playback/src/lib/playback-diagnostics/playback-diagnostics.model.ts \
+  libs/ui/playback/src/lib/playback-diagnostics/playback-diagnostics.util.ts \
+  libs/ui/playback/src/lib/playback-diagnostics/playback-diagnostics.util.spec.ts
+git commit -m "fix(playback): classify native source errors from evidence"
+```
+
+Expected: one commit containing the normalized contract, classification rules,
+and their regression coverage.
+
+### Task 2: Verify and align the Video.js HTTP boundary
+
+**Files:**
+- Modify: `libs/ui/playback/src/lib/vjs-player/vjs-player.types.ts`
+- Test: `libs/ui/playback/src/lib/vjs-player/vjs-player.component.spec.ts`
+
+- [ ] **Step 1: Add the Video.js boundary regression**
+
+Import `NativePlaybackErrorInput` in `vjs-player.component.spec.ts`, change the
+harness `currentError` type to `NativePlaybackErrorInput | null`, and add:
+
+```typescript
+it('preserves safe Video.js HTTP context in playback diagnostics', () => {
+    const issues: Array<PlaybackDiagnostic | null> = [];
+    component.playbackIssue.subscribe((issue) => issues.push(issue));
+    render({
+        sources: [
+            {
+                src: 'https://example.test/missing/playlist.m3u8',
+                type: 'application/x-mpegURL',
+            },
+        ],
+    });
+    harness.currentError = {
+        code: 4,
+        message: 'The media could not be loaded',
+        status: 404,
+        metadata: { errorType: 'NETWORK_REQUEST_ERR' },
+    };
+
+    harness.emit('error');
+
+    expect(issues.at(-1)).toEqual(
+        expect.objectContaining({
+            code: 'network-error',
+            source: 'native',
+            sourceUrl: 'https://example.test/missing/playlist.m3u8',
+            httpStatus: 404,
+            nativeErrorType: 'NETWORK_REQUEST_ERR',
+            externalFallbackRecommended: false,
+        })
+    );
+});
+```
+
+Keep the existing MKV unsupported-container test.
+
+- [ ] **Step 2: Run the focused component integration spec**
+
+Run:
+
+```bash
+NODE_OPTIONS=--experimental-vm-modules \
+node node_modules/jest/bin/jest.js \
+  --config jest.web-esm.workspace.ts \
+  --runTestsByPath \
+  libs/ui/playback/src/lib/vjs-player/vjs-player.component.spec.ts \
+  --runInBand
+```
+
+Expected: PASS after Task 1. JavaScript already passes the complete error
+object at runtime; this regression proves that the component does not rebuild
+or strip it before classification.
+
+- [ ] **Step 3: Reuse the shared native-error contract**
+
+At the top of `vjs-player.types.ts`, add:
+
+```typescript
+import type { NativePlaybackErrorInput } from '../playback-diagnostics/playback-diagnostics.model';
+```
+
+Replace the `error` signature with:
+
+```typescript
+error: () => NativePlaybackErrorInput | null;
+```
+
+No component implementation change is required because
+`VjsPlayerComponent.handleVideoJsError` already passes the complete
+`player.error()` object to `classifyNativePlaybackIssue`.
+
+- [ ] **Step 4: Verify the aligned Video.js type contract**
+
+Run:
+
+```bash
+NODE_OPTIONS=--experimental-vm-modules \
+node node_modules/jest/bin/jest.js \
+  --config jest.web-esm.workspace.ts \
+  --runTestsByPath \
+  libs/ui/playback/src/lib/vjs-player/vjs-player.component.spec.ts \
+  --runInBand
+pnpm exec tsc -p libs/ui/playback/tsconfig.lib.json --noEmit
+```
+
+Expected: both commands pass for the HTTP integration case, existing
+source/reset behavior, and the production library type graph. The behavioral
+RED/GREEN proof lives in Task 1; this task aligns the TypeScript declaration
+with the already-tested runtime payload.
+
+- [ ] **Step 5: Commit the Video.js boundary**
+
+```bash
+git add \
+  libs/ui/playback/src/lib/vjs-player/vjs-player.types.ts \
+  libs/ui/playback/src/lib/vjs-player/vjs-player.component.spec.ts
+git commit -m "fix(playback): preserve Video.js HTTP error context"
+```
+
+Expected: one commit containing only the Video.js type boundary and component
+regression.
+
+### Task 3: Render explicit HTTP evidence
+
+**Files:**
+- Modify: `libs/ui/playback/src/lib/web-player-view/web-player-view-diagnostics.utils.ts`
+- Test: `libs/ui/playback/src/lib/web-player-view/web-player-view.component.spec.ts`
+
+- [ ] **Step 1: Write the failing rendered-diagnostic regression**
+
+Add this helper in `web-player-view.component.spec.ts`:
+
+```typescript
+function createHttpDiagnostic(): PlaybackDiagnostic {
+    return {
+        code: PlaybackDiagnosticCode.NetworkError,
+        source: PlaybackDiagnosticSource.Native,
+        sourceUrl: 'https://example.com/missing/playlist.m3u8',
+        container: 'm3u8',
+        mimeType: 'application/x-mpegURL',
+        player: 'videojs',
+        audioCodecs: [],
+        videoCodecs: [],
+        httpStatus: 404,
+        nativeErrorCode: 4,
+        nativeErrorMessage: 'The media could not be loaded',
+        nativeErrorType: 'NETWORK_REQUEST_ERR',
+        externalFallbackRecommended: false,
+    };
+}
+```
+
+Add this component test:
+
+```typescript
+it('renders explicit HTTP evidence without recommending an external player', () => {
+    runtimeCapabilities.supportsManagedExternalPlayers = true;
+    fixture.detectChanges();
+    const issue = createHttpDiagnostic();
+
+    component.handlePlaybackIssue(issue);
+    fixture.detectChanges();
+
+    const banner = fixture.debugElement.query(
+        By.css('[data-test-id="playback-diagnostic-banner"]')
+    );
+    const mpvButton = fixture.debugElement.query(
+        By.css('[data-test-id="playback-fallback-mpv"]')
+    );
+
+    expect(banner.nativeElement.textContent).toContain('HTTP 404');
+    expect(mpvButton).toBeNull();
+    expect(component.getDiagnosticMeta(issue)).toBe('HTTP 404');
+    expect(component.getDiagnosticDetails(issue)).toEqual(
+        expect.arrayContaining([
+            {
+                labelKey: 'PLAYBACK_DIAGNOSTICS.DETAIL_ERROR_DETAILS',
+                value: 'HTTP 404 · NETWORK_REQUEST_ERR',
+            },
+        ])
+    );
+});
+```
+
+- [ ] **Step 2: Run the focused view spec to verify RED**
+
+Run:
+
+```bash
+NODE_OPTIONS=--experimental-vm-modules \
+node node_modules/jest/bin/jest.js \
+  --config jest.web-esm.workspace.ts \
+  --runTestsByPath \
+  libs/ui/playback/src/lib/web-player-view/web-player-view.component.spec.ts \
+  --runInBand
+```
+
+Expected: FAIL because the metadata still renders `m3u8` and the details helper
+does not include the structured HTTP context.
+
+- [ ] **Step 3: Format HTTP metadata and safe error details**
+
+At the start of `getDiagnosticMeta`, add:
+
+```typescript
+if (issue.httpStatus !== undefined) {
+    return `HTTP ${issue.httpStatus}`;
+}
+```
+
+Replace the final error-details item in `getDiagnosticDetails` with:
+
+```typescript
+{
+    labelKey: 'PLAYBACK_DIAGNOSTICS.DETAIL_ERROR_DETAILS',
+    value: formatDiagnosticErrorDetails(issue),
+},
+```
+
+Add:
+
+```typescript
+function formatDiagnosticErrorDetails(issue: PlaybackDiagnostic): string {
+    return [
+        issue.httpStatus === undefined ? '' : `HTTP ${issue.httpStatus}`,
+        issue.nativeErrorType ?? '',
+        issue.details ?? '',
+    ]
+        .filter((value) => value.length > 0)
+        .join(' · ');
+}
+```
+
+Keep the existing translation keys and template unchanged.
+
+- [ ] **Step 4: Run the view spec to verify GREEN**
+
+Run the focused command from step 2 again.
+
+Expected: PASS with visible `HTTP 404`, no MPV action, and the combined safe
+details row.
+
+- [ ] **Step 5: Run all affected unit tests and lint**
+
+Run:
+
+```bash
+pnpm nx test ui-playback
+pnpm nx lint ui-playback
+```
+
+Expected: both commands exit 0 with no failed tests or lint errors.
+
+- [ ] **Step 6: Commit the rendered evidence**
+
+```bash
+git add \
+  libs/ui/playback/src/lib/web-player-view/web-player-view-diagnostics.utils.ts \
+  libs/ui/playback/src/lib/web-player-view/web-player-view.component.spec.ts
+git commit -m "fix(playback): show explicit HTTP playback errors"
+```
+
+Expected: one commit containing the diagnostic presentation and component
+coverage.
+
+### Task 4: Document the behavior and add the release note
+
+**Files:**
+- Modify: `docs/architecture/embedded-inline-playback.md`
+- Create: `.changes/playback-accurate-source-errors.md`
+
+- [ ] **Step 1: Update the canonical playback documentation**
+
+Add this paragraph after the supported diagnostic-code list in
+`docs/architecture/embedded-inline-playback.md`:
+
+```markdown
+Native `MediaError` code 4 is not codec evidence by itself. A source already
+known to use a browser-incompatible container remains
+`unsupported-container`; otherwise the native failure stays
+`unknown-playback-error`. When Video.js exposes an explicit HTTP error status,
+the failure is classified as `network-error`, the status is shown in the
+diagnostic, and an external decoder is not presented as a likely fix for the
+same failed request.
+```
+
+- [ ] **Step 2: Add the user-facing release note**
+
+Create `.changes/playback-accurate-source-errors.md` with:
+
+```markdown
+---
+type: fix
+area: playback
+issues: [1159]
+---
+
+Unavailable streams no longer appear as unsupported codecs. When Video.js
+exposes a server error such as HTTP 404, the player shows that status; otherwise
+ambiguous source errors remain unidentified instead of guessing.
+```
+
+- [ ] **Step 3: Validate documentation and release metadata**
+
+Run:
+
+```bash
+git diff --check
+pnpm run release:notes:validate
+pnpm run i18n:check
+```
+
+Expected: all commands exit 0. The release note validates, and no translation
+catalog drift is introduced because the implementation reuses existing keys.
+
+- [ ] **Step 4: Commit documentation and release note**
+
+```bash
+git add \
+  docs/architecture/embedded-inline-playback.md \
+  .changes/playback-accurate-source-errors.md
+git commit -m "docs(playback): document native error evidence"
+```
+
+Expected: one commit containing the canonical behavior contract and the
+issue-linked release note.
+
+### Task 5: Complete verification and local Codex review
+
+**Files:**
+- Verify: all files changed from `origin/master`
+
+- [ ] **Step 1: Run the final affected validation ladder**
+
+Run:
+
+```bash
+pnpm nx test ui-playback
+pnpm nx lint ui-playback
+pnpm run typecheck:web
+pnpm run i18n:check
+pnpm run release:notes:validate
+git diff --check origin/master...HEAD
+git status --short
+```
+
+Expected: every command exits 0, all `ui-playback` tests pass, and the worktree
+is clean. E2E is not required because no player selection, route, interaction,
+or overlay layout changes; classifier, boundary, and rendered output are
+covered by focused component tests.
+
+- [ ] **Step 2: Dispatch a fresh local Codex reviewer**
+
+Resolve the exact review range:
+
+```bash
+git rev-parse origin/master
+git rev-parse HEAD
+```
+
+Dispatch a fresh reviewer agent with no implementation-history context and
+this request:
+
+```text
+Review origin/master...HEAD for issue #1159 against
+docs/superpowers/specs/2026-07-30-accurate-native-playback-diagnostics-design.md.
+Focus on correctness, regressions, unsafe disclosure, TypeScript/runtime
+contract mismatches, and missing tests. Report only actionable P0/P1/P2
+findings with exact file and line references; do not report style-only nits.
+Do not modify files.
+```
+
+Expected: a local Codex review report before any push or PR creation.
+
+- [ ] **Step 3: Verify and resolve every P1/P2 finding**
+
+For each reported finding:
+
+1. Reproduce or prove it from the code and tests.
+2. Add or update a regression test first when behavior changes.
+3. Run the focused test and observe the expected RED result.
+4. Apply the smallest valid fix.
+5. Run the focused test to GREEN.
+
+If the review has confirmed P1/P2 findings, stage the tracked files changed by
+the verified fixes and commit them with:
+
+```bash
+git add -u
+git commit -m "fix(playback): address local review findings"
+```
+
+If a finding is invalid, record the concrete code/test evidence for rejecting
+it in the task summary instead of changing the implementation.
+
+- [ ] **Step 4: Re-review after fixes**
+
+If step 3 changed any file, send the same reviewer a follow-up request to
+re-check the new `origin/master...HEAD` diff for remaining P0/P1/P2 findings.
+Repeat steps 3 and 4 until no confirmed P1/P2 findings remain.
+
+- [ ] **Step 5: Re-run final verification after review**
+
+Run the complete validation command set from step 1 again.
+
+Expected: all commands exit 0 after the final review changes, and
+`git status --short` is empty.
+
+### Task 6: Push and create the focused pull request
+
+**Files:**
+- Verify only: `.github/pull_request_template.md` when present
+
+- [ ] **Step 1: Inspect the final commit range**
+
+Run:
+
+```bash
+git log --oneline origin/master..HEAD
+git diff --stat origin/master...HEAD
+git status --short --branch
+```
+
+Expected: only the design/plan, focused diagnostic implementation, regression
+tests, canonical documentation, and release note are present; the worktree is
+clean.
+
+- [ ] **Step 2: Push the branch**
+
+Run:
+
+```bash
+git push -u origin agent/fix-playback-diagnostic-1159
+```
+
+Expected: push succeeds and configures the upstream branch.
+
+- [ ] **Step 3: Create the ready pull request**
+
+Create a non-draft PR with title:
+
+```text
+fix(playback): avoid false codec diagnostics
+```
+
+Use this body:
+
+```markdown
+## Summary
+
+- stop treating an ambiguous native `MediaError` code 4 as proof of an unsupported codec
+- preserve and show explicit Video.js HTTP error statuses such as 404
+- keep confirmed container/codec diagnostics and external fallback behavior evidence-based
+
+## Testing
+
+- `pnpm nx test ui-playback`
+- `pnpm nx lint ui-playback`
+- `pnpm run typecheck:web`
+- `pnpm run i18n:check`
+- `pnpm run release:notes:validate`
+- local Codex review completed with no unresolved P1/P2 findings
+
+Closes #1159
+```
+
+Expected: a ready PR targeting `master`, created only after local review and
+final validation succeed.