Commit Graph
14 Commits
Author SHA1 Message Date
dtourolleandClaude Opus 5 bb35665bd2 Let a paste carry some kinds of edit and not others
FR-DEV-6 asks for presets "covering a subset of the edit graph". What
landed with the named presets covered two subsets: everything, and
everything but the crop. "Match the colour but not the sharpening" had
no way to be said.

`Scope` is now a set of `Attribute` — the same six kinds every operation
already declares and the develop panel already builds its tabs from. The
photographer ticking "tone and colour" is naming the groups they
navigate by, and neither this module nor the interface has to name an
operation to do it (FR-DEV-3c).

The pleasing part is what left. Framing used to be excluded by an
explicit test against one operation's id; it is now excluded because
Geometry is not in the default set. The special case dissolved into the
general rule, and the argument for it — a crop is a decision about *this*
photograph, and carrying it across forty destroys forty compositions —
is now a statement about a kind of edit rather than about a node. All
thirty-three existing preset tests pass unchanged, which is the evidence
that the generalisation kept its promises.

One decision that is a field rather than a rule, because the two cases
genuinely differ. An operation this build cannot classify — from a newer
version, arriving over sync — travels under "everything" and "everything
but the crop", because those are claims about the whole edit and an
unrecognised operation is part of it (FR-NC-8). It does not travel under
a hand-picked set, because that is a claim about kinds, and an unknown
kind is not one of the kinds that were ticked.

The settings page's "Copy crop and rotation" checkbox is gone, replaced
by the same chips the preset sheet draws. It asked the right first
question — geometry is the kind whose accidental travel destroys work —
but it was the only question a boolean could ask. The field stays in
`Settings`, read exactly once to seed the new set, so anyone who had
ticked it keeps their behaviour.

The chips are deliberately not in the develop column. Six of them there
would set the width of the whole sidebar, which is the bug `ChipGrid`'s
comment records at length.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-29 20:38:19 +02:00
dtourolleandClaude Opus 5 bd33487054 Say which axis each export dimension is, in the one place that renders
The box modes put two numeric fields one above the other, and on screen
they are two anonymous numbers: `TextRow` draws its label behind the
field rather than above it, so "Width" and "Height" never appear. That
fault is older than this feature — the storage panel's cache sizes have
the same missing labels, and the single "Size value" row always did — and
it belongs in its own change rather than being fixed under cover of this
one.

But one unlabelled number is survivable and two are not, so the axis goes
where the page does render it: the unit. It reads "3840 px wide" above
"2160 px high", which is the sentence the user is trying to write anyway.

The `label` bindings stay correct and stay where they are, so this
becomes redundant rather than wrong the day `TextRow` is fixed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-29 13:49:21 +02:00
dtourolleandClaude Opus 5 3bb68cff69 Export at an exact resolution, and offer the panels worth naming
Export sizing could bound an image but not fix it. Long edge, short edge
and percentage all preserve the aspect ratio by letting one dimension
fall where it may, which is right for most work and useless against a
display that accepts one resolution and rejects everything else — a
television's art mode, a digital frame, a wallpaper slot.

FR-EXP-3 has always listed both halves of the answer, and this adds them.
**Fit box** scales to fit inside a width and height, so nothing is thrown
away and the result is smaller than the box on one axis unless the crop
already matches it. **Fill box** scales to cover the box and cuts the
overhang off the middle, so the file is exactly the pixels asked for.

Fill is the only mode in the file that discards image data, so two things
about it are worth stating. The overhang comes off symmetrically: the
crop tool is where a photographer decides which part of a frame survives,
and this stage having an opinion of its own would fight it. And locking
the crop to the same ratio leaves nothing here to cut, which is the
workflow the two features are meant to be used in.

With upscaling off and a source too small to cover, a fill box keeps its
*shape* rather than falling back to the source's: exporting a 3:2 file
where 16:9 was asked for is silently wrong in exactly the way the mode
exists to prevent, so the box shrinks instead. The existing rule — clamp,
never fail — is otherwise unchanged.

