diff --git a/docs/README.md b/docs/README.md index c3b42a611..172b6dd13 100644 --- a/docs/README.md +++ b/docs/README.md @@ -92,4 +92,7 @@ bounds, what's mined per issue): `docs/handoffs/chicorytv-issue-queue.md` → "K that file's standing kickoff for the two concurrent tracks (orientation ‖ selection). ersatztv#237 is a closed, archival historical tracker (superseded by `startup.parallel-orientation` in `docs/decisions.md`) — not a live pointer. +- **`docs/tracker-retrofit-triage-237.md`** — audit trail for the #524 triage of ersatztv#237's 111 + comments (method, per-comment classification, totals). Evidence for the + `docs.tracker-comment-retrofit` decision; read it only when triaging another over-cap tracker. - **`docs/handoffs/rest-api.md`** — original handoff prompt for kicking off the REST API work (#2). diff --git a/docs/decisions.md b/docs/decisions.md index b31044e22..d4cab234e 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -131,6 +131,7 @@ in-file entries. - [2026-07-21 — Decision records carry a lifecycle schema, validated by a script; append-only-by-diff is retired (#521)](#2026-07-21--decision-records-carry-a-lifecycle-schema-validated-by-a-script-append-only-by-diff-is-retired-521) - [2026-07-21 — Parallel orientation + selection is the startup protocol; #237 retired (#520)](#2026-07-21--parallel-orientation--selection-is-the-startup-protocol-237-retired-520) - [2026-07-21 — External channel-logo URLs are downloaded and cached at save time; the render path never fetches a logo (#525)](#2026-07-21--external-channel-logo-urls-are-downloaded-and-cached-at-save-time-the-render-path-never-fetches-a-logo-525) +- [2026-07-21 — Check the worked issue before the decision corpus; a closed tracker's comments need no retrofit (#524)](#2026-07-21--check-the-worked-issue-before-the-decision-corpus-a-closed-trackers-comments-need-no-retrofit-524) --- @@ -3282,3 +3283,36 @@ stored value moves too (harmless — only the QSV path ever reads it — but it on a field the user didn't touch); and a machine client that `PUT`s `0` gets a `200` and then reads back `64`, which is a silent transform of a submitted value that the OpenAPI description does not advertise. + +## 2026-07-21 — Check the worked issue before the decision corpus; a closed tracker's comments need no retrofit (#524) +`key: docs.tracker-comment-retrofit` · `status: active` · `since: 2026-07-21` · `supersedes: none` · `superseded-by: none` +**Rule:** When the knowledge exporter flags an over-cap tracker issue and excludes it from ingestion, triage its comments instead of assuming a retrofit is owed — and for each decision-shaped item check the **worked issue first**, because a tracker session comment is by construction a précis of the fuller closing record posted on the issue it narrates. Applied to #237 (111 comments) this yielded **zero** records, so server-management#642's "a fact found only in a #237 comment" retrieval row has no valid subject and its interim target (an already-migrated record) is permanent. +**Signals:** tracker retrofit, over-cap issue exclusion, worked-issue-first, tracker-is-not-a-knowledge-store, #237 comment history · paths: `docs/tracker-retrofit-triage-237.md` · issues: #524, #237, #520, #521, server-management#642 +**Mechanics:** `docs/tracker-retrofit-triage-237.md` — method, per-comment classification table, totals, and the one candidate that was raised and disproved + +**Why the worked-issue-first ordering is the load-bearing part.** The session protocol that produced +#237's log required each session to post its full closing record on the issue it actually worked, and +*then* summarize across issues on the tracker. The tracker entry is therefore the lossy copy. A +comment that looks like a unique source is nearly always a précis of a primary the exporter already +indexes — so the instinct "the tracker is excluded, therefore its knowledge is orphaned" inverts the +real dependency. Checking the corpus first and the worked issue second wastes the effort; the reverse +order settles most items in one lookup. + +**Worked example.** Comment [106/111] recorded that #497's music-video metadata reconcile deliberately +excludes Guids (not eager-loaded by `GetOrAdd`, so reconciling would duplicate-insert every scan) and +Directors (not add-persisted for `MusicVideoMetadata`). Genuinely decision-shaped, and genuinely +absent from the decision corpus — it survives a corpus-only check and looks like a retrofit target. +Issue #497's own closing comment states both exclusions with fuller reasoning, and the exporter +ingests it as `497.md`. One worked-issue lookup disposes of it. + +**Scope of the claim — deliberately narrow.** The coverage test was applied to **decision-shaped +items only**. Items classified as cross-cutting lore were classified but *not* coverage-checked, and +that bucket is **not** empty: two facts from comment [109/111] were found to have no home anywhere — +`scripts/e2e-local.sh`'s readiness probe hanging on a reused config dir, and the troubleshooting +playback API's inability to exercise channel branding. Both were swept into the handoff lore by this +PR. So the correct statement is "no *decision-shaped* orphans," not "no orphans"; a tracker triage +that skips the lore bucket will leave real knowledge on the floor. + +**A zero result is a legitimate outcome**, not evidence the triage was done wrong. What would change +the answer is a tracker whose sessions did *not* also write per-issue closing records — one where the +tracker genuinely was the primary rather than the narration layer. diff --git a/docs/decisions/README.md b/docs/decisions/README.md index e0fa4be63..9744bdc7d 100644 --- a/docs/decisions/README.md +++ b/docs/decisions/README.md @@ -47,6 +47,7 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera | `concurrency.schedule-item-child-identity` | `PUT /api/schedules/{id}/items` reconciles by an optional round-tripped child `Id` (null/absent/0 ⇒ new item), never by array position, so fill-group/shuffle state follows the logical item across reorders; an unknown or duplicate id is rejected 422 (checked after the §7a `CheckVersion`, so 412 precedes 422). | 2026-07-11 | [link](optimistic-concurrency.md#2026-07-11--stable-child-identity-for-schedule-item-replace-259-split-from-252253) | | `docs.convention-docs-session-start` | Docs-first, not source-first: conventions (api-conventions, spa-conventions, e2e-local, blazor-route-parity, domain-model, decisions, README) are read from docs, not reverse-engineered from code, via `docs/README.md`'s task-signal map — only the sections it points to for the task at hand, not the whole set. Each doc is updated in the same PR that changes what it documents, replacing deferred/follow-up doc updates. | 2026-07-07 | [link](../decisions.md#2026-07-07--convention-docs-read-at-session-start-updated-in-pr) | | `docs.decision-lifecycle` | every decision `##` record (active or archived) carries a 5-field metadata block (`key`, `status`, `since`, `supersedes`, `superseded-by`) checked by `scripts/decisions_validate.py`; a record is never deleted or line-edited to reverse a call — it is moved to `docs/decisions/archive/` with `status: superseded`/`retired` and a reciprocal `superseded-by`/`supersedes` key pair to its replacement. | 2026-07-21 | [link](../decisions.md#2026-07-21--decision-records-carry-a-lifecycle-schema-validated-by-a-script-append-only-by-diff-is-retired-521) | +| `docs.tracker-comment-retrofit` | When the knowledge exporter flags an over-cap tracker issue and excludes it from ingestion, triage its comments instead of assuming a retrofit is owed — and for each decision-shaped item check the **worked issue first**, because a tracker session comment is by construction a précis of the fuller closing record posted on the issue it narrates. Applied to #237 (111 comments) this yielded **zero** records, so server-management#642's "a fact found only in a #237 comment" retrieval row has no valid subject and its interim target (an already-migrated record) is permanent. | 2026-07-21 | [link](../decisions.md#2026-07-21--check-the-worked-issue-before-the-decision-corpus-a-closed-trackers-comments-need-no-retrofit-524) | | `ffmpeg.external-logo-graphics-engine` | External-URL channel logos pass through to the graphics engine like any other watermark source; `WatermarkSelector` must never gate them on `File.Exists` (always false for a URL) and never route them through the ffmpeg-native overlay shortcut. | 2026-07-20 | [link](../decisions.md#2026-07-20--external-url-channel-logos-pass-through-to-the-graphics-engine-never-fileexists-gated-never-ffmpeg-native-502) | | `ffmpeg.hls-cold-start-burst` | HLS cold-start latency is fixed with a bounded `-readrate_initial_burst` (gated on FFmpeg ≥6.1 capability detection), not by raising `work_ahead_limit`, which would remove the concurrency guarantee it exists for. | 2026-07-20 | [link](../decisions.md#2026-07-20--hls-cold-start-is-fixed-with--readrate_initial_burst-not-by-raising-the-work-ahead-limit-350) | | `ffmpeg.qsv-decode-encode-split` | QSV decode is decoupled from QSV encode via a single `FFmpegProfile.QsvPreferNativeDecoder` bool (default ON, Linux-only), so a QSV encode profile can decode with the more tolerant native VA-API decoder instead of the QSV decoder, mirroring Jellyfin's hybrid decode/encode toggle instead of a general decode-family enum. | 2026-07-20 | [link](../decisions.md#2026-07-20-498--qsv-decode-is-split-from-qsv-encode-via-a-single-qsvprefernativedecoder-bool) | diff --git a/docs/handoffs/chicorytv-issue-queue.md b/docs/handoffs/chicorytv-issue-queue.md index b7429ac01..a1469e69b 100644 --- a/docs/handoffs/chicorytv-issue-queue.md +++ b/docs/handoffs/chicorytv-issue-queue.md @@ -454,6 +454,20 @@ HARD CONSTRAINTS: (tiny ffmpeg testsrc MKVs + `LibraryPath` SQL rows + scan). Live-E2E is a **stated requirement** for write-path handler changes — see that doc's "When live-E2E is required" + the decisions.md entry; it's the only net for the lazy-enumeration / reload-through-read-path 500 class (#229). +- **`scripts/e2e-local.sh` hangs its readiness probe on a REUSED config dir** (surfaced in the #502 + session; recovered from #237's comment log by the #524 triage). The probe waits for + `Done migrating search index` (`e2e-local.sh` `READY_LINE`), which a **fresh** config emits — a + reused one logs `Search index is already version 50` instead and the line never comes, so the + script times out and kills a perfectly healthy server. `docs/e2e-local.md` documents the ready line + and recommends a fresh dir, but not this failure mode. **Use a fresh config dir** — the script + greps one fixed `READY_LINE`, so there is no "wait for either line" option today (widening it to + `Done migrating search index|Search index is already version` is tracked as #533). Cost that + session two failed launches before the cause was obvious. +- **The troubleshooting-playback API cannot exercise channel branding** (same origin). + `PrepareTroubleshootingPlaybackHandler` builds a synthetic channel with `Artwork = []`, and its + `ChannelId > 0` branch ignores the media item — so logo/watermark/bug behavior is **not** testable + through the troubleshoot path. Verify branding changes through a real channel playout instead; + a green troubleshoot run proves nothing about the bug overlay. - **Parallel sessions**: claim before working (`in-progress` label — the tiny read→claim race window is accepted; later claimant backs off). Claiming prevents duplicate pickup, NOT overlapping code changes — check the issue's dependency notes ("#234 after #231", "coordinate with #215") before diff --git a/docs/tracker-retrofit-triage-237.md b/docs/tracker-retrofit-triage-237.md new file mode 100644 index 000000000..db6e19fef --- /dev/null +++ b/docs/tracker-retrofit-triage-237.md @@ -0,0 +1,206 @@ +# Tracker-retrofit triage — ersatztv#237 (#524) + +Evidence for the decision record `docs.tracker-comment-retrofit` (`docs/decisions.md`). That record +states the rule; this file is the audit trail — what was examined, by what method, and how every one +of the 111 comments was classified, so the result can be spot-checked or re-derived rather than taken +on trust. + +Deliberately **not** under `docs/decisions/`: files there are parsed as decision records by +`scripts/decisions_lib.py`, and this is evidence, not a record. It is also not in +`docs/decisions/migration-map.md`, whose scope is the #521 legacy-heading→key migration. + +## Why the triage happened + +server-management#642 exports Gitea issue history into MemPalace but **excludes #237** — at 111 +comments it exceeds the miner's per-file cap. #520 separately removed #237 from session startup. The +worry behind #524: a durable fact living *only* in a #237 comment would become unretrievable, and +should be curated into lifecycle records. + +## Method + +1. **Corpus.** All 111 comments pulled verbatim from the Gitea API with per-comment provenance (id, + author, timestamp, canonical URL). Nothing sampled — every comment read. The corpus itself is not + committed (that would be the 1:1 dump #524 rejects); to re-derive or spot-check a row below, + re-pull from the permanent primary source: + `GET /api/v1/repos/timothy/ersatztv/issues/237/comments` (returns all 111 in one call). +2. **Three-way split** (from #524), applied to each *content item*, since one session comment + routinely mixes all three: + - **decision** — a durable "we do X rather than Y because Z" that governs future behavior; + - **issue-work** — per-session/per-issue narration (closures, PR/CI numbers, review loops, claims), + which already lives on the individual Gitea issue and is ingested from there; + - **lore** — a cross-cutting workflow/orchestration lesson, belonging in the handoff lore or MEMORY. +3. **Coverage test, applied to decision-shaped items only.** Semantic candidate discovery against the + `ErsatzTV-Decisions` / `ErsatzTV-Decisions-Archive` MemPalace wings, then verification against + canonical Markdown by exact search (`docs/decisions.md`, `docs/decisions/*.md`, + `docs/decisions/archive/*.md`, `docs/handoffs/chicorytv-issue-queue.md`, convention docs). No item + was declared covered on a semantic hit alone — MemPalace was discovery, never authority. +4. **Worked-issue check.** For any item surviving step 3, the issue the comment narrates was read + live. This is the step that disposes of most apparent orphans. + +## Result + +**Decision-shaped orphans: zero.** No durable decision-shaped fact exists only in #237. + +Item-level counts from the coverage pass (units are *items*, so totals exceed 111 — one comment +commonly carries a decision restatement, its issue-work narration, and a lore aside): + +| Slice | Only-in-#237 | Already covered | Issue-work | Lore | +| --- | --- | --- | --- | --- | +| comments 1–28 | 0 | 14 | 12 | 2 | +| comments 29–56 | 0 | 18 | 22 | 14 | +| comments 57–84 | 0 | 12 | 14 | 8 | +| comments 85–111 | 0 (1 raised, disproved) | 16 | 21 | 9 | +| **total** | **0** | **60** | **69** | **33** | + +Comment-level primary class (units are *comments*; sums to exactly 111): decision **14**, +issue-work **88**, lore **9**. + +### The one candidate raised, and its disproof + +Comment [106/111] (`issues/237#issuecomment-14077`) recorded #497's deliberate scope exclusions — +Guids (not eager-loaded by `GetOrAdd`, so reconciling would duplicate-insert every scan) and +Directors (not add-persisted for `MusicVideoMetadata`). Absent from the decision corpus, so it +survived the corpus check. It fails the worked-issue check: issue #497's comments **14013** and +**14072** state both exclusions with fuller reasoning, and the exporter ingests that issue as +`497.md`. Category 2, not a retrofit target. (`scan.musicvideo-reconciliation` covers #494's +neighbouring reconcile decision but not this field-scope question — distinct records.) + +### Spot-checks verified independently + +- **#357, parked player-owned-playback architecture options** — preserved on issue #357 itself (8 + comments carrying the three paths and the no-go). Note the docs copy is *not* on `main`: + `docs/player-owned-playback-poc.md` and the `docs/decisions.md` verdict entry exist only on the + unmerged `feat/357-player-owned-playback` branch, so the issue is currently the sole reachable + home. Category 2 holds, but by a narrower margin than the other items. +- **#235 / PR #267, the `Unlock: isLast` batch fix** — this *was* a fresh design decision (Fable + prescribed lock-once-per-provider batching because the naive per-source fix would starve the 2nd+ + media source; now live at `SchedulerService.cs`), not merely an application of + `locking.entitylocker-atomic-flags`. It is still category 2: issue #235 comment **8880** and PR + #267 comment **8794** both carry the rationale. + +### Known gap in this triage + +The 33 lore-class items were classified but **not** coverage-checked. That bucket is not empty: two +facts from comment [109/111] have no home anywhere and were swept into +`docs/handoffs/chicorytv-issue-queue.md` by this PR — the `e2e-local.sh` readiness-probe hang on a +reused config dir, and the troubleshooting playback API's inability to exercise channel branding. A +future tracker triage should coverage-check the lore bucket too, or state that it didn't. + +## Per-comment classification + +`primary-class` is the single dominant class of each comment; `durable home` names where that +content lives (the worked issue, a doc, or `ephemeral` for queue bookkeeping with no durable home). + +| # | primary-class | durable home | +| --- | --- | --- | +| 1 | issue-work | #231–#235 filed (triage session); ephemeral | +| 2 | decision | triage-verdict rule (Blocker/High ⇒ priority pickup); decisions.md 2026-07-11 | +| 3 | issue-work | #213, #215, #217–#221, #236 | +| 4 | decision | tracker "Pending adversarial reviews" section; ephemeral (protocol) | +| 5 | issue-work | #126, #207, #212 closed; #230, #238 filed | +| 6 | issue-work | #231, #232 via PR #241 | +| 7 | lore | re-review the fix commit, not just the initial PR diff | +| 8 | issue-work | #244 via PR #249 (ChannelsScreen extraction) | +| 9 | issue-work | #233, #234 via PR #250; #235 remainder | +| 10 | issue-work | audit sweep filing #251–#254 | +| 11 | issue-work | arc/backlog triage for #251–#254; ephemeral | +| 12 | issue-work | #251, #252 via PR #258 | +| 13 | issue-work | adversarial-reviewer#24; #255–#257 triage | +| 14 | issue-work | #238, #243, #246 body edits; ephemeral | +| 15 | decision | #253 concurrency contract (Version, ETag, 412); optimistic-concurrency.md | +| 16 | decision | #204 redirect matcher; decisions.md `spa.legacy-redirect-matcher` | +| 17 | decision | #205, #206 auth posture + rollback tag; blazor-route-parity.md §5 | +| 18 | issue-work | #202, #255–#257 via PR #262; #264 filed | +| 19 | issue-work | #253 PR1 landed; #265 filed | +| 20 | lore | api-conventions.md §7a/§7b section-number collision on parallel merge | +| 21 | issue-work | #235 via PR #267 (F9 gate closed) | +| 22 | issue-work | #91b SPA pre-work PR #272; #271 filed | +| 23 | lore | Codex "interrupted" status unreliable; trust the rollout transcript | +| 24 | issue-work | #253 PR3 landed; #269 filed | +| 25 | decision | #91b Blazor removal; rollback tag `blazor-final` = bb310f6b | +| 26 | decision | #253 PR4 phasing — collapse Phase-1/428 flip, no separate soak release | +| 27 | issue-work | #197 decomposed into #280–#289; PR #279 Phase-0 | +| 28 | decision | #197 Bundle A — single API key, `RequireKeyForReads` default true | +| 29 | issue-work | #283, #294 | +| 30 | issue-work | #289 (PR #76) — `ERSATZTV_ALLOW_WRITES` posture | +| 31 | issue-work | #295 design ratified (OIDC + local login) | +| 32 | issue-work | #245–#247 (App.tsx modularization plan review) | +| 33 | issue-work | #287, #288 (Bundle C) | +| 34 | issue-work | #271 | +| 35 | issue-work | ephemeral (arc/tracker bookkeeping correction) | +| 36 | lore | queue-status-in-prose drift; docs/handoffs/chicorytv-issue-queue.md | +| 37 | issue-work | #295 PR1, #299, #301 | +| 38 | issue-work | #269 (500-exposure slice); #303 H6/H10 | +| 39 | decision | #303 H9 append-only + H3 root-png guard; release-ci-governance.md | +| 40 | decision | #303 H10 review-verdict merge gate; release-ci-governance.md | +| 41 | issue-work | #269 closed; #308 filed | +| 42 | decision | #311 H11 rebase-not-merge + scoped format gate; contributing.md | +| 43 | issue-work | #303 close-out; #314, #315 filed | +| 44 | issue-work | #317 merge-gate double-prompt fix; #314, #319 | +| 45 | issue-work | #295 PR2, #301; #320 filed | +| 46 | lore | merge-gate needs `ETV_GITEA_BASICAUTH` in env; #315 | +| 47 | issue-work | #319 (CSP / Permissions-Policy) | +| 48 | issue-work | #253, #265 closed | +| 49 | issue-work | #238 (TopBar primary action) | +| 50 | issue-work | #286 (`/api/v1` versioning); #289 superseded | +| 51 | issue-work | #172 (scheduling API hardening); #327 filed | +| 52 | issue-work | #314 (authenticated security scan); #330 filed | +| 53 | issue-work | #197 go-live gate closed; #334 filed | +| 54 | issue-work | #335, #336 (v26.8.0 go-live) | +| 55 | issue-work | #237 tracker closure summary; ephemeral | +| 56 | issue-work | #330 (CORP header); #340 filed | +| 57 | issue-work | #333, #344–#346 | +| 58 | issue-work | #344, #347 | +| 59 | issue-work | #245, #350, #352 | +| 60 | issue-work | #354–#357 | +| 61 | issue-work | #357, #358 (player-owned playback PoC) | +| 62 | issue-work | #246, #359, #360 | +| 63 | issue-work | #243, #247 (modularization epic) | +| 64 | issue-work | #299, #363 | +| 65 | issue-work | #99, #333, #334 (queue-selection correction) | +| 66 | issue-work | #345, #357, #367 | +| 67 | issue-work | #99, #368 | +| 68 | decision | selector tier order; docs/handoffs/chicorytv-issue-queue.md | +| 69 | issue-work | #354, server-management#599 | +| 70 | decision | selector eligibility — exclude parked / closed milestones; #334, #371, #372 | +| 71 | lore | coverlet.runsettings Migrations exclusion (exit-137 OOM); #15, #373 | +| 72 | issue-work | #333, #344, #345 | +| 73 | lore | Gitea `?milestones=` filter no-ops on `:`/`+` names | +| 74 | issue-work | #70, #73, #77, #383–#386 | +| 75 | issue-work | #383–#386, #397 | +| 76 | issue-work | #72, #164, #401, #405, #414, #415, #417 | +| 77 | issue-work | #70, #376, #383, #385, #386, #419, #421 | +| 78 | issue-work | #176, #320, #385, #386, #423 | +| 79 | issue-work | #385, #386, #425, #428 | +| 80 | issue-work | #164, #385, #425, #430, #431 | +| 81 | issue-work | #386, #425, #432 | +| 82 | issue-work | #386, #425, #439 | +| 83 | issue-work | #308, #441; optimistic-concurrency.md | +| 84 | issue-work | #176, #293, #395, #425, #442 | +| 85 | issue-work | #363, #444, #445, #447 | +| 86 | issue-work | #350, #446 | +| 87 | issue-work | #381, #383, #395, #396, #448 | +| 88 | issue-work | #310, #327, #338, #367, #401, #409, #410, #447, #458 | +| 89 | issue-work | #350, #403, #404, #412, #431 | +| 90 | issue-work | #398, #420 (upload-artifact@v3 pin) | +| 91 | issue-work | #409, #458, #460 | +| 92 | issue-work | v26.10.0 release; #350, #461 (selector script) | +| 93 | issue-work | #404, #463, #464 | +| 94 | issue-work | #431, #469 | +| 95 | issue-work | #363, #444 | +| 96 | issue-work | #350, #472–#474, server-management#630 | +| 97 | lore | verify the emitter is deployed before trusting an empty log | +| 98 | issue-work | #350, #412, server-management#604 | +| 99 | issue-work | #476, #478 | +| 100 | issue-work | #478, server-management#631 | +| 101 | issue-work | #474, #477, #480, #484, #485 | +| 102 | issue-work | #350, #472 | +| 103 | issue-work | #381, #395, #480 | +| 104 | issue-work | #474, #487, #488, #491, #494 | +| 105 | issue-work | #494, #496 | +| 106 | issue-work | #497, #498, #500 (the raised-and-disproved candidate) | +| 107 | issue-work | #25, #67, #68, #502, #503 | +| 108 | issue-work | #350, #474, #487, #510, #511 | +| 109 | issue-work | #502, #510, #511 (+ the two lore facts swept to the handoff) | +| 110 | decision | retire #237 from startup; #520, #521, server-management#642 | +| 111 | lore | independent caps don't compose into a budget; #511 |