184 Commits

Include renames

Author	SHA1	Message	Date
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
dtourolle	30162e80df	Merge branch 'master' into clarity-reduced-base ... # Conflicts: # docs/traceability.md	2026-08-29 13:27:04 +02:00
dtourolleandClaude Opus 5	bff95e25ad	Let clarity's base be computed where it is still fully determined ... Clarity's Gaussian sigma is 1.2% of the frame's shorter edge, so its radius is a property of the viewport: 52 render pixels at 4K, two separable passes of 105 taps each over 8.3 M pixels. That measured 33.9 ms — seven times the entire fused point chain, for one slider — and is docs/technical-debt.md TD-4. A detail pass may now declare `output_scale`, and clarity's base is computed on a grid a quarter the size on each axis. The pass that combines needs the blur *and* the full-resolution colour, and a colour that has been through a quarter-scale target is no longer full resolution. So a scaled pass cannot simply join the ping-pong: there are two chains now. The full-resolution one carries the colour and no scaled pass touches it; the reduced one carries the base and reaches the combining pass through a second binding as `reduced_at()`. The reduce is a dispatch of its own rather than something the first blur half does on the way past, and that is the whole difference between this and the strided kernel the module documentation rules out. A stride samples an image that is not band-limited and aliases high-frequency content down into the base, which is then subtracted, and arrives in the output as mottling across smooth gradients. This band-limits first and samples after. What is discarded is content the base could not represent at any resolution, because a Gaussian at sigma = 26 px holds nothing above one cycle per 26 px and the quarter-scale grid carries one per 8 — so the reduced base is not an approximation of the full-resolution one, it is the same function sampled where it is still determined. Which is also why the scale belongs to the band rather than to the stage. Texture's sigma is a decade finer, so the reduce pass's own box would be wider than the Gaussian it was prefiltering; texture never reduces. And clarity steps 4 -> 2 -> 1 as sigma falls, because a quarter of a small sigma is not a Gaussian either — the case that gives up is the one that was already cheap. `radius` stays in each pass's own pixels and `ComposedDetail::radius` multiplies it back up, so 13 reduced pixels at scale 4 still report the 52 render pixels a tile would have to be grown by. The halo a scheduler sees does not move. The halo tests pass unchanged, which was TD-4's stated bar; they render at 1024 px and so exercise the reduced path rather than stepping around it. Added `crossing_the_reduction_threshold_does_not_change_the_picture`, because nothing yet compared the reduced form against a *less* reduced one — every other test measures one form against itself. It renders the same edit either side of the 4 -> 2 step-down and holds the peak excursion to 0.03 stops and the reach to 2% of the frame. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 13:12:29 +02:00
dtourolle	3b5d564495	Try every GPU, not only the fastest one ... `request_adapter` with `HighPerformance` returns one adapter and no second chance. That is right on a healthy machine and wrong on one with a sick GPU, which is not rare: observed 2026-08-29 on a laptop whose discrete card had hit an NVRM assertion failure and a fullchip reset. The driver still advertised it, wgpu dutifully picked it as the highest performing, and the process died on it — while a working integrated GPU and a working external card sat unused in the same enumeration. A photo editor that will not start because the *fastest* GPU is broken, on a machine holding two that are not, is worse than a slow one. So: enumerate, order by preference, take the first that yields a device. The ordering reproduces what `HighPerformance` meant, so a healthy machine picks what it always picked and pays one enumeration for it. A CPU adapter sorts last rather than being excluded — software rendering is a poor experience and a working one. Which GPU to prefer is now a policy rather than an assumption, because the fastest is not obviously the right one. A 24 MP frame is ~96 MB of RGBA and every upload and export readback crosses PCIe on a discrete card, where an integrated GPU shares memory and crosses nothing — and does not empty a battery. Measured before choosing a default, on this machine's Iris Xe against its RX 5700 XT. The fused colour pass is within 1.5x, which is the shape shared memory suits. The neighbourhood stage is 5-8x slower, and that decides it: clarity at 1920x1200 costs 20 ms on the iGPU, over the budget on its own at the smallest size tested. So `Performance` stays the default and `Efficiency` is offered rather than chosen (`DARKROOM_GPU=integrated`). docs/frame-budget.md carries the table, and says what it does *not* show: the harness renders from a resident texture and never uploads or reads back, so the transfer cost an iGPU avoids appears in none of it. Import, export and the thumbnail sweeps may well go the other way. What this cannot fix: a GPU sick enough to accept `request_device` and segfault afterwards, which arrives as a driver crash rather than an error. It moves the boundary from "the preferred adapter is unusable" to "unusable and dishonest about it".	2026-08-29 12:43:10 +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	b2250cc460	Measure a regroup on the tablet, not just on the desktop ... The GPU question needed a number nobody had: how a regroup divides on the hardware whose CPU is weakest. dr-face carries no weights and touches no display, and dr-catalog's example needs only a catalog file, so both run under adb shell against a copy of a real library. On the same 18,143 faces — desktop against the tablet — scan 0.96s / 2.61s, agglomerate 1.69s / 2.16s, score 0.26s / 0.40s. The scan is half the pass on the tablet and under a third on the desktop, because twenty cores of AVX2 pull ahead of NEON much further than the merge engine's single-threaded hashing does. So a GPU GEMM is worth roughly 2× a regroup on the tablet and 1.5× here, and it is the tablet that should decide whether it is built. The two architectures agree exactly: the same 1,531,969 evidence pairs, the same 2,518 groups holding the same 16,246 faces, the same reliability table. That is a better check on the NEON kernel than the unit test can be. Two instruments, both read-only: the example now prints its phases, and dr-face gains scan_bench, which needs no library at all and so can answer "how fast is this machine" on a device with nothing on it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-29 12:16:48 +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	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	702d83c218	Measure the borrow, rather than asserting it bounds the disk ... The claim that hydration-as-a-borrow makes peak disk the working set rather than the library was so far an argument. `--example vfs_cycle` runs it: 100 photographs of 25 MB, 90 dehydrated, a pass over all of them through the real engine. Peak 275 MB — the resting set plus one photograph — against 2,500 MB had the pass simply fetched everything. Back to 250 MB afterwards, and all ten files the user already kept still there, which is the half of the contract that matters more. Recorded in docs/storage.md §6.3 and ARCH §9.0a, because a bound argued from a number nobody measured is one that gets quietly lost.	2026-08-29 09:57:53 +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	f41b3f6e8e	Answer "what would the other device end up with" without the other device ... A tablet showed 200 of a person's 611 faces after syncing, and the obvious suspects — that suggestions deliberately do not travel, that the cross-device face match was too strict — were both wrong. Finding that out meant reading a catalog on a release-signed Android build, which cannot be done. So this stands the second device up locally: an empty catalog, given the images a scan would have found, the shards adopted into it exactly as a sync does, and the real catalog merged in as the remote. Then it counts, per person, against what the source holds. person source here Catherine 611 611 Me 242 242 Ian 219 219 Which settled it: the merge carries everything, and the shortfall was transfer — shards that never finished arriving. Worth keeping, because "did the sync lose this or has it not got here yet" is a question that will come up again, and guessing at it cost most of an evening. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-28 23:09:28 +02:00
dtourolleandClaude Opus 5	2c84aa1224	Write face shards the way the rest of the catalog writes ... The face store was the one part of the catalog still on SQLite's default rollback journal at `synchronous = FULL`. The catalog itself runs WAL at `NORMAL` (`schema::configure`) and so does the thumbnail store; nothing decided this one should differ, it was simply never set. Measured on this project's own filesystem, that is **21.3 ms per commit against 0.05 ms** — four hundred times. And an export commits four times per photograph: the shard's transaction, then three separate autocommitting writes to the index. Ten thousand images is on the order of fourteen minutes spent doing nothing but waiting for fsync, before a byte goes to the server. That is the "checking faces…" that appeared to hang. So: WAL and `synchronous = NORMAL`, matching the rest, and the three index writes fold into one transaction. `NORMAL` is the same trade the catalog makes — a shard is derived data, and losing the last commit to a power cut costs one image re-exported. WAL brings an obligation with it, because **a shard is uploaded by reading its file**: the newest commits live in a `-wal` sidecar that no upload sends, so without a checkpoint the server would receive a database missing exactly the faces just written, and a peer would adopt it and see nothing wrong. `checkpoint` folds the logs back in, with `TRUNCATE` rather than the default passive mode, which gives up when a reader holds the log and would leave the same gap while reporting success. Two tests: that the store is in WAL like everything else, and — the one that matters — that a checkpointed shard copied *without* its `-wal` still holds every face. That second one fails without the checkpoint, which is how it was confirmed to be testing something. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-28 23:08:10 +02:00
dtourolleandClaude Opus 5	cf614efa61	Send a re-indexed image's faces to the other devices ... `export_to_shards` asked `store.contains(file_id)` and skipped anything the shard store had already heard of. So an image was exported exactly once, and re-indexing it updated the catalog and nothing else — every other device kept the first answer for ever. That is not hypothetical. This library was re-indexed after the detection floors changed and crops were added, going from 1,807 faces to 15,194; the shard store still held the original 1,807, written before any of it. Nothing the re-index produced could reach another device. The shard's `indexed` table now carries the catalog's own `indexed_at`, and the export compares against it. A re-indexed image goes again; an unchanged one still costs nothing. Copied from the catalog rather than stamped when the shard is written, because a shard-local write time advances even when nothing changed and could not answer the question. The column is nullable so a shard written before it still reads: absent means "cannot vouch for it", which forces one re-export and then settles. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-28 09:27:16 +02:00
dtourolleandClaude Opus 5	c7cdf7e5f0	Merge: face quality gates, and identity filters that combine ... Two things the People screen was missing. Faces were being indexed at any size and any sharpness — 40 pixels on the box and no blur gate at all — so most of what the library held was background strangers and motion-blurred passers-by, and the blurred ones were quietly bridging unrelated clusters. Both floors are now measured on the real library with `face_index --quality` rather than guessed: 64 source pixels across the aligned crop, and a contrast-invariant sharpness of 0.020. And the grid could only ever be narrowed to one person, which cannot express "the pictures the two of them are in together". The filter now holds a set with a union/intersection mode, built a person at a time from the Identity screen and taken apart chip by chip on the filter bar. fmt, clippy -D warnings and the full workspace suite pass on the merge. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-27 22:23:37 +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
dtourolle	329c388d30	Merge: each canvas overlay goes home to its own domain ... Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> # Conflicts: # docs/traceability.md	2026-08-27 21:43:31 +02:00
dtourolleandClaude Opus 5	4fa914cbb1	Send each canvas overlay home to its own domain ... The crop rectangle, the gradient handles and the repair discs were 480 lines inside `canvas-area` in app.slint, while the panels that drive them already lived in adjust.slint, masks.slint and spots.slint. spots.slint even opens by describing "what is drawn over the photograph, and what a finger can take hold of" -- which was not in it. They stayed behind because all three are positioned against `shown-*`, the fitted image rect the develop view derives because Slint does not report it. That is now the interface rather than the obstacle: each overlay is *given* that rect as its own bounds, so every position inside is a plain fraction of `root.width`, and none of them reaches out to `canvas-area` for an origin any more. GradientHandles joins MaskPanel in masks.slint and SpotHandles joins SpotPanel in spots.slint, each file now holding one domain's panel and its canvas overlay together, matching masks_ui.rs and spots_ui.rs. CropOverlay gets crop.slint of its own rather than growing adjust.slint. Arithmetic is unchanged: the old fraction-x subtracted shown-x from a coordinate measured relative to canvas-area, and the new one measures from an origin that already is shown-x. The handles stay unconditional rather than gaining an emptiness guard, so the repeater identity that spots_ui::sync_handles warns about is untouched. app.slint: 2834 -> 2490 lines. Verified by running the desktop app on a photograph, with the crop overlay's guard temporarily forced open so all three instantiate -- no binding loop, no panic. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-27 21:35:08 +02:00
dtourolleandClaude Opus 5	ffc9e1aea7	Give the develop view's chrome a file of its own ... StatusBar and InfoPanel sat above AppWindow in app.slint, which read as though they were part of the application shell. They are not: neither is instantiated anywhere but the develop view, and the shell's actual job -- choosing which of the five screens is up -- is easier to follow without two unrelated components standing in front of it. Moved verbatim to develop.slint, matching src/develop.rs. No behaviour change; app.slint loses 232 lines and gains one import. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-27 21:33:17 +02:00
dtourolleandClaude Opus 5	c10dca984f	Regroup the library without stopping the window ... Pressing Regroup on a real library did not come back. Clustering 1,813 faces is the textbook agglomeration — compute every pairwise cosine, then repeatedly scan all live group pairs, score each with average link, and merge the best — and the scan is inside the loop. Each merge rescans every surviving pair, and each score is recomputed from scratch over every cross pair. Some 1.6 million pair scores per merge, some 700 merges to do. Three changes, none of which alter the answer. Only above-threshold pairs can ever matter. An average that reaches the threshold must have at least one term at or above it, so two groups with no qualifying pair between them can never merge — not now, and not after any sequence of merges, since merging only adds terms. The new `neighbours` module produces exactly that sparse list: 7,875 pairs rather than 1.6 million on the reference library. It also means the n^2 matrix is never materialised, so memory goes from O(n^2) to O(edges) — 2.5 GB to a few hundred KB at 25,000 faces. Merges cannot cross components, so the connected components of that graph are independent problems: four hundred small agglomerations instead of one large one. Average link is additive — sum(A u B, C) = sum(A, C) + sum(B, C) — so a merged group's scores follow by addition. Kept as running (sum, count) per adjacent pair, a score costs one division instead of a nested loop, and a heap with lazy invalidation replaces the rescan. Measured on the reference library: 0.28s, release, for all 1,813 faces. An exact ANN index was tried and removed, and neighbours.rs records why so it is not rediscovered as a good idea. IVF with a triangle-inequality bound is exact and prunes beautifully on synthetic clusters; on real embeddings it prunes *nothing* — 946 of 946 cell pairs survive. Median pair angle is 88.5 degrees and the merge threshold is 66.2, so the bound needs cells of radius under ~10 degrees, but two photographs of the same person sit 36-60 degrees apart. No ball-based partition of a 512-d near-orthogonal space can be tight enough. So the scan stayed exhaustive and got an unrolled dot product and its blocks spread across cores instead. Correctness is held by keeping the old implementation as an oracle: three tests run both engines over the same population — plain, under co-occurrence and anchor constraints, and with a size-weighted calibration — and assert the clusters are identical. Determinism is asserted at a size where the threaded path is in play. 62 tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-27 21:16:55 +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
dtourolleandClaude Opus 5	5e4c7424ed	Fail the build when the interface names an operation ... FR-DEV-3a is the property the declarative pipeline rests on: a node in `ops/` is one file because nothing in `ui/` has to learn about it. It was true — all fifteen ids grepped across `ui/` yield one hit, a localisation test — and it was held by discipline alone. That is the wrong mechanism for it. The failure is silent and cumulative: special-casing one operation to fix a layout problem is defensible on its own, and by the fifth the panel names half the chain and "a new operation is one file" has stopped being true without any single commit having broken it. Nothing would have told us. The ids are read from `ops/*.yaml` rather than listed, so a node added tomorrow is covered without anyone remembering this file — the same reason `traceability` parses its denominators from `requirements.md` at run time. Two decisions worth recording, because both are the difference between a test that holds and one that gets deleted: **Only string literals count.** `texture`, `contrast` and `clarity` are also ordinary graphics and English terms, and `texture` appears throughout `dr-ui` meaning a GPU texture. Matching bare words would fail constantly for reasons unrelated to the invariant. **`#[cfg(test)]` items are exempt, and finding them needs more care than it looks.** The first version cut each file at the first textual match of `#[cfg(test)]`, which in `develop.rs` is a *doc comment discussing the attribute* at line 969 — it read 18% of the most important file in the scan and passed. It now matches the attribute only as a whole line and skips the item by brace depth, and `MIN_SHIPPING_FRACTION` fails the test outright if the scan ever swallows the file again. The dangerous failure here is not a false alarm, which someone investigates; it is examining nothing and reporting success. Verified both ways: it passes on the tree, and an `"exposure"` planted at develop.rs:3635 — past two `#[cfg(test)]` attributes, exactly where the first version was blind — fails with the file, the line and the reason. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-27 19:33:20 +02:00
dtourolle	fb1b44ce47	Merge branch 'worktree-agent-a1e5c8cb565255f5b' into master ... # Conflicts: # docs/traceability.md	2026-08-27 19:21:10 +02:00
dtourolleandClaude Opus 5	8202c05d9d	Format the zoom test the way the gate asks for it ... Whitespace only. `cargo fmt --check` is a required step and the FR-DSP-5 test arrived disagreeing with it — kept as its own commit so it can be skipped wholesale rather than read for a change that matters. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-27 19:20:23 +02:00
dtourolleandClaude Opus 5	0cd3ef1b3f	Run a node declaration without compiling it ... `ops/*.yaml` plus `build.rs` has been the class-1 plugin format since the declarative nodes landed — it was simply resolved at build time. Nothing about a declaration requires the compiler: everything it produces is data plus a WGSL string, and the composer already assembles WGSL at run time from whatever operations are active. So this is not a new mechanism. It is the existing one, loaded later (FR-PLG-2). `DeclaredOp` implements `Operation` from an owned `Declaration` — one interpreter over many declarations, where `build.rs` emits generated code per node. The generated path stays, as FR-PLG-2 says it should: a generated `match` is faster than an interpreted one, the built-ins' declared `tests:` have to run under `cargo test`, and generated source is inspectable in a way an interpreter's state is not. **The reader is now one file, read by both.** `src/declared/decl.rs` and `src/declared/expr.rs` are `#[path]`-included by `build.rs` as well as being modules of the crate, and they produce a neutral `Declaration` that names no Rust type. The build script's job is reduced to *rendering* that declaration as Rust; `DeclaredOp` converts the same declaration into descriptors and `Expr::eval` walks the same tree the renderer writes out. There is one grammar, one set of validations and one set of error messages, so "a plugin is the same kind of thing as a built-in" is structural rather than aspirational. What remains genuinely written twice is the pair of backends — an arithmetic node rendered as Rust here and evaluated there — and that is what the parity test stands between. `tests/declared_parity.rs` parses every built-in declaration at run time and asserts the composed WGSL is byte-for-byte what the generated implementation produces, with the uniform block bit-for-bit identical, at both ends of every parameter's range and at four interior points; then again over the whole develop chain with the declared nodes swapped in, which is what covers uniform slot ordering and helper de-duplication between operations. A third test asserts the declared and hand-written nodes partition `ops/` between them, so coverage cannot shrink silently. Bit-for-bit rather than within a tolerance, because a tolerance is where a real divergence hides. The one thing that had to be got right for that to hold is number literals: `expr::as_f32` rounds a decimal exactly once, through the same shortest-round-trip text the compiler is handed, rather than rounding an `f64` a second time. Not in scope, and deliberately untagged: load-time WGSL validation (FR-PLG-11), id namespacing, a plugin directory read at startup, and pass nodes (FR-PLG-2a). Those are separate work, and tagging them from here would be the overstatement the spec's own §7 warns about. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-27 19:08:34 +02:00
dtourolle	3b5bba62f1	Merge branch 'worktree-agent-aa9f4356c13893373' into master ... # Conflicts: # docs/traceability.md	2026-08-27 19:08:32 +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	13deaa2fbb	Assert the frame budget, and commit the numbers behind the FR-DSP-2 verdict ... FR-DSP-3 states a latency requirement and nothing checked it, which makes it a wish. This adds the check and the measurements it guards. `docs/frame-budget.md` is the bench's output with the reading of §2's decision rule attached. The short version: every point-operation chain at every viewport size, fit and at 1:1, is inside 16 ms at the 99th percentile — the widest is 4.5 ms of GPU at 4K — so FR-DSP-2 should be rewritten rather than implemented. The measurement did find a stage that misses the budget, and it is the one §2 predicted: clarity's 52-pixel separable kernel costs 34 ms at 4K. Tiles make that worse rather than better, since a tiled convolution reads a halo per tile; the fix `local_contrast` already names for itself is a base computed at reduced resolution. The test guards the fused path and says so, at length, rather than quietly excluding the expensive stage and letting the tag imply otherwise (§7). What it asserts is exactly the claim the recommendation rests on: one dispatch over a viewport-sized target, at a full chain, is comfortably inside a frame. Two things the numbers forced: - The two cases are one `#[test]`. As two they ran on a thread each, contended for the same device, and took the 1:1 case from 2.5 ms to 14.9 ms — a measurement of the harness that would have flickered either side of the budget forever. - The CPU half of the frame is judged only in an optimised build. Composition is real per-frame work on the UI thread and belongs in the budget, but the workspace builds its own crates at `opt-level = 0` in dev and `cargo test` is a dev build, so measuring it there measures rustc. The GPU half is asserted either way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-27 19:00:45 +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	69dccaa061	Count the platform layer's traceability tags ... `platform` was missing from the traceability scanner's source roots, so every TRACES tag in `dr-plat` — the secret store, volume discovery, and now display-profile acquisition — was invisible to the matrix. The FR-PLAT-* family is exactly what that crate exists to satisfy, so the omission understated coverage by the requirements it was meant to count. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-27 18:46:59 +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	7235972ca4	Measure what a frame costs, so FR-DSP-2 is decided by numbers ... `docs/display-and-extension.md` §2 fixes a decision rule in advance: if the 99th percentile of a frame sits inside 16 ms, tiled computation is rewritten as a scheduling concern for export rather than built on the interactive path. Nothing in the tree could answer that, so the rule had nothing to act on. This is the instrument. It renders a 60 MP synthetic source through the real `render_detailed` at three viewport sizes and four chain lengths, fit and zoomed to 1:1, and reports nearest-rank percentiles rather than means — a slider drag is judged by its worst frame. Three things it does that a simpler timer would not: - It separates the fused pass from the neighbourhood stage. "Every operation active" mixes one dispatch together with a chain of convolutions, and §2's question is about the first of those. `point` is every operation that contributes a fragment to the fused shader; `all` adds the four with kernels, and M3 times those alone by moving only a detail parameter so `render_detailed`'s colour reuse skips the fused dispatch. The reuse is reported rather than assumed — the `colour` column counts fused dispatches and must be zero for an M3 row to mean what it says. - It times the CPU half separately. Composition runs per frame in `DevelopSession::render`, so it is inside the budget whether or not anyone has looked at it, and if shader assembly were the expensive half then no tile scheduler could help. - It builds the "every operation" chain from `EditGraph::capabilities` rather than from a list, so declaring a new node does not quietly turn that row into a shorter chain wearing a longer chain's label. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-27 18:32:00 +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	8155f5276e	Spec how the display contract closes, and how the pipeline opens ... Two families were the weak points: FR-DSP at 37%, which is the architecture's central performance claim, and FR-PLG at zero. They are one document because the same property decides both — the pipeline composes its work from declarations, which is why the display path is fast and is also, already, most of a plugin format. Two findings change the size of the job. **Some of FR-DSP is done and untagged.** Zoom already meets FR-DSP-5: the sampled region shrinks while the render target keeps its size, so zooming raises the resolution the pipeline works at — arrived at without tiles. **FR-DSP-2 may not be worth satisfying as written.** It predates the fused shader and assumes a chain of passes over a large buffer, where recomputing per frame is ruinous and tiles are the way out. What exists composes every active operation into one dispatch over a viewport-sized target. So the spec fixes three measurements and a decision rule *in advance*: if a frame sits inside budget at the 99th percentile, the requirement is rewritten rather than implemented, and tiling becomes what it actually is here — a scheduling concern for export, which already runs off the frame path. Writing a tile scheduler the design does not need would be the most expensive way to find that out. **The plugin format already exists**, resolved at build time: `ops/*.yaml` through `build.rs` produces something indistinguishable from a hand-written operation, and everything it produces is data plus a WGSL string. The blocker is one line — `descriptor()` returns `&'static` — and the rest is an interpreter over declarations, load-time WGSL validation, and namespaced ids, because the sidecar stores parameters by id and a collision is a wrong edit silently applied. Recorded last and deliberately: traceability counts a tag, not a behaviour. `FR-DEV-8` is tagged against plumbing a future spot-removal op would use and `FR-DEV-7` against a history row for a frontend that does not exist, so 51% is an overstatement of unknown size. Every requirement this plan closes should be closed by a test that fails if the behaviour is removed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-27 13:28:39 +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