Commit Graph

214 Commits

Author SHA1 Message Date
Simon Pinfold
0b28b50ff2 refactor(assets): remove file_path from public asset responses
The top-level `file_path` served the global-namespace-root path
("models/checkpoints/flux.safetensors") to every API client, duplicating what
`display_name` and `loader_path` already carry in the forms clients actually
consume. Nothing in the response contract needs the namespace-rooted form, so
it is surface no caller has to be given.

Dropped from schemas_out.Asset and from both response builders, and from the
Asset and AssetUpdated schemas in openapi.yaml. AssetCreated inherits Asset, so
list, detail, create/upload, from-hash and update all lose it together.

Every internal path stays: ReferenceData.file_path, AssetContent.path, the
preview URL computation, and system_metadata["file_path"] are untouched, as is
the persisted loader_path this file's tests guard.

Kept as a standalone top commit so the policy can be reverted on its own.
2026-08-27 20:48:55 -07:00
Simon Pinfold
18b6fd53aa test(assets): move M3 mismatch docstrings onto their assertion messages
Comment-gate walk (M3): the docstrings on the two mismatch tests stated what
each test is evidence for. Under the gate's test rubric there is no keep for
tests — that claim belongs on the assertion it backs, where it surfaces on a
red build instead of only to someone already reading the source.

The metadata-swap test's docstring also claimed the row "stays a candidate
for the next enrichment pass"; dropped rather than moved, since this test
never exercises a retry and so is not evidence it provides.
2026-08-27 18:57:17 -07:00
Simon Pinfold
04e5796517 chore: comment cleanup
Comment-Gate: 3 quarantined
2026-08-27 18:56:52 -07:00
Simon Pinfold
39dafd63ff fix(assets): scope /upload/image dedup to the written path
register_file_in_place writes bytes at the locator BEFORE registering, then ran a
global lookup_for_upload_dedup. Both match branches could point at a different file
that merely shares bytes: the Asset branch returned that file's record, the
AssetContent branch created a record against that file's path. Either way the
just-written locator stayed untracked and the caller got back an asset describing
someone else's path.

Ruling (2026-08-27): /upload/image accepts weaker content-dedup than the multipart
endpoint. Drop the global lookup and fall through to create_content unconditionally.
_reconcile_live_content_at_path already leaves at most one live row at the locator,
so unchanged bytes reuse that content row and changed bytes retire it; a distinct
path holding equal bytes now gets its own content row and record, and the hash still
finds both. Re-registering an unchanged file writes a new delivery record against the
reused content row, which is what a repeat save through /upload/image is.

upload_from_temp_path keeps its global dedup-before-write unchanged.
2026-08-27 16:54:14 -07:00
Simon Pinfold
4a7d6c012a fix(assets): refresh file facts when the off→on drain confirms an unchanged hash
drain_transition_queue branched on null-hash and changed-hash with no else, so the
success case — a recomputed digest equal to the stored one, which is proof the bytes are
unchanged — fell through and left size_bytes/mtime_ns untouched. A row whose stored stat
was stale then failed lookup._stat_consistent and stayed unservable by hash until the next
full scan, even though the drain had just verified it.

Adopt the observed stat on the equal-hash arm, matching what drain_pending_verifications
already does on its matching-hash path.
2026-08-27 14:56:40 -07:00
Simon Pinfold
ae65f1ac08 fix(assets): pair enrichment metadata and hash to one verified file observation
snapshot_hash returns the stat it verified brackets the bytes it read, and
enrich_asset threw it away. Metadata came from the stat taken on entry, the
digest from whatever the file was during the read, and nothing checked the two
described the same file. A writer landing between them welded old metadata to a
new-bytes hash — permanently, because a row with non-NULL system_metadata is
never an enrichment candidate again.

