3419 Commits

This Branch

This Branch

All Branches

Author	SHA1	Message	Date
timothy	98f9e6ec34	Merge pull request 'fix(622): bind H10 merge consent to the reviewed sha via a required commit status' (#630) from feat/622-verdict-status-check into main	2026-07-25 21:33:15 +00:00
timothy	9f5317bfa5	fix(622): classify every non-success CI state, and word a negative verdict correctly ... Fourth review round, two Low findings, both in the condition-(a) message change I added last round. Both are message-only — every path still denies — but both would have printed something flatly false at the moment someone is trying to understand why a merge is blocked. 1. The filter selected only `pending` and `failure`, but Gitea also has `error` (and `warning`). With `review-verdict/h10=pending` and a build in `error`, the errored build fell out of the set, leaving the verdict as the lone entry — and the hook then claimed "every CI check is green" while a build was erroring. Non-green is now anything that is not `success`. `skipped` is deliberately still counted as GREEN: the image-push job skips on every PR (#593 — a skipped context is not red), so treating it as non-green would have permanently suppressed the H10-specific message in the one situation it exists for. 2. If the sole non-green context was `review-verdict/h10=failure` — i.e. someone reviewed this head and REJECTED it — the message said no verdict existed yet and told the reader to post MERGEABLE. It now branches on the verdict's own state: pending means nobody has reviewed this head, failure/error means it was reviewed and rejected, so resolve the findings. Also folds the two combined-status fetches into one. Verified against eight payloads: verdict-only-pending, verdict-pending-plus-error, verdict-failure-alone, skipped-plus-verdict-pending, two-pending, a real build failure, a warning state, and an unreadable body — each producing the intended message and no other. Both review channels independently confirmed this round that the rename validation is correct at both call sites, that the tests meaningfully pin it, and that no ALLOW/GRANT path became more permissive.	2026-07-25 23:16:30 +02:00
timothy	07e1e8cfbc	fix(622): validate rename rows too; name the real blocker when only the verdict is pending ... Third BLOCKED verdict, third legitimate finding. The per-row guard validated the DESTINATION only, so `{"filename":"docs/x.md","status":"renamed"}` with no `previous_filename` passed while its source silently vanished — precisely the path-hiding that collecting both sides exists to prevent. A rename row must now carry a non-empty `previous_filename`. Required for rename rows ONLY. Demanding it globally would reject every ordinary modified/added row and make the gate refuse all exemptions — which every "withholds the exemption" test would happily pass through, so that direction gets its own positive control. Predicate checked against ten shapes before adopting; mutation-verified in both directions (dropping the clause fails 2 tests, applying it globally fails 3 including the controls). Also fixes a wart this PR introduced. `review-verdict/h10` is itself folded into the COMBINED status, so a PR awaiting its verdict reports combined 'pending' and the hook's condition (a) reported it as a CI problem — sending a reader to build logs when the missing thing is the review, and exiting before the H10 branch that would have said so. The message now names the outstanding contexts, and says plainly when the verdict is the only one left. Scope boundary, stated in the record: real Gitea populates `previous_filename` on renames and returns well-formed pages. Everything past this point defends against shapes with no evidence of existing, so the guard's claim stays "any page we cannot fully classify withholds the exemption" rather than growing to cover unobserved responses. Decisions-Edit: yes	2026-07-25 22:43:30 +02:00
timothy	fc547b2d46	fix(622): validate file pages per ROW, and de-vacuum the exemption tests ... Second BLOCKED verdict, also correct. My previous fix validated only the TOP-LEVEL type of each page, which left the same hole one level down: `[{}]` is a well-formed JSON array whose rows carry no `filename`, so it contributes zero paths, looks like a short final page, and completes the enumeration from a PARTIAL list — the exact failure the guard exists to prevent, just wearing a valid-looking wrapper. An array of scalars additionally tripped `set -e` during extraction without producing a decision. Both call sites now require every row to carry a non-empty string `filename`. An empty array remains valid, since that is what a real end-of-pagination is. Predicate checked against nine shapes before adopting: `[]` and well-formed rows valid; `[{}]`, `[{"filename":""}]`, `[{"filename":null}]`, `["x"]`, `[1,2]`, a bare error object, and a mixed good/bad array all rejected. The reviewer also caught that my `test_first_page_failure_*` was VACUOUS: with page 1 failing the path list is empty, and the hook independently withholds the exemption on an empty list, so it passed with the guard removed. It is kept as a smoke case with that limitation stated in its docstring, and three tests that actually pin the behaviour were added — malformed rows and scalar rows after a FULL page 1 (so the path list is non-empty and the exemption would really fire), plus MAX_PAGES exhaustion. Added a positive control too: an empty final page must still exempt. Without it, an over-strict guard would make every "withholds the exemption" test pass for the wrong reason — the suite would be asserting "never exempt", which is not the contract. Verified by mutation: top-level-only validation fails 2 tests, an over-strict guard fails the positive control, and widening MAX_PAGES fails the exhaustion test. Decisions-Edit: yes	2026-07-25 22:23:54 +02:00
timothy	f6d01c4924	fix(622): make the exemption paging fail CLOSED; read statuses latest-per-context ... Cross-family re-review returned BLOCKED on the fix commit, correctly. Two of the three findings were on the FAILURE path, which is why every happy-path run and the whole first test suite looked fine. 1. (High/Medium) Both paging loops treated a failed or non-array page as a legitimate short final page: empty output counts as zero rows, zero rows reads as "end of list", and the enumeration completed from a PARTIAL list — failing OPEN at exactly the point the guard exists to fail closed, while the decision record claimed the opposite. Each page is now validated as a JSON array before its rows are counted; anything else withholds the exemption. 2. (High) The workflow's "is there already a verdict" pre-check read `/statuses/{sha}`, which returns one row per status POST rather than per context and pages at 50 — so a head with a few CI reruns can push a real verdict off the first page (this PR's head already carries 15 rows for 11 contexts). Missing it there is not benign: the job would post `pending`, or an exemption `success`, OVER a human verdict. Both the workflow and the hook now read `/commits/{sha}/status` (latest-per-context), and the workflow refuses to post anything when that read fails rather than inferring "no verdict yet". Credit: raised independently by the parallel session on this issue. 3. (Low) The record claimed both callers withhold the exemption when enumeration cannot complete. True only after (1); the claim and the code now agree. Adds scripts/tests/test_merge_consent_exemption.py — 9 tests over the hook's exemption path via a curl shim, asserting behaviour when a page ERRORS or returns a non-array body, not merely when it returns data. Mutation-proven: reverting the array validation fails both failure-path tests; dropping `previous_filename` fails the rename test. One test I wrote was wrong and the run caught it: I asserted a `.gitea/... -> docs/...` rename must break the hook's exemption, but that hook deliberately exempts `.claude/`/`.gitea/`/`.husky/` too (to a human PROMPT, never an auto-grant), so both sides legitimately match. The rename guard matters there for a genuinely non-exempt source; the stricter PROTECTED semantics belong to the workflow. Both cases are now pinned, including a positive control that a docs->docs rename stays exempt. Decisions-Edit: yes	2026-07-25 22:04:18 +02:00
timothy	e03c36e371	Merge pull request 'fix(491): unique index on LibraryFolder(LibraryPathId, PathHash) + tolerate concurrent insert' (#613) from fix/491-libraryfolder-unique-index into main	2026-07-25 20:00:44 +00:00
timothy	a9e3e23abf	docs(622): record head-execution as verified, not conditional ... The workflow this PR introduces does not exist on `main`, yet its job ran on #630 and posted a status — so Gitea runs `pull_request` workflows from the PR HEAD. Limitation 4 was written as a conditional ("if Gitea runs..."); it is now a confirmed property, which makes PROTECTED definitively a guardrail against accident rather than a control against a PR that rewrites the classifier. Same run also confirms the good case: #630 was correctly judged non-exempt on its `.claude/`+`scripts/` paths and got `review-verdict/h10=pending`. Decisions-Edit: yes	2026-07-25 21:53:57 +02:00
timothy	628ae46f7f	fix(622): bind H10 merge consent to the reviewed sha via a required commit status ... `pretooluse-merge-consent.sh` proves all three consent conditions at the moment the merge tool is called. With `merge_when_checks_succeed=true`, condition (a) is delegated to Gitea, which then merges whatever head is green at ITS merge time — while (b) Done-when and (c) the head-referencing verdict were proven against the head at SCHEDULING time. Every commit pushed in between merges unreviewed. The gate is not bypassed; it is satisfied against a snapshot that stops being true. Demonstrated as a controlled A/B rather than inferred, with a CI check left pending so Gitea waits as it really does: without a required verdict context: unreviewed commit B MERGED with it: same sequence REFUSED after reviewing B and posting it: merges — blocked, not deadlocked NOTE the anecdote in #622 is wrong and is corrected in the docs rather than repeated: PR #619 does carry `Review-verdict: MERGEABLE @ 02c82b35`, posted six seconds before the merge, explicitly re-reviewing the follow-up commits. #622 was filed off a lagging API read. The hole is real regardless, and structural — nothing FORCED that re-review inside the window Gitea would have merged in. This turns a property that held by diligence into one that holds by construction. The fix is the sha, not a smarter check. A Gitea commit status belongs to exactly one commit, so a status written for a parent cannot be inherited by a child that did not exist yet. `review-verdict/h10` becomes a REQUIRED status check on main: push a new head and the context is simply absent, which Gitea reads as not-passing (verified against 1.25.4: missing AND pending both block, and auto-merge re-checks the current head). It also covers merge paths the hook never sees — Gitea UI, raw API, another agent's session. - scripts/post-review-verdict.sh writes the comment and the status together so they cannot drift, and re-reads head after commenting: if a commit landed mid-flight it writes NO status and exits non-zero rather than retargeting the verdict at a commit nobody read. - .gitea/workflows/review-verdict.yml auto-passes the two exempt classes that would otherwise deadlock — Renovate-authored (platformAutomerge) and docs-only — and marks everything else pending. Exemptions are void when the PR touches .claude/, .gitea/, .husky/, scripts/ or docker/ci/. - The hook refuses to SCHEDULE an auto-merge unless that status is green on head, and no longer claims "CI green" on the mwcs path, where it never read CI. Two silent false negatives in the exemption path, both found by verifying rather than reasoning, both fixed at BOTH call sites (workflow and the hook's pre-existing docs-only carve-out): - The files endpoint caps at 50 rows and IGNORES a larger `limit` — PR #619 has 194 changed files and `?limit=100` returns 50. A single page saw ZERO protected paths there where the full enumeration finds ten. Both now page to exhaustion and withhold the exemption if they cannot complete. - A rename is ONE row whose `filename` is the destination, the source only in `previous_filename`. Verified live: `.gitea/workflows/renovate.yml` -> `docs/innocuous-note.md` presented as docs-only with no protected path visible. Both now read BOTH sides. Limits are documented, not papered over: base changes leave the head sha (and so the verdict) unchanged, and a PR editing the workflow is judged by its own edited copy — so PROTECTED is a guardrail against accident, not a tamper-proof control. fixes #622	2026-07-25 21:51:02 +02:00
timothy	960145348b	ci(491): split the MySql dedupe fixture out of CI, tracked by #627 ... The gate went red three times in CI with three distinct root causes (stale pooled session after a drop, lost isolation from a shared database name, connect-before-create). An intermittently-red gate is worse than none: it trains everyone to re-run instead of read, which is how the two collation defects escaped in the first place. The production fix is reviewed and green, so it should not stay blocked behind test-harness reliability. The fixture is kept and stays opt-in via ETV_TEST_MYSQL_CONNECTION (visible skip without it); only the CI wiring is removed, with a note where it belongs. Decision record corrected — it described a CI step that no longer exists. [decisions-edit]	2026-07-25 21:43:17 +02:00
timothy	8e83183b8b	docs(491): migrate the decision record to the per-file corpus format ... #610 split docs/decisions.md into one YAML-frontmatter file per record while this branch was open, so the inline record could not be merged -- it had to be converted. Same content and metadata, now at docs/decisions/records/scan/libraryfolder-unique-identity.md with an index entry and a regenerated catalog.	2026-07-25 21:16:51 +02:00
timothy	04ea2abb40	test(491): restore per-test databases; keep the pool clear (fixture determinism) ... Second CI failure of this fixture, and my previous fix caused it: collapsing to one shared database traded isolation for a wipe that has to succeed, and when it silently did not, the second test seeded onto the first's rows and failed with 'Duplicate entry 1 for key LibraryPath.PRIMARY' in 539ms - too fast to have re-run the migration chain, i.e. MigrateAsync no-opped against an already-current __EFMigrationsHistory. The coordinator's read was right and I verified it rather than assuming: the leak came from never clearing pools, not from names being unique. Harness against a real 8.4 server, unique database name per iteration WITH ClearPoolAsync on that connection string: 0 leaked threads over 30 iterations. So isolation costs nothing and the shared name was solving a problem pool-clearing already solved. Restore a fresh etv491_<guid> database per test, never created out of band (the test's own MigrateAsync(PreviousMigration) creates it, keeping EF the single owner of the schema), dropped in TearDown via the guarded EnsureDeletedAsync and followed by ClearPoolAsync on that exact connection string. The stale-session hazard from the first failure needs the connection string to be REUSED after the drop, which a never-repeated name makes impossible; clearing the pool is the belt to that brace and closes the leak. Verified: 10 consecutive runs 10/10 green, server threads flat at 2 and zero leftover schemas throughout; each test run ALONE twice; both tests with the order REVERSED - order-independence being the evidence this failure would have needed, since contamination is invisible when a test runs first. Negative control re-confirmed after the change: restoring COLLATE utf8mb4_bin still fails with survivors [1,4,5,6,7,9] vs [1,4,5,6,7,9,10], then restored and green again. No Retry anywhere. One shared fixture body across providers; still fail-closed under ETV_REQUIRE_MYSQL_TESTS. Refs #488 #308 fix #491	2026-07-25 21:13:31 +02:00
timothy	cdb67e26f2	test(491): make the MySql dedupe fixture deterministic (pool + database lifetime) ... The new MySql coverage failed intermittently in CI with 'Unknown database etv491_...' at connect time, and passed on the immediately preceding commit - a latent race in the fixture, not a regression. Diagnosed against a real 8.4 server rather than by inspection; two measured mechanisms: 1. A pooled session outlives DROP DATABASE. Reopening the dropped database's connection string SUCCEEDS, because MySqlConnector hands back the still-alive session whose default schema no longer exists. Whether a caller sees success or 1049 then depends on whether the pool reuses that session or opens a fresh one - a fresh handshake names the dropped schema and fails. That is the non-determinism, and it is invisible in any single green run. 2. A per-test database name leaks a pool per test. MySqlConnector keys pools by connection string, so each test built a new pool that was never cleared, leaking ~1 server thread per iteration (measured); hammering it exhausted max_connections outright, and a saturated server also inflated this fixture from ~1m20s to 13 minutes. Fix is structural, not a retry: ONE fixed database name (hence one pool), EF owns the lifecycle via EnsureDeletedAsync (guarded, unlike a raw DROP), the pool is cleared after every drop, and the drop moves to the START of a test so no test leaves a live pool pointing at a database it just destroyed. ServerVersion is resolved once per SetUp instead of twice. Determinism verified: 10 consecutive runs, 10/10 green, server threads flat at 2 throughout. Negative control re-confirmed after the change - restoring COLLATE utf8mb4_bin still fails with survivors [1,4,5,6,7,9] vs [1,4,5,6,7,9,10]. Both providers still share one fixture body; still fail-closed under ETV_REQUIRE_MYSQL_TESTS. Refs #488 #308 fix #491	2026-07-25 21:13:31 +02:00
timothy	b587f80f43	test(491): clear the nullable warnings in the dedupe fixture ... Review follow-up. The new MySql-half members introduced CS8600/8604/8625/8618 and were inconsistent with this same file's established late-init pattern (_databasePath = null!). Nullable-annotate the genuinely-nullable ones and use null! for the Dapper-populated row properties.	2026-07-25 21:13:31 +02:00
timothy	83cd36e0de	test(491): run the dedupe fixture against MySql in CI; correct the collation claim ... The dedupe DML had zero automated coverage on MySql: the migrations job only applies migrations to a fresh EMPTY database, so no dedupe row ever executed there. Two MySql-only collation defects escaped that gate in this session and were caught only by hand-run containers. Parameterize LibraryFolderDedupeMigrationTests over both providers from ONE fixture body - same seeded rows, same expected survivors - rather than adding a MySql-only copy that would drift and recreate the gap. Assertions no longer use WHERE Path = '...', which is itself collation-dependent and would quietly mean something different per provider; rows are read once and compared ordinally in memory. A new step in the existing migrations job runs it against that job's mysql:8.4 service, on a per-test database of its own. Proven red when the collation is wrong: restoring COLLATE utf8mb4_bin fails the MySql half with survivors [1,4,5,6,7,9] - the trailing-space sibling deleted - while SQLite stays green. Proven non-skippable: without ETV_TEST_MYSQL_CONNECTION the fixture ignores visibly, and with ETV_REQUIRE_MYSQL_TESTS=1 (which CI sets) that skip becomes a hard failure, so it cannot pass having connected to nothing. Local runs need no MySql. Also correct an overstated comment. The schema pins only the utf8mb4 charset, never a collation, so the effective comparison is the server default: always case-insensitive, but PAD SPACE only on utf8mb4_general_ci - 8.4's default utf8mb4_0900_ai_ci is NO PAD, verified on the real column. The migration bug was independent of that because the old code applied an EXPLICIT utf8mb4_bin, which is PAD SPACE everywhere; the runtime simply tolerates both. Refs #488 #308 fix #491	2026-07-25 21:13:31 +02:00
timothy	48d41b9235	fix(491): make the MySql dedupe byte-exact, not just case-exact (PAD SPACE) ... Cross-family review of 1b4dd6d6 found that utf8mb4_bin - chosen to keep the dedupe case-exact - is a PAD SPACE collation, so trailing spaces are insignificant under it. Verified on MySQL 8.4: '/media/Foo' = '/media/Foo ' is TRUE, while case correctly compares unequal. Two distinct legal directories therefore grouped together and the second was DELETED irreversibly, even though PathUtils.GetPathHash hashes them differently and the unique index about to be created would have accepted both. The dedupe destroyed data the constraint never required it to destroy. Group and join on CONVERT(Path USING binary) instead - NO PAD and byte-exact, matching the hash. utf8mb4_0900_bin is also NO PAD but carries a server-version floor. This is the only path comparison in either migration (every other predicate keys off an integer id), so there is no mix of padded and unpadded comparisons across the keeper-selection, repoint and delete steps. SQLite's = on TEXT is byte-exact with no padding, so that migration was already correct - which is exactly why a SQLite-only test could not see the divergence. The two providers are now semantically equivalent, and the dedupe fixture is shared: same rows, same expected survivors (1,4,5,6,7,9,10), asserted by the SQLite test and reproduced by hand on MySQL 8.4. Runtime was never affected, and this is now stated and tested rather than assumed: GetFolder's SQL equality is a superset narrowing (both collation quirks make it more permissive, never less, so it cannot miss a byte-exact match) and ResolveExact settles identity with StringComparison.Ordinal, which compares length first. Added ResolveExact coverage for the trailing-space axis. Refs #488 #308 fix #491	2026-07-25 21:13:31 +02:00
timothy	50eff83628	docs(491): reattach the case-exact lookup rationale to GetFolder ... Review follow-up. ResolveExact was inserted between GetFolder's summary and GetFolder itself, leaving ResolveExact with two summaries and GetFolder with none -- so the H1 rationale (SQL equality is only a narrowing filter; OrderBy(Id) for determinism) described neither of the things ResolveExact does. On a fix that turns on exactly that reasoning, a misattached explanation is what misleads the next reader.	2026-07-25 21:13:31 +02:00
timothy	ecb763ea58	fix(491): review polish — heal cannot abort a scan, ordinal settle unit-tested ... Final low-severity items from the re-review of ee10f932. L2: DbUpdateConcurrencyException derives from DbUpdateException but carries no provider exception, so IsUniqueConstraintViolation does not classify it. A row deleted by a concurrent library edit between the heal's read and its save would propagate and fail the scan, contradicting the invariant stated directly above it. Admit it in the filter. L1: lift the in-memory ordinal settle into LibraryRepository.ResolveExact and unit-test it with both spellings in the candidate list. No SQLite-backed test can exercise it (SQLite's = on TEXT is already binary), so this converts the half that rested on hand-run MySQL evidence into automated coverage. The end-to-end companion test's comment no longer claims to be provider-independent. L4: assert PRAGMA foreign_keys is 1 before migrating, so the enforcement guard cannot silently degrade into the weak pre-fix form it was added to replace. L3: detach the failed heal, matching the insert path. N3: the heal's inner predicate now matches its IsNullOrEmpty outer guard, so a PathHash = '' row cannot enter the branch and silently never heal. N4: record that GetFolder returning null for a case-differing spelling makes MySQL insert a second row where it used to reuse one — correct, and now matching SQLite, but a real behaviour change on a case-insensitive filesystem. Refs #488 #308 fix #491	2026-07-25 21:13:30 +02:00
timothy	14e9b03433	fix(491): wire the unique-violation classifier in the scanner; make folder lookup case-exact ... Review of 491f5099 found the fix inert in the only process that runs it, plus a MySQL collation defect in the lookup. B1 — TvContext.IsUniqueConstraintViolation was assigned only in ErsatzTV/ Startup.cs, but ErsatzTV.Scanner is a separate executable and every production caller of GetOrAddFolder/SetEtag lives there. The classifier kept its '_ => false' default, so the catch never ran and the DbUpdateException failed the whole scan - worse than the duplicate row it replaced. Wire both provider branches in ErsatzTV.Scanner/Program.cs, and add ProviderStaticsWiringTests (architecture) asserting the scanner assigns every TvContext static the host assigns, with IsSqlite documented as the one exemption. H1 — GetFolder's 'Path == folder' is case-insensitive on MySQL while PathHash is case-sensitive, and FirstOrDefault was unordered: a scan of '/x/foo' could resolve the '/x/Foo' row and stamp the wrong hash onto it (verified on MySQL 8.4: the WHERE matches both, LIMIT 1 returns the wrong one). Treat the SQL equality as a narrowing filter, order by Id, and settle identity ordinally. Route the heal through EF and drop a classified violation, so an opportunistic maintenance write can never abort a scan. Also: run the dedupe migration test with foreign keys ON (matching prod), clear the keeper's etag, null out a self-parent, and document the cleanup's limits (NULL paths excluded, Down does not restore deleted rows, CI's fresh-DB apply covers none of the data mutation). Refs #488 #308 fix #491	2026-07-25 21:13:30 +02:00
timothy	9a4f3e832d	fix(491): unique index on LibraryFolder(LibraryPathId, PathHash) + tolerate concurrent insert ... GetOrAddFolder was a check-then-insert with no unique constraint behind it, so two callers racing the same folder could both miss the lookup and both insert. Enforce identity in the schema and make the loser adopt the winner. - LibraryFolder gains a SHA-256 PathHash (the MediaFile.Path/PathHash precedent): Path is MySQL longtext, which cannot be indexed without a prefix length and collates case-insensitively, so the unique index is on (LibraryPathId, PathHash) instead. - GetOrAddFolder and SetEtag catch a classified unique violation via the existing TvContext.IsUniqueConstraintViolation seam (#308) and re-read. - Dual-provider migration audits and collapses pre-existing duplicates (repointing MediaFile, ParentId and ImageFolderDuration) before creating the index; legacy rows keep a null hash and heal on the next scan. - Tests: deterministic cross-connection race, 8x10 barrier stress with an insert-attempt vacuity guard, classifier-inversion negative control, and a real-migration dedupe test. Refs #488 #308 fix #491	2026-07-25 21:13:22 +02:00
timothy	8e84191a03	Merge pull request 'feat(610): split the decision corpus into one YAML-frontmatter file per record' (#619) from feat/610-decisions-one-file-per-record into main	2026-07-25 18:36:08 +00:00
timothy	02c82b35ea	fix(610): make the frontmatter READ path dependency-free — CI has no PyYAML ... This is why `decisions lifecycle` went red, and it was NOT the known flake. I came close to dismissing it as one for the second time this session, because an earlier red on another branch genuinely was. The dual-format parser imported PyYAML to read frontmatter. `decisions-guard` does `setup-python` and installs NOTHING, so once the corpus was migrated every record became unparseable there: ModuleNotFoundError, job fails. The same would hit the Husky pre-commit hook and every contributor's machine. Installing PyYAML in CI is the wrong fix: READING happens everywhere -- CI, the hook, every dev -- while WRITING happens once, in a migration a human runs deliberately. So the read path is now dependency-free and only `migrate_decisions_split` (the writer) still imports yaml. A hand-rolled parser is only safe if it provably matches the library that WROTE the files, so `test_frontmatter_reader_matches_pyyaml_on_every_real_record` compares the two field-by-field across all 169 real records (importorskip, so it is skipped rather than failing where PyYAML is absent) with a >100-file guard against near-vacuity. It is narrow by construction: the frontmatter is machine-generated with default_flow_style=False and width=10**9, so every value is a single-line scalar, and the reader bails to None on anything nested. Verified by running all four affected entry points against a shim that makes `import yaml` raise: validate --base/--head, catalog --check, the kickoff guard, and the plain validate the pre-commit hook calls. All exit 0. refs #610	2026-07-25 19:57:22 +02:00
timothy	52786a545c	feat(610): re-key the body-diff guard to key; enforce path<->key; pin the filesystem invariant ... Three Done-when items that were genuinely unfinished. I audited the checklist before ticking it rather than after, and these were not done. 1. BODY-DIFF GUARD RE-KEYED FROM HEADING TO `key`. Heading-keying made a pure RENAME look like a removal plus an unrelated addition, so retitling a record failed CI as an "unlogged removal" -- a trap that has bitten this repo before. Records are now MATCHED by key and REPORTED by heading (a key alone is not enough for a human to find the record). Demotion, which is inherently about a record losing its key, is detected on the heading axis instead, and a demoted record is no longer double-reported as removed. Key-matching is also what lets the comparison work straight through the legacy->frontmatter migration, since `key` survives both the format change and the move between files. Pinned by a test driving the REAL git-backed diff engine: retitle a record, same key, byte-identical prose -> removed/rewritten/demoted all empty. Mutation-verified: restoring heading-keying makes it fail with exactly "a retitle was reported as a removal". 2. PATH <-> KEY ENFORCED BY THE VALIDATOR. The filename is derived from the key, which is what makes one-active-per-key a filesystem property rather than a check -- but only if the two cannot drift. Now an error when they disagree, with legacy multi-record files explicitly exempt (they have no key-derived path to match). 3. ONE-ACTIVE-PER-KEY PINNED AS A FILESYSTEM PROPERTY. Two records sharing a key derive the same path, so the filesystem refuses the second; the migration aborts on a destination collision rather than silently overwriting. Both are now tested, the latter end-to-end through `main()`. Also fixed a vacuous assertion ruff caught in my own new test -- `assert X or True` is always true. Note on verification: my first positive control for the guard reported it NOT firing. That was the probe, not the code -- "load 340" appears in both the `sources:` frontmatter field and the body, and replace(...,1) hit the metadata copy, which is correctly token-free. Re-run against genuine body prose, the guard fires with exit 1. refs #610	2026-07-25 19:53:08 +02:00
timothy	fba5233caf	feat(610): split the decision corpus into one YAML-frontmatter file per record ... 168 records -> docs/decisions/records/<area>/<topic>.md (163 active, 23 dirs) and docs/decisions/archive/<area>/<topic>.md (5 archived). The filename IS the key, so one-active-record-per-key becomes a filesystem property rather than a validator check, and supersession becomes a `git mv`. WHY: the monolith was a concurrency problem before an aesthetic one. A 3,900-line append target made parallel sessions collide -- PR #605 and PR #614 both hit append-vs-append conflicts during routine rebases, and hand-resolving those inside the corpus is exactly the operation the rationale-rewrite guard exists to police. HOW IT IS VERIFIED: a ~170-file diff cannot be meaningfully read, so correctness does not rest on reading it. The parser was taught BOTH formats first, so the body-diff guard parses the old form at the merge-base and the new form at head -- the migration validates itself, no bypass. The proof is a field-level equivalence harness: 168 records before and after, zero lost, zero gained, zero field mismatches, zero rationale bodies differing. Reviewers should scrutinise the harness; it is the actual evidence. What measuring caught that reading would not have: - ~500 lines sit OUTSIDE any record -- decisions.md's lifecycle schema and each topic file's preamble, mostly the only copy. Source files are kept and stripped, never deleted. They also cannot be filed per-area: topic files hold several areas and 4 of 23 areas span several files. - Archive discovery was a non-recursive glob; after the split it found ZERO archived records, surfacing as four bogus "supersedes points to unknown key" errors rather than an obvious failure. - ~32 live docs point into the corpus BY DATE, which the split dangles. Each stripped file now ends with a generated "Records formerly in this file" index, which also rescues the identical breadcrumbs in old issue comments. - decisions.md's "In this file:" list was 97 same-file anchor bullets that the split makes WRONG, not merely stale. Dropped; the generated index replaces them with links that resolve. The equivalence harness now runs against a checked-in FIXTURE, not the live corpus. The earlier version migrated the real tree, which made it a one-shot: the moment the migration landed there was nothing left to move and the tests failed for reasons unrelated to the code. A fixture keeps them testing the SCRIPT rather than the repo's current state. Keys preserved verbatim, warts included: `sched` (12) and `scheduling` (1) remain two directories for one concept. Renaming a key is not a move -- it changes identity, breaks the equivalence proof, and invalidates MemPalace's per-key drawers. Taxonomy normalisation is separate work. refs #610	2026-07-25 19:45:09 +02:00
timothy	8578dc1ca7	fix(610): de-brittle the count assertion; stop conflating index lines with preserved prose ... Both surfaced when main gained two records mid-flight. - test_record_count_is_the_expected_166 hardcoded the total, so it failed the moment a record landed on main -- a merge turning an unrelated test red. The real invariant is before == after; the count only needs to prove the harness isn't parsing a stub corpus, so it is now equality plus a floor. - The migration's "lines preserved" figure silently absorbed the generated where-did-it-go index once that was threaded into the preamble string, jumping 507 -> 759 with no new prose preserved. It now reports the two separately: 514 lines of original prose, plus 245 generated index lines. A number that quietly changes meaning is worse than no number.	2026-07-25 19:08:58 +02:00
timothy	64b65fd2db	feat(610): generated where-did-it-go index on each stripped file ... Live docs point into the corpus BY DATE -- "see `decisions.md` 2026-07-10" -- about 32 such references across 12 files, plus the same form in historical issue comments. The split would dangle every one of them. Each stripped file now ends with a generated "Records formerly in this file" index: date, title, and a link to the record's new path. A reader following a date pointer lands on the file it names and resolves from there. That is far cheaper and less error-prone than rewriting 32 references by hand, and it also covers the issue-comment breadcrumbs, which cannot be rewritten at all. Caught while verifying it: the generated `## Records formerly in this file` heading is itself an H2, so the record parser counted one legacy-unmigrated record per stripped file -- the notice went 0 -> 6. Same treatment as the existing `## Index` section: skip it by name. SKIP_HEADINGS moved to decisions_lib as the single source of truth, since three modules now need it. Found by reading the validator's notice output on a trial migration, not by inspection -- the corpus still validated OK, so nothing else would have flagged it.	2026-07-25 19:08:58 +02:00
timothy	8ec1f527e4	fix(610): catalog path links only for the split layout, not every file under docs/decisions ... The first cut tested `relative_to(TOPIC_DIR)`, which also matches the LEGACY multi-record topic files -- so every record in workflow-process.md et al. lost its anchor and linked to the top of the file instead of to its own record. Caught by `build_decisions_catalog --check` going stale on the unmigrated corpus, not by reading. Narrowed to `RECORDS_DIR in src.parents`, which by construction only matches one-record-per-file.	2026-07-25 19:08:58 +02:00
timothy	1af65b7bee	feat(610): budget counts PROSE, excluding YAML frontmatter ... The line budget exists to bound how much narrative a reader or agent must get through. Under the split each record carries ~11 frontmatter lines plus two fences -- 1789 lines across 166 records -- which are the structured restatement of what used to be one dense backtick line. Counting them inflates the metric without any new knowledge being added. Stated plainly because it flatters the number: this is a change of METRIC, not a consolidation. It re-measures the same corpus, it does not shrink it. Whole-file counting put the migrated corpus at 6837 against a 5600 budget; prose-only puts the same content at ~5048. The consolidation work is still worth doing -- it is simply no longer being signalled by a warning that was partly measuring punctuation. Inert pre-migration: no legacy file has frontmatter, so the branch is never taken and today's number is unchanged.	2026-07-25 19:08:58 +02:00
timothy	ef67be924c	feat(610): teach the validator and catalog the split layout ... Both surfaced in a trial migration against a disposable copy of the real corpus, not from reading: - Archive discovery was a NON-recursive glob. After the split, archived records live at archive/<area>/<topic>.md, so every one of them became invisible -- which surfaced as four bogus "supersedes points to unknown key" errors rather than as an obvious "no archive found". rglob at both sites. - Catalog links: a record is now a FILE, so the link is a plain relative path with no anchor -- nothing to slug, nothing to keep in sync with a heading. The legacy anchor form is kept for records still living inside a multi-record file, so the catalog is correct on either side of the migration.	2026-07-25 19:08:58 +02:00
timothy	d52268f2ed	feat(610): migration script + field-level equivalence harness ... The migration is a MOVE, so correctness is provable rather than reviewable: parse the corpus before, migrate, parse after, assert the Record sets are identical field-for-field with byte-identical rationale. scripts/migrate_decisions_split.py 166 records -> docs/decisions/records/<area>/<topic>.md (active) or docs/decisions/archive/<area>/<topic>.md (superseded/retired), 26 directories. Path is DERIVED from the key, so identity stays the key. Refuses to run if any record lacks a key, and aborts on a destination collision. Source files are KEPT, stripped to their narrative -- not deleted. 506 lines of the corpus sit outside any record: decisions.md's lifecycle-schema header (the status vocabulary, supersession rules, the edit-token contract) and each topic file's preamble explaining why those records exist. For most of it that is the only copy. It also cannot be filed per-area -- topic files hold several areas and 4 of 23 areas span several files -- so the files themselves stay. scripts/tests/test_migration_equivalence.py Runs the real migration against a COPY of the real corpus in tmp_path, never the working tree. Asserts: all 166 records survive with the same keys, every metadata field round-trips, titles round-trip from headings, every rationale body is byte-identical, path matches key, archived records land in the archive wing, the legacy files keep their narrative, and no parseable record is left behind in them. Proven non-vacuous: corrupting one migrated record's prose is caught by the byte-identical check, and deleting one is caught by the survival check. One test-authoring note: an early assertion string-matched "## " to prove no records were left in decisions.md. That is wrong -- the schema header quotes an illustrative "## 2026-07-17 ..." example in prose. Whether records remain is a PARSING question, so the parser-based leftover test is the real invariant.	2026-07-25 19:08:58 +02:00
timothy	e063fd065f	feat(610): dual-format decision-record parser (backtick line + YAML frontmatter) ... First step of the one-file-per-record split. `decisions_lib.parse_text` now dispatches on a leading `---`: the legacy "many ## records per file, metadata on a backtick line" form and the new "one record per file, YAML frontmatter" form both produce the same Record. This is the load-bearing move for the migration. Because the parser understands both, the body-diff guard can parse the OLD form at the merge-base and the NEW form at head -- both keyed on `key` -- so the commit that relocates 166 records validates itself with no one-time bypass and no escape hatch. Details: - Frontmatter uses the SAME field vocabulary as the backtick line (`superseded-by`, `stale-after`), so on-disk names don't shift mid-migration. - YAML `None` (a key with no value) is preserved as "" rather than collapsing to None, keeping the empty-vs-absent distinction `stale-after` depends on (#603). - `active_files()` also walks `docs/decisions/records/**`, so both layouts can coexist while the migration lands. - Malformed frontmatter (unterminated fence, non-dict, YAML error) yields no record rather than raising; the validator then reports it as a missing record.	2026-07-25 19:08:58 +02:00
timothy	798ecd1f53	Merge pull request 'fix(609): the decisions rationale-edit marker is a git trailer, not a bare substring' (#618) from fix/609-decisions-edit-token into main	2026-07-25 17:07:49 +00:00
timothyandClaude Opus 5	f49556b6ef	docs(609): qualify the two ci-cd.md mentions as non-merge ... Last review Low, marked safe-to-defer but it is two words in a file this PR already edits. Both prose mentions of the marker now say "non-merge commit", matching the decision Rule, the catalog, the module comment and the function docstring. Docs-only. fixes #609 Decisions-Edit: yes Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-25 18:40:55 +02:00
timothyandClaude Opus 5	e9f4444fec	docs(609): qualify two comments as non-merge, matching the implementation ... Review Low: the module comment and the _edit_trailer_armed summary still said "a commit message ... does arm" / "some commit", while the matcher excludes merges. The decision Rule and catalog already carried the qualification; these two did not. Comment-only. fixes #609 Decisions-Edit: yes Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-25 18:38:58 +02:00
timothyandClaude Opus 5	f00dfde0c5	docs(609): correct the --no-merges rationale — merges are discouraged, not blocked ... Review Low x2, both correct and both the stale-comment class: The claim that prepush-rebase-check.sh forbids merging main into a PR branch is false. That hook refuses a branch that is BEHIND origin/main; a merge makes origin/main an ancestor, so the push is allowed. Merging main in is discouraged by convention only. So --no-merges does cost a real false negative: an author who marks ONLY a conflict-resolving merge commit gets a legitimate rewrite rejected. Keeping --no-merges and stating the trade explicitly -- that failure is loud and costs one extra commit, whereas honoring forge-composed merge bodies disables the guard silently, which is #609 itself. The module comment also still claimed a quoted example cannot arm the guard, which contradicts the residual the decision record now states accurately. Aligned both, and narrowed the record's Rule line from "some commit" to "some NON-MERGE commit" so the stated contract matches the implementation. No logic change -- comments, docstring, record prose and regenerated catalog only. fixes #609 Decisions-Edit: yes Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-25 18:36:07 +02:00
timothyandClaude Opus 5	cc9481f541	fix(609): review fixes — exclude merge commits, unfold folded values, correct the fail-posture doc ... Cross-family review (High + Medium + Low), all three reproduced before fixing: High -- on a pull_request event actions/checkout lands on a synthetic merge commit whose body the forge composes from the PR description, so a description ending in an example marker armed a guard no author armed. Excluded merge commits from the range; merging main into a PR branch is separately forbidden, so no author-written commit is skipped. Medium -- a folded value (`no` + continuation ` yes`) was split into independent lines and the continuation armed on its own, inverting the value the author wrote. Read with `unfold` so the value is judged whole. Low -- the module docstring promised fail-open while marker resolution deliberately fails closed. The posture is right; the docstring was wrong. Documented as the one exception. Both new negative controls verified red against the unfixed matcher. Decision record amended to state the residual honestly rather than overclaim: a quoted example that is the FINAL paragraph of an ordinary commit is a trailer by git's own grammar and does arm. What the change buys is that discussing the marker can no longer disable anything. fixes #609 Decisions-Edit: yes Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-25 18:29:46 +02:00
timothyandClaude Opus 5	c597c49f02	fix(609): arm the decisions body-diff with a git trailer, not a bare substring ... The rationale-edit exemption was a substring test over the whole commit range, so any message containing the literal marker armed it -- including prose explaining why no marker was needed, which is how it fired live in PR #605: a green --base/--head run that was vacuous on the body-diff dimension, in the one PR that hand-resolved a merge conflict inside the corpus the guard exists to police. Now read as an affirmative `Decisions-Edit:` git trailer. Git parses trailers only in the final paragraph, so a quoted example message cannot arm it -- which matters because this commit and its decision record both quote one. A non-affirmative value (`no`) does not arm it either; the retired substring arms nothing and gets a ::warning:: nudge. Tests: negative controls (prose mention, quoted mid-body trailer, `no` value, retired substring) plus positive controls (trailer, uppercase, alongside Co-Authored-By). All four negative controls verified red against the old matcher. fixes #609 Decisions-Edit: yes Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-25 18:21:12 +02:00
timothy	54ed75624a	Merge pull request 'fix(496): per-library server identity for music videos — itemId diff + soft trash' (#607) from fix/496-musicvideo-identity into main	2026-07-25 16:08:12 +00:00
timothy	b5b6e7f636	test(496,484): record projection failures during enumeration, not eagerly [decisions-edit] ... Re-review Low. The substitute incremented the failure counter inside .Returns(...), i.e. when the enumerable was handed out, while the real paginator records from ProjectToMusicVideo's catch DURING enumeration. A refactor that snapshotted Count before the enumeration completed would then break production while both replacement tests kept passing — exactly the regression the guard exists to prevent. Moves the recording into an async iterator, and corrects the decision record to describe #484's removed music-video test accurately and name its two replacements.	2026-07-25 17:36:34 +02:00
timothy	4bdad4bf52	fix(496,484): thread #484's projection-failure guard through the new music-video scanner [decisions-edit] ... Rebasing onto #612 exposed a silent gap rather than a conflict. #612 added a `projectionFailureCount` parameter to MediaServerReconciliationGuard.ShouldFlagMissing that refuses the sweep when the enumeration reported swallowed projection exceptions — but the parameter is OPTIONAL with a default of 0, so this scanner compiled unchanged while opting out of the protection entirely. ProjectToMusicVideo has exactly the swallowing catch #484 exists to defend against, so the music-video sweep would have been the only one unguarded. - MediaServerMusicVideoLibraryScanner creates one MediaServerProjectionFailureCounter per enumeration (never a field on the singleton api client, so concurrent scans of different libraries can't leak failures into each other's sweep decision), passes it to GetMusicVideoLibraryItems, and feeds its Count to ShouldFlagMissing. - The single guard call also gates the #496 legacy path diff, which is more exposed: a legacy row has no etag to fall back on. - ProjectToMusicVideo now returns MediaServerProjectionResult<JellyfinMusicVideo>, combining #612's Skipped/Failed distinction with #496's identity type. - main's own #484 music-video test targeted the pre-#496 hard-delete scanner (FindMusicVideoPaths /DeleteByPath) and no longer applies; it is replaced by two tests in the new architecture covering the identity sweep and the legacy sweep. Both proven non-vacuous — removing projectionFailures.Count from the guard call fails exactly those two. 4,277 tests green; format/BOM clean; decisions validator OK.	2026-07-25 17:27:50 +02:00
timothy	147166b053	fix(496): harden the legacy path sweep against MySQL collation (re-review Low) ... Re-review returned MERGEABLE with one Low, MySQL-collation-dependent edge in FlagFileNotFoundByPaths: the C#-side Except diff is ordinal, but `MF.Path IN @LocalPaths` runs under MySQL's case-insensitive default collation, so a still-reported identified row differing only in case from an absent legacy row could be matched and flagged missing. Match on the indexed PathHash instead of collated Path text, and re-state the NOT EXISTS (JellyfinMusicVideo) guard so the identity pass and the legacy pass are disjoint by construction rather than by the caller's diff being correct. SQLite was unaffected.	2026-07-25 17:20:53 +02:00
timothy	6bd1d954bd	fix(496): review fixes — thread the replaced local path, scope identity per library, sweep legacy rows [decisions-edit] ... Independent cold review (Codex) returned BLOCKED. Findings 1, 3 and 4 are fixed here; each has a regression test proven non-vacuous by a negative control. 1. Blocker — the replaced local path was discarded. The scanner computed localPath but GetOrAdd only received `incoming`, so the repository re-derived the path from the UNREPLACED projection. On any install with path replacements, adoption hashed the server-side path, missed the existing row, ALSO slipped past MediaFileAlreadyExists (which hashes that same wrong string) and inserted a duplicate row under a server path, leaving the original collection-linked row identity-less forever. The test harness hid this because its path-replacement stub was an identity function. → GetOrAdd now takes localPath explicitly and never reads the projection's path; BuildPathReplacement takes a real mapping and the new test genuinely replaces. 3. Medium — GetByItemId matched on ItemId alone, so two media sources presenting the same item id (cloned Jellyfin DB) resolved to each other's row, letting one library repoint another's. → filtered by LibraryPath.LibraryId. 4. Medium — a row predating the identity that the server had ALREADY stopped reporting was never adopted (adoption only runs for an incoming item) and carried no identity, so the itemId diff could not see it either: it sat Normal and schedulable forever, strictly worse than the hard delete it replaced. → GetExistingLegacyMusicVideoPaths + FlagFileNotFoundByPaths reconcile legacy rows by local path, and they are counted into the #477 empty-fetch guard (on the first scan after this ships they ARE the whole library, so a guard counting only identity rows would sweep all of them on a transient empty fetch). Finding 2 (the issue's Done-when #2) is a scope question, not a defect, and is unchanged: one file path is still one MediaItem row globally, so this lands music videos at parity with movies rather than eliminating shared-row trashing. Recorded honestly in the decision record; raised for an explicit call before the issue is closed. fixes #496	2026-07-25 17:20:53 +02:00
timothy	64decd492e	fix(496): give music videos a per-library server identity; itemId diff + soft trash ... Music videos carried no server identity, so JellyfinMusicVideoLibraryScanner had to reconcile by a (LibraryPathId, path) diff and HARD-delete the remainder. A file served by two libraries with overlapping local paths is a single row owned by whichever library scanned it first, so that owner's sweep destroyed a row another library still served — taking collection membership and playout references with it, irreversibly. This is #494's deferred "option 2": - New JellyfinMusicVideo : MusicVideo (ItemId/Etag), mirroring JellyfinMovie — TPT table, varchar(36), ItemId index. Dual-provider migration Add_JellyfinMusicVideo. - New IMediaServerMusicVideoRepository + JellyfinMusicVideoRepository: itemId-keyed existing-set/lookup and Flag{Normal,Unavailable,FileNotFound} seams, all scoped per library via LibraryPath.LibraryId. - New MediaServerMusicVideoLibraryScanner base; JellyfinMusicVideoLibraryScanner folds onto it and keeps the #177/#488/#497/#500 metadata-reconcile logic verbatim. - The sweep now soft-trashes (FileNotFound) instead of deleting, so removal is reversible and EmptyTrash-governed. DeleteEmptyArtists consequently no longer fires from a sweep. - Pre-identity rows are ADOPTED in place: the identity row is inserted against the same MediaItem id, scoped to the scanned library's own LibraryPath, so collection membership survives and a local/second-library row is never hijacked. - AddMusicVideo normalizes Path/PathHash to the path-REPLACED local path; the projection fills them from the server-reported path, which would break every later PathHash lookup. Docs: scan.musicvideo-reconciliation relocated to docs/decisions/archive/scan.md as superseded; new active record scan.musicvideo-server-identity. fixes #496	2026-07-25 17:20:53 +02:00
timothy	eb4de0cc2b	Merge pull request 'feat(484): suppress the media-server sweep on projection failures; reject the ratio threshold' (#612) from feat/484-scanner-guard-threshold into main	2026-07-25 15:08:07 +00:00
timothy	7cc12881de	docs(484): correct the counted-enumeration counts on the api client interfaces ... Review follow-up. The comments still said three (Jellyfin) / two (Emby) library-level enumerations after the nested season and episode ones were counted, which understates the reach of the very safety property this branch establishes -- a future auditor reading them would conclude the nested sweeps are unprotected.	2026-07-25 16:36:35 +02:00
timothy	6f4497e1ce	fix(484): guard the nested TV season and episode sweeps against projection failures ... Review finding 1 (blocking). ScanSeasons' FlagFileNotFoundSeasons and ScanEpisodes' FlagFileNotFoundEpisodes had no guard at all — neither #477's nor #484's — so ProjectToSeason / ProjectToEpisode returning Failed() was computed and discarded. #477 scoped those out because "the blast radius is one show's seasons / one season's episodes", which holds for a per-parent EMPTY fetch but not for a projection failure: that is systematic by construction. One bad code path fires on every parent, so every season enumerates zero episodes, existing.Except([]) is the whole episode library, and EmptyTrashHandler deletes it permanently. Threads the counter into GetSeasonLibraryItems / GetEpisodeLibraryItems(WithoutPeople) for Jellyfin and Emby using the same optional-trailing-param shape, and guards both sweeps with MediaServerReconciliationGuard.ShouldFlagMissingDescendants — the same class and the same private failure predicate as ShouldFlagMissing, deliberately WITHOUT #477's empty-fetch branch so per-parent empty behaviour (and #476's cascade, which depends on it) is unchanged. Also from the review: - finding 3: tests now pin the same-instance JOIN at every level (movie, show, season, episode, music video) by driving the real ScanLibrary entry point and recording the failure from inside the enumeration, so a refactor handing the api client a fresh counter goes red. - finding 4: the missing-library Failed() branch is documented as defensive and unreachable. - finding 2: the mass-Skip residual (Emby's response-shape-dependent MediaSources guard, Plex's pre-projection filter) is stated as a known limitation in the decision record. - finding 5: the log-contract change (only the #484 message when both refusals apply) is noted. fixes #484	2026-07-25 16:36:35 +02:00
timothy	e3645a2840	feat(484): suppress the media-server sweep on projection failures; reject the ratio threshold ... Extends MediaServerReconciliationGuard (#477) with a second deterministic refusal: when the enumeration that produced the incoming set silently dropped items whose projection THREW, the file-not-found sweep is refused. A dropped item the server did return is indistinguishable from a deletion at the reconcile step, so a projection regression could otherwise mass-flag a healthy library FileNotFound (which EmptyTrash then deletes permanently). Deliberate guard-clause skips (STRM files, virtual items, unsupported types) are explicitly NOT failures and never suppress a sweep — counting them would permanently disable reconciliation for any library holding a single STRM file. The ratio / missing-fraction threshold is REJECTED, not deferred: it is a two-sided heuristic with no tunable default and no telemetry, and the failure it approximates is exactly observable via the projection-failure count (a genuine bulk deletion produces zero failures). Seam is deliberately narrow — the private ProjectTo* contract inside each api client changed from Option<T> to MediaServerProjectionResult<T> (projected/skipped/failed), the paged helper counts IsFailure in one place, and the scanner reads it through an optional trailing MediaServerProjectionFailureCounter on only the five library-level methods that feed a sweep. The counter is per-enumeration state created by the scanner, never a field on an api client. fixes #484	2026-07-25 16:36:35 +02:00
timothy	fd70e6eb4f	Merge pull request 'fix(503): map WatermarkLocation.MiddleCenter to a centered overlay position' (#611) from fix/503-watermark-middlecenter into main	2026-07-25 14:35:27 +00:00
timothy	c3beee1629	Merge pull request 'feat(603): adopt OKF's optional stale-after and Sources decision-record metadata' (#605) from worktree-603-decisions-stale-after-sources into main	2026-07-25 14:15:59 +00:00
timothy	f04412a95b	docs(603): correct the record's own no-backfill claim, which the backfill commit falsifies ... Caught in re-review. The record shipped in 0ad02db6 said 'no backfill was done here'; 8e1e15e4 then backfilled three ci.* records on the same branch. Corrected now while the record is still NEW at HEAD, so the body-diff guard does not apply and no edit token is needed -- after merge this would be surviving rationale prose and the same fix would cost one. Deliberately avoiding the literal bracketed token string here: decisions_validate matches it as a bare substring over the whole commit range, so a message merely DESCRIBING the mechanism arms it and suppresses the rationale-rewrite guard for the entire PR. Found exactly that way on the first draft of this commit.	2026-07-25 15:30:43 +02:00
timothy	8e1e15e4d7	docs(603): backfill stale-after + Sources onto the three ci.* outside-world records ... The mechanism shipped unused. First adopters are the records that encode measured host behavior, which is what drifts silently: - ci.runner-placement 2027-01-15 — bumblebee sizing (25 GiB/12 cores) + container caps. Premise has already partly moved: the media transcoders left for jazz on 2026-07-20 and the runners were retuned since. - ci.infra-shaped-red-under-load 2027-02-15 — a triage heuristic calibrated against load 243 on a host that has since been retuned. - ci.peak-anon-measurement 2027-03-15 — weakest of the three (mostly our own script) but it does assert that memory.peak is cache-inflated, a cgroup fact. Dates staggered so they don't all come due in the same week. Each Sources line cites the incident evidence already named in the record's own prose.	2026-07-25 15:22:50 +02:00