# DarkRoom — Code health and the cost of a contribution

**Status:** Audit · 2026-08-27
**Companion to:** [architecture.md](architecture.md), [technical-debt.md](technical-debt.md),
[view-composition.md](view-composition.md)

What it costs to add something to this codebase, measured rather than estimated, and the work that
would lower the price.

[technical-debt.md](technical-debt.md) records compromises that were *chosen* — each one has a
reason that outlived the person who took it. This document records the opposite: friction nobody
chose, which accumulated because no single commit was responsible for it. The distinction matters
when deciding what to touch. A TD entry is load-bearing until its "done when" is met; an entry here
is not defending anything.

It is also not a bug list. Everything below compiles, passes 2,042 tests and ships.

---

## 1. What was measured

Every figure in this document is reproducible from a clean checkout. Worktrees under `.claude/` and
build output under `target/` are excluded from all counts — including them roughly triples the line
totals and was the first thing to get wrong.

```bash
# Lines of Rust per crate
for d in core/* ui/* platform/* apps/* tools/*; do
  [ -d "$d/src" ] && echo "$(find $d/src -name '*.rs' -exec cat {} + | wc -l) $d"
done | sort -rn

# unwrap() in production code only — split each file at its #[cfg(test)] marker
# (a naive grep counts ~1,500 and tells you nothing)

# Distinct window properties written per UI module
for f in ui/dr-ui/src/*.rs; do
  echo "$(grep -oP '\b(w|window|win|ui)\.\Kset_[a-z0-9_]+(?=\()' "$f" | sort -u | wc -l) $f"
done | sort -rn
```

| Measure | Value |
|---|---|
| Rust across 19 crates | ~112,000 lines; ~78,000 after comments and blanks |
| Comment density | 28% overall, 20–34% per crate |
| Test functions | 2,042, plus 21 integration test files |
| `.unwrap()` in production code | **3** — one in `dr-gpu`, two in `dr-ingest` |
| `.unwrap()` in test code | ~1,500, which is where it belongs |
| `unsafe` blocks | 9 — three of them the face scan's SIMD kernels (faces.md §9) |
| `TRACES` tags / orphan tags | 793 / 0 |
| Resolved dependencies | 826 |
| Largest function | `dr-ui::run` — 1,855 lines |
| `AppWindow` members | 282 properties + 190 callbacks |

CI gates on `cargo fmt --check`, `cargo clippy --workspace --all-targets -- -D warnings`,
`cargo test --workspace`, a release build, and an Android cross-check. The traceability matrix is
regenerated and compared, with a pre-commit hook that keeps it in step.

---

## 2. The seams, graded

Six things a contributor might plausibly want to add. Each grade was checked against the tree.