Four panel sizes are offered as buttons beside the fields. Getting 3840 x
2160 by typing four digits twice is a step at which the mistake is
discovered after the upload rather than before it. They fill in the
numbers and nothing else, in particular not the fit/fill choice: both are
legitimate against a screen, and guessing would discard the edges of a
photograph for a user who wanted them. The list is panels rather than
platforms, because a screen has one exact pixel count for ever where
"what a photo site wants" would rot in the file.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-29 13:49:20 +02:00
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.
2026-08-29 09:57:52 +02:00
dtourolleandClaude Opus 5 242374fd0f Let the interface hold a backend without knowing whose it is
`dr-sync` defines `RemoteBackend` and a capability model the engine adapts
to, so a second backend can be added without touching the code that uses
one. That boundary was documentation. Seven files in `dr-ui` constructed a
`NextcloudBackend` directly, ten functions took one by concrete type, and
exactly two call sites in the tree — both inside `dr-sync` itself — ever held
the trait object. A WebDAV or local-folder backend would have had a
well-written trait to implement and nowhere to go afterwards.

The change is smaller than the finding suggests, because the trait was
already right. Every method the UI has ever called on a backend — `get`,
`put`, `list`, `delete`, `create_dir`, `move_to` — was already on it, so
nothing had to be added and no behaviour moved. Ten signatures widened to
`&dyn RemoteBackend`, sixteen constructions became `remote::connect`, and
`remote.rs` is now the only file in the interface that names a connector.

`connect` returns `Result<Box<dyn RemoteBackend>, RemoteError>`. The error
type is `dr-sync`'s rather than the connector's, which is why every call site
kept its shape — the `match`, the `let Ok(..) else`, and
`.map_err(ScanFailure::local)?` all still read as they did.

One wrinkle worth recording: `&Box<dyn Trait>` does not reach `&dyn Trait` on
its own. The compiler reaches for unsizing, which wants
`Box<dyn RemoteBackend>: RemoteBackend`, and reports a confusing missing impl
rather than suggesting a deref. Twelve call sites therefore say `&*backend`,
and two say `let backend: &dyn RemoteBackend = &*backend` where a borrow is
shared across lanes.

What this does *not* do is abstract credentials. `AppCredentials` is an app
password from Login Flow v2 — a Nextcloud protocol, not a general notion of
authenticating to a remote — and seven files still name it. An OAuth token, a
bucket key pair and an app password have no useful common shape, so deciding
what an account is across backends before a second one exists would be a
confident guess. code-health.md CH-2 now records that as the remaining half,
and it should wait for the backend that forces it.

Verified: fmt clean, clippy clean at -D warnings, 2041 tests pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-27 19:51:23 +02:00
dtourolleandClaude Opus 5 25c88d9dbd Start face indexing from Settings
Beside the thumbnail sweep, because it is the same kind of thing: a job
that runs for an hour, is asked for once, and reports into the activity
list above it. It is also downstream of that sweep -- detection reads the
proxies it builds -- so the two belong in that order, and the coverage
line says how many images are waiting on a proxy rather than only how
many are left to index.

The button drives the Identity Manager's own state rather than a second
copy, so it cannot disagree with that screen about whether a pass is
running, and either place can start or stop it.

The pass now opens an activity row. The caption promises progress will
appear in the list above, and without a row it would not: the button
would be the only sign anything was happening, invisible from every
other screen.

Coverage is read when the Settings page opens. The figures live in the
catalog and this page deliberately holds no session, so they arrive
through a closure rather than being kept current -- they are only ever
looked at while the page is on screen, and the check is two counts and an
indexed scan.

Also adds DARKROOM_NO_SYNC. Redirecting XDG_DATA_HOME isolates a test
launch's catalog and thumbnails but not its server, and I found that out
by pushing a test catalog over the live one. The guard sits in
start_derived_sync rather than at its three call sites, because the sweep
firing a sync is correct and a flag checked in three places is one that
gets missed in a fourth.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-27 09:22:43 +02:00
dtourolleandClaude Opus 5 fa4ad6e2d6 Drag the date range on the axis it is chosen from
The range could be turned on with a finger and not aimed with one. Its two
ends were typed as `YYYY-MM-DD` into 108px fields behind a soft keyboard, to
name days already drawn on the axis a thumb away; and the chip that seeds them
takes its span from the timeline's zoom and pan, which are a wheel and a middle
button. A touch screen has neither, so on Android the filter was a switch with
no aim.

