f5edd49b6b79f025eb78c112975002d59aa72d03
5
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  dtourolleandClaude Opus 5
|
ab0ef6a26d |
Say which requirements the code was already satisfying
Thirteen requirements were surveyed as built but untagged. Eight of them were: R3, R6, FR-DEV-1, FR-UI-6, FR-NC-6d, NFR-OPS-3, NFR-PORT-2 and NFR-SEC-3. Each was read against its full text in requirements.md and against the code before the tag was added, because a tag that is wrong is worse than an absent one — it turns a visible gap into an invisible one. The five that were refused, and why, because the reasoning is the part worth keeping: R2 carries "(figure TBD)" in its own acceptance criterion and asks for a stated prefetch margin and cache-hit rate; neither figure exists anywhere in the tree and neither quantity is measured, while TD-2 and TD-3 both describe the thumbnail path falling short of it. R5 asks for three things and the code does one. The display pipeline does run at viewport resolution, but "only visible tiles are computed" and "panning recomputes only newly exposed tiles" need a tile scheduler that does not exist — and frame_budget.rs currently argues for striking tiled computation from the interactive path rather than building it. FR-RAW-2 asks for a trait taking a SourceRef, so that a second decoder can be added without changing callers. What exists is free functions over &[u8]. That meets the requirement's stated *purpose* — the same decoder serves a local file, a SAF document and a byte range, which is exactly why it takes bytes — but there is no trait and no second implementation seam, so the requirement should probably be amended rather than tagged. NFR-ARCH-1 asks for named executors with stated thread counts. architecture.md §7.1 states the table; nothing implements it. Workers are twenty-odd ad-hoc std::thread::spawn sites, each building its own one-worker tokio runtime, with no decode pool, no GPU-submit executor and no I/O pool. The requirement's own text says R4 and NFR-P9 "assert an outcome with no stated means", and that is still true. NFR-SEC-4 is satisfied by absence — there is no telemetry — and absence has no module to tag. A tag would point at nothing. NFR-OPS-3 was the closest call of the eight taken. The store is single, separate from the catalog, survives a catalog rebuild and does not sync between devices; it has no version *field*, deliberately, and settings.rs argues why and names the condition that would need one. The substance is met and the reasoning is recorded where it belongs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
dtourolle
|
4168d67cfa |
Do not push a catalog over one we could not read
`sync_catalog` is a read-modify-write over a file another device also
writes: take theirs, merge, push the union. It was shaped
if let Ok(bytes) = backend.get(&RemoteId::Path(target), None).await {
which folds *every* failure into "there is no remote catalog" and carries
straight on to the upload. On a placeholder library the snapshot in
`.darkroom-derived/` is dehydrated like anything else, so the read failed
every time and each sync pushed our catalog over theirs unmerged —
taking the other device's collections and their members with it.
The same shape as the sidecar bug, and the same fix: a read that fails
for anything other than `NotFound` stops the upload and says why. An
unreadable or unopenable snapshot stops it too — "will not parse" is not
"is not there". This is what `NotFound` and `NotMaterialised` being
separate errors is *for*: one means ours is the whole truth, the other
means do not dare.
Shard downloads go through the same fetch-on-demand read. They logged
and skipped before, which on a library the client keeps dehydrated is
every shard, every pass, and a peer's thumbnails and faces silently
never arriving.
And `put` over a placeholder now replaces it rather than refusing.
Refusing was over-cautious of me: derived state lives inside the library
folder, so a folder the client had dehydrated could never be written to
again. An unconditional write replaces the whole file, so there is
nothing in the stub to keep — content first, then the placeholder, since
in a synced tree an absence is a deletion that propagates. `IfMatch`
still refuses, because a stub's validator describes the stub; `IfAbsent`
fails, because the file is there and only its content is not.
|
||
|
dtourolle
|
5768100816 |
Borrow the library to index it, and give it back
The passes that need every photograph's bytes — thumbnails, face indexing — now borrow each one and release it at the end. On a placeholder library that is the difference between peak disk being the working set and being the whole library. Including on cancellation, which was nearly missed: the face sweep returns mid-loop when the user presses Stop, and without releasing there the disk is spent and nothing is delivered for it. `materialise` now answers whether *it* fetched the content. The pool used to work that out by listing a file's parent directory — one listing per file across a library — when the backend already had to `stat` it to decide whether to ask. One syscall instead of a directory walk, and it removes the bug class the tests found earlier: a file at the library root has no `parent()`, so every one of them read as already-downloaded. **Pinning is the retention control**, and it drives the model the catalog already had rather than a second one. `tier_desired` is what the user asked to keep hydrated, `pending_pins` is the resumable work list, and a pinned collection is never dehydrated for the same reason it was never evicted. It was in fact *broken* here before: `get` on a stub failed, and the pin worker logged "one unreadable file must not abandon the whole pin" and silently did nothing. Pinned originals on such a library are recorded with `path = NULL` (`Cache::record_in_place`) rather than copied under `originals/`. Two reasons, and the second is the important one. A copy would hold every pinned photograph twice, with the budget able to evict the half that was not costing the disk. And `release` deletes the file a row names — so a row that names none cannot delete anything, which puts the one catastrophic operation out of reach by construction rather than by remembering not to call it. Deleting a materialised file inside a synced tree removes the photograph from the server and every other device. Handing disk back is `spawn_dehydrate`, which asks the client. Two gaps written down rather than papered over (docs/storage.md §7): a hydrating pass cannot yet quote its cost, because a stub reports no size; and the two sweeps hold separate pools, so a library indexed for both fetches twice. |
||
|
dtourolle
|
c102ba9df2 |
Treat a placeholder as the photograph, not as a one-byte file
The folder connector was pointed at a Nextcloud VFS tree and got three things wrong, the first of which loses work. **A dehydrated sidecar read as absent.** `a.drsc` does not exist when the client has dehydrated it — only `a.drsc.nextcloud` does — so `get` missed, `.ok()` swallowed the `NotFound`, and the sidecar writer took that for "there is no sidecar yet" and wrote a fresh document over the existing one. Every edit another device had put there went with it. That function's own doc comment calls this the exact loss the format's unknown-key preservation exists to prevent. **A stub was catalogued as a 1-byte image**, and ARCH §9.0 measured this machine at 121,785 placeholders against 10,267 real files — so a folder library on a synced tree was ~92% broken rows. **Identity changed on hydration**, so downloading a photograph looked like a delete and an add, orphaning its thumbnail and its face rows. Entries now carry the photograph's own name and a `materialised` flag; `get` on a stub returns the new `RemoteError::NotMaterialised`, which is distinct from `NotFound` precisely because the sidecar writer must treat them differently — it fetches the sidecar and merges, or leaves the entry queued. Hydration is a **borrow**. `BorrowPool` records what was on disk before it asked, so `release_all` dehydrates only what a pass brought and leaves what the user already had. Reference counted: the thumbnail pass and the face pass meet on the same RAW, and without counting the first to finish dehydrates the file the second is reading. A borrow against a plain folder or a server does nothing, so a pass written for VFS runs everywhere. Releasing means asking the client to dehydrate and never deleting: a deletion inside a synced tree propagates to the server and removes the photograph from every device. Not a second backend — the capability is per *connection*, not per type, since the same folder hydrates only while the client runs. The convention arrives through a detector the registry supplies, so `dr-sync-folder` still knows nothing about any client's protocol. ARCH §9.0a records this as an amendment: finding 3 rejected hydration because it costs 100× a range read, and that comparison assumed a connector was available. A folder library has none. |
||
|
dtourolle
|
f12aece07e |
Make storage pluggable, and prove it with a folder backend
`RemoteBackend` existed from the first release and bought nothing it was
designed for. Seven files in `dr-ui` constructed a `NextcloudBackend`
directly, an account *was* a server URL beside a DAV user id, the local
cache directory was named after a hostname, and the launch screen knew
that signing in meant a browser handshake. The trait was real; the seam
was documentation.
A trait over operations is only a quarter of it. Pluggable storage needs
four things, and this adds the other three:
- **Capabilities** — already there, and the reason the engine can drive
two backends at the speed each actually runs at.
- **Configuration** — `dr_sync::Account`: where a library lives, in
whatever form its connector addresses, with no server in it. Loads
every existing config unchanged (`backend` defaults to `nextcloud`,
`endpoint` is stored under its historical `server` key), and
`Account::namespace()` reproduces the old catalog directory byte for
byte, because changing it would abandon a catalog, its thumbnail
shards, and the sidecars holding unsynced offline work.
- **Registration** — `BackendProvider` and `BackendRegistry`.
`ui/dr-ui/src/remote.rs` is now the only file above `dr-sync` that
names a connector.
`Connection` (an account plus an optional `Secret`) replaces the
credentials-and-user-id pair that was threaded through fifteen
signatures in an order that could be swapped. `Secret`'s inner string is
reachable only through `expose()` and its `Debug` prints `Secret(***)`,
so the indirect leak — a `{:?}` on anything holding one — no longer
compiles into a leak.
Nextcloud is unchanged and keeps every peculiarity: propagating ETags,
chunked upload v2, `oc:fileid`, the `oc:permissions` probe on a refused
PUT, the 423 retry classification, Login Flow v2. Those are what the
capability model exists to serve, not something to hide.
`dr-sync-folder` is the second connector: a local disk, a network mount,
an external drive, or a folder a Nextcloud client already syncs. No
account, no credential — the route that works where no secrets daemon
does. It declares `LocalEtags` rather than claiming propagation a POSIX
directory cannot provide, which costs nothing because 50k `stat` calls
are not 50k PROPFINDs. Identity is a path hash, not an inode: an inode
survives a rename but differs between devices and is reused after a
delete, so two machines would disagree about which photograph a
thumbnail belonged to. Re-deriving a thumbnail is a cost; showing the
wrong one is a bug.
docs/storage.md is the contract — the traits, the four steps to add a
backend, and what each connector declares. ARCH §8.0 and §8.4a, and
FR-NC-13, say why.
|