Architectural rules

‍Verified against 42eca0e8fe on 2026-09-29.

The five rules, and the evidence behind each. Most are enforced by CI, but not all and not all

on every change — see the note below. CLAUDE.md states them as imperatives; this file says why, and what each one cost to learn.

‍ Each finding below is dated with the commit that established it. A finding is only as good as its hash: before acting on one older than the code you are changing, re-measure it — and re-date it here when you do. Where an earlier version of a claim was wrong, that is recorded rather than quietly corrected: how a claim was wrong is usually the more useful thing to know.

‍What CI actually enforces, measured 2026-09-29. An earlier version of this line said "the five rules CI enforces on every change". That is too strong, and the sentence was introduced by the migration rather than carried from CLAUDE.md. Rule 1 is checked on every build (pragma_once_to_guards.py --verify, ci.yml:189). Rule 2 runs on pull requests only and in one matrix cell (check_unused_includes.sh --changed, ci.yml:217). Rule 3 is covered, but by gates this document does not name — the SQL-handle and SQL-outside-the-module ratchets in check_module_boundaries.sh (ci.yml:177). Rules 4 and 5 have no gate at all: nothing in tools/ checks params threading or a stored-format version bump. CLAUDE.md hedges this correctly ("CI enforces most of them"); this file did not.

<tt>#pragma once</tt> is FORBIDDEN — use an include guard

Found 8de7446ff7, 2026-08-06.

Every header in src/ uses an explicit #ifndef DT_<PATH>_H / #define / #endif guard, named after the path relative to src/ (src/develop/masks/masks_history.h → DT_DEVELOP_MASKS_MASKS_HISTORY_H). Do not add #pragma once to an existing header, and do not start a new one with it.

This is not a style preference. #pragma once and an include guard behave identically at the preprocessor level, but #pragma once silently makes a cyclic include graph compile: a header re-entered mid-definition is skipped, and the first inclusion finishes with whatever it had at that point. That is how three include cycles survived unnoticed in this codebase for years, each one a header trailing-including a header that includes it back (see doc/include-graph.md). Explicit guards make the same situation greppable and reviewable instead of invisible.

Enforcement: python3 tools/pragma_once_to_guards.py --verify exits non-zero if any #pragma once reappears, and runs in CI's "Check include hygiene" step. It sweeps every header spelling — .h, .hh, .hpp, .hxx — from the repository root the tool itself sits in, not from the working directory: a sweep restricted to .h, or one run from the wrong directory, is a gate that passes by finding nothing. python3 tools/include_graph.py --summary must keep reporting cycles 0.

**darktable.h (at src/, not in a module) has no guard either — it has a TRIPWIRE.** It ends up included by at most one path per translation unit (an entry point calling dt_init(), or a subsystem that owns one of the darktable members), so a second inclusion is never legitimate: it means the header arrived through a path nobody intended. A guard would absorb that silently; instead the file #errors on re-inclusion. If you hit it, do not add a guard — find who included it and give that code the specific lib it needs (common/logging.h, system/mem_alloc.h, …) or the accessor for the global it wants (dt_dev_get_global(), dt_control_get_global(), …). No header may include it; as of this writing none does.