The band is now on the timeline. Two ends with grips, dragged along the bars,
released to filter — the histogram was already how a period is found, and this
makes it how a period is stated. Both ends snap to whole days, which is what
the typed fields mean, what `show_range` reads back out, and a floor under a
range dragged shut. The fields stay for what dragging cannot do: name an exact
day, and say in words what the range is.

For that to work the axis had to stop following the range. Redrawn to the band,
it moved the ground under the very handles doing the narrowing, and there was
nothing outside the range left to widen back into.

While there: a fixed number of equal bins instead of calendar buckets. Between
one calendar unit and the next the bar count is free to wander by a factor of
twelve, so zooming in halved it two steps out of three — the same picture drawn
wider until it jumped back to fine. Equal bins also include the empty ones, so
a bar's position on the track and the date under it are finally the same
quantity; before, a library with gaps drew a February six months wide and the
marker, the band and a click all pointed somewhere else. The count is a
setting, 32 or 64, because the right answer is a question about the screen.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-23 18:42:42 +02:00
dtourolleandClaude Opus 5 44f0a4971b Show the folder picker on the platform that needs it most, and upload at once
Two faults, both of my own making, reported from the tablet as "I cannot
select a location" and "it does not upload".

**The picker button was gated on `target-selected == 1`.** That index was
Remote's position while both targets were offered. Making the target list
platform-aware narrowed Android's to Remote alone, so Remote became index 0
and the button disappeared — on the one platform where the picker is the
*only* way to set a destination, since a device folder is not reachable there
at all. It is gated on a boolean derived from the target now. An index into a
list whose length varies is not a fact about the target, and writing it as one
is what made a correct change break the thing it was meant to fix.

**A queued export waited for a sync pass.** Staging first is deliberate — an
export is finished on disk the moment it is written, and offline is then just
a longer queue — but nothing drained the outbox until the next sync, so
"Queued for Exports" sat unchanged and read, fairly, as an upload that never
happened. A finished batch that wrote anything now drains immediately. The
sync-pass drain stays: the first makes an upload feel immediate, the second is
what eventually delivers the exports made in a tunnel.

Committed without the parallel session's in-flight collection work, which is
mid-save and does not compile; verified by stashing it and building this tree
alone. 281 dr-ui tests pass, clippy clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-17 12:01:12 +02:00
dtourolleandClaude Opus 5 8ea92545df Stop the export folder forgetting itself, and let Android reach one
Three faults, compounding into an export that could not be made to work on a
tablet at all and a destination that appeared to reset on its own.

**Switching target destroyed the destination.** One field held both a
filesystem path and a remote folder, so changing the target had to clear it —
`/home/x/Exports` carried to the server would have offered to create a folder
called `home` at the library root. The consequence was that merely looking at
the other option threw away the destination already chosen, which reads,
correctly, as a setting that will not stick. There are two fields now. Each
target remembers where it was pointed and switching is free; `active_destination`
picks between them so no caller can reach for the wrong one.

**The library root read as "unset".** The picker opens at the root, so
confirming it where it opens stored an empty string — indistinguishable from
"ask each time" and looking exactly like the picker had done nothing. Empty
now means the library root for a server destination, which is a real folder
and the one the photographs are already in; it means "ask" only for a device
folder, where no path is worth assuming. The page labels it so.

**Android defaulted to a target it cannot use.** A device folder there means
the Storage Access Framework, which provides no filesystem path (ARCH §6.9)
and is not implemented — so the default target could never succeed however the
destination was filled in. The export button said "no export folder is set",
the settings page offered no way to choose one, and the only way out was to
guess that the other target was the working one. The device target is now
absent from `ExportTarget::available()` on Android and the default there is the
server, which needs no platform work at all. A settings file carrying an
unreachable target — copied from a desktop, say — is corrected on read rather
than left to fail at the last step.

Seven tests, each named for the fault it prevents returning. The compatibility
one matters most: a file written before `remote_destination` existed keeps its
device path and gains an empty remote one.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-17 09:25:15 +02:00
dtourolleandClaude Opus 5 cb1d2be240 Choose the export folder by walking the server, not by typing it
The destination for a Nextcloud export was a text field. Nobody recalls the
exact spelling of a path three levels down, and getting it wrong does not
fail — `create_dir` makes whatever was typed, so a misremembered folder
becomes a new one at the root and the exports are somewhere nobody looks.

