S122 — music rescan: skip unchanged files (mtime/size), bound read concurrency at 4, fix the dead getID3 tag path

[detain]

S122 — music rescan: 6.1 h → minutes

Stops re-reading ~54,585 unchanged files over sshfs on every music rescan. Baseline, from
the vault diagnostic and the first-ever completed scan: 2.78 files/s = 360 ms/file =
6.1 h for 61,135 files, with items_added = 0 / items_updated = 6,550 — 89 % of the
work was for nothing.

(a) mtime/size skip — the biggest lever

New MusicScanSkipIndex loads the library's (path → mtime, size) map in ONE query
and the walk skips a matching file BEFORE probeMetadata().

Predicate: skip iff mtime AND size both match what was recorded the last time
the tags were actually read. Stated failure mode: an in-place edit that preserves BOTH
(a padding tag editor plus touch -r, cp --preserve=timestamps) is missed until
something touches the file. That is asserted as a known trade-off, not left implied, and
the escape hatch is operator-reachable without a code change:

UPDATE media_items
   SET metadata_json = JSON_REMOVE(metadata_json, '$.file_mtime', '$.file_size')
 WHERE library_id = '…' AND type = 'track';

No migration. The identity goes in media_items.metadata_json as
file_mtime/file_size — the convention AudioScanner:218-219 and
MusicLibraryManager:334-335 already use for exactly this datum — so media_items is not
ALTERed and the 13-member type ENUM is not touched. Measured 90.9 bytes/entry, 5.30 MB
for the real 61,135-file library (16x cheaper per entry than the whole-tree map S95
removed).

Two one-query-per-scan gates keep it from regressing S96(e): the fast path is only
available when nothing is unhealed and no orphan is adoptable, and $mayAdopt is
re-read per file so a caught mid-walk failure disables it for the remainder. The
identity map JOINs music_tracks, so a media_items row with no track row is never
skippable — without that, a lost file would be permanently lost instead of retried.

⚠ The first rescan after this ships is still slow. Every pre-S122 row carries no
stamp, so the first pass reads everything (as today) and stamps as it goes; the second is
the fast one. There is deliberately no filesystem backfill migration — recording
today's mtime for a row whose indexed tags might predate a change would manufacture
exactly the silent miss the predicate is designed to avoid.

(b) Read concurrency 1 → ≤ 4

New MusicScanPrefetcher runs up to 3 reader processes LOOKAHEAD files ahead of the
walk, warming the page cache for files it is about to probe. Hard-capped at 4:

threads	ms/file	files/s	speedup
1	117.0	8.5	1.00x
4	67.7	14.8	1.73x
8	197.8	5.1	0.59x — worse than serial
16	115.9	8.6	1.01x

The figures are cited in the class, in config/scanner.php, and asserted by a test, so
raising the cap means deleting a falsifiable claim rather than editing an integer. There is
no in-process alternative on this runtime: SWOOLE_HOOK_FILE is excluded from
SwooleRuntime::SAFE_HOOK_NAMES (it crashed workers), there is no ext-parallel, and
config/process.php count must stay 1 for LibraryScanWorker's reaper invariant.
The pool is a page-cache warmer only — it cannot influence what is indexed — and
submit() drops work rather than ever blocking (pinned with a FIFO-wedged reader).
scanner.music_read_concurrency = 1 disables it entirely.

(c) Tag reads — this one is a BUG, not a tuning result

getID3::analyze() never populates $info['comments']. That merged view is built only
by getid3_lib::CopyTagsToComments(), which no getID3 module and no part of analyze()
ever calls. So probeViaGetId3() returned NULL for every file ever scanned and
100 % of the library fell through to ffprobe — after paying getID3's full read first.

probe	ms/file	read syscalls
ffprobe (what actually ran)	114.9	232 (strace -c)
tuned getID3 (what was meant to run)	2.2	—

52x, ≈1.95 h of pure ffprobe over the production library on warm local disk, before any
sshfs latency. Fixed by calling CopyTagsToComments(). Plus measured option tuning:
−69 % bytes, −66 % reads, −68 % seeks on an encoder-written fixture, and
1,891,240 → 33,256 bytes retained per analyse.

⚠ CORRECTION to the vault diagnostic, measured: option_save_attachments = false does
NOT reduce bytes read. module.tag.id3v2.php:139 reads the whole ID3v2 frame region
contiguously before any frame is inspected; the option is consulted afterwards, at
:1448, only to decide whether to KEEP the picture — 598,585 bytes either way, to the
byte. It buys memory, not I/O. Removing that read needs a vendor/ patch, which is
forbidden; the bytes lever for it is (a): an unchanged file is not opened at all.