| Feature | What you touch | Cost |
|---|---|---|
| A develop operation<br>*split toning, channel mixer* | One file in `core/dr-pipeline/ops/`. Nothing else. | **Trivial** |
| A RAW format | A `Format` variant and magic-byte recognition. `dr-decode` is a generic TIFF walker carrying only 3 format-specific branches. | **Easy** |
| A neighbourhood operation<br>*dehaze, a sharpener* | Rust in `dr-pipeline/src/ops/` implementing `Operation` + `DetailStage`, plus a stub YAML declaring `rust:` and `order:`. | **Moderate** |
| A parameter widget<br>*a colour wheel* | A `WidgetKind` variant, `develop::supported()`, the panel model, a Slint component. | **Moderate** |
| A second sync backend<br>*S3, WebDAV, a local folder* | `RemoteBackend` is the easy half. Seven UI files construct `NextcloudBackend` directly and ten signatures take it concretely — see [CH-2](#ch-2). | **Hard** |
| Anything with its own UI | `app.slint`'s root component, a ~1,000-line `wire()`, and `run()` at 1,855 lines — see [CH-1](#ch-1). | **Hard** |

The top half of that table is the good half, and it is very good. The bottom half is one problem
wearing two hats: **`dr-ui` has no seams, so every UI feature lands in the same three files.**

---

## 3. What is load-bearing, and must not be "tidied"

Listed before the findings deliberately. A remediation document that only enumerates problems invites
someone to fix something that was right.

**The operation declaration format.** `ops/*.yaml` + `build.rs` is a working plugin system that
happens to resolve at build time — see [display-and-extension.md §6](display-and-extension.md). Its
deliberate smallness is the point: the expression grammar is restricted so a declaration cannot
become a second, worse place to write code. Do not "improve" it by letting a node name arbitrary
Rust.

**No operation is named in `ui/`.** Verified: all fifteen built-in op ids grepped across every `.rs`
and `.slint` file in `ui/` yield exactly one hit, a localisation test in `labels.rs:353`. This is
what makes "a new operation is one file" true rather than aspirational, and it is the single
property most likely to be destroyed by a well-meaning special case in the panel. See [CH-5](#ch-5).

**Unimplemented widgets degrade rather than break.** `develop::supported()` lists every `WidgetKind`
explicitly instead of using a wildcard, so a new kind added to the core surfaces as a compile error
rather than as silence, and a widget nothing draws falls back to sliders with the edit still
working (ARCH §4.3a).

**The mpsc-plus-timer worker shape.** Slint's event loop must never block (NFR-P9). Threading is not
what any finding below proposes changing.

**`sync_rows` mutating rows in place.** Replacing the model breaks slider dragging.

---

## <a id="ch-1"></a>CH-1 — `dr-ui` has no view layer, and the cost is compounding

**Where:** `ui/dr-ui/src/lib.rs:836`, `library_ui.rs:4374`, `collections_ui.rs:1457`,
`ui/dr-ui/ui/app.slint`

### What it is

Every UI feature lands in the same three places: the Slint root component, one of the `wire()`
functions, and `run()`.

| Function | Lines |
|---|---|
| `lib.rs::run` | 1,855 |
| `library_ui::wire` | 998 |
| `collections_ui::wire` | 964 |
| `identity_ui::wire` | 478 |
| `masks_ui::wire` | 357 |
| `settings_ui::wire` | 317 |

Above them sits one `AppWindow` carrying 282 properties and 190 callbacks, with exactly one Slint
global in the whole `ui/` directory — and it is not exported. Cross-view state therefore has nowhere
to live except the root component, and every interaction is a callback registered inside a `wire`.

The core crates are healthy by the same measure: their largest functions are `compose_full` at 437
lines and `build.rs::emit_node` at 558, both of which earn their length. This is specific to `dr-ui`.

### Why it matters more than it did

**[view-composition.md](view-composition.md) already diagnosed this and specified the fix**, in
three independently landable stages, on 2026-08-09. None of the three has landed. In the eighteen
days since, the numbers that document itself used have moved:

| Its measure | 2026-08-09 | 2026-08-27 |
|---|---|---|
| `run()` | 500 lines | **1,855** |
| `library_ui` distinct window properties | 24 | **54** |
| `lib.rs` distinct window properties | 23 | **52** |
| `collections_ui` | 9 | 11 |
| `launch_ui` | 16 | 16 |
| `set_show_*` call sites | 7 | 9 |
| View-state booleans on `AppWindow` | 2 | **5** |

Four modules it did not list now write window properties too: `settings_ui` (39), `import_ui` (24),
`identity_ui` (18), `masks_ui` (16).

The prediction it made has also come true literally. It described `app.slint` compensating for two
mutually exclusive booleans with `if !root.show-launch && root.show-library` chains. There are now
five such booleans — `show-launch`, `show-library`, `show-identity`, `show-settings`, `show-import` —
and the chains at `app.slint:1405`, `:1445` and `:1654` are five-term conjunctions. Each new view
multiplies the conjunctions rather than adding to them.

None of this is difficult work. It is simply work that every feature must now do, in files every
other feature is also editing — which is why two contributors working in parallel conflict by
construction, and why a newcomer must read a 1,855-line startup sequence with real ordering
constraints before safely inserting a line into it.

### What to do

Execute [view-composition.md](view-composition.md) as written. Its analysis holds and its staging is
right; stage 1 alone removes the nullable-callback knot and the duplicated post-load sequence, and
is worth landing whether or not stages 2 and 3 follow.

One thing to add to it, because it was not in scope there: the `wire()` functions. They are already
sectioned internally by comment, so lifting each section into `fn wire_ratings(window, ctl)`,
`fn wire_keywords(...)` and so on is mechanical, checked entirely by the compiler, and can go one
section per commit. It gives a feature a *function* to own rather than a region of one, which is
what removes the conflict, and it does not wait on stage 1.

Do the extraction behind new features rather than as a big-bang refactor. The pile grows either way;
the question is only whether each new feature adds to it or subtracts.

**Done when:** no function in `dr-ui` exceeds 300 lines, a new view registers itself instead of
adding a boolean to `AppWindow`, and `active-view` has replaced the boolean set so both-true is
unrepresentable.

---

## <a id="ch-2"></a>CH-2 — `RemoteBackend` is an abstraction nothing above `dr-sync` uses

**Where:** `core/dr-sync/src/lib.rs:46`; seven files in `ui/dr-ui/src`

### What it is

The trait is carefully built. Capability negotiation decides the sync strategy; range reads are
documented as a hint rather than a guarantee so correctness holds either way; chunked upload is
deliberately kept internal so one server's protocol cannot leak into the interface. Every choice is
explained where it is made.

And then `trash.rs`, `import.rs`, `export.rs`, `derived_sync.rs`, `launch_ui.rs`, `library.rs` and
`settings_ui.rs` each construct `NextcloudBackend` directly — 34 references — and ten functions take
`&NextcloudBackend` rather than `&dyn RemoteBackend`. Exactly **two** sites in the tree take the
trait object, both inside `dr-sync` itself.

### Why it matters

The abstraction currently buys nothing it was designed for. Worse, it reads as though it does: a
contributor who wants a WebDAV or local-folder backend will find a well-documented trait, implement
it correctly, and only then discover that nothing above `dr-sync` can be handed the result.

There is no defence of this in the tree, which is what makes it an entry here rather than in
`technical-debt.md`. It is what a single-backend application looks like when the second backend has
not yet been attempted.

### What to do

Change the ten signatures to `&dyn RemoteBackend` and construct the backend once, behind something
the UI does not name — the same discipline `develop.rs` already applies to operations. The trait is
already correct, so this is a mechanical change, and it is much cheaper now than during a second
backend when it would be entangled with that backend's own problems.

Worth doing even if no second backend is ever written: it makes the sync layer testable against a
fake, which today it is not.

**Done when:** `NextcloudBackend` is named in at most one file in `ui/`, and a stub backend can be
substituted in a test without touching the UI.

**Done**, with one half deliberately left. `ui/dr-ui/src/remote.rs` is now the only file in the
interface that names a connector; the ten worker functions take `&dyn RemoteBackend` and will accept
a stub. Every method the UI ever called on a backend — `get`, `put`, `list`, `delete`, `create_dir`,
`move_to` — was already on the trait, so nothing had to be added to it.

What remains is **credentials**. `AppCredentials` is an app password obtained through Login Flow v2,
which is a Nextcloud protocol rather than a general notion of how one authenticates to a remote, and
seven files still name it. Abstracting it needs a decision about what an account *is* across
backends — an OAuth token, a bucket key pair and an app password have no useful common shape — and
making that decision before a second backend exists would produce a confident wrong answer. It is a
design problem rather than a mechanical one, and it should wait for the backend that forces it.

---

## <a id="ch-3"></a>CH-3 — There is no path in for a contributor who is not already here

**Where:** repository root

### What it is

No `CONTRIBUTING.md`. No `rust-toolchain.toml`, though CI pins 1.92.0 exactly. No issue or PR
templates.

The documentation that exists is excellent — 7,990 lines across 14 files, including 177 numbered
requirements — and all of it is written for someone who has already decided to work on this. Nothing
tells a newcomer which document to read first, that `core/dr-pipeline/ops/README.md` is the door
with the lowest bar, or that a clone without `git-lfs` needs one command before the build succeeds.

That last point is handled well in the code: `dr-segment`'s build script detects an LFS pointer file
and fails with an instruction rather than embedding 130 bytes and dying at inference time. It is
simply not written anywhere a first-time cloner would look.

### Why it matters

It is the cheapest item in this document and it gates every other contribution. A person who cannot
get a first build is not going to reach the parts that are good.

### What to do

Write `CONTRIBUTING.md` and point the first door at the operation format. "Add a develop operation"
is a genuinely one-file contribution with declared tests that run under `cargo test` — the best
first experience this codebase can offer, and it happens to teach the architecture's central idea on
the way through.

Then state the three things that are currently folklore: `git lfs` is a prerequisite, the first
build resolves 826 crates and takes a while (saying so stops it reading as a hang), and the
toolchain is 1.92.0. Add `rust-toolchain.toml` so that last one is enforced rather than documented —
a contributor on an older stable currently gets confusing type errors instead of a version message.

**Done when:** someone who has never seen the repository can clone it, build it, and land a new
`ops/*.yaml` node without asking a question.

---

## <a id="ch-4"></a>CH-4 — Coverage is counted by tagging, not by behaviour

**Where:** `docs/traceability.md`, `tools/traceability`

### What it is

Not a new finding — [display-and-extension.md §7](display-and-extension.md) states it plainly, and
`traceability.md` itself says coverage is the intersection of tagged and defined IDs. The tooling is
genuinely good: 732 tags, zero orphans, denominators parsed from `requirements.md` at run time
rather than hardcoded, regenerated in CI and guarded by a pre-commit hook.

What it cannot do is check that the code under a tag does the thing. `FR-DEV-8` is currently tagged
against instance-buffer plumbing a future spot-removal operation *would* use; `FR-DEV-7` against a
history row for a frontend that does not exist. Both read as covered.

### Why it matters

The 55.4% figure is an overstatement of unknown size, and the risk is that it is used as a planning
input. It is recorded here so that the number keeps its asterisk when read outside the document that
already qualified it.

### What to do

Nothing structural — the honest framing already exists in two places. Adopt
display-and-extension.md's rule going forward: **close a requirement with a test that would fail if
the behaviour were removed**, and let the percentage move slowly and mean something.

**Done when:** the rule is stated in `CONTRIBUTING.md` alongside the tag syntax, so it reaches
someone adding their first tag.

---

## <a id="ch-5"></a>CH-5 — The best invariant in the codebase is unprotected

**Where:** `ui/dr-ui/src`, `ui/dr-ui/ui`

### What it is

"No code in `ui/` names an operation" (FR-DEV-3a) is what makes the whole declarative pipeline pay
off, and it is currently maintained by discipline alone. Nothing fails if someone special-cases
`exposure` in the panel to fix a layout problem at five in the evening.

### Why it matters

It is one grep, it would take an hour, and it protects the property this audit rates highest. The
failure mode is silent and cumulative: the first special case is defensible, and by the fifth the
panel names half the chain and "a new operation is one file" has quietly stopped being true.

### What to do

A test that greps `ui/dr-ui/src` and `ui/dr-ui/ui` for every id in `ops/*.yaml` and fails on a hit,
with the current `labels.rs` localisation test as its one allowed exception. It belongs in CI beside
the traceability check, which is the existing precedent for a structural gate.

**Done when:** adding `window.set_exposure_slider(...)` to the panel fails CI with a message naming
FR-DEV-3a.

---

## 4. Order of work

Ordered by value per hour rather than by size.

| | Item | Effort | Why first |
|---|---|---|---|
| 1 | [CH-3](#ch-3) — `CONTRIBUTING.md` + `rust-toolchain.toml` | Half a day | Gates everything else; unblocks the trivial seam that already works |
| 2 | [CH-5](#ch-5) — CI gate on operation names in `ui/` | An hour | Protects the property everything else in the pipeline rests on |
| 3 | [CH-2](#ch-2) — make `RemoteBackend` load-bearing | 1–2 days | Mechanical now, entangled later; also makes sync testable |
| 4 | [CH-1](#ch-1) — split the `wire` functions | Incremental | No behaviour change, compiler-checked, one section per commit |
| 5 | [CH-1](#ch-1) — [view-composition.md](view-composition.md) stages 1–3 | Sustained | Largest and most invasive; every deferred month adds to the pile |

Items 1, 2 and 3 are independent of each other and of the rest. Item 4 does not wait on item 5.

---

## 5. What this document does not claim

It measures structure, discipline and coupling. It does **not** assess runtime correctness, GPU
shader behaviour, security posture, or whether any tagged requirement is actually implemented —
[CH-4](#ch-4) is precisely the observation that the last of those is unmeasured.

Line counts and function sizes are proxies. `run()` being 1,855 lines is a real problem because
every feature must edit it, not because 1,855 is a bad number; `build.rs::emit_node` at 558 lines is
not a problem at all. Where a figure appears above, the sentence around it says which of the two it
is.

The audit was first measured against `origin/master` at `d4a34ef`, then re-measured after
`android-bundled-face-models` merged in: `run()` moved from 1,810 lines to 1,855, and the whole-
library face sweep added its fetching path to `library.rs`. The seam grades are unchanged by that
merge — the work went into the seams that already existed rather than cutting new ones, which is
itself the pressure CH-1 describes.