So it is picked the way the library root is picked, using the same
`FolderBrowser` model the launch screen drives: up, into, and "use this
folder", confirming the folder currently *shown* rather than one selected in
the list. Same rule in both places, so the phrase means one thing.

The model is shared; the worker is not. `settings_ui::spawn_folder_list` is a
near-twin of the launch screen's, because that one reaches into the
`LaunchController` for its session and reports onto the launch screen's error
line, while this one is handed credentials and writes to the settings page.
Factoring them together needs a function taking both controllers or a trait
implemented twice to abstract two call sites — more machinery than the twenty
lines it saves. What matters is shared already: navigation behaves identically
because both drive the same model.

The callbacks are wired in `lib.rs` rather than in `settings_ui::wire`,
because listing a remote folder needs credentials and the settings page holds
no session on purpose — it is reachable before a library is opened and must
not depend on one existing. With no account the picker says to sign in first,
rather than showing an empty list that reads as a server with no folders.

Details that are decisions rather than accidents: the picker opens at the
library root rather than at whatever half-typed path is in the field, which
would list nothing and look broken. The listing area is a fixed 180px, since a
folder with sixty children would otherwise push the rest of the settings page
off the bottom. "Up" is disabled at the root rather than hidden, so the row
does not jump as the user navigates. A failed listing leaves the picker open
on the folder it was showing — where the user had got to is not something to
discard over a dropped request. And the chosen folder saves immediately like
every other setting on a page that has no Save button.

The poll timer lives on the controller for the reason `LaunchController` keeps
its own there: a `slint::Timer` stops when dropped, so one local to the
function that starts it would be collected before the listing arrived.

Carries in-flight work from a parallel session — a segmentation pass in
dr-gpu, a sidecar cache, and the develop panel's continuing changes.

1020 tests pass, fmt clean. One clippy warning remains and is not mine:
`sidecar_cache::dir` is unused while that work is in progress.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-17 07:12:01 +02:00
dtourolleandClaude Opus 5 e00c99b864 Let a photograph leave: an export button, and a cache to leave from
dr-export could turn a frame into bytes and nothing could ask it to. This is
the button, and the place the bytes go.

**Everything is staged first.** An export bound for the server is written to a
local outbox and uploaded afterwards; offline is not a special case, it is the
same path with a drain that finds the server absent. Doing it the other way —
upload directly, stage only on failure — makes the failure path the one that
is rarely exercised and always broken, and a network drop mid-batch leaves
some exports existing and some not with nothing recording which. Staged first,
an export is finished the moment it is written and the upload is a promise
kept later.

The outbox sits beside the catalog rather than under the cache. dr_catalog's
cache already draws that line: passive entries are a convenience and go under
LRU, pinned ones are a promise and never do. An export awaiting upload is a
promise — the user was told it succeeded — and sweeping it for disk would
destroy the only copy. Bytes are written before the destination record, so a
kill between the two leaves an orphan the drain ignores rather than a record
pointing at nothing.

The status line says "Queued for Exports/2026", never "Exported to Nextcloud",
until it has actually landed. There is a test asserting that wording, because
the tempting shorter sentence is a claim the app cannot keep.

The drain runs on the sync pass, before the shards: a thumbnail shard can be
rebuilt from the originals and the catalog is an index, but a queued export
exists nowhere else.

`DevelopSession::render_for_export` renders the framed size rather than reusing
the frame on screen, which is deliberately viewport-sized (FR-DSP-1) — encoding
that would hand the user a soft, screen-sized file with nothing to say anything
had been lost (FR-EXP-9).

One compromise, recorded rather than hidden: the export runs synchronously on
the UI thread, so the window is unresponsive for the few hundred milliseconds
a full-resolution render and encode takes. Moving a DevelopSession and its GPU
pass to a worker is a larger change than one button earns, and it is batch
export that makes the wait intolerable rather than merely noticeable.

Still missing: the Nextcloud folder *picker*. The destination is typed into
Settings for now. `FolderBrowser` in launch.rs is already the reusable model
for it — it browses a remote tree and nothing about it is specific to choosing
a library root — but wiring it into the settings page needs a listing worker
and browser UI there, which is its own piece of work.

Carries in-flight work from a parallel session — presets, the develop copy and
paste, and the node schema's `presentation` and `enum` support. One misplaced
callback in settings_ui.rs is moved from `render` to `wire`: registered in
`render` it borrowed a `&SettingsController` into a 'static closure and would
not compile, and that file's own docs say render pushes properties while wire
connects callbacks.