Anti-tautology: measured, not claimed

12 of 50 new unit tests fail against master (762c2c99) with only
MusicLibraryScanner.php + config/scanner.php reverted. The other 33 specify NEW
classes for which there is no prior, so each load-bearing claim is pinned by mutation
instead — 11 of 11 mutations RED, 11 of 11 RESTORED_OK:

mutation	verdict
drop JOIN music_tracks from load()	RED
compare only mtime in isUnchanged()	RED
drop min() from clampReaders()	RED
hasUnhealedMusicMediaItem() → false	RED
canSkip() ignores $mayAdopt	RED
load() also loads for a NULL library id	RED
skip moved AFTER probeMetadata()	RED
no stamp on the skipped branch	RED
CopyTagsToComments() removed	RED
skip does not increment scanned	RED
drop JOIN (lost-file retry)	RED

Two tests initially proved nothing and were fixed, not explained away (both fixture
defects, both found by mutation):

• the heal test nulled music_artists.media_item_id but LEFT the media_items row, making
  it an orphan — already caught by the other gate, so the heal gate was never under
  test;
• the mid-walk-flip test used a small fixture, and a skipped file never opens an album,
  so the 32-album window could not overflow and $mayAdopt could not flip at all. Now 40
  albums with the first 33 touched: 40 probes with the per-file read, 33 without it.

A third defect was in the double: keying its branch on JOIN music_tracks meant deleting
that join from production SQL made the statement fall through and return nothing, where
real MySQL returns more rows. The branch is now recognised by the columns it asks for
and the join filter applied only when the statement contains it.

Real-DB tests for what a double cannot express

MusicScanSkipUnchangedIntegrationTest (5 tests, 58 assertions) drives the real getID3
over a committed MP3 fixture against real MySQL 8.0, with 3 more mutations RED:

1. JSON_SET(COALESCE(metadata_json, JSON_OBJECT()), …) — JSON_SET(NULL, …) returns
   NULL, so without the COALESCE the stamp UPDATE erases the document (sub_type
   and name with it). Mutation R1 RED.
2. metadata_json->>'$.file_mtime' against a real JSON column + the JOIN. R2 RED.
3. The heal gate's SELECT … UNION ALL … LIMIT 1 must parse and be classified as a
   select by Connection::query()'s leading-keyword dispatch. Reformatting it so the
   keyword is missed (the workerman/mysql landmine) is R3 RED.

Verification

• PHPUnit Tests: 7596 → 7652 (+56), all pass, run against real MySQL 8.0 with all 95
  migrations applied. Only Tests: is quoted — Assertions:/Skipped: drift.
• PHPStan L9 src/: [OK] No errors (exit 0).
• phpcs --standard=PSR12 -n src/: exit 0, zero output.
• phpcs --standard=PSR12 src/Server/: exit 0. composer validate: valid.
• Coverage: MusicScanSkipIndex 69/69 = 100 %, MusicScanPrefetcher 84/91 =
  92.3 %, MusicLibraryScanner 636/676 = 94.1 %.
• No scan was started anywhere — every behaviour is proved by fixtures and tests.
• Infra levers (mount options, sysctls, phlix-vault-tune.service) untouched: out of scope
  and already applied on prod.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[codacy-production]

Not up to standards ⛔

🔴 Issues 46 medium · 54 minor

Alerts:
⚠ 100 issues (≤ 0 issues of at least minor severity)

Results:
100 new issues

Category	Results
UnusedCode	2 medium
BestPractice	21 medium
Documentation	7 minor
ErrorProne	5 medium
CodeStyle	27 minor
Complexity	18 medium
Comprehensibility	20 minor

View in Codacy

🟢 Metrics 506 complexity · 0 duplication

Metric	Results
Complexity	506
Duplication	0

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}

[codecov]

Codecov Report

❌ Patch coverage is 93.85965% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.48%. Comparing base (762c2c9) to head (039bfd1).

Files with missing lines	Patch %	Lines
src/Media/Music/MusicLibraryScanner.php	91.25%	14 Missing ⚠️
src/Media/Music/MusicScanPrefetcher.php	93.20%	7 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master     #569      +/-   ##
============================================
+ Coverage     65.34%   65.48%   +0.13%     
- Complexity    20957    21061     +104     
============================================
  Files           656      658       +2     
  Lines         66023    66311     +288     
============================================
+ Hits          43142    43421     +279     
- Misses        22881    22890       +9     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.