495 Commits

Include renames

Author	SHA1	Message	Date
dtourolleandClaude Opus 5	c38df01bf7	Hang the auto-crop off the slider's commit, not off the pointer passing over ... The straighten auto-crop worked once and then stopped. It was keyed on `PlainSlider::drag-changed`, whose name is a lie inherited from what it forwards: `SliderTrack` defines `engaged` as `has-hover || claimed`, and it has to — a `Flickable` withholds the press for 100ms, so hover is the only signal that arrives in time to stand the scrolling ancestor down. That is the right definition for the job it was written for and the wrong one for this. Keyed on hover, the correction fires when the pointer first crosses the track — before anything has been dragged — and then does not fire again for as long as the pointer stays on it, however many times the angle is changed. Which is exactly what "it only works once" looks like. `SliderTrack` already publishes the signal this wants. `committed` fires on release, once per gesture, after the final `changed`, and its doc comment says so in as many words. It was simply not forwarded through `PlainSlider`, so it now is, and the geometry panel's callback is a `committed(float)` rather than a `drag-changed(bool)`. The angle needs no re-applying here: the track emits its last `changed` before it commits, so the value is already in the graph by the time this runs. What is left is the correction that has to happen exactly once. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 13:49:21 +02:00
dtourolleandClaude Opus 5	52c655e6cf	Turn the ratio lock with the photograph when it is turned ... A quarter turn carries the crop with it — that is what makes turning a photograph keep its composition rather than sliding the selection onto a different part of the picture. So a rect locked to 16:9 comes out of the turn at 9:16, of a frame whose axes have also swapped, and the lock was left claiming landscape over a portrait rect. The next drag would then snap it back upright and undo what the turn had just done. The orientation switch now turns with it, on odd numbers of quarters. `Original` is deliberately excluded, and getting that wrong flips it twice: it is resolved against the framed size every time it is asked for, and the turn has already swapped that frame's axes — so it has turned by the time anything asks. `turns_with_the_frame` is the one predicate that separates the two cases, with a test that pins both. `CropAspect` arrived without tests of its own; it has them now, including the round trip this fixes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 13:49:20 +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
dtourolleandClaude Opus 5	d9eb8faffd	Crop away the corners a straighten exposed, once the slider is let go ... Turning a rectangle inside its own bounds exposes its corners: there is no source pixel out there, and the shader renders it black. Nothing in the render prevents that, deliberately — a free angle does not change the output size, which is what leaves the frame where the user put it while the slider moves. Correct during the drag; four black wedges on the finished photograph. Letting go of the slider now pulls the crop inside the area the angle leaves defined. `Framing::max_inscribed_crop` already computed that bound and had no caller; this is the caller its doc comment described. **Once, at the end of the gesture.** Applied per frame it would shrink the crop on every step of the slider and never grow it back, so a user who overshot to 20° and came back to 3° would be left with a crop ratcheted down by the excursion rather than by the angle they settled on. Per gesture it is bounded by the angles actually rested at, and undo steps back through them. **The crop is fitted into the bound, not replaced by it.** A crop placed deliberately off-centre is a decision, and an automatic correction that recentred it would undo the user's work to fix a problem they did not have. `CropRect::fitted_into` scales only as far as the bound demands and then slides the rect the shortest distance needed to be inside — so a ratio locked in the crop panel survives the straighten too, since the shape is never touched. It returns the rect unchanged, bit for bit, when nothing needed to move. That matters more than it looks: this runs on every release of the slider, including releases at zero, and a rect that drifted by a rounding error each time would be an edit recorded for no reason. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 13:49:19 +02:00
dtourolleandClaude Opus 5	e6ad906bc1	Let the crop be held to a ratio while it is dragged ... A photographer cropping for a print, a phone wallpaper or a 16:9 frame is not choosing four edges — they are choosing one edge and a known shape. Free-dragging every corner made them do that arithmetic by eye on every drag, and get it slightly wrong. The panel now offers Free, Original, 1:1, 3:2, 4:3 and 16:9, with a Portrait switch for the ones that have two orientations. Original follows the frame rather than naming a number, so it stays right on the next photograph from another body and after a quarter turn. **The ratio is of output pixels, and the rect is not.** `CropRect` is stored in fractions of a frame that is not itself square, so holding a shape needs the frame's size — `ratio * height / width` of the frame. Skipping that gives a "1:1" crop that is square only on a square photograph, which is the one case nobody would test on, so the conversion lives in `CropRect::with_aspect` where it is explained and pinned by a test that asserts the fractions are *not* equal. Two decisions worth recording: The reshaped rect **grows** onto the ratio rather than shrinking onto it, then scales down only as far as the frame's edge demands. Fitting inside instead makes a one-axis drag do nothing at all — the other axis clamps the first straight back, and the handle simply refuses to move. The overlay now reports **which corner the drag is holding**, because reshaping onto a ratio has to know which corner is nailed down and only the handle that took the press knows that. A move reports no corner and keeps its shape: reshaping about a centre would pull an over-moved rect smaller instead of sliding it along the edge. The lock lives with the window rather than the session. A `DevelopSession` is per image, and cropping a set of frames to one shape is exactly when the lock earns its place. It is not an edit and reaches no sidecar — what is saved is the rectangle it produced. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 13:48:51 +02:00
dtourolleandClaude Opus 5	da20d42d33	Merge master: pluggable storage, and a name that anchors ... Conflicts were docs/traceability.md alone, and it is generated — so it was regenerated rather than hand-merged. dr-face was untouched on the other side; ui/dr-ui/src/faces.rs and identity_ui.rs auto-merged, the first around recluster's anchoring and the second around load_faces. Worth recording because the two branches met on the same problem from different ends. Master's "Let a name hold a group together" is the fix for the sixteen Catherines — fourteen of them empty — that this branch found while measuring the library and reported without fixing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 12:30:01 +02:00
dtourolleandClaude Opus 5	ebb7d3cf5c	Score a suggestion against the people the user has named ... The number beside a suggestion was the mean calibrated probability between the face and the rest of its group, which measures the wrong thing twice. It punishes coverage: a person with two hundred faces over fifteen years is *meant* to have members a given photograph is orthogonal to, so a correct suggestion onto a well-photographed person scored low for being well photographed. And it never asked who else the face might be — a face matching Anna at 0.95 and nobody else, and one matching Anna at 0.95 and her sister at 0.93, came out identical, when the second is the only one worth the user's attention. dr_face::assign answers both, and multiplies them: the mean of the best ten calibrated matches into the identity (the old mean, capped, which is what stops coverage counting against it), times that identity's share of the evidence against every *named* rival. Only named people compete, and per person rather than per group. Both halves of that had to be measured on a real 18,000-face library rather than reasoned about. Normalising across every group made the number useless — median suggestion 21%, four in five under half — because clustering leaves one person spread over many groups, so a face competed against itself; and keying rivals by group left Catherine competing with Catherine, median 39%. Per named person: median 99.5%. Rivals are gathered below the merge threshold, down to even odds: a named person matching at 0.6 will never be merged into but is exactly the competition to discount for. That would be a second similarity scan, the expensive half of regrouping a library, so cluster_scored scans once at the looser floor and hands the merge engine the subset at or above the threshold — pair for pair what it would have scanned for itself, held to that by a test. Leave-one-out over that library's 2,702 confirmations across 54 named people: 99.33% of faces placed on the right person against the old mean's 99.15%, and the number shown for the right person moves from a median of 90.4% to 99.3%. It errs low — 100% correct wherever it states 80% or more — which is the safe direction, and docs/faces.md §9.1 says plainly that the low bands are not calibrated. The example that measures it comes too: this is a claim about a library's numbers, and nobody should have to take it on faith. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 10:44:01 +02:00
dtourolle	21d599b420	Test the guard, since the bug was a branch nobody ran ... The catalog clobber existed because `if let Ok(bytes)` had a failure arm that was never exercised. Fixing it without covering that arm leaves the next person free to collapse it back. Three tests against a backend whose read fails in a chosen way, counting writes — because what went wrong was not a wrong value but a write that should not have happened at all: - a dehydrated snapshot uploads nothing - an unreadable one uploads nothing either, since "refused" is no more "absent" than "not downloaded" is - and a genuine first sync still uploads, which is the half that keeps `NotFound` distinct from `NotMaterialised` rather than merely cautious Checked against the original shape: the first two fail on it and the third passes. A guard test that cannot tell the bug from the fix is decoration. `async-trait` joins dev-dependencies to stand the double up behind `dyn RemoteBackend`.	2026-08-29 10:40:38 +02:00
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.	2026-08-29 10:36:26 +02:00
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.	2026-08-29 09:57:53 +02:00
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.	2026-08-29 09:57:52 +02:00
dtourolle	64462e9fd3	Test the folder sign-in instead of trusting it ... The whole of a folder library's sign-in lived inside a Slint callback, which cannot run without a display server — so the one path that decides whether a mistyped folder becomes a *stored* account had no test at all. That failure is quiet and lasting: an account for a directory that is not there skips the launch screen on the next start and reads as a library that has lost its photographs. `open_folder_library` is that logic, lifted out whole. Four tests: it stores an account with no credential and reaches the keyring for nothing, a typo is refused before anything is written, the messages read as instructions because they go straight to the screen's error line, and a file is not a library.	2026-08-29 09:57:52 +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	c8e05831f4	Show the confidence, and say which curve it came from ... FR-CULL-9 was read as "no fit, no number", so every library without 200 confirmed positive pairs showed "Confidence unavailable" on every suggestion — which is every library, until enough confirmations exist to fit one. The confirmations are made on this screen, ranked by the number it was withholding, so the degraded state was also the permanent one. There has always been a curve: Calibration::default is the reference implementation's fitted MBF sigmoid, which is what clustering already operates at. It is a published operating point, not an invention, and what the requirement forbids is presenting it *as though it were measured on this library*. So the percentage is shown, and the screen says once, above the grid, where the curve came from. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 09:57:38 +02:00
dtourolleandClaude Opus 5	1b8b7998a2	Let a name hold a group together, the way a confirmation does ... Sixteen people called Catherine, fourteen of them holding no faces at all, and her actual photographs split across the two that did. That is not a sync fault; it is this function, and it has been quietly doing it on every Regroup. Naming a cluster does not confirm its faces. They stay *suggestions* — and only confirmed faces anchored here, so the next pass cut them loose, regrouped them into a brand new person, and left the named one holding nothing. `prune_empty_unnamed` will not clean that up, because it has a name. Name the new group the same thing, and it happens again. Repeat over a few sessions and you have sixteen of her. A name is a judgement about *this group*, of exactly the kind FR-CULL-12 says travels and inference does not — the same argument that already anchors a group the user set aside. So all three kinds of ruling anchor now: confirmed, ignored, and named. It also fixes the quieter half of the same fault. A face indexed later that matches a named person now merges *into* them, rather than arriving as a rival group the user has to name all over again. Two tests, and the first fails without the change — it reports "Catherine" finishing the pass with zero faces while a fresh unnamed person holds the two she was named for. This does not retro-fit an existing library: the fourteen empty Catherines stay until they are merged by hand, and the two holding faces are separate identities that only the user can say are one person. What it stops is making more. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 09:48:32 +02:00
dtourolleandClaude Opus 5	2fb3eb5d2d	Upload a shard that was sealed after the server saw it ... The tablet showed 442 of a person's 648 faces and could never catch up. Its shard ledger said why: it had adopted the laptop's shard 0 at 2,711,552 bytes, and that shard is 20,402,176 bytes on disk. The upload skipped it. A sealed shard the server already had under that name was taken to be "byte-identical by construction", so its mere presence was proof enough and the size was never consulted. That premise is false, and this library is the counter-example: an *open* shard is uploaded on every pass as it fills — that is how a growing shard reaches the other devices — so the server ordinarily holds a partial copy of a shard that is later completed and sealed. Sealing then froze that partial copy in place for ever. Nothing downstream could recover from it. The tablet's `has_adopted` check is keyed on name *and* size and would have re-downloaded a changed shard gladly; it was never offered one, because the sending side had stopped looking. So the comparison is on size, sealed or not. The client id is in the name, so nobody else can have written the file and size is a sound test. A sealed shard whose size already matches is still skipped on the first comparison, which is all the original cheapness was worth. The thumbnail upload had the identical test and the identical hole, and is fixed with it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 08:32:01 +02:00
dtourolleandClaude Opus 5	d70fe9da8d	Decide whether to send a shard before reading it ... The upload read every shard off disk and only then asked whether it needed sending. For a library with 94 MB of face shards already on the server, every idle sync read 94 MB to conclude it had nothing to do. The question does not need the bytes. A sealed shard the server already has is byte-identical by construction, and the client id is in the name, so nobody else could have written it — the name settles it. The open shard is compared on size, which `stat` answers. The progress line moves after the skip for the same reason: announced before it, an idle pass claimed to be sending five shards and sent none. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-28 23:08:16 +02:00
dtourolleandClaude Opus 5	a1e8f494b9	Say what the face sync is doing while it does it ... The face pass set the status to "checking faces…" once and then said nothing until it was finished. On a library whose first export after a re-index is 9,849 photographs and 85 MB of shards, that is eight minutes of a progress bar sitting still — which is indistinguishable from a hang, and was reported as one twice. Nothing was wrong with the sync. The only fault was that it was silent. Three places now report, which are the three that take real time: - **Preparing**, per image with a count, since this is the long one and the only one whose length the user cannot guess from anything on screen. - **Sending**, per shard with its size, because a face shard carries crops and runs to tens of megabytes — one of them is a visible wait on any connection. Announced before the upload rather than after, since the wait *is* the upload. - **Taking in** a peer's shard, which is a download and then a row-by-row merge. The export reports every 25 images rather than every one, so the channel behind it stays lost in the write it accompanies. `export_to_shards` keeps its old signature and delegates, so the callers that do not want progress do not grow a parameter for it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-28 21:56:16 +02:00
dtourolleandClaude Opus 5	41daa4cca7	Keep a group set aside actually set aside ... "Not interested" hid a group, and the next Regroup brought it straight back. Reclustering anchors the faces the user has ruled on so a pass cannot move them. It took *confirmations* as the only kind of ruling — but setting a group aside is a ruling too, and the faces it covers are only ever suggestions. So an ignored group's faces entered clustering loose, regrouped into a fresh person that carried no ignore flag, and reappeared in the rail. The original group was left behind holding nothing, hidden and empty. Anchoring them on `ignored` as well as on `confirmed` fixes it, and does one better: a face indexed later that matches a group which was set aside now merges *into* it, so a stranger photographed again stays set aside instead of arriving as somebody new. That is the case that would otherwise have made the feature feel like it only half worked. Three tests, and the first fails without the change — it reports the group coming back with its two faces while the original sits ignored and empty. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-28 08:30:16 +02:00
dtourolleandClaude Opus 5	af5a13b3f7	Narrow the grid to several people at once, either way ... "Show photos" could only ever mean one person. The two questions a photographer actually asks are "every picture of Anna or Bob" and "the pictures they are both in", and the second is not reachable by any sequence of single-person filters — no amount of switching between one person and another finds the frame they share. So the filter holds a *set* of people and a mode. `RatingFilter` was already the right home, as its own doc says: every query path threads it, so the count in the header and the cells in the grid are narrowed by the same thing, and this composes with stars, flags and the date range for free. The union is `EXISTS ... person_id IN (...)`. The intersection counts **distinct** people per image and compares against the size of the selection — one subquery rather than one per person, and it does not grow the statement with the selection. `DISTINCT` is what makes it correct: three faces of Anna in one frame must not satisfy a filter asking for Anna and Bob, and there is a test that says so. Any rather than All is the default. With one person the modes are the same filter, and adding a second to a union can only ever show more — so a user who has not noticed the toggle never ends up staring at an empty grid wondering what they broke. The toggle only appears at two people, because a control that demonstrably does nothing is a control that teaches the user to ignore it. Building the set needs no picker of its own: the Identity screen gains "And also…" beside "Show photos", offered only once the grid is already narrowed to somebody. Each person is a chip on the filter bar and each chip removes just that person, so a selection of three can be taken apart one at a time rather than only cleared wholesale. `RatingFilter` stops being `Copy`, since it now holds a `Vec`. Every query path already took it by reference; the casualties were two struct updates and one `Cell` that becomes a `RefCell`. 484 dr-ui tests pass, including the union, the intersection, that one person reads the same in both modes, and the repeated-faces trap. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-27 22:17:59 +02:00
dtourolleandClaude Opus 5	a1790c3e67	Stop indexing faces too small or too blurred to be anyone ... The library was storing faces at 52 source pixels and embedding whatever came back. There was a size floor, but it was 40 pixels on the *bounding box*, and there was no blur gate at all — so a subject walking through a half-second exposure detected confidently, aligned cleanly, and produced a perfectly ordinary-looking 512-vector. Nothing downstream can tell that apart from a real face, and because blurs resemble each other more than they resemble the people they were, they cluster together and weld unrelated identities into one group. Two floors, both measured rather than guessed. `face_index --quality` runs the detector over real proxies with both gates disabled and prints the distribution; over 1,503 faces in 600 images of the reference library: percentile crop px sharpness 1% 16 0.0006 25% 23 0.0025 50% 38 0.0071 75% 76 0.0284 99% 352 0.4282 The median face in a personal library is 38 pixels. Most of what the detector finds is background: people across a square, a face on a poster, a stranger at the next table. They are real detections and useless identifications. **Size, on the crop rather than the box.** "At least 64x64" has to mean the pixels the *embedder* sees, and the box is not that — the ArcFace template reaches past it for forehead and chin, so the aligned crop spans roughly 1.3x the box's shorter edge. The floor is therefore `min_source_px` on the aligned crop, applied after the warp fixes the scale, and `min_face_px` drops to 48 as what it always really was: a cheap pre-filter set low enough that it cannot reject a face the real floor would have kept. **Sharpness.** Variance of the Laplacian divided by the variance of the luma it was taken over. The division is the part that matters: raw Laplacian variance scales with contrast, so a threshold on it would quietly discard every backlit portrait in the library. The ratio asks how much of the crop's variation is edges rather than broad gradients, and is invariant to exposure. What each pair removes, cumulatively, of everything the detector finds: min crop min sharp size cut blur cut kept 64 0.000 70% 0% 30% 64 0.010 70% 3% 27% 64 0.020 70% 7% 23% 80 0.010 76% 2% 21% 64 and 0.020. The size floor does most of the work, and the blur floor removing only 7% on top of it is the point rather than a disappointment: at 64 pixels most faces are already sharp, and what it takes out is the large-but-soft one — precisely the face that would otherwise contribute a confident, wrong embedding. The two gates are not independent and the doc comments say so: a face under 112 pixels was upsampled to reach the embedder, and upsampling invents no edges, so small faces score low on sharpness even when the original was crisp. That is why `--quality` prints them together. **This will re-index.** Around 70% of what the current settings store falls below the new floors — faces between 20 and 40 pixels that nobody could identify. The People screen gets shorter and every group in it gets better. 66 dr-face tests pass, including that a blurred crop scores below a sharp one, that halving the contrast does not move the score, and that an upsampled face scores below the same face at full size. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-27 22:17:38 +02:00
dtourolleandClaude Opus 5	cafa63ca6f	Let the develop column ask how wide it needs to be ... The column was 280px, a number chosen for a tablet, with 380px bolted on later for a desktop. Both were guesses at how much room the widest row inside needs, and a guess is what cannot work here: the mode strip is one chip per attribute the *operation set declares*, so the row is generated and no constant in app.slint can track it. When the guess came up short the failure was not a tidy clip. The Flickable inside the column never had its `viewport-width` set, so the viewport took its content's preferred width, and a viewport wider than its Flickable is *centred* in it — the same rule the note on the seam's `x: 0` already records a few lines below. So the column lost half of each edge rather than one of them: "HISTOGRAM" read "ISTOGRAM", "Straighten" read "aighten", Copy sat centred while Paste ran off the far side. It looked like a rendering fault and it was an alignment one. So the column asks instead of guessing. Every panel that can appear in it — image, histogram, geometry, settings transfer, masks, repairs, adjust, history — now publishes a `content-width`: how wide it has to be before it starts clipping itself, read off its own layout rather than asserted. Each declares that as its `min-width` too, and that is what makes the aggregation automatic: `column` is a layout, so it already reports the largest minimum among its children, and it does so for the panels that come and go with the mode as well, which live inside `if`s and cannot be named from outside. Grep `content-width` in ui/dr-ui/ui to see every panel with a say in the answer. The mode strip is named explicitly only because it is pinned outside that layout, so nothing else measures it. There is no floor left. A floor is one more guess and the panels state their own minimums now. The only thing still above the measurement is `panel-max-width`, which is not a size but a policy — a column may not take the window from the photograph it exists to serve — and it comes from Rust beside `layout-class` because a width read from `root.width` inside the layout that `root.width` depends on is a binding loop. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-27 21:23:44 +02:00
dtourolleandClaude Opus 5	d6380fecc8	Make the People screen a place work can be done ... Five faults, all on one screen, and the Slint and Rust halves of each have to land together. **Regroup froze the window.** It ran inside the Slint callback, on the UI thread. It is much faster now, but fast is not bounded — the work grows with the library, and the one thing that must not grow with the library is how long the window stops answering. It runs on a worker thread with an mpsc channel and a 250ms poll, like every other long pass in this module, and the button says what it is doing instead of the window going quiet. Cancellation is dropping the receiver. Reclustering also prunes the empty groups the previous pass left, so pressing the button twice no longer fills the rail with "Unnamed (0 faces)". **The faces were a single row running off the screen.** The comment on the layout claimed to be a wrapping row; Slint has no flow layout and a HorizontalLayout does not wrap, so a person with forty faces was a person whose faces could not be reviewed past the fifth. It is now laid out the way the library grid lays out thumbnails, with the same arithmetic: choose how many columns of roughly the requested size fit, then divide the width between them so the cells fill the row exactly and nothing overhangs. **The header did not fit a phone.** A 240px name field beside five buttons is wider than an Android screen — and worse than not fitting, a layout cannot be narrower than its children's minimums, so the row reported that oversized minimum upwards and inflated the whole screen. The faces grid is its sibling, so it would have been measured against a width that was never on the display. The header is now two rows, the actions sit in a Flickable that scrolls rather than overflowing, and the rail narrows to 132px on the compact class. **Strangers crowded out the people who matter.** Most clusters in a real library are passers-by and other people's guests. "Not interested" sets a group aside; the rail hides it and says how many are hidden, with one button to bring them back. Reversible, and never a deletion — see the catalog commit for why. **A face was a dead end.** Identifying someone and then having no way to see their photographs is a filing cabinet with no drawer handles. "Show photos" narrows the library grid to that person and leaves a chip on the filter bar saying so, which is also how it is cleared. It is a term on `RatingFilter` rather than a grid scope of its own, exactly as that struct's own doc says new narrowing terms should be — so the count and the cells are narrowed by the same thing, and it composes with the others for free. Suggested faces count, not only confirmed ones, or a freshly grouped person would show an empty grid. Crops are read from where they are now stored, falling back to cutting one out of the proxy for faces indexed before that existed. 480 tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-27 21:21:31 +02:00
dtourolleandClaude Opus 5	7dfbe3184a	Merge master into the face branch ... Master moved 24 commits while this branch was building the face sweep, and one of them changed how the interface reaches a server: `remote.rs` is now the only file that names a connector, and everything else takes `&dyn RemoteBackend`. The face sweep was written before that landed and built a `NextcloudBackend` directly. Git merged the text without complaint and the result did not compile, which is the useful kind of conflict — it goes through `remote::connect` now, like every other pass. Two documentation conflicts, both resolved toward master. `code-health.md` was an add/add: master's copy carries the CH resolution for the backend seam and a better provenance note, so it wins outright, with its measured figures re-taken against the merged tree rather than either side's — `run()` is 1,855 lines now, 2,042 tests, 793 traceability tags. `traceability.md` is generated, so it was regenerated rather than hand-merged. The seam grades in code-health.md are unchanged by this merge. That is worth noticing rather than glossing: the face work went into the seams that already existed — `run()`, `library.rs`, `AppWindow` — which is exactly the pressure CH-1 describes rather than evidence against it. All four CI jobs pass: desktop (fmt, clippy, test, build), layering, traceability, and the Android cross-build. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-27 20:27:45 +02:00
dtourolle	28c046130c	Merge branch 'master' into android-bundled-face-models	2026-08-27 20:22:42 +02:00
dtourolleandClaude Opus 5	7b4263ceb9	Bring master's display and parity work under the new checks ... Master moved sixteen commits while these fixes were being written — per-display colour, the frame-budget measurement that decides FR-DSP-2, and a declared node running without being compiled. Merged here rather than on master so the conflicts are resolved where they can be tested. Two files overlapped and neither was interesting. `lib.rs` gained `mod remote` from this branch and `mod display_ui` from master, which git resolved on its own. `docs/traceability.md` is generated, so it was regenerated from the merged tree rather than hand-resolved — hand-editing a generated matrix produces one that agrees with neither side. Coverage reads 59.9% (106/177), up from 55.4%, entirely from master's tagging. The check worth having run is `the_interface_names_no_operation` against master's new `display_ui.rs` and its 195 changed lines of `develop.rs`: a new UI module written without knowledge of this gate passes it. That is the evidence the gate is not merely satisfiable by the code that shipped with it. fmt clean, clippy clean at -D warnings, 2087 tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-27 20:10:56 +02:00
dtourolleandClaude Opus 5	371038614f	Fetch the repairs first, or they never happen ... The repair added in 051447b was appended to the work list: wanted.extend(repair); Behind every un-indexed image in the library. On this library that is position 23,000-odd — roughly two hours of fetching before the first repair is reached, which inside one session is indistinguishable from the feature not existing. The log said it had found them and the screen stayed empty, which is the worst combination of the two. They go first now. There are a few hundred of them against tens of thousands of un-indexed images, and they are precisely the images the People screen is failing to draw at this moment — so the ordering costs nothing and is the difference between the grid filling in within a minute and not filling in at all. A failure to build the un-indexed half no longer discards the repairs either: the pass runs with whatever it has rather than returning empty. Verified on the live catalog: 455 images hold faces, 251 already have a proxy from the fixed sweep, and the remaining 204 are what now sits at the front of the queue. The stored proxies check out — a valid JPEG at the large class, 49 KB — so the storing half of 051447b was already working. 471 tests pass, including one that the orphan is ordered ahead of the library. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-27 19:58:32 +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	051447bda6	Keep the proxy a found face will be cropped from ... Every cell on the People screen read "no preview" while the sweep was happily reporting 893 faces found. Both were true. Faces were being detected and stored correctly; there was simply nothing left to draw them from. A face is stored normalised and drawn by cropping the proxy it was found on — `identity::decode_proxy` reads `FACE_TIER` out of the thumbnail store. The fetching sweep fetched a preview, detected on it, wrote the faces and dropped the pixels. So every face it found pointed at a proxy that had never been stored, and the grid had nothing to cut. Worse, that state could not repair itself: the image has its `face_index` row, so it is not outstanding work and no later pass would look at it again. Two fixes, and the first is nearly free. The sweep now keeps the proxy — it has already paid the round trip and the decode, and the crop needs those same pixels the moment the user opens the person. Kept **only where a face was found**: two thirds of a personal library is landscapes and documents (docs/faces.md §7a), those will never be cropped, and skipping them keeps this well clear of the whole-library cost `SWEEP_THUMB_SIZE` deliberately avoids. The downscale to the large class happens after detection, which is the last use of the full buffer. Second, the sweep now picks up images that have faces with no proxy, whatever put them in that state — this bug, or an ordinary cache eviction, which would have produced exactly the same empty grid. Re-running detection repairs it and loses nothing: `record_detections` replaces rather than appends and carries the user's confirmations across the replacement. That makes the screen self-healing rather than dependent on nobody ever evicting a thumbnail. The proxy is stored *before* the detections. A kill between the two then leaves a proxy with no faces — which the next pass simply re-indexes — rather than faces with no proxy, which is the state that cannot recover. Note for the library already part way through a sweep: the 986 images indexed before this will be picked up by the repair route on the next run. 470 tests pass, including one that a face whose proxy is gone becomes work again. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-27 19:35:12 +02:00
dtourolle	fb1b44ce47	Merge branch 'worktree-agent-a1e5c8cb565255f5b' into master ... # Conflicts: # docs/traceability.md	2026-08-27 19:21:10 +02:00
dtourolle	510d1a26cb	Regenerate the traceability matrix ... The platform layer was never scanned, so every FR-PLAT-* and NFR-PORT-* tag in dr-plat was invisible. Coverage 51.4% -> 57.6%, almost all of it pre-existing tags that were simply not being counted.	2026-08-27 19:08:23 +02:00
dtourolleandClaude Opus 5	b858fc029a	Record what per-display colour actually cost ... The status table said FR-DSP-8 was absent and §5.2 assumed Slint reports window moves. It does not — there is no `on_moved` on any backend — so the position is sampled instead. §5.4 records the two trades that are worth someone finding later: a display profile is matched to the nearest of four spaces rather than applied through a CMM, and on Wayland the canvas follows the first output rather than the window, because a Wayland client is never told where its window is and the protocol's own answer needs a `wl_surface` that Slint does not expose. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-27 19:03:24 +02:00
dtourolleandClaude Opus 5	0c3b8cb1c4	Hand out descriptors a declaration could produce ... `Operation::descriptor()` returned `&'static OpDescriptor`, and that lifetime is the whole reason a build-time node is free and a run-time node is impossible: only a compile-time literal can satisfy it, so no amount of reading `ops/*.yaml` at startup could ever produce a descriptor the rest of the application would accept. FR-PLG-2 says a bundled operation and a third-party plugin are the same kind of thing, differing only in where the file was found — and a lifetime outsiders cannot meet is exactly the second, weaker format that requirement forbids. So a descriptor is now owned and handed out as `Arc<OpDescriptor>`, with `Vec` where it held `&'static` slices. `Arc` rather than a `&self`-borrowed reference because the callers want to *keep* it: the develop panel collects descriptors and then mutates the graph, and a borrow would tie the descriptor's lifetime to a borrow of the operation it came from, which is the one thing `&'static` was doing right. The identifier newtypes deliberately did not follow. `ParamId` is `Copy`, is compared in `match` arms against generated constants, is a map key in the sidecar and history, and reaches Slint model rows; an `Arc<str>` there would cost a refcount on every one of those and would take `match id { EXPOSURE => .. }` away from the generated code. They gain an interner instead, which is honest about its lifetime rather than pretending to one — the set of ids is bounded by deduplication and is process-lifetime by construction, because the sidecar on disk names its parameters and an id has to stay resolvable for as long as any edit naming it can be opened. No behaviour changes. Every descriptor that was a `static` is a `LazyLock` initialiser now, `Operation::helpers` borrows from `self` instead of being `'static` so a future run-time node can own its list, and `Warp` and `Framing` follow `Operation` so there is one shape rather than two. The one place a descriptor is read per frame is `compose_full`, which takes `descriptor().id` to prefix each active operation's uniforms, and `dr-ui` composes on every frame it draws. That is a dozen atomic increments beside a composition that is already building several kilobytes of WGSL on the same call; it is noted at the trait method rather than left for a profiler to find. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-27 19:02:25 +02:00
dtourolleandClaude Opus 5	3e5840e413	Let a test hold the words the About page shows ... FR-DSP-8 asks for a fallback that is *defined*, and a reason that reaches the code but never the screen satisfies half of it. The three readouts are now built by a free function over the survey rather than written straight into the window, so a test can assert that "sRGB assumed" arrives with the reason attached, that an approximated profile says "nearest to" rather than claiming the space, and that the second monitor is described on the page read from the first — which is the display the requirement is actually about. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-27 19:00:43 +02:00
dtourolleandClaude Opus 5	c8c6368542	Index the whole library by fetching what it has not seen ... "Index faces in the whole library" could not. Its work list was intersected with the thumbnail store at `ThumbSize::Large`, and nothing fills that class for a whole library — `SWEEP_THUMB_SIZE` is deliberately `Grid`, because the large class is ~860 MB of shards against ~200 MB and every syncing device pays it. So the only images with a large proxy were the ones the user had personally zoomed into or opened in the loupe. On this library that was 220 of 23,529. The comment defending it misread the requirement: // Requesting one here would put face indexing on the network path, // which FR-CULL-8 explicitly keeps it off. FR-CULL-8 keeps indexing off the **full decode**, not the network, and then says the opposite in the same paragraph: "where no proxy exists, the job requests one at background priority rather than decoding inline". faces.md §7 repeats it. Neither was implemented. So the pass fetches. Same two-stage route the thumbnail sweep uses — the header, then the located preview's own byte range (FR-NC-3) — so no whole file is pulled and no RAW is decoded, because an embedded preview is a JPEG. The work list is now every visible image with no `face_index` row for the model: 23,308 here, against nearly none before. **It indexes at the resolution the preview actually has**, not the 1024 the old tier would have given. `locate_preview` already picks the largest embedded preview, and the thumbnail sweep was decoding it and throwing the detail away at `downscale_to(256)`. A face 2% across the frame is 5 px on a grid thumbnail and ~61 px at the cap here — and 112 is what the embedder samples, so this is the difference between an upsampled crop and a real one. `crop_px` records which, per face, as §7 intended. Capped at 3072 rather than truly full: `index_proxy` needs packed `f32` RGB at 12 bytes a pixel, so a 24 MP frame is ~288 MB and the fetch lanes hold one each. The constant is named and sits next to the reason. Orientation is applied **before** detection, not after downscaling. That costs a permutation of a larger buffer — ~15 ms against a ~150 ms decode — and buys the entire class of bug this codebase keeps having: detection then runs on the photograph rather than the sensor, so every box and landmark is already in the space the catalog stores and the overlay draws, with no second mapping to get backwards. One detector and one embedder serve every lane. The lanes are concurrent futures on a single thread, not threads, and inference contains no await, so a `RefCell` borrow never overlaps another — a pair per lane would duplicate ~16 MB of weights for no parallelism. Images with no face in them are recorded too. `face_index` records that detection *ran*, and zero is its most valuable value: without the row every landscape and document scan returns on every pass, for ever, and in a personal library that is most of it (§7a). The old store-only pass survives as `spawn_store_face_sweep` for `examples/face_index.rs`, which indexes a local store with no network. The settings copy no longer claims indexing reads "the photographs already thumbnailed above", and the audit line says "to fetch" rather than "awaiting a proxy", which had become a blocker that no longer blocks. Verified against the real catalog: the new work list returns 23,308 where the old one returned effectively nothing. 469 tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-27 18:51:53 +02:00
dtourolleandClaude Opus 5	131004393d	Follow the canvas from one display to the next ... The rest of FR-DSP-8. The develop session now carries the space its canvas is encoded into, and `render` composes for it instead of for sRGB — which is the whole of the change to the pixel path, because the output space was always a parameter of composition and always entered the structure hash. A display change is a recomposition. The space is set on the way into every render rather than pushed when the window moves, so a photograph opened while the window already sits on the second monitor is right on its first frame instead of flashing the wrong colour until the next poll. Which display that is comes from sampling the window's position and scale factor twice a second — Slint reports neither a move nor a display change — and re-surveying only when they differ. Settings shows what came back under ABOUT: the display, the space, why, and the other monitors, because the failure FR-DSP-8 names is one that is invisible from the display you are reading the page on. Fractional scaling: the canvas is now rendered at the physical pixel size of the box it occupies rather than the logical one, so the compositor presents it 1:1. At 1.25 it was previously handed 1600 samples to fill 2000 device pixels, and the softness that produces reads like a bad demosaic rather than like a scaling bug. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-27 18:44:51 +02:00
dtourolleandClaude Opus 5	57c0cc0d35	Keep the name typed into a cluster when the next one is opened ... Naming a cluster and moving straight to the next is the gesture this screen exists for, and it discarded the name every time. Two causes, both in the same four lines. `Field.text` is two-way bound to its `TextInput`. Binding it to `selected-name` therefore works exactly once: the first keystroke writes through the `<=>` and **replaces** the declarative binding, after which the field follows nothing. Switching clusters left the previous cluster's half-typed text on screen, attached to the new person. And `Field` only reported `accepted`, which is Enter. A name typed and then abandoned by clicking the next face never reached Rust at all. So `Field` gains an `edited` callback, the screen keeps the draft with the person it was typed for, and the draft is written when the selection moves or the screen closes. The field is then reset from a revision counter the screen watches. A counter rather than `changed selected-name`, because the name is not a key: naming six clusters "Anna" in a row never changes `selected-name`, and the field would keep the half-typed text from the cluster before. Nor `changed selected-person`, since accepting a namesake merge lands the user back on a person they may already have been on. The draft carries its `PersonId`. A reload can move the selection out from under a half-typed name — a merge arriving through a sync, a deletion — and applying it to whoever is selected now would rename a stranger. If the person is gone when the draft lands, it is dropped rather than resurrecting a row the rail no longer shows. `None` and `Some("")` are kept distinct. A user who cleared the field means to clear the name; a user who never touched it means to leave it alone. Collapsing those two erases names by walking past them. An implicit commit does not raise the namesake merge offer. That question is about a screen the user has already left, and answering it on their behalf while they look at the next cluster is not a question at all — the offer stays on the explicit submit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-27 18:32:00 +02:00
dtourolleandClaude Opus 5	2d95807542	Package the models on every platform, not just the phone ... The Android bundling landed the weights under that platform's asset directory, which was the wrong home the moment a second packager wanted them. `makepkg -si` produced a desktop install with no model at all — the same "no face model is installed" the phone used to show, for the same reason: nothing put the files anywhere the app looks. So `models/face/` at the root is the one copy, and both packagers read it: assemble-apk.sh bundles it as APK assets, and the PKGBUILD installs it to /usr/share/darkroom/models. Both refuse an LFS pointer rather than shipping a 130-byte file that fails inside the graph loader on a user's machine. `face_models` now searches three places, most specific first: the account's own directory, the shared user directory, then $XDG_DATA_DIRS. So a packaged pair is found automatically and a pair the user placed by hand still outranks it — which is what keeps a deliberate choice of weights from being overridden by an upgrade. $XDG_DATA_DIRS rather than a hard-coded /usr/share: that is the variable a distribution, a prefix install or a Nix-style store already sets to say where its data went, and its documented default is exactly the two paths that would otherwise have been hard-coded. Empty on Android, which has no such directories — there the APK's copy is unpacked into the shared user directory instead, because an asset inside a package is not a path anything can read from. Verified: the APK still carries both models at assets/models/, the PKGBUILD parses and installs from the new path, 467 tests pass. Includes the pkgver 0.6.0 → 0.7.0 bump that was already sitting uncommitted in the working tree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-27 18:05:23 +02:00
dtourolleandClaude Opus 5	725f7bf77f	Offer the merge when two people turn out to share a name ... Over-clustering is the normal state of a freshly indexed library — FR-CULL-10 says so — which means one person arrives as several groups and the user names each of them the same thing. Until now that produced several people called Anna and no way to join them: `identity::merge` existed, `faces::merge_people` existed with its redirect tombstone, and `merge-into(int)` sat in identity.slint declared, never emitted and never wired. The screen had a split button and no merge. So a rename that collides now offers one. Type a name another person already carries and a strip appears under the field: "Someone else is already called Anna (14 faces). Merge them?" Offered, not performed. `people.uuid` is the identity and the name is not — the schema comment on that column is explicit that two devices naming the same cluster independently is the case it was built for — so two people sharing a name is legal, and folding them together on a keystroke would be the screen making an identity decision on the user's behalf. That is the thing this screen spends a whole button avoiding. The rename always lands first, and declining leaves it exactly as typed. There is nothing to undo because nothing was done. Details that are not arbitrary: The comparison is trimmed and case-insensitive. "anna" on a phone keyboard and "Anna" on a desktop are one intention, and an offer that appeared only when the capitalisation matched would read as a bug. An empty name collides with nothing. Every unnamed cluster renders as "Unnamed (n faces)"; if that counted as a collision the offer would appear on every cluster in a fresh library, and accepting it would fold the library into one person. The newly-named person folds into the one that already held the name, not the reverse. The older person is the one other devices have seen and the one whose confirmations are more likely to be real. Selection follows the merge, because landing on an empty screen after a successful action reads as a failure. The offer is retired when the person changes, and when a refresh finds its target gone — merged from the other side of a sync, or deleted. An offer left standing would fold whoever happens to be selected now. A merged-away person is not a namesake: `faces::people` already excludes redirects, so the offer does not reappear the instant it is accepted. Four tests over the collision rules, and the existing 467 still pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-27 13:33:36 +02:00
dtourolleandClaude Opus 5	eaafacc3fb	Give the phone the model it had no way to obtain ... Face indexing was compiled into the APK all along — dr-ui takes dr-face with `inference` on every target, so SCRFD, alignment, MBF, calibration and clustering were all in there. What was missing was the weights, and on Android there was no way to supply them. Route C (docs/faces.md §2.2) says the user obtains the model and the app loads it. On a desktop that is a real gesture: drop two files in ~/.local/share/darkroom/models/ and indexing starts working. On Android it is not a gesture at all. `internal_data_path` is app-private, `run-as` needs a debuggable build, and the in-app fetch route C specifies was never built — so the settings page reported "no face model is installed" on every launch with nothing behind the message. Not "off until you supply weights"; off. So the shape-fixed pair goes into LFS under the APK's assets, assemble-apk.sh copies it into the package, and `android_main` unpacks it to the shared models directory before anything asks whether a model is present. Three things that are not incidental: The models directory is now shared across accounts rather than per-account. Weights are identified by `faces.model_id`, not by who is signed in, so two accounts had no reason to hold two copies — and the unpack runs before any session exists to key a per-account path off. `face_models` still prefers a per-account directory when one is populated, so anyone mid-migration keeps the ability to pin one library to its own pair. The unpack writes under a temporary name and renames. `face_models` decides availability on `is_file()` alone, so a copy truncated by the process being killed would leave a file that passes that test and fails inside tract — reported to the user as a broken model rather than a missing one. assemble-apk.sh refuses an LFS pointer. At ~130 bytes it looks exactly like a model to `cp`, and unchecked it reaches the device and fails in the graph loader instead of telling someone to run `git lfs pull` — the same guard dr-segment's build script applies to yolo26n-seg.onnx. The licensing half is unchanged and recorded in §2.2a: the InsightFace grant is research-only, this is a private repository and a self-installed build, and these files come back out before anything is published. The weights are still not a cargo build input — dr-face has no `models/` directory and no `embedded-model` feature, and nothing in the build reads them. The APK assembly step copies two files and is the only thing in the tree that knows they exist. Verified on device: both models unpack on first launch (2524817 and 13616095 bytes) and the APK carries them at assets/models/. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-27 13:18:07 +02:00
dtourolleandClaude Opus 5	b846b312b8	Run the formatter over the face branch before it reaches CI ... The merge of the SCRFD/MobileFaceNet work brought 69 rustfmt diffs across dr-catalog, dr-face and dr-ui with it, so `cargo fmt --all -- --check` fails on master and the Desktop job stops at its Format step — before clippy, the tests or the release build have run at all. That makes the whole desktop half of CI blind: a real compile error behind this would look exactly the same from the outside. There was nothing behind it, as it turns out — with the formatting fixed, clippy, the test suite and the release build all pass. Every .rs hunk is `cargo fmt --all` on the pinned 1.92.0 toolchain, not a hand edit, but it is worth being precise about what that moved, because it is more than whitespace. Besides reflowing signatures and call chains, rustfmt reordered the `pub mod` and `pub use` items in dr-face/src/lib.rs so the `#[cfg(feature = "inference")]` entries sort in place, added the trailing semicolon inside `let ... else { return }` bodies in identity_ui.rs, wrapped a bare closure body in braces in cluster.rs, adjusted trailing commas, and dropped a stray blank line at the end of identity_ui.rs. All of it is semantically inert; none of it changes behaviour. docs/traceability.md rides along because it has to. The matrix records each TRACES tag by line number, and reflowing develop.rs, lib.rs, faces.rs, identity.rs and identity_ui.rs moved them — FR-CAT-8, FR-CAT-9, FR-CULL-10, FR-DEV-3, FR-DEV-3a and FR-DEV-3c all shift by a line or two. The matrix was verified up to date on d777f7f before this commit, so this is drift these formatting changes introduced, not pre-existing staleness being swept up. Leaving it for a follow-up commit would hand traceability-check.yml a failure caused entirely by a whitespace change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-27 13:12:49 +02:00
dtourolle	d777f7f44d	Merge branch 'master' into worktree-faces-scrfd-mbf ... # Conflicts: # docs/traceability.md # ui/dr-ui/src/develop.rs # ui/dr-ui/src/segmentation.rs	2026-08-27 11:57:38 +02:00
dtourolleandClaude Opus 5	7e1c33ebed	Draw the subjects on the photograph, not on the sensor ... "Find subjects" would recognise a person on a portrait frame and then paint the outline into the hillside behind them. The detection was right and the mask was right; what was wrong was the picture drawn to show them. Instance masks live in sensor space, and correctly so — the generated shader samples them at `uv_src`, after the framing map, which is what keeps a mask on its subject through a zoom, a pan and a crop. The overlay is the one consumer that is *not* sampled by that shader. It is a flat image handed to the compositor to lay over a photograph that has already been through the framing map, so it has to arrive in the same space that photograph is in, and it did not. On a frame from a camera held sideways the outlines were drawn a quarter turn away from the subjects they described. `overlay_clip` had the same fault one layer down, and it is the more insidious of the two because it looks right. The crop and the viewport are fractions of the photograph as the user sees it — the prologue maps an output pixel through `crop_rect` *before* it unturns the frame — and they were being measured against the sensor's width and height. Two numbers, correct type, wrong axis. Both now go through `Orientation::into_shown`, so the overlay and its clip are in the photograph's space and the turn is the same one the render and the thumbnails make. Neither was noticeable until this week, and the reason is worth writing down: before the detector was given an upright frame it found almost nothing on a portrait photograph, so there was rarely an outline to be in the wrong place. Fixing the detector is what made this visible. Landscape frames were never affected, which is most of them, and is why an overlay that ignored orientation entirely survived this long. Verified on `_MG_9080.CR2`, a portrait frame of two people and a dog: the overlay was a 1599x1066 image drawn onto a 1066x1599 canvas, with the colour sitting in the mountainside above the subjects. It is now 1066x1599, and each outline is on the thing it names. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-27 09:46:48 +02:00
dtourolleandClaude Opus 5	ce201c7dd6	Name the two spaces a photograph lives in, so a turn cannot go the wrong way ... Every orientation bug this codebase has had has been the same bug: a turn of the right size applied in the wrong direction. That failure is worth naming precisely, because it does not look like one — a quarter turn applied backwards lands 180 degrees from right, so the result is a plausible transform of the picture rather than anything obviously broken, and on landscape frames it is not wrong at all. It was the straighten shear, and it was the segmentation overlay, and each time it was found by eye rather than by a test. The reason it keeps happening is that "rotate 90 degrees clockwise" cannot be checked by reading it. The reader has to hold in their head which of the two images is being rotated and which way the y axis runs, and there were four hand-written copies of the permutation to hold it for: the shader prologue, its CPU twin, the thumbnail path, and the segmentation. So nothing added here says clockwise, anticlockwise, horizontal or vertical. The functions say *which space they take and which space they return* — `into_shown` and `into_stored`, `source_pixel` and `shown_pixel`, `into_shown_rect` and `into_stored_rect` — and each takes the dimensions of the space it reads from, so no caller has to work out which pair it is holding. `StoredRect` and `ShownRect` are separate types because they are the same four numbers meaning different things, which is exactly the case where a mistake is silent: a shown rect measured against stored dimensions produces a rectangle in the wrong place, not an error. Underneath there is one permutation. `source_pixel` was already shared by the prologue and the thumbnails; `source_point` is its normalised twin, written beside it so the two cannot drift, and everything else is those two read forwards or backwards. `Orientation::inverse` is the group inverse rather than `4 - turns`: mirrors apply after the turn, so undoing means undoing them first, and a mirror seen from the far side of an odd turn is about the other axis. That is the diagonal-mirror case, tags 5 and 7, and getting it wrong renders as — again — 180 degrees. Three call sites lose their own copy: the thumbnail path, `dr-ui`'s segmentation, and `dr-gpu`'s `local` example. "Upright" now means one thing across the application rather than one thing per caller. The gate that matters most is `the_render_and_the_orientation_map_agree`. The shader prologue and `Orientation` answer the same question by different routes, and until now nothing checked that they answered it the same way. It now checks every EXIF tag against every user rotation and mirror on top of it, because the composition is where the two could agree singly and disagree together. The rest earn their place by having caught something. Writing these found two real errors in this commit's own new code before it ran anywhere: `shown_pixel` was handed the dimensions of the wrong space and overflowed, and the rect map turned the wrong way for the diagonal mirrors. A round trip that returns what went in is the only check worth having here, since every wrong answer is still a picture. No behaviour changes. The permutations are the ones that were already being applied; they are simply applied from one place now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-27 09:46:24 +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	f00b92a0e6	Turn face boxes back into sensor space before matching regions ... Faces are found on the thumbnail, which is cached the right way up -- the grid would lie on its side otherwise. Segmentation runs on a proxy rendered through a neutral edit graph, which carries no orientation and is therefore in sensor order. For anything shot in portrait the two differ by a quarter turn, so a face and the person containing it were being compared in spaces 90 degrees apart: no match, or worse, a match against somebody else's region. The transform goes on the face rather than on the proxy. Instance masks are defined in the proxy's space and sampled long afterwards, so turning that space would be a far larger change than naming a region warrants. Also two things the first screenshot of the running app showed that no test would have: 110 of 23,528 displayed as "0%", which reads as the feature having done nothing. One decimal below ten percent, and a floor so real progress never shows as none. The rail picked some near-black covers, because the largest face in a group is often the nearest one in a badly lit frame and a black square beside a name identifies nobody. It now cuts the best few and takes the first legible one, falling back to the largest when a person's every photograph is dark -- which happens, and showing it beats showing nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-26 23:28:52 +02:00
dtourolleandClaude Opus 5	61c4547b9c	Make the Identity Manager a peer of the library and develop ... It was reachable only from the library header, which made it a side trip rather than a mode. It is now reachable from develop's header too, beside the way back, because that is the same kind of move -- leaving this photograph for somewhere else in the library -- and a screen you can only reach from one of the other two is not a peer of them. Leaving returns to whichever screen opened it, and the button says which. A back button that read "Library" while returning to develop would be lying about the one thing a back button has to be right about. The develop session is only hidden, never torn down, so returning to it costs nothing and keeps the photographer's place. The header now matches the other two screens rather than using a close cross: three screens whose headers disagree read as three applications. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-26 23:14:17 +02:00
dtourolleandClaude Opus 5	b55812812a	Name segmented people from the faces already recognised in them ... The segmenter knows it found a person; the face index knows which person. Joining them turns "person" in the mask list into "Anna", which is the difference between a vocabulary of eighty COCO classes and one that includes the user's family. Selecting a subject in a group photograph stops being a guessing game between three identical rows. Containment, not IoU. A face is a small part of the person it belongs to, so a correct pairing has an IoU near zero and anything IoU-based would reject every true match. Confirmed names only. A suggestion is the system's guess, and printing a guessed name onto a mask region would launder it into a fact. Writing the tests corrected the design once: a tight head-and-shoulders portrait, where the face fills most of the person box, is the case where naming is most certain, not least. An earlier guard rejected exactly that and has been removed, with the reasoning left as a test because it is easy to get backwards a second time. The names hang on the develop session, set when the image opens because that is the one moment the catalog and the image id are both in reach. Every segmentation run afterwards picks them up for free, and a library with no face indexing behaves exactly as it did before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-26 23:10:46 +02:00
dtourolleandClaude Opus 5	d7a81375ee	List the steps, and let a photographer step straight to one ... Undo answers "take back the last thing", which is the question asked about a mistake just noticed. It is the wrong instrument for one noticed six adjustments later: eight presses, each changing the picture, with no way to see how far back the mistake is without passing through it. A step is a whole state, so arriving from six away costs what arriving from one does — which is what makes a row worth making clickable rather than decorative. `Edit::Discrete` had to go for the list to be worth drawing. Seventeen call sites recorded the same anonymous step, which is fine for deciding whether two changes are one gesture and useless for a panel: seventeen rows reading "Discrete" is not a history. Every variant now carries enough to name itself, and the compiler enumerated the sites that had to start saying so. A step that moved a parameter is still named out of the descriptor, so an operation added as a YAML declaration appears in the history correctly named with nothing written for it (FR-DEV-3c). Choosing a film stock was not undoable at all. The pick went straight to `choose_film`, which nothing on the history's path ever sees. `pick_film` records it, and is separate because the same call is also how a *restored* edit gets its tables back — recording that would push a step for the undo the photographer had just asked for. The list is rebuilt off a revision rather than off every redraw. A drag ends in a redraw per frame while folding into one step, so the unconditional version would tear down and recreate every row sixty times a second to arrive back at the list already on screen. The counter is process-wide: a per-instance one starts every photograph at the same number, so a frontend holding "the revision I last drew" would keep the previous image's steps on screen — invisible while every image opens with one identical row, and a wrong-photograph bug the moment persisted history means it does not. The step names that no descriptor can supply are constants with a roll, and a test walks the roll rather than a second copy of it. `resolve` splits so that "is this catalogued?" can be asked: `derive` turns `history.mask_toggled` into "Mask Toggled", which names a field rather than an act and, being perfectly readable, is a mistake nobody would look at twice. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-26 23:04:35 +02:00
dtourolleandClaude Opus 5	b89f1cfece	Snapshot the whole edit in the history, so a drawn mask can be taken back ... The undo stack snapshotted a `Preset` — the parameter map — and a mask layer is deliberately not a parameter. So drawing one changed nothing the history could see: `record` returned `false`, no step opened, and the layer the photographer had just painted had no way back. The interface went on calling `record` in good faith, including from the mask controls, and nothing failed. A film stock went missing the same way. The snapshot is an `EditState` now, so the history is complete by construction rather than by anyone keeping a list in their head. `undo` and `redo` return a `Step` rather than a `bool`. Stepping is not the only outcome a caller has to act on — a step across a change of film leaves the graph without its tables, and only the caller can bake them — and a `bool` would let that be dropped by writing nothing at all, which is the shape of mistake this module had already made once. `DevelopSession` settles the debt either way; a step that found nowhere to go is left alone, since clearing the film because undo hit the floor would take the stock off the picture. Five tests, all of which fail against the old snapshot: a drawn layer is undoable and redoable, a layer's own settings are a step of their own, a change of stock is a step and names what it needs baked back, clearing the film is undoable, and an exposure move does not deep-copy the mask stack. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-26 22:54:55 +02:00