992 tests pass, clippy and fmt clean. Traceability 48.3% -> 51.0%.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-16 23:58:17 +02:00
dtourolleandClaude Opus 5 151dcc3c02 Make a file out of a photograph
Export existed as a settings page and nothing else: format, quality, colour
space, five sizing modes, a filename template and a metadata switch, all
configurable in detail, and no way to produce a single file. dr-export is the
other half.

**It returns bytes and a name, and writes nothing.** An export has three
destinations with nothing in common — a path on Linux, a SAF document on
Android where there is no path at all (ARCH §6.9), and a PUT to a Nextcloud
folder — so a crate that opened the file itself would serve one of them and be
rewritten for the other two. The caller places the bytes.

Resize, then sharpen, then encode, in that order and for a reason: output
sharpening compensates for the softening the resample introduced, so its
strength scales with how much scaling actually happened, and sharpening before
shrinking would throw the result away. Lanczos-3, separable, with weights
computed once per output row — FR-EXP-4 asks for Lanczos or better because a
box filter turns a distant fence into moiré.

Collision handling takes the "is this name taken" test as a closure rather
than looking at a directory, because there is no directory it could look at
that works everywhere. That shape is not politeness toward Linux: Android's
createDocument renames on collision by itself and cannot overwrite at all, so
all three CollisionPolicy settings need the answer *before* anything is
created. Overwrite, Skip and Increment are each tested, and Increment gives up
after ten thousand rather than spinning against a destination that reports
everything as taken.

Three things are honest rather than done:

  - **Colour space.** sRGB only. The shader encodes and clips to sRGB before
    this crate sees a pixel, so tagging a file Display P3 would claim a gamut
    it does not contain. Refused with a typed error instead of mislabelled;
    honouring it is a pipeline change (FR-EXP-2).
  - **AVIF and JPEG XL.** No encoder. libaom and libjxl are C, ravif is slow
    enough to change what a batch feels like, and the settings page offers
    both because FR-EXP-1 lists them — so asking for one says so rather than
    writing a JPEG under a .avif name.
  - **16-bit TIFF** is a real 16-bit file carrying eight bits of information,
    because AdjustPass renders to Rgba8Unorm. Widened by *257, not <<8, so
    white lands on 65535 rather than a quarter-percent grey. Making it mean
    what it says needs the composer told what format to write.

Metadata is not written at all, which satisfies the half of FR-EXP-8 that
matters most: strip_location defaults to on, and a file with no EXIF block has
no GPS tag. Retaining camera and copyright when asked is not implemented and
cannot be faked by omission.

Also here:

  - `AdjustPass::export_pixels`, ungated where `read_output` is behind a
    feature. The two are the same transfer and opposites in intent: reading
    pixels back to *display* them is what ARCH §6.1 forbids and AC-8 asserts
    against, while reading them back to encode a JPEG is the only way a file
    has ever been made. Separate methods so the instrumentation can count one
    without counting the other.
  - `ExportTarget`, so a destination can be a folder on the server. On Android
    that is the only destination needing no platform work whatsoever — a PUT
    against create_dir, already on the RemoteBackend trait, behaving
    identically on both platforms. Switching target clears the destination,
    since a path is not a remote folder and carrying one across would offer to
    create a folder called `home` at the library root.

Verified end to end rather than by unit test alone: `cargo run -p dr-export
--example export` decodes a frame, runs the develop chain on the GPU at full
resolution, reads it back, and writes all five formats — 27 ms for a
full-size JPEG, 165 ms with a Lanczos reduction to 1200px. ImageMagick agrees
the 16-bit TIFF is 16-bit. dr-export cross-compiles clean for
aarch64-linux-android; all three encoders are pure Rust, which is why they
were chosen. 944 tests pass, clippy and fmt clean.

