758436cc28a6d1447f89ccffe4224ce6b4eb2fd9
100
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
758436cc28 |
Keep the runner's borrow alive as long as the connection it reads
The first compile this branch had. One borrow error, in the four-thread contention test: the `Runner` was the block's tail expression, and a tail's temporaries are dropped after the block's locals, so it outlived the `conn` it borrowed. Bound to a local, with the ordering rule written down beside it -- it is exactly the shape someone tidies back. Everything else stood: clippy clean at -D warnings, and all 18 runner tests pass, including the four-thread four-connection claim and the `UPDATE ... RETURNING` rewrite the author flagged as the riskiest line in the diff. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
846a249156 |
Drain the queue that nothing has ever drained
`jobs` has been a complete durable work queue since the catalog was written, and nothing has ever taken a job out of it. `claim_next`, `complete`, `fail` and `recover_orphaned` had no callers outside their own tests; `enqueue` had three. So the table grew one row per photograph and kept it forever, and FR-PLAT-AND-3's resumability was a property of code that never ran. `runner` is the missing half. It owns no thread, no clock and no policy, and that is the whole design: on Android the process does not decide when background work may run. WorkManager does, subject to Doze, battery saver and FR-NC-6's network constraints, and it revokes permission mid-job by calling onStopped(). So the runner exposes `run_one` — claim, run, record — and `drain`, which repeats it against a budget, a deadline and a cancellation flag the host owns. A `Worker.doWork()` with ten minutes calls drain with a deadline; a desktop idle pass calls it with none. That is the seam the Android service plugs into, and it needs no Android to test. Handlers are supplied from above, because the catalog knows what needs doing and nothing about how: a thumbnail needs a decoder and a fetch needs a network stack, neither of which belongs under core/dr-catalog. A runner claims only kinds some handler declares, so a queue holding work this device cannot do is left alone rather than failed five times. Four outcomes, and only two of them are the job's fault. Done deletes the row; Retry backs off; Abandon gives up now, for a failure no retry can fix; Interrupted releases the claim with its attempt refunded and ends the drain, because the host stopped rather than the job — five backgroundings in a row must not mark good work as failed. Process death is the fifth and cannot report itself, which is what `recover` is for. Recovery is called from `show_catalog_now`, which is the one place a catalog is opened for a session and already returns early if one is open. It has to be exactly once and before any worker starts: there is no owner column, so a second pass while a worker held a claim would take it away. The attempt a dead claim consumed is deliberately kept — a job that takes the process down with it is indistinguishable from one that fails, and the attempt counter is the only evidence that survives a death. The tests cover claiming under contention twice over: sequentially across two connections, and with four threads on four connections against one catalog on disk, asserting every job ran exactly once. Plus completion, backoff, giving up, abandoning, interruption, budget, deadline, cancellation, and a job orphaned by a simulated crash being reclaimed and run once rather than lost or repeated. Not wired to a handler yet, and deliberately not: the only enqueue site the app actually reaches is the remote scan's, whose thumbnails are already served by the async grid worker, and `walk`'s two sites are reachable only from the scan_local example. Inventing a handler to make the plumbing look used is how a requirement comes to read as covered by code that does not implement it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
d63872e5a9 |
Make a claim one statement, and give the queue what a runner needs
The claim was a deferred transaction around a SELECT and an UPDATE, and under a single connection that is fine. Under two it is not what it looks like: the SELECT takes only a read lock, the UPDATE tries to upgrade, and in WAL a worker that read the same snapshot as another gets SQLITE_BUSY_SNAPSHOT on its write. That is not an error a busy handler can retry away — the fix is to roll back and start over — so the queue was "safe" only in the sense that the loser failed loudly instead of taking a job someone else was holding. `UPDATE jobs SET state = 1, attempts = attempts + 1 WHERE id = (SELECT ...) RETURNING ...` is one statement and so one implicit transaction that takes the write lock immediately. Two workers serialise, the loser waits out its busy timeout, and neither can see a row the other already holds. The existing tests are unchanged by it, because from one connection the two forms are indistinguishable — which is exactly why it was never noticed. The rest is the surface a runner has to have and did not: - `claim_next_matching` takes only kinds a worker can actually do. Without it a device with no connector claims `FetchOriginal`, fails it, and pays five wakeups and five backoffs per photograph to reach a conclusion known before it started. Filtering after a claim cannot work: the claim has already marked the row running. - `abandon` gives up now, for failures no retry can fix. `fail` uses it for its own MAX_ATTEMPTS branch, so there is one statement that ends a job. - `release` hands a claim back with its attempt refunded, for a worker that is being stopped rather than a job that is going wrong. `attempts` stands in for the owner column the table does not have: it is bumped by every claim, so a stale worker's release matches nothing and changes nothing. - `reap_orphan_subjects` deletes jobs whose photograph is gone. Coalescing keeps the table one row per unit of work and nothing ever shrank it when the work stopped existing. `ScanFolder` is excluded because its subject is a folder id, and joining that against `images` deletes by coincidence of numbering — hence `JobKind::subject_is_image`, and `JobKind::ALL` so the next kind added cannot quietly fall out of the filter. - `counts` is the number a foreground service's notification is built from. One behaviour change worth stating: a kind this build does not recognise is now parked with an error rather than read as `ExtractMetadata`. The old `unwrap_or` would have run a job of an unknown kind as some arbitrary known one, which is worse than not running it at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
a2a0131693 |
Merge: face grouping the photographer can tune, people on the filter bar, and a tap that means it
Four fixes to the identity work, and one to the grid. The name field lets the keyboard go when a name is finished, instead of leaving it up over the faces the user pressed Enter to get back to. Filtering to two people at once has worked since people became a selector term and was unreachable behind a two-screen round trip. It is a tray on the filter bar now, where filters are. The merge probability and the smallest group the clusterer will call a person were constants tuned on one library. They are settings, edited beside the Regroup button that applies them, with a read-only preview that answers what they would do to *this* library. And a hand brushing past a photograph no longer opens it: a finger has to stay down long enough to have meant it, on a scale between the graze and the hold that starts a selection. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
b9891e2c04 |
Merge master into tablet-selection
Two real conflicts, both from work that landed either side of the same lines rather than against them. `lib.rs`: the settings controller was hoisted above the People screen's wiring, and Android's thumbnail-tier eviction registered itself at the same point. Independent, so both stay. `library.rs`: manual collection ordering and burst folding each added a clause to the same two queries. The scoped range read now carries both — the folding matters there for one step further on than it does in the grid, because a collapsed burst is one cell, so an ordinal counted over a list still holding every frame names a photograph several places away from the one the user pointed at. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
23c74d7063 |
Put the grouping dials where the regrouping is
The merge probability was `dr_face`'s constant and the smallest group was a bare `< 2` in the clustering pass. Both were tuned on one library — 1,813 faces of one photographer's family — and the quantity they optimise is a property of the population, not of the model. A household at close family resemblance and two thousand strangers at a wedding want different answers, and neither of them is the reference library. The doc comment already conceded the point and pointed at `face_index --tune`; a photographer does not have a terminal. So they are `FaceSettings` now, saved per device beside the cache budgets and edited from the People screen — beside the Regroup button that applies them and the rail that shows what they did, because a value changed three screens away from its effect is one nobody can tune. Moving them is safe by construction, which is why nothing asks for confirmation: a regroup writes only the suggested half, and confirmations, names and ignores enter as anchors and come back unchanged. The smallest-group rule is applied only to groups the system invented — a group the user named or set aside survives it whatever its size, because a display preference does not overrule a judgement. **Withdrawal, without which the setting does nothing visible.** Raising the smallest group stops the pass creating small groups; it does not remove the ones a previous pass made, because those still hold their suggestions, so they are not empty, so the prune leaves them. The pass now releases every unanchored face it did not place before pruning. And a dial you cannot see the effect of is not a dial. "What would this do?" runs the same population through the clusterer without opening a transaction and reports groups, faces grouped and largest group — one row of `--tune`'s table, on the user's own library, on a worker thread. The line leads with the group count because that is the number that says which side of the right setting you are on: it climbs as fragments are gathered into people and falls as separate people start being welded, while the grouped-face count rises straight through both. The preview parks its poll timer in a slot of its own. A preview and a regroup are allowed to be in flight together, and sharing the sweep's single slot would have the second to start drop the first's timer — visible as a Regroup that finished on its worker and never said so. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
a4b9deaf96 |
Put the people filter where the filters are
Narrowing the grid to two people at once has worked since people became a selector term, and it was effectively unreachable. The only control that could add a second person lived on the People screen, behind selecting them there, and it appeared only once the grid was already narrowed to somebody — so "photographs with both of them" needed a two-screen round trip the user had to guess at. A filter belongs on the filter bar. A "People" chip there opens a tray of everyone the library knows; tapping a name adds or removes them, and the any/all chip beside it — already there, and already the thing nobody found — now has something to sit next to that explains it. The caption leads the row so a pair of chips means something before either is pressed. The tray is a strip under the bar rather than a popup, the way the develop column's film picker is: the view scrolls as one, so an inline strip is taller content and not a second overlay to dismiss. It scrolls horizontally for the same hard reason the bar above it does — a layout cannot be narrower than its children's minimums, and forty people would otherwise set the minimum width of the whole view. The roster is built on open, not kept in step: indexing and regrouping change who exists, and a list cached at startup would be stale for exactly the user who has just been naming people. Named first, then by how much of them the library holds — the catalog orders by face count alone, which puts a dozen unnamed strangers ahead of the two people the user actually cares about. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
2403f355d6 |
Tell a tap on a photograph from a hand going past
A brush across the grid opened whichever photograph was under it. Travel was already answered — the Flickable claims the pointer and the press is cancelled — but a contact that neither travels nor lasts reaches a TouchArea as an ordinary press and release, and it was landing the user in develop. So a finger now has to stay down for `TAP_MIN_MS` before letting go counts as opening anything. That is the floor under a tap where the 450 ms `HOLD_DELAY_MS` is the ceiling: below is a graze, between is a tap, above is a hold that starts a selection. One scale, three gestures. Only a finger is held to it. A mouse click is a discrete decision made by a button and is routinely over in thirty milliseconds, so `cell-pressed` now reports whether a finger did it — the same finger-id convention the pinch arbitration beside it already uses — and the dwell applies to touch alone. A graze still *selects* the cell it landed on, because the press already did that. That is the right failure mode: something visible and reversible rather than a silent nothing, and rather than develop. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
bbbc23f376 |
Let go of the keyboard when a name is finished
Pressing Enter on a person's name committed it and then kept the field focused, so on a tablet the on-screen keyboard stayed up over the faces the user had pressed Enter to get back to. The name looked accepted and the screen looked stuck. `Field` grows `release-focus()`, the other half of the `take-focus()` it already had, and the Identity screen calls it from `accepted`. A function rather than a property for the reason the existing one gives: focus is an event, and bound to a property it would fight anything else that took it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
e027d34b65 |
Regenerate the matrix over a night's merges
Coverage 59.8% to 67.0% (120/179). Eight of those came from tagging what was already built; the rest are tonight's features -- FR-CULL-3's peaking half, FR-CULL-5, FR-PLAT-AND-5. FR-PLAT-LIN-3 and NFR-COMPAT-2 stay untagged deliberately. Both were satisfied by a manifest and a document, and there is no source file to hang a tag on that would fail if the behaviour went away. This is also the first commit tonight the pre-commit hook has checked. Every branch used --no-verify, because the hook runs the traceability build and the branches were under a no-build rule; the matrix each of them left untouched is regenerated here, once, over all of them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
82d9d077b0 |
Merge: answer Android's memory warnings, and stop reporting a lost root as an empty library
FR-PLAT-AND-5 in full, FR-PLAT-AND-2 in part -- the recovery is built and live for Nextcloud roots, the SAF cause it names does not exist yet. FR-PLAT-AND-4 and FR-PLAT-AND-6 are not here, both blocked behind the same gap: assemble-apk.sh compiles no Java, so the APK cannot carry a Service or a FileProvider. The container has JDK 17 and build-tools 36; the build step is what is missing. Verified: fmt, clippy --workspace --all-targets -D warnings, and 1043 tests across dr-catalog, dr-sync, dr-sync-folder, dr-sync-nextcloud, dr-plat and dr-ui. The aarch64 target was checked before the branch was finished but not after; no device was available. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
75fd5619ca |
Refuse a scan whose root has gone, instead of reporting it empty
FR-PLAT-AND-2, and a silent failure on both platforms. `dr_sync::scan` stepped over a NotFound or PermissionDenied the way it does for a child that vanished mid-walk -- correct for a child, wrong for the root, where it ended the walk, returned Ok with nothing in it, and reported a successful scan of a library that was no longer there. A lost root is now its own error. The images under it are marked Availability::Offline per FR-CAT-9 and no catalog row is deleted; `library::persist` clears the mark per file as each one is listed again, so a root that comes back needs no repair step. Partly satisfied rather than closed, and the gap is worth stating. The recovery half is real and reachable on Android today, because `map_status` turns Nextcloud's 403 and 404 into it and Nextcloud is how a phone actually gets a library in this build. The causes the requirement names -- revocation, reinstall, a removed card -- are properties of a persisted tree permission, and there is none: SAF does not exist here, `SourceRef::Document` is constructed only in test modules, and `LocalStorage` rejects the variant outright. When SAF lands it becomes a third producer of this error and nothing above it changes, which is why the discovery belongs in the connector. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
2b812ebe21 |
Give memory back in the order the user will miss it least
FR-PLAT-AND-5. Android asks for memory back through onTrimMemory and kills the process if it is not given; until now nothing listened, so the answer was always "no". A tiered registry answers instead: GPU caches first, then proxies, then thumbnails, driven from android_main on MainEvent::LowMemory and MainEvent::Stop. The order is the argument. A backgrounded app has no window to draw and therefore no use for a render pipeline, while its thumbnails are exactly what the user will be looking at half a second after they come back -- so going into the background frees only the GPU tier, and only being measured against death frees everything. Sinks register beside the cache they free and hold weak handles, so the registry cannot keep a controller -- and every decoded portrait in it -- alive past the interface it belonged to. `try_borrow_mut` and skip: a warning can land mid-render, freeing textures under the code drawing with them is worse than missing one, and a warning not acted on is always followed by another. The GPU test is the one that matters: an eviction must change no pixel. A freed intermediate pool whose `colour_key` promise still stands renders an empty texture, and nothing else would have caught it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
6246a2e346 |
Merge: group the frames of one moment, and let a burst fold away
FR-CULL-5. Frames join a burst when they are adjacent in time and look like the frame before them -- both, because time alone groups a whole ceremony and similarity alone groups a studio setup across two days. Adjacent pairs only, chained; there is no all-pairs step and there must never be one. No selection of any kind. The representative is the earliest frame, a fact about the clock rather than a judgement about the photograph, and a newly found burst arrives open, so the pass never takes a row off the screen. Verified: fmt, clippy --workspace --all-targets -D warnings, 346 dr-catalog tests, 511 dr-ui tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
fc4157a1e0 |
Keep the test scene's own arithmetic from overflowing a u64
The first compile this branch ever had. `cargo fmt` reflowed four files and clippy passed at -D warnings untouched, but one test panicked: `the_signature_does_not_change_with_scale`, on "attempt to multiply with overflow". It is the fixture, not the feature. `scene()`'s little LCG multiplied the block's y by the golden-ratio constant with a plain `*` while the term beside it already used `wrapping_mul`, so any scene taller than about 104 pixels overflowed in debug. Only the scale test builds one that large, which is why 345 of 346 passed around it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
36ea53cceb |
Merge: stop a subject's name from setting the width of the develop column
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
3e19628324 |
Stop a subject's name from setting the width of the develop column
The column moved sideways the moment segmentation finished. It takes the widest minimum any panel declares, each panel publishes `layout.preferred-width` as that minimum, and `MaskPanel` gained rows whose width came from the model's output -- so a photograph the user was looking at jumped because a label said "traffic light". `overflow: elide` did not prevent it and was never going to. Eliding is what a `Text` does when it draws; at layout time it still asks for the width of its whole string, and it is the asking that reaches the column. Both the subject rows and the mask entries are bounded, because a mask is itself a segmentation result -- without the second half the column moved when a subject was clicked instead of when one was found. The bound is stated for the reason `ChipGrid` declares its width from its column count rather than from its options ( |
||
|
|
4a04496c78 |
Merge: focus peaking, so a frame can be judged without zooming to 100%
FR-CULL-3's peaking half. The raw histogram and raw clipping indicators remain unbuilt -- what exists is a display histogram tagged FR-DSP-7, counting AdjustPass's 8-bit output, which reports a highlight as gone precisely where FR-CULL-3 needs it to report the highlight recoverable. Verified before merge: fmt clean, clippy --workspace --all-targets -D warnings green, 11 focus GPU tests, 79 baseline dr-gpu tests, 511 dr-ui tests. The cfg(target_os = "android") arm is unverified -- the host-target clippy never compiled it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> # Conflicts: # ui/dr-ui/src/lib.rs # ui/dr-ui/ui/app.slint |
||
|
|
2168cdd1c4 |
Mark what is in focus, so a frame can be judged without zooming to 100%
FR-CULL-3's focus peaking. One compute dispatch measures local contrast in WGSL and writes an overlay texture; on desktop it reaches Slint through the same zero-copy wgpu import the canvas uses, so nothing per-pixel touches the CPU on the frame path. With peaking off the cost is zero and structurally so: focus_overlay opens with `let settings = self.peaking?;` before the frame is touched, and clearing drops both overlay textures, so no VRAM is held either. NFR-P14 is met by construction rather than by measurement -- one dispatch, no second render, no pipeline compile after session open, and a test asserting allocations stay at 2 over eight frames. The budget test asserts 50ms at 4K rather than a tight bound, deliberately: a tight bound fails on a loaded machine and gets deleted, which is worse than a loose one that still catches the regression that matters. TD-1 is amended rather than joined by a TD-6: on Android the overlay rides the readback that already exists there, roughly doubling that transfer while peaking is on, and TD-1's own "Done when" removes both because both are the same missing capability. Verified: cargo fmt clean; clippy --workspace --all-targets -D warnings green, which also compiles peaking.slint through dr-ui's build.rs; 11 focus GPU tests and 79 baseline dr-gpu tests pass; 511 dr-ui tests pass. Not verified: the cfg(target_os = "android") arm, which the host-target clippy never compiled. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
b52be080ff |
Merge: say which requirements the code already satisfied, and which it does not
Eight requirements were implemented and untagged; tagging them takes coverage from 59.8% to 64.2% without a line of feature code. Five more were refused rather than tagged -- R2's own acceptance criterion still reads '(figure TBD)', R5 has one of three clauses built, FR-RAW-2 meets the requirement's purpose but not its stated mechanism. docs/outstanding.md records what is genuinely unbuilt, so the gap reads as a decision rather than an oversight. It also names four requirements the matrix reports as covered that are not, including two covered only by string literals inside the traceability tool's own test fixtures. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
4d0ad0601c |
Merge: a Flatpak that asks for no filesystem, and the channels v1 ships through
FR-PLAT-LIN-3 and NFR-COMPAT-2. The manifest grants no filesystem permission of any kind, which is not an oversight: a folder library is chosen by typing an absolute path, nothing in the tree calls the FileChooser portal, and a static --filesystem= grant would have made that design appear to work by not testing it. docs/distribution.md records what a portal-based picker would take. No Flatpak has been built -- flatpak-builder is not installed here -- so the finish-args set is reasoned from what the binary links, not observed to be sufficient. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
115653a262 |
Mark a burst in the grid, and let it be folded away
The counterpart to the grouping: where the signatures come from, and how a group reaches a cell. Signatures are computed from the 256px thumbnails dr-thumbs already holds -- vastly more resolution than a 9x8 reduction can use -- so a library that has been browsed, or that has synced somebody else's shards, has already paid for them and no RAW is decoded for this. The consequence is stated rather than hidden: an image with no thumbnail gets no signature and never joins a burst. That is self-correcting, and it is why the pass runs when the thumbnail sweep finishes rather than on a timer. Nothing happens at import and nothing happens at query time. The mark is drawn as a child of the cell's TouchArea, for the same reason the star strip is: a click on it must not also reach `cell-clicked` and throw the user into develop, and children are hit-tested before the element they sit in. It is never hidden on hover the way the stars are -- a collapsed burst stands in for frames that are not on screen, and something has to say so whether or not a pointer is nearby. Folding changes what the grid's *query* returns rather than what its cells draw, because the grid is a window over an ordered query and the frames a fold hides are mostly not loaded. So the predicate joins VISIBLE in every query that lists or counts cells -- the window, the header's count, the run a shift-click resolves, and the ordinal a scrub lands on -- under the discipline VISIBLE's own comment sets out: present in four places of five is worse than absent, because the counts disagree with the cells and neither looks wrong on its own. There is a test for exactly that. `the_window_read_walks_the_ordering_index` now includes the burst clause. It asserts on the query plan while holding its own copy of the query, so left alone it would have gone on reporting green against a query the grid no longer runs. If the clause costs `images_grid_order` and puts the sort back, that fails here rather than becoming jitter someone measures in six months. The pass keeps its own drain timer in a thread-local instead of taking fields on the library controller, so everything the feature needs to run lives in one file and the screen that starts it holds nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
d3dadbd725 |
Group the frames of one moment, by when they were taken and what they look like
A burst is the commonest thing in a cull and the least interesting: twelve frames of the same gull at 10 fps occupy twelve cells, are scrolled past twelve times, and end with the photographer keeping one. FR-CULL-5 asks for them to collapse to one representative and be judged as a unit. Two signals, because neither alone survives a real library. Time alone groups a whole wedding ceremony -- a photographer working steadily never leaves the gap that would end the run. Similarity alone groups a studio setup shot across two days, which is a project rather than a moment. Together they are specific: adjacent in time *and* looks like the frame before it. Two seconds is the time bound, and the reason is worth recording because the figure looks absurd next to a 10 fps camera. `images.captured_at` is whole seconds -- EXIF's DateTimeOriginal has no sub-second field and SubSecTimeOriginal is optional and widely omitted -- so a burst arrives in the catalog as ten frames sharing one timestamp. Any threshold finer than a second is a threshold on information that is not there. Where the pace really is faster, the similarity bound is what separates the frames. Similarity is a 64-bit difference hash over a 9x8 box-averaged reduction, compared between *adjacent* frames only. Chained rather than anchored on the first frame, because by frame twenty a camera following a bird has nothing in common with frame one while no two neighbours differ by much; the time bound is what stops the chain running away. There is no all-pairs step and there must never be one -- that is what turns a grouping pass into something nobody can afford to run over 50k images. Nothing here ranks a frame. FR-CULL-5 names the failure it is avoiding, which is rejecting the only frame of an important moment because somebody blinked, so there is no sharpness score and no best-of-burst. The representative is the earliest frame -- a fact about the clock, not a judgement about the photograph -- and the user's own choice lives in its own table so that rebuilding the grouping cannot erase it. Same argument `people.ignored` makes one subsystem over: nothing short of remembering a decision survives re-clustering. A newly found burst is recorded *open*. Collapsing on discovery would be tidier, and would also mean a background pass taking photographs off the screen part way through a cull. The pass marks; the user folds. It is a pass rather than a job kind for the reason catalog.md 10.2 gives for face clustering: a burst is a property of a run of frames and has no natural subject_id, so a per-image job would rebuild the world once per photograph. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
8d5dc423ea |
Say that all three of FR-CULL-3's bullets are unbuilt, not one
The first draft got this half right and half wrong. It correctly said the existing histogram and clipping indicators are not FR-CULL-3's, but it described them as adjacent — as though the work left were mostly focus peaking and a relocation. They are not adjacent. The histogram reads AdjustPass's 8-bit output and counts clipping as r == 255, so it describes the frame the display is about to show, after the entire develop chain. FR-CULL-3 asks for the histogram of the sensor data, and gives its reason in the requirement itself: a rendered image "systematically lies about what is recoverable in the raw". A readout taken from the render cannot answer that question however it is presented, which means two of the three bullets need a new measurement rather than a new placement. Found by the agent building focus peaking, who had to go looking at the counters to find out. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
0d9f802f88 |
Regenerate the matrix that this branch is about
Eight requirements gained a tag in this branch's first commit, so the generated matrix is stale until it is rerun. Coverage moves from 59.8% (107/179) to 64.2% (115/179), and the untagged list drops from 72 entries to 64. Regenerating here rather than leaving it to the pre-commit hook, because this branch's subject *is* the matrix: a reader comparing the two commits should see the number move for a stated reason. The eight are R3, R6, FR-DEV-1, FR-UI-6, FR-NC-6d, NFR-OPS-3, NFR-PORT-2 and NFR-SEC-3, and none of them is new work — every one was already satisfied by code that had simply never said so. Six of the thirteen tag sites were new `TRACES:` lines rather than additions to existing ones, so the tag count moves 896 → 902 while the covered count moves by eight. Orphan tags remain at zero. Run from source with `cargo run -p traceability -- report`, never from a prebuilt binary, and note that the tool tracks line numbers — the six prepended module tags shift every subsequent line in their own files, which accounts for the churn in this diff that is not a coverage change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
88ef7148d6 |
Write down what is not built, so the gap reads as a decision
The traceability matrix reports one number and cannot say what it means. An untagged requirement is either one nobody built or one somebody built and did not label, and both look identical in the summary table. Eight of the second kind were tagged in the previous commit. This is the first kind, written out with the reasoning, so that the distance between the register and the binary is something a reader can see rather than reconstruct from a percentage. Eleven clusters, each verified against the tree rather than inherited from a survey. Several turned out to be more interesting than "not done": Plugins are 21 untagged requirements — nearly a third of the shortfall — and §7 already lists the Plugin API as out of scope for v1 while §3.10 spends 280 lines specifying it. The two that *are* built, FR-PLG-2 and FR-PLG-2d, are the declarative operation format, which is a plugin system that resolves at build time. The fix here is an edit to requirements.md, not code, and D16 has to be answered before any format is called stable. FR-DSP-2 is not merely unbuilt; it is under challenge. frame_budget.rs and TD-4 independently argue that tiling costs more than it saves on the interactive path, so the open question is whether the requirement should survive, not when it will be met — and the spike that would settle it, S6, has not run. NFR-A11Y-1's hard half is done and its easy half is not. LocalizedKey already keeps display strings out of core/ and labels::resolve is a single resolution point; what that point does is a hardcoded English match, so a translation needs a recompile, which is the one thing the requirement forbids. The §4.1 performance targets are unverified rather than unmet. §8 requires a per-commit benchmark suite whose regressions fail the build; there is no benches/ directory, no criterion, and no benchmark step in any of the three CI workflows. The one guard that does live in CI skips itself without a GPU adapter and asserts its CPU half only in release, while CI tests in dev. Nothing says the targets are missed. It says nobody would find out. And three requirements are reported as covered while not being met: R1 and NFR-OPS-1 are tagged only by string literals inside the traceability tool's own tests, which it scans along with everything else, and FR-CAT-13's single tag sits on keyword storage while no XMP is parsed or written anywhere. CONTRIBUTING already warns that a tag proves a tag exists; these are the specific ones. Focus peaking, burst grouping, Flatpak packaging and the Android platform integration are being built in parallel and are marked in progress rather than listed as absent, so those lines can be struck as they land. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
8165619065 |
Describe the project the README is actually in front of
It said the status was "early", that v0.1 is a remote library viewer, and that the zero-copy display path was "not yet working" and its replacement was the highest priority. All three were true when they were written and none of them has been true for eight releases. The zero-copy line was the damaging one, because it is the project's central architectural constraint and the README stated the opposite of what happened. Desktop keeps the zero-copy path — the compute pass writes a texture Slint composites directly — and the readback survives in exactly one place, the Android develop view, because wgpu's Vulkan swapchain tears a portrait window on a tablet whose panel is mounted landscape. That is TD-1, with the on-device measurements and the three things any one of which would remove it. A reader who took the old text at face value would have gone looking for a bug that was fixed, and missed a compromise that was chosen. "Current state" now says what runs, checked item by item against the tree rather than from memory: the first draft claimed AVIF and JPEG XL export, which the settings page offers and dr-export refuses on purpose, and sixteen declared operations where there are fifteen. Both are the kind of error this commit exists to remove. The requirement count moves from 122 to 179, the documentation table gains the five documents somebody would actually want next, and the "Not built" paragraph points at docs/outstanding.md rather than leaving the reader to infer the gap from silence. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
ab0ef6a26d |
Say which requirements the code was already satisfying
Thirteen requirements were surveyed as built but untagged. Eight of them were: R3, R6, FR-DEV-1, FR-UI-6, FR-NC-6d, NFR-OPS-3, NFR-PORT-2 and NFR-SEC-3. Each was read against its full text in requirements.md and against the code before the tag was added, because a tag that is wrong is worse than an absent one — it turns a visible gap into an invisible one. The five that were refused, and why, because the reasoning is the part worth keeping: R2 carries "(figure TBD)" in its own acceptance criterion and asks for a stated prefetch margin and cache-hit rate; neither figure exists anywhere in the tree and neither quantity is measured, while TD-2 and TD-3 both describe the thumbnail path falling short of it. R5 asks for three things and the code does one. The display pipeline does run at viewport resolution, but "only visible tiles are computed" and "panning recomputes only newly exposed tiles" need a tile scheduler that does not exist — and frame_budget.rs currently argues for striking tiled computation from the interactive path rather than building it. FR-RAW-2 asks for a trait taking a SourceRef, so that a second decoder can be added without changing callers. What exists is free functions over &[u8]. That meets the requirement's stated *purpose* — the same decoder serves a local file, a SAF document and a byte range, which is exactly why it takes bytes — but there is no trait and no second implementation seam, so the requirement should probably be amended rather than tagged. NFR-ARCH-1 asks for named executors with stated thread counts. architecture.md §7.1 states the table; nothing implements it. Workers are twenty-odd ad-hoc std::thread::spawn sites, each building its own one-worker tokio runtime, with no decode pool, no GPU-submit executor and no I/O pool. The requirement's own text says R4 and NFR-P9 "assert an outcome with no stated means", and that is still true. NFR-SEC-4 is satisfied by absence — there is no telemetry — and absence has no module to tag. A tag would point at nothing. NFR-OPS-3 was the closest call of the eight taken. The store is single, separate from the catalog, survives a catalog rebuild and does not sync between devices; it has no version *field*, deliberately, and settings.rs argues why and names the condition that would need one. The substance is met and the reasoning is recorded where it belongs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
a56ac87e2c |
Build a Flatpak that asks for no filesystem at all
FR-PLAT-LIN-3 says that where DarkRoom is distributed as a Flatpak, filesystem access uses portals. There was no manifest, so the sandboxed path had never been exercised and the requirement had never been tested against the code. The manifest is YAML rather than JSON because it has more to explain than to declare, and every permission in finish-args carries the argument for itself. The two that are not obvious: - --device=dri is not an optimisation. The develop pipeline is compute shaders through wgpu with nothing behind it while NFR-R8 is open, so without the render node the application starts and cannot develop. - --talk-name=org.freedesktop.secrets rather than the Secret portal. These are different things and the requirement's wording invites the wrong one: the portal hands an app a master key for a store it keeps itself, whereas the session's secret daemon is what keeps the Nextcloud app password visible to secret-tool and Seahorse, and therefore individually revocable by the user. FR-NC-2's degraded mode is what happens if nothing answers, inside the sandbox exactly as outside it. There is no --filesystem= line, and that absence is the substance rather than an oversight. It leaves the document-portal path working — a photograph opened from a file manager arrives in argv under /run/user/$UID/doc and opens with no code change — and leaves library selection broken, because dr-sync-folder takes a typed absolute path and nothing in the tree calls the FileChooser portal. --filesystem=host would fix that and is precisely what the requirement forbids; --filesystem=xdg-pictures would fix it by not testing the design, in a smaller directory. docs/distribution.md §4 records what actually closes the gap and the flatpak override to use in the meantime. The runtime version is chosen from what the binary needs rather than from what is newest. ldd on a release build names fontconfig, freetype, expat, libpng, zlib, brotli and bzip2 and nothing more: wgpu dlopens libvulkan.so.1, and x11rb and wayland-client speak the wire protocols in Rust rather than binding libxcb or libwayland. So what the runtime must supply at runtime is a Vulkan loader and an ICD, which is the GL extension's job, and freedesktop 25.08 carries a rust-stable extension at 1.98.0 — comfortably above the workspace's 1.92 minimum. That extension is not rustup, so rust-toolchain.toml's pin is ignored here; the manifest says why that is correct rather than a violation of CONTRIBUTING.md, since the pin exists to make fmt and clippy agree and neither runs in a packaging build. Like the PKGBUILD, it builds from the local checkout, so a build is of what you are working on. That needs network for cargo, which Flathub forbids — the comment says what a submission there would need instead and why generating 30,000 lines of vendored sources buys nothing yet. The LFS pointer check is carried over from the PKGBUILD for the same reason it exists there: a dir source copies a 130-byte pointer in without complaint, and the failure would land on a user's machine rather than the packager's. Not verified: nothing has been built. flatpak-builder is not installed here and the machine is under a build embargo. The manifest parses, its keys are the ones flatpak-builder reads, and the desktop entry and metainfo it installs both validate — but no Flatpak has been produced from it and no permission has been observed to be sufficient. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
47a40d1afa |
Describe the application once, in a file every channel installs
A desktop entry gives a software centre a name and a one-line Comment, and nothing else — no description, no licence, no age rating, no statement of what hardware the interface was laid out for. GNOME Software and Discover both show an application with no metainfo as an unexplained icon, and both packages this repository produces were in that position. packaging/paris.tourolle.darkroom.metainfo.xml is the AppStream component, and it is installed by the PKGBUILD as well as by the Flatpak manifest because the description, the licence fields and the OARS rating are facts about the application rather than about how it was packaged. Writing it twice is how the two packages start disagreeing. Two things in it are easy to get wrong and are commented in place. The two licence fields differ on purpose: metadata_license covers the file itself and has to permit the unconditional redistribution and reformatting a catalogue does, which GPLv3 does not, so it is CC0-1.0; project_license is the application's own and reads GPL-3.0-or-later to match D8. And the component id is not a fifth name but the same string as the desktop basename, the Flatpak application id, and the app_id dr_ui::run sets — a rename that misses one costs the icon or the association, and neither failure announces itself. D15's decision that the target devices are a tablet and a desktop is stated as a display_length requirement rather than left implicit, so a software centre does not offer this on hardware where the photograph and the parameter panel cannot both be on screen. Validates clean under appstream-util validate-relax; appstreamcli --pedantic reports only that the gitea URLs are unreachable from a machine that cannot see that host. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
95847e3a31 |
Say which channels v1 ships through, and that the Flatpak cannot reach a library
NFR-COMPAT-2 asks for the v1 channels to be stated, and says why in its own second sentence: the channel decision and the storage design are coupled. There was nowhere that statement lived. packaging/ held a PKGBUILD and a desktop entry, which is a recipe rather than a decision, and the coupling the requirement points at was therefore invisible. docs/distribution.md states five channels and, more usefully, which two of them exist only as promises. It also records what every channel has to get right independently of format — the one identifier that appears in four places, the metainfo, Vulkan being a requirement rather than a preference while NFR-R8 is open, a secrets daemon being optional rather than required, and the LFS pointer check that stops a package shipping 130 bytes where an 11 MB model should be. §4 is the part worth reading. Preparing a Flatpak is what surfaced that FR-PLAT-LIN-3 is not satisfied and cannot be satisfied by packaging alone: a folder library is chosen by typing an absolute path into an EndpointOnly field that checks it with std::fs, and nothing in the tree calls the FileChooser portal. Inside a sandbox that path does not exist, so the launch screen refuses it. Import fails one step earlier, because a sandboxed process reads its own mount namespace and a card mounted on the host is not in it. That is written down rather than fixed with --filesystem=host, and the argument for not fixing it that way is §2: the Arch package and an AppImage both hand the application the same unrestricted process the developer runs it in, so Flatpak is the only Linux channel that tests whether a design assumed unrestricted access. Granting the permission removes the only reason to ship it. The reverse coupling on Android is recorded too. NFR-COMPAT-2 says Play distribution is what makes ARCH §6.9 binding; §6.9 is verified rather than assumed, so SAF is already unconditional and a sideloaded build would gain nothing by asking for more. Play is deferred over the GPLv3 question, which is a licence-reading exercise and blocks nothing in the storage design. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
eb39599f12 |
Merge: named presets (FR-DEV-6)
Build and test / Desktop (Linux) (push) Failing after 1h21m9s
Build and test / Layer separation (push) Successful in 48s
🐳 Android image / Build and push (push) Successful in 3s
Build and test / android-image (push) Successful in 3s
Traceability / Requirement traces (push) Successful in 43s
Build and test / Android (aarch64) (push) Successful in 20m42s
|
||
|
|
5a8327824f |
Keep an edit under a name, not just on the clipboard
FR-DEV-6 asks for three things — named presets, copy/paste between images, and batch-apply to a selection. The last two have been here for a while; this is the first. The format is the sidecar's, deliberately. A preset *is* the non-default half of a version, so the lines are the same lines keyed the same way, which makes the two files diffable against each other and lets someone debugging an edit paste a block from one into the other. One file rather than one per preset: a preset per file makes the name a path, and every name then has to survive a filesystem — a `/` becomes a directory, a name differing only in case collides on one platform and not another, and renaming becomes two operations that can half-fail. As a key in a document it is none of those. Unknown *parameters* needed no machinery. `Preset` already holds whatever keys it is given and resolves them against the descriptors only at apply time, so one written by a newer build survives by being stored. Only lines that are not `op.param = float` at all are preserved verbatim, which is the sidecar's version-skew promise made here too. Applying is the paste path with a different source, so a preset reaches a selection through the sidecar read-modify-write that was already there: no graph, no decode, no GPU, forty files or one. Two smaller decisions worth the record. A library that fails to parse is held empty in memory and *not* written back over — settings regenerate themselves and this is work, so a parse failure must not be the moment it is destroyed. And every save persists immediately and rolls the in-memory copy back if the write fails, so the sheet never lists a preset the file does not have. The grid's "Presets" button is gated on the selection alone, unlike the "Paste to 40" beside it. That button needs a clipboard armed this session; the preset list is whatever was saved last month, and hiding it behind an unrelated action is what makes a feature only its author knows about. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
|
574107bc39 |
Let a manual collection be put in the order it is meant to be seen in
`collection_members.position` and `Sort::CollectionPosition` have been in the catalog since collections were, and nothing above dr-catalog has ever written or read either: `collections::set_order` had no callers, and the grid ordered everything by capture time whatever it was scoped to — dr-ui does not construct a `Query` at all, it has its own `GRID_ORDER` constant. So a manual collection was a set with an order nobody could see or change. Three pieces, because it could not be fewer: `grid_order_for` decides the ordering from the scope, and both readers take it from there. That is the load-bearing part. An ordinal only names a photograph relative to an ordering, so the window read and the span read have to agree — a shift-click resolved through a different ORDER BY than the cells were drawn with selects a different run than the one on screen, and the user finds out when the export runs. `read_ids_span` already stated that invariant about `GRID_ORDER`; this widens it to an ordering that depends on the scope. Only a single manual collection has one. A set draws its descendants' images too, and two children's positions are unrelated integers that interleave arbitrarily; a smart collection has no member rows to carry a position at all. Both fall back to capture time and refuse the drop rather than pretending. The drop is on the cell, on whichever half of it the finger landed — the trailing edge is the only way to name the last place in a collection, since there is no cell beyond the last one to drop in front of. `reordered` is pure and the membership is rewritten whole. `set_order` sets the positions it is given and leaves the rest, so a partial write would interleave the moved run with rows nobody touched; and it is read unfiltered, so what the filter is hiding keeps its place relative to what the user can see. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
6d25d85f18 |
Make the range gesture visible, and offer the whole grid at once
Touch has had a range gesture for as long as selection mode has: double-tap the far end. It is invisible, it is unreliable on a grid that scrolls under the second tap, and it extends from the anchor *before* the two taps moved it — a rule subtle enough that the code needs two paragraphs to explain it to itself. Nobody who was not told about it has ever used it. "Select to…" is the same operation with state you can see. Press it, the strip stops reporting and says "Tap the last photograph", and the next cell taken is the far end. It reaches Rust as shift on `cell-pressed`, so it lands in `apply_press` as the ctrl+shift it already is, and there is no third selection policy to keep in step with the other two. This is deliberately not the sweep gesture. A drag that paints cells can only reach what is on screen, and the ranges that hurt on a tablet are longer than a screenful — between the two taps here the user may scroll as far as they like, and the run is resolved by the catalog rather than by what happened to be loaded. A sweep is still worth having for short runs; it is not what this should have rested on. "Select all" beside it, asked of the catalog for the same reason: a select-all that quietly meant "the hundred cells that happen to be loaded" is a lie the user cannot see until the export runs. The double-tap stays. It is tested, and an accelerator that costs nothing is worth keeping for whoever has already learnt it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
9ac1447139 |
Ask for the collection's name where the keyboard can reach it
"New collection from selection" created the collection under a placeholder name and then opened the rename field in the sidebar tree. On a tablet the sidebar is not on screen. It is instantiated all the same — app.slint collapses it to zero width and `visible: false` rather than using an `if`, because an `if` there is a layout loop Slint panics on — so the rename field was created, its `init` took focus, and Android raised the on-screen keyboard over a box nobody could see. Nothing else on the screen is focusable, so the keyboard had nowhere to go: it stayed, the name could not be typed, and the collection was already written under the name the user did not want. Asked in a sheet instead, on the same card as the filing and keywording sheets, before anything is written. That also fixes what was hiding behind it: an abandoned rename used to leave a "New collection" in the tree, because the collection existed before the name did. `Field` gains `take-focus()` so a sheet whose field is the only thing to do in it can answer the keyboard for the user — a function rather than a property, because focus is an event and a bound property would re-take it on every unrelated re-evaluation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
5133e53bc8 |
Give back what a straighten took, when the angle comes back
Build and test / Desktop (Linux) (push) Successful in 2h16m6s
Build and test / Layer separation (push) Successful in 52s
🐳 Android image / Build and push (push) Successful in 4s
Build and test / android-image (push) Successful in 4s
Traceability / Requirement traces (push) Successful in 51s
Build and test / Android (aarch64) (push) Successful in 47m10s
The auto-crop only ever shrank. Straighten to 20 degrees and the corners are cropped away correctly; come back to 3, or all the way to zero, and the crop stays at the size 20 degrees demanded. Nothing on screen explains why the photograph is still small, and the only way back was undo. The cause was that each correction was computed from the previous correction's output, so it accumulated: every angle the slider rested at took its cut and none was ever returned. The fix is to stop accumulating and recompute. The applied crop is now always the user's own rectangle fitted into the current angle's safe area, so as the angle falls and that area opens up the crop grows back — and stops, exactly, at the rectangle they chose. At zero the safe area is the whole frame and the fit is the identity, which is what carries it the last of the way home. There is deliberately no early exit for the upright case now: that exit is precisely what would strand the crop small. **The intent is remembered as a pair, so it repairs itself.** The session keeps `(applied, intended)` — what the correction wrote, and what it was derived from — and trusts the remembered intent only while the graph still holds `applied`. Every other route to the crop leaves something else there: a handle dragged, a ratio chosen, a sidecar loaded, a paste, an undo. That mismatch is the signal the memory is stale, and the current rectangle becomes the new intent. The alternative was a write into this field from each of those paths, which is the kind of bookkeeping that is correct until someone adds a seventh path. Dragging a handle therefore *is* the user choosing, including at a non-zero angle: the correction will not later grow the crop past what they dragged it to. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
ff6c313bab |
Wrap the chip rows, so one six-choice parameter stops sizing the sidebar
The develop column asked for 482px. Every comment in it, a dozen of them, describes it as a 280px column — and on a tablet it was taking 40% of the screen from the photograph it exists to serve. Measured rather than guessed, because none of it is visible in the source: the column takes the widest width any panel declares, `AdjustPanel` wanted 482 of it, its rows wanted 458, and after the 44px scroll gutter the widest single row was 414. That row is `film_sim`'s `format` — six film formats from 35mm to 8x10, laid out by `Segmented` as six 64px chips in a `HorizontalLayout` that cannot wrap. 6 x 64 + 5 x 6 = 414, exactly. Nothing about that is the film simulation's fault. An operation declares its parameters and the panel decides how to draw them (FR-DEV-3a), so a node is entitled to offer six choices; it is the drawing that has to cope. Any future operation with a five-choice enum would have done the same thing, silently, to every screen in the application. `ChipGrid` is the general answer: chips placed by index arithmetic inside a plain `Rectangle`, wrapping at a column count, declaring a width that depends on the columns rather than on the number of choices. `Segmented` takes a `columns` property and uses it when asked — zero, one row however many chips, stays the settings page's behaviour, where the page is full-width and reading the alternatives side by side is the whole argument for chips over a dropdown. The generated enum rows and the curve-channel picker now wrap at three, and the crop ratio chips use the shared grid instead of the private copy of it they shipped with last week. The column measures 351 now, down from 482, and what sets it is the mode strip rather than a parameter — which is a control the user chose to have on screen rather than an accident of one node's variant list. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
bbd26f05f9 |
Release 0.9.0
Build and test / Desktop (Linux) (push) Successful in 2h8m7s
Build and test / Layer separation (push) Successful in 37s
🐳 Android image / Build and push (push) Successful in 1s
Build and test / android-image (push) Successful in 1s
Traceability / Requirement traces (push) Successful in 1m42s
Build and test / Android (aarch64) (push) Successful in 1h4m36s
Thirty-five commits since 0.8.0, and two of them are the reason this is a minor rather than a patch. **Storage became pluggable.** A folder backend now sits beside the Nextcloud one behind the same seam, proved by a test rather than by a trait, and the launch screen offers three routes into a library instead of one. **Faces gained a confidence that means something.** A suggestion is scored against the people the user has actually named, the curve it came from is stated rather than implied, and a regroup runs roughly three times faster — the similarity scan and the merge engine both use the machine's own SIMD kernel now, measured on the tablet where the NEON path is the one that runs. Alongside those, the framing tools got the quality-of-life pass this release is named for: a crop can be held to a ratio, a straighten crops away the corners it exposed, an export can be pinned to an exact resolution with the common panel sizes offered as buttons, and dragging the crop rectangle no longer tracks the pointer at half speed. `pkgrel` returns to 1: a new `pkgver` is a new archive name, so there is nothing left for a release number to disambiguate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
bd33487054 |
Say which axis each export dimension is, in the one place that renders
The box modes put two numeric fields one above the other, and on screen they are two anonymous numbers: `TextRow` draws its label behind the field rather than above it, so "Width" and "Height" never appear. That fault is older than this feature — the storage panel's cache sizes have the same missing labels, and the single "Size value" row always did — and it belongs in its own change rather than being fixed under cover of this one. But one unlabelled number is survivable and two are not, so the axis goes where the page does render it: the unit. It reads "3840 px wide" above "2160 px high", which is the sentence the user is trying to write anyway. The `label` bindings stay correct and stay where they are, so this becomes redundant rather than wrong the day `TextRow` is fixed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
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> |
||
|
|
2c8768ac29 |
Lay the ratio chips out by hand, because Slint will not lay them out
The ratio chips shipped inside a `GridLayout` with `row` and `col`
computed from the repeater's index. It compiles. On screen every chip
piles into a single row that runs off the panel, and the terminal fills
with
Internal error in Slint: RepeatedItemTree::grid_layout_input_data()
not implemented
once per frame. A `for` inside a `GridLayout` is not supported, and
nothing says so until it is running.
A `HorizontalLayout` is not the answer either: six chips side by side
need over 400px, and this column's width is the largest width any panel
declares — so one row would widen every other panel in the application to
fit a control that is on screen only while cropping.
So the grid is arithmetic over the index inside a plain `Rectangle`. It
costs the layout engine nothing, it wraps a seventh ratio onto a third
row by itself, and — because a bare `Rectangle` declares no preferred
width — it takes the width the column already has instead of setting it.
The Portrait chip gets a container of its own for the same reason: a
`ChoiceChip` dropped straight into a `VerticalLayout` is stretched the
full width of the panel and reads as a button for the section rather than
as one more chip.
The three conditional pieces are also now individually-conditional
children of the one layout rather than a nested layout under a single
`if`, which is the convention `app.slint` and `masks.slint` already carry
notes about: a conditional nested layout under-reports its height here
and the panels below it draw on top of one another.
All of this was invisible in the source and obvious in a screenshot.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
dbaf5358d1 |
Measure a crop drag against the frame, not against the rect it is moving
Dragging the crop rectangle tracked the pointer at half speed: the rect slid out from under the cursor, and the handle being held stopped being the one under the finger. On a photograph where the whole point is to place an edge by eye, that made the tool close to unusable. The cause is that a `TouchArea` reports `mouse-x` relative to itself, and every area in the overlay is positioned *by the very rect the drag is editing*. Rust echoes the applied rect back on each step, so the area moves under the pointer and the reported position falls by exactly the amount it had just risen. `mouse-x - pressed-x` therefore subtracts the drag from itself, and the fixed point of that feedback is a rect that moves half as far as the pointer does — which is why it looked like sluggish tracking rather than like a coordinate bug. Both terms are now taken in the frame's own coordinates: `parent.x + mouse-x`, where `parent.x` tracks precisely the movement `mouse-x` lost, with the press captured in those same coordinates on the way down. The two movements cancel and the rect follows the pointer exactly. `GradientHandles` in masks.slint was already written this way, and for the same reason — its handles are placed by the mask they drag. The header note here now says why the shape matters, since the wrong version compiles, looks plausible, and is only wrong once the rect starts moving. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
480c46c41a |
Quote the percentile the measurement can actually support
The before/after published a p99 ratio of 6.0x at 4K. It is not supported and the correction is worth more than the number was. The baseline run's `fit` rows spread 2.3x between median and 99th percentile while every row of the after run spreads about 1.1x. A stage costing `radius x pixels` has no reason to be bimodal, and `fit` is the memory-bound configuration — it walks the whole 482 MB source on a stride where `1:1` reads a contiguous window. Something else had the machine. The merge brings in the cross-check that settles it: "Try every GPU, not only the fastest one" measured the same baseline code on the same card and reports 4.67 ms p99 for M3 clarity fit at 2560x1600, against 10.40 ms here. Two measurements of one thing differing by 2.2x mean the noisier one is wrong. So both percentiles are now published and the p50 column is the claim: 2.8x at 4K rather than 6.0x. The p99 improvement is real and larger; this run cannot say by how much, and says so. What the caveat does not touch: every after figure is inside the 16 ms budget with a p99 within 26% of its median at every size and both views, and clarity against all-four is a within-run comparison. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
30162e80df |
Merge branch 'master' into clarity-reduced-base
# Conflicts: # docs/traceability.md |
||
|
|
8b251b84a0 |
Record what the reduced base actually bought
TD-4 asked for the measurement as well as the change, and this is it: 25.05 ms to 4.17 ms at 3840 x 2160, six times faster, with clarity no longer dominating the neighbourhood stage it used to be 97% of. Measured before and after on the same machine and the same adapter minutes apart, baseline at the branch's merge-base, so the only variable is the change. That adapter is not the RTX 3050 the rest of this document was measured on, so the new table says to read it on its own rather than against the ones above — the before/after is comparable, the absolute figures are not, and quietly replacing the existing tables would have changed the instrument. Also recorded: the declared halo is now quantised to multiples of the output scale, because the 2-sigma truncation rounds on the reduced grid. 29 px becomes 28 at 1920x1200 and 38 becomes 40 at 2560x1600. It is inside what the cross-form test holds — 0.03 stops of peak, 2% of reach — but it is a change in reach and not only in cost, and a tile scheduler would be handed it. Better written down now than found later as a seam. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
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> |
||
|
|
3b5d564495 |
Try every GPU, not only the fastest one
Build and test / Desktop (Linux) (push) Successful in 2h7m41s
Build and test / Layer separation (push) Successful in 46s
🐳 Android image / Build and push (push) Successful in 3s
Build and test / android-image (push) Successful in 3s
Traceability / Requirement traces (push) Successful in 41s
Build and test / Android (aarch64) (push) Successful in 21m1s
`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". |
||
|
|
0407fb8d2d |
Format the two new examples
They were written after the last fmt run and CI gates on --check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
3944e17aa5 |
Merge: face confidences that mean something, and a regroup three times faster
Two things the People screen was getting wrong, and then the arithmetic underneath it. It refused to show a confidence at all until the library had fitted its own calibration, which needs 200 confirmed positive pairs — made on the screen that was withholding the number used to rank them. The default curve is the reference implementation's fitted one, a published operating point rather than an invention, so the percentage is shown and the screen says which curve it came from. The number itself was the mean similarity between a face and every other member of its group, which punished well-photographed people and never asked who else the face might be. It is now the mean of the best ten matches into the identity, times that identity's share of the evidence against every other identity the user has ruled on. Leave-one-out over this library's 2,702 confirmations across 54 named people: 99.33% placed on the right person, and the number shown for the right person moves from a median of 90.4% to 99.3%. Both of those were measured rather than argued, on a copy of a real 18,143-face library, and the instrument is committed with them. Measuring is also what found the rest. A regroup went from 10.0s to 3.1s: the similarity scan was walking the whole embedding array once per row and not vectorising at all, and the agglomeration was spending 3.06s having every component scan the entire pair list for its own pairs. The scan now picks its kernel per machine — AVX2 where the CPU has it, NEON on the tablet, where the whole suite and a full regroup have both now been run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
39c34d4e44 |
Say what actually counts as a rival, now that a name anchors too
assign's denominator is the identities the user has ruled on, and it reads Cluster::person to find them. Master's "Let a name hold a group together" widened what sets that field: a confirmation, a name, or an ignore, where before it was a confirmation alone. The behaviour is right either way — a named person is exactly the identity a suggestion should be discounted against — but the module note and faces.md §9.1 both said "a confirmation", which is now too narrow. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
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> |
||
|
|
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> |
||
|
|
f4395bd17c |
Run the face tests on the tablet, where the NEON kernel actually runs
The similarity scan picks its dot product per machine, and the NEON one is the kernel that ships to the phone and the tablet — and the one a desktop cargo test never executes. A wrong lane index or a mishandled tail there is a silent wrong answer on exactly the devices nobody runs the suite on, which is a poor place for the only untested code path. dr-face carries no weights and touches no display, so its tests are a plain ARM64 binary that runs under adb shell with nothing installed. The script builds it against the SDK's newest NDK, pushes it, runs it and cleans up. It checks for the device first, so a tablet that is not plugged in costs a second rather than the two minutes it takes to compile for it. Not wired into CI, which has no device attached. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
8f596262b8 |
Record where a regroup's time actually goes
The §9 note said the scan was half a regroup and implied the agglomeration was an irreducible sequential walk. Both halves of that are now wrong, and the numbers are the point of the section. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
a67402961d |
Let the merge engine's dot product use the machine's kernel too
Engine::cross is the one place a dot product is computed during agglomeration — when two groups become adjacent through a third and their sub-threshold pairs, never summed because they were never interesting, have to be accounted for. It was calling the portable loop while the scan beside it had AVX2 or NEON, which on the reference library was 1,753,514 dot products taking 0.54s. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
b4d39ba33a |
File each pair under its component once, not once per component
Seeding the merge heaps was 3.06s of a 5.93s regroup on the reference 18,143-face library — more than half the pass, spent before a single merge was considered. Every component scanned the whole pair list looking for the pairs that were its own: 475 components against 804,499 pairs, 382 million set lookups to place 804,499 of them. A pair can only ever join two faces of one component, since that is what a component is, so the union-find that finds the components can file the pairs at the same time and hand each agglomeration the list it needs. The membership set inside agglomerate goes with it — it existed only to run that filter — and the heap can be sized up front now that the pair count is known. Ordering is preserved deliberately: pairs are filed in the order they arrive, which is the global (i, j) order, so the seeded heap breaks its ties exactly as before and the merge order is unchanged. Same 2,518 groups holding the same 16,246 faces on the reference library, at 3.5s rather than 5.9s. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
e596eb0657 |
Give the similarity scan the machine's SIMD, and its cache
The scan is O(n²) dot products and nothing else, so its speed is the face subsystem's speed — and it was running at 0.7 flops per cycle. Two separate faults, both measured over the reference 18,143-face library on twenty cores. It walked the whole embedding array once per row, ~336 GB of traffic, where a column tile that fits in L2 is read once per tile of rows: 4.64s → 2.81s. And the workspace builds for baseline x86-64 — SSE2, no FMA — into which the portable loop was not being vectorised at all: 2.81s → 0.86s, 195 GFLOP/s. So the dot product is now chosen per machine. AVX2 + FMA where is_x86_feature_detected! finds it; NEON unconditionally on aarch64, since Advanced SIMD is in that baseline and every Android device the app builds for has it — with the explicit vfmaq, because LLVM will not fuse a multiply and an add without being told to. The portable loop stays as the definition the others are tested against, and the_fastest_kernel_agrees_with_the_portable_one is the only check the NEON path gets on a machine that is not aarch64. Faces::embeddings is one flat buffer rather than a Vec per face: the pointer chase defeated both the prefetcher and the tiling, and it is also the layout a GPU pass would want. Behaviour is unchanged and that is checked rather than asserted — the same 1,531,969 pairs from all three kernels, and on the real library the same 2,518 groups holding the same 16,246 faces with the same confidence distribution. A full regroup there goes from 10.0s to 5.9s; the rest is the agglomeration, which is a sequential heap walk and is where the next look should go. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
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> |
||
|
|
21d599b420 |
Test the guard, since the bug was a branch nobody ran
Build and test / Desktop (Linux) (push) Successful in 2h5m41s
Build and test / Layer separation (push) Successful in 50s
🐳 Android image / Build and push (push) Successful in 2s
Build and test / android-image (push) Successful in 2s
Traceability / Requirement traces (push) Successful in 1m37s
Build and test / Android (aarch64) (push) Successful in 59m40s
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`. |
||
|
|
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.
|
||
|
|
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. |
||
|
|
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. |
||
|
|
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. |
||
|
|
6c363cee97 |
Let the launch screen scroll, now that it offers three routes
The signed-out screen was a `VerticalLayout { alignment: center }` with
no scroll. That was already tight with two routes; the folder option
made the column taller than a 900x560 window, and a centred layout that
overflows clips at *both* ends — so the masthead and the last route
disappear together, with nothing on screen to suggest either existed.
A Flickable whose viewport follows the content, and a top padding that
centres the column only while it fits. Both cases checked against the
running app: centred at 1000x1800, top-aligned and scrollable at
900x560.
|
||
|
|
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. |
||
|
|
cbe5c4fcde |
Measure the folder walk against a real tree, not a claim
The folder connector declares `LocalEtags`, which means the engine walks the whole library on every scan with no pruning. That is the honest capability, and the argument for it being affordable was so far an assertion about `stat` versus `PROPFIND`. `--example scan` runs the real path — `dr_sync::scan` over the connector, then a ranged read of the kind the thumbnail worker makes. Read-only; it never writes into the folder it is pointed at. 2,299 images across 233 directories in 137 ms, and 380 across 13 in 29 ms. Against 34.1 s for 17,185 RAWs over WebDAV *with* pruning available. Recorded in docs/storage.md §5.2 and ARCH §8.4a, because a capability trade-off argued from a number nobody measured is the kind that gets quietly reversed later. |
||
|
|
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.
|
||
|
|
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> |
||
|
|
1b8b7998a2 |
Let a name hold a group together, the way a confirmation does
Build and test / Desktop (Linux) (push) Successful in 2h6m43s
Build and test / Layer separation (push) Successful in 46s
🐳 Android image / Build and push (push) Successful in 1s
Build and test / android-image (push) Successful in 2s
Traceability / Requirement traces (push) Successful in 29s
Build and test / Android (aarch64) (push) Successful in 21m27s
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> |
||
|
|
2fb3eb5d2d |
Upload a shard that was sealed after the server saw it
Build and test / Desktop (Linux) (push) Successful in 2h7m52s
Build and test / Layer separation (push) Successful in 57s
🐳 Android image / Build and push (push) Successful in 12s
Build and test / android-image (push) Successful in 13s
Traceability / Requirement traces (push) Successful in 57s
Build and test / Android (aarch64) (push) Successful in 20m58s
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> |
||
|
|
f9e331eed2 |
Let a test build be opened up, when asked
`adb shell run-as` refuses on a release build — "package not debuggable" — and the app's private storage is then unreachable from the host. That storage is where the face shards, the thumbnail store and the catalog live, so when a device disagrees with the desktop about what it has synced there is no way to find out which of them is right. An evening was spent guessing at exactly that. `DARKROOM_DEBUGGABLE=1 ./docker/android/package.sh --install` now sets `android:debuggable` through aapt2's `--debug-mode`, and nothing else changes. Set through aapt2 rather than written into `AndroidManifest.xml` on purpose: the flag then exists only for the build that asked for it, and a release build cannot inherit it because somebody forgot to take it out again. A debuggable APK lets any process on the device read this app's files, so it belongs on a test tablet and nowhere else. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
f41b3f6e8e |
Answer "what would the other device end up with" without the other device
Build and test / Desktop (Linux) (push) Successful in 2h7m52s
Build and test / Layer separation (push) Successful in 59s
🐳 Android image / Build and push (push) Successful in 3s
Build and test / android-image (push) Successful in 4s
Traceability / Requirement traces (push) Successful in 37s
Build and test / Android (aarch64) (push) Successful in 22m22s
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>
|
||
|
|
bb3b512b81 |
Merge: a face sync that can actually complete
Three faults between a laptop that had indexed a library and a tablet that only ever received part of it. The face store was the one part of the catalog still on the rollback journal at synchronous=FULL — 21.3 ms a commit against the 0.05 ms everything else pays, four commits per photograph, on the order of fourteen minutes of pure fsync for ten thousand images. It now writes the way the catalog and the thumbnail store do, with a checkpoint before upload so the file that goes to the server is complete without its write-ahead log. Every idle pass read 94 MB of shards to decide it had nothing to send; the name and the size answer that. And the 60-second total request timeout was a floor on link speed rather than a hang detector: a 25 MB shard needed a sustained 425 KB/s or it failed, and then retried and failed again indefinitely. It now times out on inactivity. The merge itself was never at fault — a probe that stands up an empty catalog, adopts the shards and merges the real one in gets all 611 of a person's faces, not 200. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
d2af6a3981 |
Time out on a stalled transfer, not on a slow one
The HTTP client had a 60-second *total* request timeout. That is not a hang detector; it is a floor on link speed. A face shard runs to 25 MB, so it demanded a sustained 425 KB/s or the transfer failed — and having failed it was retried on the next pass and failed again, for ever. A tablet on ordinary wifi could therefore never finish taking in a library's faces, and nothing said why: each attempt looked like a network blip rather than an arithmetic impossibility. The catalog snapshot is 36 MB and has the same problem. `read_timeout` fires when no bytes arrive for the period, which is the condition actually worth failing on. A slow transfer that is still moving now finishes, however long it takes; a connection that has genuinely died is still caught in a minute. The connect timeout is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
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> |
||
|
|
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> |
||
|
|
0bba882fb1 |
Read the Android API level out of the ELF, not out of file(1)
Build and test / Desktop (Linux) (push) Successful in 2h8m13s
Build and test / Layer separation (push) Successful in 1m3s
🐳 Android image / Build and push (push) Successful in 2s
Build and test / android-image (push) Successful in 2s
Traceability / Requirement traces (push) Successful in 41s
Build and test / Android (aarch64) (push) Successful in 1h0m55s
Every push has failed the Android job for weeks — back through fourteen runs —
with "FAIL: linked for Android 'unknown', expected 28", on a .so that was linked
perfectly correctly at API 28 the whole time.
The check ran `file` on the linked object and pulled the level out of its
description with a regex. `file` only prints "for Android 28" when its magic
database is new enough to decode `.note.android.ident`, and this image's is not
— it stops at "dynamically linked, not stripped". The regex matched nothing, and
`${API:-unknown}` turned that silence into a confident-looking failure about the
artefact rather than about the tool inspecting it.
The API level is the first word of that note, little-endian, so it is read
straight out of the ELF with `readelf` — which is in the image via
build-essential, and cannot go out of date the way a magic database can. An
absent note is now its own message rather than being folded into the mismatch
case, since "nothing states an API level" and "states the wrong one" are
different faults.
`file` stays in the image and in the log: it names the NDK that built the
object, which is worth having when this does go wrong. Nothing depends on it.
Verified inside the real container against the linked .so: the note reads
1c 00 00 00, and the step prints "OK: linked for Android 28".
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
|
e997be8c72 |
Say which version this is: 0.8.0
Build and test / Desktop (Linux) (push) Successful in 2h14m44s
Build and test / Layer separation (push) Successful in 54s
🐳 Android image / Build and push (push) Successful in 3s
Build and test / android-image (push) Successful in 3s
Traceability / Requirement traces (push) Successful in 1m45s
Build and test / Android (aarch64) (push) Failing after 53m50s
A hundred commits since 0.7.0, and the face subsystem in them is not a set of fixes — it is the difference between a feature that was shipped and one that works. Clustering finished at all for the first time: the old agglomeration rescanned every live pair and recomputed average link from scratch after every merge, and on a real library it did not return. A sparse above-threshold graph, connected components and Lance-Williams sums put 1,813 faces at 0.28s, and the merge threshold moved to 0.80 because the old 0.90 was measured — not guessed — to leave a third of the library ungrouped. Faces are no longer indexed at any size or any sharpness. Both floors were measured over the real library with `face_index --quality`: 32 source pixels across the aligned crop, and a contrast-invariant sharpness that catches the large-but-blurred face whose confident, wrong embedding used to weld two people together. The crop is cut once and kept, so the People screen is no longer a derivative of a thumbnail cache entitled to evict anything at any moment. The screen itself became usable: the faces wrap into a grid instead of running off the edge, the header fits a phone, a group can be set aside, and a person's photographs are a button away — as a union or an intersection of several people. And face sync now reaches the other device. Re-indexed images re-export, shards written before the crop and index-time columns are repaired rather than failing every insert, people and the user's judgements about them cross the wire at all, and the pass says what it is doing while it does it. The Android versionCode follows without being restated — package.sh packs MAJOR*10000 + MINOR*100 + PATCH, so 0.8.0 is 800, above the 700 already on devices and therefore an upgrade rather than a refusal. `pkgrel` returns to 1, since this is a new version rather than a rebuild of the last one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
a8ce1ff19c |
Merge: a face sync that says what it is doing
Build and test / Desktop (Linux) (push) Successful in 39m22s
Build and test / Layer separation (push) Successful in 52s
🐳 Android image / Build and push (push) Successful in 4s
Build and test / android-image (push) Successful in 5s
Traceability / Requirement traces (push) Successful in 53s
Build and test / Android (aarch64) (push) Failing after 53m19s
The face pass announced itself once and then went quiet for the whole export, upload and adopt. On a first export after a re-index that is eight minutes of a motionless progress bar, which reads as a hang. It now reports the images it is preparing, the shards it is sending and their size, and the shards it is taking in. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
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> |
||
|
|
35febf31dd |
Merge: a face sync that finishes
Build and test / Desktop (Linux) (push) Successful in 35m30s
Build and test / Layer separation (push) Successful in 53s
Traceability / Requirement traces (push) Successful in 38s
🐳 Android image / Build and push (push) Successful in 1s
Build and test / android-image (push) Successful in 2s
Build and test / Android (aarch64) (push) Failing after 58m37s
The export asked whether each image was already in the shard store at its current index time, and answered by opening the shard database — schema batch, pragma probes and all — once per photograph. 9,849 opens per sync pass for this library, before any face was written, which showed up as a sync stuck on "checking faces…" and never coming back. The index time moves to the store's own index, which is already open, and the write handle is held across a run instead of reopened per image. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
0729dfa359 |
Stop the face sync opening a database per photograph
"Checking faces…" never finished. `export_to_shards` asks, for every indexed image in the library, whether the shard store already holds that image at that index time — and `indexed_at` answered by opening the shard database, running its six-statement schema batch and two `pragma_table_info` queries, then querying. Once per image. 9,849 times for this library, on every sync pass, before a single face had been written. The index time now lives in the store's `index.sqlite` alongside the shard number, so the question is one indexed lookup on a connection that is already open. It stays in the shard as well — that copy is the one that travels — but nothing reads it from there on the hot path. The write side had the same shape: `put_image` opened the shard afresh for each image, which mattered little when exports were a handful of new photographs and matters a great deal now that a re-index sends thousands. The handle is kept and reused, invalidated by shard id so sealing a full one and moving to the next drops it without anything having to remember to. `INDEX_SCHEMA` is `CREATE ... IF NOT EXISTS` like the shard schema, so the new column is added on open for an index already on disk — the same trap, caught the same way. Two tests: that the index time survives reopening the store, and that an index written before the column can still be opened and written to. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
9052b1a1de |
Merge: repair shards that predate the crop and index-time columns
Build and test / Desktop (Linux) (push) Successful in 33m10s
Build and test / Layer separation (push) Successful in 34s
🐳 Android image / Build and push (push) Successful in 3s
Build and test / android-image (push) Successful in 3s
Traceability / Requirement traces (push) Successful in 38s
Build and test / Android (aarch64) (push) Failing after 52m58s
The schema batch is all CREATE IF NOT EXISTS, so two columns added to it never reached any shard already on disk — and every export into one failed on "no such column", quietly, logged at warn while the sync reported success. That, rather than anything in the export logic, is why this library's shard stayed at 1,807 faces while the catalog reached 15,194. Shards are now upgraded when opened, and a peer's read-only shard is read as it stands. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
76eece8500 |
Add the columns an existing shard never got
`SHARD_SCHEMA` is entirely `CREATE ... IF NOT EXISTS`, which does exactly nothing to a table that already exists. So `crop` and `indexed_at`, both added to that batch, never appeared in any shard that had been written before — and the `INSERT` naming them failed with "no such column". Which took face export down completely, on every library that had ever synced a face. Silently: `export_to_shards` returns the error, `sync_face_shards` logs it at warn, and the sync goes on looking successful while the catalog fills with faces no other device will ever see. This library's shard sat frozen at 1,807 faces with 15,194 in the catalog, and the reason was this rather than anything in the export logic. Shards are upgraded on open now: both columns are additive and nullable, so catching up is one `ALTER` each. There is deliberately no version counter — "does this column exist" is the question actually being asked, and asking it directly cannot fall out of step the way a counter can. A peer's shard is opened read-only and cannot be repaired, so one written before crops is read as it stands, with a `NULL` standing in for the column. An adopted face simply has no crop, which is the truth about it. Four tests, built against the pre-crop schema written out in full rather than derived from the current one — the point being that it is *not* the current schema and must not track it. Verified against the real 1,807-face shard on this machine: the ALTERs apply, writes succeed, and nothing already in it is lost. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
43097f5033 |
Merge: make face sync actually reach the other device
Build and test / Desktop (Linux) (push) Successful in 31m3s
Build and test / Layer separation (push) Successful in 36s
🐳 Android image / Build and push (push) Successful in 1s
Build and test / android-image (push) Successful in 1s
Traceability / Requirement traces (push) Successful in 30s
Build and test / Android (aarch64) (push) Failing after 50m38s
Two independent faults, both of which had to be fixed before a tablet could show what a laptop had indexed. An image was exported to the face shards exactly once, so a re-index updated the catalog and nothing else — this library went from 1,807 faces to 15,194 and kept syncing the original 1,807. And people never crossed a device boundary at all: the catalog merge handled collections and keywords only, so the far end received every face and no groups, and drew an empty People screen over a full catalog. Names, confirmations, rejections and set-aside groups now merge by uuid, with faces matched across devices by photograph and box overlap. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
4ed10f7b23 |
Let a person cross from one device to another
The face shards carry boxes, landmarks and embeddings. What they deliberately do not carry is who anybody **is** — the person rows, their names, and the assignments joining the two. Those travel in the catalog snapshot, which is a whole-file copy and does contain them. But the snapshot is *merged*, not adopted, and this merge only ever looked at collections and keywords. `face_shard`'s own module note says people travel in the snapshot; nothing implemented it. So a second device received every face and no people at all, and drew an empty People screen over a full catalog. Exactly what a tablet showed after syncing thousands of faces from a laptop. What travels is what the user decided, following the rule the rest of this module already follows — judgements travel, inference is rebuilt: - **People**, by uuid on `revision`, exactly as a collection is: the name, and whether the group was set aside. - **Confirmations**, and **rejections** — "this is not her" is a fact too, and is why re-clustering does not put it back. - **The suggestions inside an ignored group**, which are otherwise ordinary inference but are what anchors the ignore. Without them a group set aside on one device reappears on the other, the same fault that made "Not interested" not stick locally. Ordinary suggestions are not carried. Both devices hold the same embeddings and clustering is deterministic, so each recomputes them and arrives at the same answer; shipping them would double the merge for no new information. **A face has no cross-device identity**, and unlike a collection there is no uuid to give it one. Both devices do agree on `oc:fileid` and roughly on the box, so a remote face is matched to the local face on the same photograph whose box overlaps it most, above 0.5 IoU. That is not a new rule — it is the one `record_detections` already uses to carry a confirmation across a re-index, and it is loose on purpose: the question is "the same face in the frame", not "the same rectangle". A local confirmation is never overwritten. Two devices confirming one face as different people is a real disagreement and an assignment carries no revision to settle it with; taking the remote's answer would let a sync undo what the user just did on the device in their hands. The remote's schema is probed rather than assumed: `remote_is_mergeable` admits any catalog at or below this version, so one written before faces existed, or before V10 added `ignored`, is ordinary. An absent table skips this half instead of aborting a merge that would otherwise have succeeded. Nine tests, including that the name lands on the overlapping face and not its neighbour in the same frame, that a set-aside group stays set aside, that an ordinary suggestion does not travel, and that merging twice changes nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
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> |
||
|
|
b60f9d10d9 |
Merge: 32-pixel faces, a header that does not overlap, and set-aside that sticks
Build and test / Desktop (Linux) (push) Successful in 30m54s
Build and test / Layer separation (push) Successful in 44s
🐳 Android image / Build and push (push) Successful in 1s
Build and test / android-image (push) Successful in 1s
Traceability / Requirement traces (push) Successful in 43s
Build and test / Android (aarch64) (push) Failing after 52m25s
The size floor comes down to 32 source pixels with the blur floor moved to match — they are coupled, since an upsampled face scores low on sharpness whatever its original quality, and dropping one without the other would have undone itself. The Identity header stops pinning its rows shorter than the controls in them, so the name field no longer draws through the buttons below it. And a group set aside now survives Regroup: its faces anchor the way confirmations do, so they stay where the user put them instead of regrouping into a fresh person with no ignore flag. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
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> |
||
|
|
c81d6865a7 |
Stop the name field running through the buttons under it
The Identity header was two rows pinned to 32px and 36px. Neither number was big enough for what the row held: a `Field` is `Theme.touch-target` — 44px — and a `Button` is `Theme.control-height`. Slint honours a child's own height and lets it overflow the box the layout gave it, so the name field drew 44px from the top of a 32px row while the button strip began at 38px. The overlap was 6px of text box sitting on top of "Confirm all". Neither row states a height any more. The first takes the height of what is in it, and the strip takes the height of a control — read from the theme rather than from the row inside it, since `actions` sizes itself from the Flickable's viewport and measuring it back would be a binding loop. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
1d4c3348af |
Let a 32-pixel face count, and move the blur floor with it
64 source pixels was too strict: it threw away 70% of everything the detector
finds, and plenty of what it took were faces a person could name.
Lowering it is not a one-line change, because the two floors are coupled. A face
under 112 pixels is *upsampled* to reach the embedder and upsampling invents no
edges, so a small face scores low on sharpness however crisp the original was.
Re-measured over the reference library with `face_index --quality`:
min crop min sharp size cut blur cut kept
32 0.000 44% 0% 56%
32 0.002 44% 3% 53%
32 0.005 44% 8% 48%
32 0.010 44% 16% 40%
32 0.020 44% 27% 30%
64 0.020 70% 7% 23%
Holding the blur floor at 0.020 while dropping the size floor to 32 would have
rejected a further 27% — for being small rather than for being blurred — and
kept only 30%, barely more than the 23% the strict pair kept. Most of the point
of lowering the size floor would have gone straight back out through the other
gate.
0.005 removes 8% of what the size floor leaves, which is the same job 0.020 was
doing at 64 (7%): the large-but-soft face this gate exists for. Together they
now keep 48% of what the detector finds, against 23% before.
The box pre-filter follows down to 24, staying below what the real floor accepts
so it cannot reject a face that would have cleared 32.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
|
c7cdf7e5f0 |
Merge: face quality gates, and identity filters that combine
Build and test / Desktop (Linux) (push) Successful in 32m56s
Build and test / Layer separation (push) Successful in 50s
Traceability / Requirement traces (push) Successful in 39s
🐳 Android image / Build and push (push) Successful in 3s
Build and test / android-image (push) Successful in 3s
Build and test / Android (aarch64) (push) Failing after 52m43s
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> |
||
|
|
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> |
||
|
|
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>
|
||
|
|
329c388d30 |
Merge: each canvas overlay goes home to its own domain
🐳 Android image / Build and push (push) Successful in 2s
Build and test / android-image (push) Successful in 2s
Build and test / Desktop (Linux) (push) Successful in 1h23m17s
Build and test / Layer separation (push) Successful in 41s
Traceability / Requirement traces (push) Successful in 25s
Build and test / Android (aarch64) (push) Failing after 52m22s
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> # Conflicts: # docs/traceability.md |