Keep the verified stat and reject the whole result when it disagrees with the
metadata's observation. The row stays NULL on both fields, so the next pass
retries it once change detection reconciles the stored stat.
2026-08-27 14:24:47 -07:00
Simon Pinfold
21382fbcad test(db): fail the lock-release test with what a stranded lock costs
setup_database logs and continues when assets are disabled, so a failed init
that keeps the lock blocks every other instance for its whole lifetime. The
assertion message says so; the acquire it replaces said nothing.
2026-08-26 20:12:04 -07:00
Simon Pinfold
1d5cc1e36e test(assets): fail the out-of-root drain test with its real consequence
An escaped ValueError reaches setup_database, which sys.exit(1)s the app when
--enable-assets is set. Put that in the failure output rather than a docstring
nobody reads when the build goes red.
2026-08-26 20:12:04 -07:00
Simon Pinfold
94ff05ce9e chore: comment cleanup
Comment-Gate: 454 quarantined
2026-08-26 20:12:04 -07:00
Simon Pinfold
002c1d7ae8 docs(assets): ratify ruling-6 — upload dedup runs in both hashing modes
The prior wording ('only hash mode permits upload deduplication') never
described the code. upload_from_temp_path hashes unconditionally and
lookup_for_upload_dedup never consults mode.hashing_enabled (unlike
lookup_for_from_hash, which does) — the deliberate product decision from
4bced38a. Only scanner/output hashing is gated by --enable-asset-hashing.

Ruling text updated to match; no behaviour change.
2026-08-26 19:14:34 -07:00
Simon Pinfold
29410e4eb7 test(assets): drive real workflows in the last primitive-only behaviour tests (round-3) 2026-08-26 18:27:32 -07:00
Simon Pinfold
5aa47063d5 fix(assets): stop treating size equality as byte equality in path reconciliation (round-3)
_reconcile_live_content_at_path adopted a freshly-computed hash onto an
existing hash=None row whenever the recorded size_bytes matched the file.
Equal size is not equal bytes: via /upload/image (which writes the
replacement BEFORE calling register_file_in_place) a same-sized overwrite
left the original record and content live while silently adopting the NEW
digest, so records describing the old bytes claimed the new ones.

Rather than guess, take the signal from the caller that has it. server.py
already computes image_is_duplicate - true exactly when compare_image_hash
proved the bytes were already on disk and the write was skipped - so
register_file_in_place now takes content_written and reconciles on fact:
written -> retire, not written -> adopt as before. The no-write path is
untouched, so genuinely unchanged unhashed files are still never retired.

upload_from_temp_path has no caller signal, but it has something better:
the incumbent bytes still exist when it runs. It now settles the
destination row against that file's OWN hash before both the dedup lookup
and the move, so the post-move reconciliation always compares known
hashes and never has to read size as identity.

Second defect: a matching hash returned without refreshing the stored
stat, so a same-bytes mtime-only re-registration left the row failing
lookup._stat_consistent - unservable by hash, and routed into creating a
DUPLICATE record. A matching hash IS proof of byte equality, so the
observed stat is now written back.