Not yet wired to a button. The develop view has no export action, so nothing
in the running app can reach any of this yet.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-16 22:26:37 +02:00
dtourolleandClaude Opus 5 03326242a1 Make the CI checks say what they mean, and format the workspace
Build and test / Desktop (Linux) (push) Successful in 1h23m26s
Build and test / Android (aarch64) (push) Failing after 2s
Build and test / Layer separation (push) Successful in 50s
Traceability / Requirement traces (push) Failing after 1m9s
The Android job's "Verify minimum API level" step has never verified the
minimum API level. It took the first `*.so` anywhere under the target
directory, which is a host proc-macro from debug/deps — an x86-64 object
built by the runner's gcc, whose .comment section cannot mention Android
and so can never contradict the expected value. It now reads the artifact
under the target triple, compares against MIN_API parsed from the
Dockerfile rather than a second copy of the number, and fails on a
mismatch. Both sides are checked non-empty first: two failed parses would
otherwise compare equal and pass, which is the same silent success in a
new costume.

The Android image installs one SDK package per layer and keeps the
output. sdkmanager is a JVM program that aborts when it cannot get memory,
and the single `> /dev/null` step reported that as a bare "exit code 134"
while a retry re-downloaded everything that had already succeeded.

tools/ci-local.sh runs all four jobs — desktop, android, layering,
traceability — against the host toolchain, which is pinned to the same
1.92.0 CI installs. Its matrix check compares regeneration against the
working tree rather than against HEAD: CI starts from a clean checkout, so
git's answer is the right one there and reports every local run stale here.

The rest is rustfmt across the workspace, and the clippy findings that
surfaced once it did: manual_contains in dr-thumbs and collections_ui, a
map iterated as pairs for its keys, an index loop over a slice, and two
runtime assertions on a constant now made at compile time.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-16 12:02:51 +02:00
dtourolleandClaude Opus 5 fa12afed18 Keep originals on this device, by pin and by use
Build and test / Desktop (Linux) (push) Failing after 1s
Build and test / Android (aarch64) (push) Failing after 0s
Build and test / Layer separation (push) Failing after 1s
Traceability / Requirement traces (push) Failing after 2s
Fills in `image_cache`, which the previous commit's "On this device" filter
read but nothing wrote. Also carries in-flight work that shared these files:
the Android TLS root store, the settings page, and a regenerated
traceability report.

# Two populations, deliberately separate

An original is kept here for one of two reasons, and conflating them produces
the exact failure the feature exists to prevent.

**Pinned** originals were asked for. Pinning a collection before a trip is a
promise, so pinned rows are never evicted and never counted against the
budget — a cap that could silently delete a pinned trip would make pinning
worthless, because it could not be relied on without checking.

**Passively cached** originals are a side effect of working: develop already
downloads the whole file, so keeping it costs no bandwidth and saves the
entire transfer next time. This population is what the budget bounds, evicted
least-recently-used, because it otherwise grows until a day of culling fills
a disk.

Sharing one budget would let a large pin starve the passive cache, or let
browsing evict a pin. They are separate.

# What was built

`dr_catalog::cache` owns the bookkeeping — held tier, size, last use, pinned
— and writes the bytes; deciding to download stays with the caller, which is
what keeps a crate with no network out of the network's business. Files are
written to a temporary and renamed, so a dropped connection cannot leave a
truncated file recorded as a complete original. They are named by image id,
not filename: `Photos/IMG_0001.CR2` and `Trips/IMG_0001.CR2` are different
photographs, and a flat cache keyed on the name would serve one for the other.

`spawn_full_fetch` became read-through. A hit is a disk read; a miss stores
what it downloads and enforces the budget. A cache that cannot be opened is a
miss, not a failure to open the photograph.

Pinning writes intent — `tier_desired` — without downloading, so the button
responds immediately, and `spawn_pin_fetch` fills it in sequentially
afterwards. Sequential because these are tens of megabytes each: the lanes
that make the thumbnail sweep fast buy little against one connection's
bandwidth and cost a great deal of memory. A pin interrupted by a lost
connection resumes from where it stopped.

Schema v5 adds `pinned` and `path`. `pinned` is a column rather than something
inferred from `pinned_by_rule`, which is ON DELETE SET NULL and so cannot
answer for an image whose rule was deleted. A v4 catalog migrates in place;
existing rows default to unpinned, the safe direction.

The budget and "keep opened originals" come from the settings page rather than
a constant, and are applied at startup rather than only on change — a cache
capped at 2 GB last session would otherwise spend this one filling to the
default. Turning off keeping leaves what is already cached readable: those
bytes are paid for, and refusing them would re-download images sitting right
there, including pinned ones.

Also removes a doubled `#[test]` introduced in the previous commit.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-11 21:12:01 +02:00