When auditing this, grep for darktable\.h"</tt> and check the spelling. Includes can be written relative to the including file's own directory, which is how several files hid from earlier audits while the header still lived in <tt>src/common/</tt>. It now sits at <tt>src/</tt>, so <tt>\#include "darktable.h"</tt> IS the canonical root-relative spelling. Three files (and one header, <tt>common/colorchecker.h</tt>) hid behind that spelling through several audits of this series; the compile-time tripwire is what finally caught them. <tt>tools/include_graph.py</tt> resolves both spellings and was right when the ad-hoc greps were wrong. Five headers deliberately have NO guard at all and must never get one: <tt>common/module_api.h</tt>, <tt>views/view_api.h</tt>, <tt>libs/lib_api.h</tt>, <tt>imageio/format/imageio_format_api.h</tt>, <tt>imageio/storage/imageio_storage_api.h</tt>. They are X-macro headers, re-included several times in the same translation unit with different macros defined, and expanded inside struct bodies to generate members. For the same reason a top-level <tt>\#include</tt> in one lands inside those structs. The precise rule, as the imageio pair actually implement it: real includes must sit inside the <tt>\#ifdef FULL_API_H</tt> block — that macro is defined only in full-API mode, while the struct-body expansion defines <tt>INCLUDE_API_FROM_MODULE_H</tt> instead and skips the block — and only other X-macro headers (<tt>common/module_api.h</tt>) may be included unguarded. <blockquote>&zwj;Two corrections, 2026-09-29. This used to say <tt>common/module_api.h</tt> "has no includes itself". It has one — <tt>\#include \<glib.h\></tt> at <tt>:32</tt>, four lines below its own comment saying no include may ever be added. It is harmless only by accident: glib's own guard makes the in-struct re-expansion empty. And only the imageio pair actually implement the rule as stated; <tt>views/view_api.h:27</tt> and <tt>libs/lib_api.h:26</tt> carry real system includes (<tt>\<gtk/gtk.h\></tt>, <tt>\<glib.h\></tt>) at struct-body-expansion level. So a reader auditing all five against this paragraph will find two that do not conform, which the paragraph did not say. Symbols used </blockquote>outside that block (<tt>dt_version()</tt>, <tt>dt_print()</tt>, <tt>IS_NULL_PTR</tt>) are the consuming <tt>.c</tt> file's responsibility. @subsection autotoc_md39 A header includes only what its own declarations need Found <tt>8400a289b4</tt>, 2026-08-08. Everything else belongs in the <tt>.c</tt>. A header that includes more becomes a supply line its consumers never asked for and cannot see: they compile because something upstream happened to pull in what they use, and the day anyone tidies that include away the breakage surfaces somewhere else entirely, in a file that was never touched. This is not theoretical. Removing <tt>gui/gtk.h</tt> broke a dozen IOPs because it had been the only thing pulling <tt>sqlite3.h</tt> in ahead of <tt>common/points.h</tt> (whose vendored SFMT <tt>\#define N</tt> then collided with <tt>sqlite3_compileoption_get(int N)</tt>). And within the same series, deleting an unused <tt>widgets/label.h</tt> from <tt>widgets/dialog.c</tt> removed <tt>dt_free</tt> — arriving through <tt>label.h</tt> -> <tt>system/mem_alloc.h</tt> — from a file that had never named either header. Concretely: if a header declares <tt>void f(GtkWidget *w)</tt>, it includes <tt>\<gtk/gtk.h\></tt> and nothing more. Implementations do not belong there either — <tt>widgets/label.h</tt> carried five <tt>static inline</tt> helpers, and those five forced four extra includes on all ~30 of its consumers. Moving them to <tt>label.c</tt> left the header needing only <tt>\<gtk/gtk.h\></tt>. The one legitimate exception is a header whose published interface is inline code (<tt>widgets/draw.h</tt>), which necessarily includes what that code calls. Keep those rare, and keep them honest: they are a deliberate performance trade, not a convenience. <tt>tools/check_unused_includes.sh</tt> gates the include lines a change adds. <tt>tools/header_consumers.py</tt> reports what each includer of a header actually takes from it, separating a header's own symbols from what it merely forwards. **<tt>header_consumers.py</tt>'s "files using nothing from it" bucket does NOT mean the file can drop the include.** It means this include is redundant — the file reaches those symbols through one of its other includes. Those are exactly the files that are relying on the supply line described above, so under this tree's rule they need the include added explicitly, not removed. Deleting all seven such includes when <tt>colorprofiles/colorspaces.h</tt> was split still compiled in Release and Debug, and broke <tt>build-nofeatures</tt>, where <tt>control/jobs/control_jobs.h</tt> lost the two types <tt>dt_control_export()</tt> is declared with — the other supplier only existed in the feature-full configurations. Confirm against the symbols the file actually names (<tt>grep</tt> for the header's types and functions) before removing anything, and never trust one build configuration to prove an include is unnecessary. @subsection autotoc_md40 No SQL in GUI modules Found <tt>22f623c0be</tt>, 2026-06-25. <tt>src/libs/</tt> and <tt>src/views/</tt> modules must contain no raw SQL. Database access belongs behind named functions in <tt>src/common/</tt> and <tt>src/database/</tt>. When a GUI module needs data, add or extend a <tt>dt_collection_*</tt> / <tt>dt_film_*</tt> / <tt>dt_tag_*</tt> function and call it. Reuse existing helpers (<tt>dt_film_get_id</tt>, <tt>dt_selection_select_list</tt>) rather than re-issuing SQL. <blockquote>&zwj;Two corrections, 2026-09-29. This named <tt>dt_collection_get_extended_where</tt> as a helper to reuse: it does not exist anywhere in <tt>src/</tt> — the only survivor is the file-static <tt>_extended_where()</tt> in <tt>src/database/collection_query.c</tt>, removed from the public surface by <tt>ec5b7de3f0</tt>. Two orphaned doc comments in <tt>common/collection.h</tt> still describe it. And the rule named <tt>common/collection.c</tt> and <tt>common/film.c</tt> as the places database access belongs: both now contain zero SQL (measured: 0 matches for <tt>sqlite3_prepare</tt>, <tt>DT_DEBUG_SQLITE3_PREPARE</tt> or <tt>sqlite3_exec</tt> in either). The SQL moved to <tt>src/database/</tt> and its seven repositories; <tt>c75eaef473</tt>'s subject says it outright — "Collection: rules cross the boundary, not SQL". Following the old text, you would add a query to <tt>common/collection.c</tt> and learn otherwise only when CI's boundary ratchet failed. </blockquote> Examples added during the collect rewrite: <tt>dt_collection_get_property_values()</tt>, <tt>dt_collection_get_images_for_rule()</tt>, <tt>dt_film_relocate()</tt>. @subsection autotoc_md41 Pipeline↔module interface is history Found <tt>22f623c0be</tt>, 2026-06-25. The ONLY thread-safe interface between the pixel pipeline and an IOP module is history (guarded by <tt>dev-\>history_mutex</tt>). <tt>module-\>params</tt> and <tt>module-\>blend_params</tt> belong to the GUI thread and are NOT thread-safe — the pipeline thread must never read or write them. Do NOT call <tt>dt_iop_commit_params(module, module-\>params, ...)</tt> from pipeline code. Commit from the history snapshot (<tt>hist-\>params</tt>), never the live module params. To push live/transient state to the pipe (e.g. drawlayer realtime stroke, ashift edit mode), either (a) write it through history under <tt>history_mutex</tt>, or (b) use the transient-resync interface <tt>dt_dev_transient_params_{set,clear,get,active}</tt> in <tt>dev_history.{h,c}</tt>. See <tt>doc/reorganisation.md</tt> for the threading model (GUI diamond nodes vs. pipeline round nodes). @subsection autotoc_md42 A stored format's version is bumped only once it has shipped in a round-numbered release Found <tt>af78aa4e42</tt>, 2026-09-21. Three formats outlive the build that writes them: a module's params (<tt>DT_MODULE_INTROSPECTION</tt>), the database schema (<tt>CURRENT_DATABASE_VERSION_LIBRARY</tt> / <tt>_DATA</tt>, <tt>database/database.c</tt>) and the XMP sidecar (<tt>DT_XMP_EXIF_VERSION</tt>, <tt>common/xmp_sidecar.cc</tt>). Bump one only if its current version was distributed in a round-numbered release: Ansel 1.0, 2.0, … or, for a version inherited from darktable, a darktable release. A version that has not shipped in one is still open, and a change goes into it without a bump. For the database schema and the XMP, the bump itself is made as late as possible, just before Ansel's version number changes. A bump rejects nothing: the build that makes it converts every older version (<tt>legacy_params()</tt>, the schema migration steps, the readers of older XMP versions). What it adds is permanent, since each version keeps its conversion code for good, so versions follow round-numbered releases instead of piling up one per change in nightlies. Changing an open version in place has its own conditions, since nightlies have already written it: - Module params: append, never insert. The conversions from older versions commonly copy the old layout as a prefix over the defaults, so existing members keep their offsets. A blob of the same version but another size makes <tt>_sync_params()</tt> call <tt>legacy_params(N, N)</tt>, which is not told the stored size and has no branch for it, so the step is dropped: an edit saved with the shorter layout loses that module. <tt>_sync_params()</tt> does not fall back to copying the common prefix. - Database schema: a library already at version N never re-runs the step that brought it there (<tt>_upgrade_library_schema_step()</tt>), so what is added to N in place must also be applied, idempotently, to a library already at N: <tt>_sanitize_db()</tt> runs at every open. - XMP: a new key is optional, its absence meaning the default, and an existing key keeps its meaning: a sidecar written earlier under the same version still reads, and an older build reading a newer one skips what it does not know. Schema 36, data 9 and XMP 5 come from darktable releases. The bumps planned for Ansel 1.0 (schema 37, XMP 6) wait on the dedicated branch described in @ref "/home/runner/work/ansel/ansel/doc/masks_history_dedup.md" "`masks_history_dedup.md`" (this named it masks-history-dedup; no such branch exists locally or on the remote), see doc/masks_history_dedup.md.