Tests: the existing unhashed-changed-file test used different-SIZED bytes
and passed under the old buggy logic too; it now uses same-sized bytes so
it actually discriminates. Adds stale-stat and same-size-different-bytes
coverage for both register_file_in_place and upload_from_temp_path.
2026-08-26 18:05:50 -07:00
Simon Pinfold
d72c3feb77 fix(assets): drop the unverifiable hash when accepting a same-size mtime change (round-3) 2026-08-26 17:55:17 -07:00
Simon Pinfold
de1f1870d3 fix(assets): normalize stored content paths; release the DB lock on failed init (round-3) 2026-08-26 17:53:06 -07:00
Simon Pinfold
c7ca8ca5d0 fix(db): hold the file lock across migrations, diverging from master's incorrect rationale 2026-08-26 17:27:39 -07:00
Simon Pinfold
21737709ba test(assets): drive real workflows in intended-behaviour tests (review-10 round 2) 2026-08-26 14:45:08 -07:00
Simon Pinfold
051154c0fc fix(assets): do not retire unhashed content on re-registration (regression from ef92de23) 2026-08-26 13:52:09 -07:00
Simon Pinfold
9ff3e18cbd fix(assets): make SQL path-prefix predicates case-sensitive and component-bounded (regression from 42463e99) 2026-08-26 13:44:57 -07:00
Simon Pinfold
10ab7a0907 fix(assets): refresh stored stat when a same-size mtime change is accepted (regression from 435dd323) 2026-08-26 13:44:38 -07:00
Simon Pinfold
5cfd1a7bd6 fix(assets): restore zero-record fallback in resolve_hash_to_path (regression from 632c5a49) 2026-08-26 13:42:02 -07:00
Simon Pinfold
dfe329c1b7 test(execution): pin S10.5 cache-purity across async re-entry 2026-08-26 12:50:11 -07:00
Simon Pinfold
e37f65a6a8 refactor(assets): sweep dead code and dead parameters; expose protected tag bucket (review2-18) 2026-08-26 12:36:09 -07:00
Simon Pinfold
736e426eb9 test(assets): make intended-behaviour tests falsifiable (review-10) 2026-08-26 12:13:20 -07:00
Simon Pinfold
42463e9938 perf(assets): push enrichment and temp-wipe filtering into SQL; skip empty-batch scan sleep (review-8, review-12) 2026-08-26 12:03:56 -07:00
Simon Pinfold
dd8cb74bf3 fix(assets): narrow create_content integrity handling; report protected tags honestly (review-12, review2-18) 2026-08-26 12:02:27 -07:00
Simon Pinfold
62641f9d12 fix(assets): clean orphan content on failed ingest (review-11) 2026-08-26 11:46:00 -07:00
Simon Pinfold
632c5a490f fix(assets): serve /view filename and Content-Type from the same record (review2-18) 2026-08-26 11:44:21 -07:00
Simon Pinfold
0ff3a47e14 fix(assets): never cascade record deletion into other assets; 4xx on unknown preview_id (review2-4) 2026-08-26 11:31:57 -07:00
Simon Pinfold
d849b8544b fix(assets): repair 0007 downgrade indexes and guard the OFF→ON drain path classification (review2-15, review2-16) 2026-08-26 11:31:34 -07:00
Simon Pinfold
435dd3233d fix(assets): unified split policy — mtime+size identity, NULL-metadata replacements, re-enrichable splits (review2-7, review2-12) 2026-08-26 11:14:50 -07:00
Simon Pinfold
6d55a5edae fix(assets): list missing records consistently across catalog surfaces (review-1, review2-11) 2026-08-26 11:10:35 -07:00
Simon Pinfold
bb5d8e9e64 fix(assets): store system_metadata None as SQL NULL so enrichment matches (review2-1) 2026-08-26 09:36:12 -07:00
Simon Pinfold
bb4dd0eb47 fix(assets): restore display_name and file_path as served fields; unify error envelopes (review-5, review2-8) 2026-08-26 09:29:50 -07:00
Simon Pinfold
ef92de23af fix(assets): retire the live content row before re-registering a path in place (review2-3) 2026-08-26 09:29:13 -07:00
Simon Pinfold
89e62fcee4 fix: satisfy whole-tree ruff; resolve owner_id leftover deliberately (review2-10)
CI runs bare `ruff check .` (whole tree); the earlier local gate only checked
changed files, so 10 pre-existing errors reached HEAD:

- 9x E701/E702 in tests-unit/assets_test/test_intended_behaviour.py: semicolon/
  colon-chained statements (scenarios 7, 11, 17, 27) split onto separate lines.
  Logic untouched; de-tautologising is deferred to a later todo.
- 1x F841 unused `owner_id` in server.py's /view blake3 branch.

/view unknown-user decision (F841): `get_request_user_id` returns "default" in
single-user mode but RAISES KeyError for unknown users and system-prefixed users
in multi-user mode. `resolve_hash_to_path(filename)` never consumed `owner_id`,
so the value is dead -- but the call is a load-bearing access gate: it rejects
unknown/system users before a blake3 hash is resolved to an on-disk path.
Removing the line entirely would silently weaken multi-user access control on
/view?filename=blake3:... . Decision: keep the call, drop the binding (bare call,
return discarded); a comment documents the deliberate side-effect-only call to
prevent a future "dead call" cleanup regressing the gate.

