[DO NOT MERGE] Assets: integration branch for #16358 + #16393

[synap5e-bot]

What this is

Do not merge. This branch is #16358 and #16393 combined on top of current master, so the two can be exercised together before either one lands. It contains no work of its own beyond the conflict resolution described below, and nothing should be based on it — it goes stale the moment either PR head moves.

Contents: #16358 at 9d71c9b7 + #16393 at ca8aba7a + master at b0f4b7b2.

Why it exists

Both PRs are individually mergeable against master, so the conflict only appears in the combination: 15 files, 5 source and 10 test. They overlap because they change the same functions for different reasons — #16358 changes where work happens relative to the SQLite write lock (stats and hashes hoisted out, chunked bounded transactions), and #16393 changes what happens when that work fails. In seed_asset_specs and tick_watch_list those are two answers to the same question, so the combination needs a decision rather than a merge strategy.

How the conflicts were resolved

The principle throughout: #16358's transaction discipline and #16393's failure-path hardening both survive. Three places were genuine design interactions rather than textual conflicts.

seed_asset_specs — opposed failure policies. #16358 wraps the spec loop so any fault rolls the whole batch back, discards orphaned content and re-raises; #16393 replaced that with per-spec absorption into a returned first_error so one bad file cannot discard the batch. Resolved by layering: each spec keeps its own savepoint, faults are absorbed and returned, and the outer compensation is kept for what still escapes.

insert_asset_specs — a problem neither PR has alone. #16358 moved the commit into run_write_txn, which retries its callback on lock contention; #16393 needs the batch fault available after a failed commit. The fault is therefore captured out of band and reassigned on every attempt, so one recorded by an attempt whose work was discarded cannot outlive it.

tick_watch_list — drop the entry or keep it. The two PRs carry directly contradictory assertions here. Resolved by separating an absorbed fault (bad stat, bad spec, returned seed error → drop the entry with telemetry, which is what non-wedging was about) from an exception escaping the write transaction (→ put the entry back, because a failed write is the database's fault and not the file's).

Two smaller notes: mark_missing_outside_prefixes_safely's two return contracts turned out compatible rather than conflicting — None only when the prune failed having committed nothing, a partial count otherwise — and both PRs' existing tests pass unchanged. And #16393's guard for a file vanishing during the recovery hash is structurally absorbed by #16358 hashing before the transaction opens.

What a reviewer should look at

Three things are unresolved by design, and the trade is stated rather than defended.

1. Under the layering above, the only fault that still escapes seed_asset_specs is MemoryError, so for the ordinary case the orphan compensation existed for — create_record failing after create_content inserted — that compensation no longer runs and savepoint rollback is the whole guarantee. That holds on the WAL writer engine this combination gets from fix(assets): prevent SQLite concurrent writer failures #16358. _discard_unreferenced_content's own docstring says it does not hold on a no-WAL engine and that the binding is not knowable from the call site, which makes this a question about fix(assets): harden the asset catalogue's failure paths #16393 standing alone rather than about the combination. Closing it properly means per-spec compensation, which neither PR writes, so it was flagged rather than invented here.
2. The tick_watch_list split above is a judgment call. It was resolved toward fix(assets): harden the asset catalogue's failure paths #16393, and fix(assets): prevent SQLite concurrent writer failures #16358's queue-drain characterization test — which documented the pre-hardening behaviour of an unreadable watched file propagating out of the tick — was rewritten to document the new behaviour rather than deleted.
3. Four changed signatures or return types rippled into call sites outside the conflict hunks, where the merge silently kept whichever spelling it picked. Those are reconciled in a separate commit (1adfd4b5) so they read apart from the resolution itself.

Tests

python -m pytest tests-unit/assets_test tests-unit/seeder_test tests-unit/app_test tests-unit/test_assets_event_log_static.py tests-unit/feature_flags_test.py -q \
  --ignore=tests-unit/app_test/model_manager_test.py \
  --ignore=tests-unit/app_test/test_db_init_locking.py \
  --ignore=tests-unit/app_test/test_prompt_worker.py \
  --deselect tests-unit/assets_test/test_event_log_sites.py::test_noassets_emits_no_enabled_event

863 passed, 72 skipped, 1 deselected, 91 warnings in 45.67s

ruff check is clean on app/assets, app/database and the touched test trees.

The four excluded targets are a gap in local verification, not a claim about the branch: they fail identically on master in the same environment, which is what pins it on the environment rather than on this merge. CI on this PR is the real check for them.

🤖 Generated with Claude Code