IHE-RO-Tooling/dicom-browser
IHE-RO-Tooling/dicom-browser
2
0
Fork 0
Code Issues 3 Pull requests Projects Releases 5 Packages Wiki Activity Actions

add-de-identification #151

Merged
christof.schadt merged 13 commits from add-de-identification into main 2026-09-02 18:42:32 +02:00
Conversation 0 Commits 13 Files changed 57 +14057 -1080
christof.schadt commented 2026-09-02 18:42:16 +02:00
Owner
Copy link

This includes file-system backend fixes from a parallel branch.

This includes file-system backend fixes from a parallel branch.
christof.schadt added this to the 0.9.7 milestone 2026-09-02 18:42:16 +02:00 
plan-34: patient/study/series folders for filesystem storage. Promotes the
ZIP export's naming convention into a shared `dicomhelper::store_layout` and
makes the filesystem backend store through it. Hierarchy becomes the default
for new and existing connections; the read side needs no change because the
scan is already recursive and the index is keyed by full path.

plan-35: watch open folder connections for changes via notify +
notify-debouncer-mini, with an incremental path-scoped sync instead of a full
rescan. Documents why an automatic tree reload must re-key by identity: both
TreeViewState and TreeSelection are positional, and the patient list is sorted
on load. Phased so the DB sync ships before the tree merge.

Land #150 before #149 — see plan-35 §8.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Storing into a folder connection was flat while reading was already recursive
and the ZIP export already wrote patient/study/series — the store was the one
place that disagreed. It now writes through the same layout, from one shared
implementation in `dicomhelper::store_layout` that `create_zip_tab` also uses,
so the two cannot drift.

`StoreLayout` (Flat | Patient | PatientStudySeries) is a per-connection setting
with a combo box in the connection dialog. It defaults to PatientStudySeries
for new *and* existing connections: nothing on disk is migrated, and a root
holding both flat and nested files indexes correctly because every row is keyed
by its own full path.

Folder names are unique by identity, not by label: two studies of one patient
can share a date and description, and two series a modality and description,
which previously merged them into one folder (in the ZIP export too). Each
component now ends with a short FNV-1a digest of that level's UID unless the
sanitized label already contains the identity verbatim — so a UID-fallback name
and an intact PatientID keep their clean form, while a mangled PatientID gets
one. The digest is hand-rolled because std's DefaultHasher is not stable across
Rust releases and a folder name must not move with the toolchain.

A store never fails because of the layout: an unreadable object, an over-long
path (Windows MAX_PATH) or an uncreatable directory degrades to the flat root
with a warning. Deleting now prunes the folders it empties, stopping at the
archive root.

Implements src/doc/plan-34-filesystem-store-layout.md.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
While a filesystem tab is open, its folder is watched and the index is kept up
to date without the user asking. Implements phase 1 of
src/doc/plan-35-folder-watch.md — the DB half; the automatic tree reload
(phase 2) is deliberately not included, see below.

The reaction is incremental, not a rescan. `scan_and_sync` walks and stats the
entire root, which is the expensive phase on a network share, so reacting to a
handful of files must not pay it. Its diff/parse/insert body is factored into
`apply_sync`, shared by the full scan and the new path-scoped `sync_paths`; only
the way `disk_files` is gathered differs, so the two cannot drift in semantics.

Directory events reconcile the whole subtree rather than trusting per-file
events — that is how a new patient folder arrives now that #150 writes into
subdirectories, and it closes the inotify race where files created inside a
brand-new directory are never reported individually.

A vanished root (unmounted share, disconnected drive) leaves the index alone
rather than reading as "every file was deleted": that index is expensive to
rebuild and still describes what was there. Paths outside the root are ignored
so a stray event cannot attribute another archive's files to this one.

Watching is exposed as a `WatchableRoot` capability on `BuiltBackend`, beside
the existing `reference` and `delete` handles, so `DicomBackend` is untouched
and nothing downcasts. `FolderWatchState` mirrors `FhirState`: app-owned,
pumped once per frame, stopped on Drop; one watcher per root shared by every tab
open on it, and the watcher set is only rebuilt when the tabs actually change.
Events are coalesced by notify-debouncer-mini so a send of hundreds of files
produces a few batches, not hundreds.

Per-connection `WatchMode` (Native | Poll{secs} | Off), defaulting to Native.
Poll exists because inotify and FSEvents only report local activity: a write
made by another host to an NFS/SMB mount never fires one.

Phase 2 is separate on purpose. `refresh_tab_from_backend` clears `patients`,
`tree_state` and `selection`; both of the latter are keyed by *position* while
`on_patients_loaded` sorts the patient list, so an automatic reload would move
the user's expansion and selection onto different nodes. Until that reload
re-keys by identity, a change flags the tab with a badge next to the refresh
button instead.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Completes plan-35: the tree now follows a watched folder, not just the index.

It is an update, not a reload. Re-querying to discover what the watcher already
told us is slower, discards every lazily-loaded series, and manufactures the
positional re-keying problem that made an automatic refresh look risky in the
first place — `refresh_tab_from_backend` clears the tree and `on_patients_loaded`
rebuilds and sorts it. Mutating in place has none of those properties, and its
cost is proportional to what is on screen rather than to the archive: a delta
landing in a NotLoaded study is a no-op, because its series are fetched on
expand anyway.

`sync_paths` now returns an `IndexDelta` of `TouchedSeries` coordinates instead
of a bool. The delta deliberately carries no display fields: if it did, applying
it would become a second implementation of `group_into_patients` and
`instance_attributes_from_record`, free to drift from the one every query uses.
Node content instead comes from the existing scan-free calls — `query_series`,
and a new `refresh_studies` that is `group_into_patients` over
`records_for_scope`, narrowed to the studies a delta mentioned.

`app/tree_update.rs` owns every mutation together with its positional
side-arrays, so there is one place to get that right. Its tests pin down the
invariant that motivated the design — a selection or an expanded node must never
end up on a *different* patient after an insert or a removal — plus the shape
invariant that patients_open/studies_open/series_open stay in step with the tree
they index.

Two deliberate losses, stated rather than incidental: `refs_open` entries under
an affected study are dropped (keyed by four indices with no identity to anchor
to), and `search_marks` is cleared on a structural change (a transient search
result the watcher has no business recomputing). Anything the update cannot
apply falls back to phase 1's badge, and a tab with a query already in flight is
left to that query, which is fresher than the delta.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Filesystem storage layout (#150) and folder watching (#149), plus a fix for
the saved-connection list being truncated to 20 entries.
Picks up the filesystem store layout (#150) and folder watching (#149).

# Conflicts:
#	src/ui/create_zip_tab.rs
christof.schadt deleted branch add-de-identification 2026-09-02 18:42:32 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
bug
Something is not working

  
duplicate
This issue or pull request already exists

  
enhancement
New feature

  
help wanted
Need some help

  
invalid
Something is wrong

  
question
More information is needed

  
wontfix
This won't be fixed

No labels
bug
duplicate
enhancement
help wanted
invalid
question
wontfix
Milestone
Clear milestone
No items
No milestone
0.9.7
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
IHE-RO-Tooling/dicom-browser!151
No description provided.