Verified: `uvx ruff check .` clean (whole tree); assets_test 378 passed, 1 skipped.
2026-08-26 09:25:27 -07:00
Simon Pinfold
32f7237693 fix(assets): align openapi.yaml with served contract and drop dead response kwargs (review-5) 2026-08-26 08:13:51 -07:00
Simon Pinfold
473337dbed fix(assets): pair verified snapshot stat with its digest and stop hash-mode poisoning (review-2, review-7)
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
2026-08-26 08:11:37 -07:00
Simon Pinfold
3850c92baa fix(assets): make temp cleanup robust when DB is unavailable; keep startup ordering (review-4) 2026-08-26 08:08:44 -07:00
Simon Pinfold
9b7b446d41 fix(assets): remove unused variable in test_lookup.py (ruff F841) 2026-08-26 04:47:04 -07:00
Simon Pinfold
e7cd965656 refactor(assets): hoist mid-file imports to module scope in tests (D21)
Re-measured inventory: 56 nested imports across 14 test files. All hoisted
to module scope. Empirically tested every import folder_paths hoist — none
caused regression. No folder_paths exceptions needed.

Side effect: hoisting contextmanager + SASession out of the
mock_create_session fixture body in services/conftest.py fixed 3 pre-existing
test failures (monkeypatch was targeting stale module references when the
imports were nested inside the fixture).

Ruff cleanup: removed duplicate import tempfile (services/conftest.py),
unused sqlalchemy.select (test_delete_b.py), unused os (test_lifecycle.py)
— all were pre-existing issues hidden by the nested-import pattern.

Zero assertion changes — only import lines moved.

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
2026-08-26 04:10:54 -07:00
Simon Pinfold
ee55797677 refactor(assets): hoist mid-file imports to module scope in product code (D21)
Re-derived inventory: 47 branch-introduced nested import statements across 10
product files (plan said 48/11; delta: asset_enrichment.py deleted by D6 removed
2, D7 added 1). Two kept nested to break introduced cycles:

- lifecycle.py:start_asset_seeder keeps 'from app.assets.seeder import asset_seeder'
  (breaks lifecycle -> seeder -> scanner -> lifecycle)
- scanner_admission.py:tick_watch_list keeps 'from app.assets.scanner import ...'
  (breaks scanner -> scanner_admission -> scanner)

All other 45 statements hoisted. Three test patch paths updated to target the
importing module (app.assets.services.ingest.*) instead of the defining module.

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
2026-08-26 04:07:56 -07:00
Simon Pinfold
322d152e39 refactor(assets): delete dead pre-split code; relocate SeedAssetSpec (D18)
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
2026-08-26 03:51:23 -07:00
Simon Pinfold
058570c11a fix(assets): store blake3-prefixed hashes; unify every read and comparison (D7) 2026-08-26 03:26:58 -07:00
Simon Pinfold
2947eec3c2 fix(assets): clean temp files at startup when assets are disabled (D9) 2026-08-26 03:07:44 -07:00
Simon Pinfold
33be00c57e refactor(assets): register outputs at emission with an ID-free cache; retire post-hoc classification and its consumers (D6) 2026-08-26 02:50:50 -07:00
Simon Pinfold
8280187d9d refactor(assets): registration primitives — rename, defer output hash, synchronous metadata, no cached fallback (D6, D8, D14a) 2026-08-26 02:17:20 -07:00
Simon Pinfold
b6ee82e945 fix(assets): port tag listing and refine to the B schema (D3, D4) 2026-08-26 02:05:37 -07:00
Simon Pinfold
646690a583 fix(assets): survive files disappearing mid-scan without aborting the batch (D17) 2026-08-26 01:49:32 -07:00
Simon Pinfold
898bb78f7b fix(assets): restore the full GET /api/assets query contract (D2) 2026-08-26 01:49:27 -07:00
Simon Pinfold
5bbe32255f fix(assets): exclude temp content from hash lookups at the iterator (D10) 2026-08-26 01:46:52 -07:00