Merge: t-paliad-074 architecture improvement audit (ada)

2026-04-30 03:02:13 +02:00
parent 2c67299740 61766161b7
commit 270f7d7ddc
1 changed files with 721 additions and 0 deletions
--- a/docs/improvement-audit-2026-04-30.md
+++ b/docs/improvement-audit-2026-04-30.md
@@ -0,0 +1,721 @@
+# Paliad — Architecture Improvement Audit
+
+Prepared by `ada` (consultant) on 2026-04-30 for task **t-paliad-074**.
+Scope: read-only architecture audit after the 9-merge push of t-paliad-066..073.
+This doc supersedes neither `docs/improvement-audit.md` (the original
+2026-04-18 product audit) nor `docs/audit-polish-2-2026-04-29.md` (UX polish).
+It complements them: this is the structural/maintainability lens.
+
+Today is 2026-04-30. The repo is at commit `2c67299` on `main`.
+
+Finding tags:
+
+- **Severity** — 🔴 active risk · 🟠 friction now · 🟡 future-proofing
+- **Effort** — 🟢 ≤30 min · 🟡 1-2 h · 🔴 half-day+
+
+---
+
+## Executive summary — if I do nothing else, fix these 3 things
+
+1. **`AdminDeleteUser` queries dropped tables `paliad.department_members` /
+   `paliad.departments`.** Migration 027 (2026-04-29) renamed those to
+   `partner_unit_members` / `partner_units`. The code at
+   `internal/services/user_service.go:768` and `:773` was missed by the
+   t-paliad-070 rename sweep (last edit 2026-04-27 by `c697fe34`, predates
+   the rename). Any admin who clicks "Delete user" in `/admin/team` today
+   will hit `pq: relation "paliad.department_members" does not exist`.
+   **Live production bug, blast radius: admin-only, but blocks user
+   off-boarding.** See **F-1**.
+
+2. **Seven live-DB integration tests skip silently when
+   `TEST_DATABASE_URL` is unset, and the repo has no CI.** This is the
+   exact pattern that masked the t-paliad-069 reminder placeholder bug
+   for ~24 h. F-1 above is another bug that a passing
+   `TestAdminDeleteUser` would have caught the moment migration 027
+   landed. Fix the visibility of the test gate: either add a Gitea
+   workflow with an ephemeral Postgres, or convert the silent skips to
+   `t.Fatal` when `CI=true`. See **F-9**.
+
+3. **Visibility predicate is centralised in
+   `internal/services/visibility.go` but inlined in 10 hot-path SQL
+   sites across `dashboard_service.go` (4×), `agenda_service.go` (2×),
+   `reminder_service.go` (2×), `team_service.go` (1×), and
+   `deadline_service.go` (1×).** This is the same security-critical rule
+   that t-paliad-058 already extracted — duplication crept right back
+   in. Every change to the team-visibility model (Chinese-wall
+   restrictions in design v2 §8) has 11 places to update, and the
+   inlined sites quietly skip the `global_admin` shortcut. See **F-2**.
+
+The next ~20 findings are a mix of naming drift, dead code, schema
+documentation gaps, and missing tests. None of them are emergencies.
+
+---
+
+## 1 — Findings by lens
+
+### 1.1 Service boundaries
+
+#### F-1. `AdminDeleteUser` writes to dropped tables — live bug
+
+🔴 active risk · 🟢 ≤30 min · stand-alone task
+
+**Files:** `internal/services/user_service.go:768`, `:773`
+
+```go
+if _, err := tx.ExecContext(ctx,
+    `DELETE FROM paliad.department_members WHERE user_id = $1`, id); err != nil {
+    return fmt.Errorf("delete department_members: %w", err)
+}
+// A Department this user led keeps existing — the lead seat just goes empty.
+if _, err := tx.ExecContext(ctx,
+    `UPDATE paliad.departments SET lead_user_id = NULL WHERE lead_user_id = $1`, id); err != nil {
+    return fmt.Errorf("clear dept leads: %w", err)
+}
+```
+
+Migration 027 (2026-04-29) renamed `paliad.departments` → `paliad.partner_units` and
+`paliad.department_members` → `paliad.partner_unit_members`. `git blame`
+shows lines 763-775 last touched by `c697fe34` on 2026-04-27 — the
+t-paliad-070 rename sweep (76785da) missed this site.
+
+**Fix:**
+
+```go
+`DELETE FROM paliad.partner_unit_members WHERE user_id = $1`
+`UPDATE paliad.partner_units SET lead_user_id = NULL WHERE lead_user_id = $1`
+```
+
+Also update the comment on line 725: `project_teams / department_members`
+→ `project_teams / partner_unit_members`. And the comment on line 771:
+`A Department this user led` → `A partner unit this user led`.
+
+**Warrants its own task:** yes — it's a customer-visible production bug
+even if the surface (admin-only) is small. `t-paliad-NN` titled "fix
+AdminDeleteUser SQL after partner_units rename".
+
+---
+
+#### F-2. Visibility predicate inlined in 10 sites despite central helper
+
+🔴 active risk · 🟡 1-2 h · stand-alone task
+
+**Files:**
+
+- `internal/services/dashboard_service.go:158, 214, 244, 274` (4 sites)
+- `internal/services/agenda_service.go:138, 204` (2 sites)
+- `internal/services/reminder_service.go:312, 325` (2 sites)
+- `internal/services/team_service.go:162` (1 site)
+- `internal/services/deadline_service.go:422` (1 site)
+
+All ten sites inline the same path-walk fragment:
+
+```sql
+EXISTS (SELECT 1 FROM paliad.project_teams pt
+         WHERE pt.user_id = $X
+           AND pt.project_id = ANY(string_to_array(p.path, '.')::uuid[]))
+```
+
+Worse, the inlined sites do **not** include the `global_admin` shortcut
+that `visibilityPredicate` / `visibilityPredicatePositional` add. A
+global_admin without an explicit `project_teams` row is visible to the
+RLS-style helper but invisible to the inlined sites — unless the
+surrounding query carries `p.firm_wide_visible OR pt.user_id = ...`
+elsewhere. Audit each site carefully; some queries might be relying on
+project-team auto-membership (every project creator gets a team row,
+per `project_service.go:467`) to mask the gap.
+
+The agenda-service ship memory (t-paliad-030, 2026-04-22) explicitly
+called this out as deliberate: "We deliberately did not factor this
+into a helper because the three call sites have slightly different
+JOIN shapes." That justification is weaker now that t-paliad-058
+proved the central helper works. The drift cost is real (Chinese-wall
+in design v2 §8 will need 11 simultaneous edits).
+
+**Fix:**
+
+1. Extend `visibility.go` with a third variant — `visibilityPredicateLateral(alias string, userArg int)` — that fits the dashboard/agenda LATERAL-JOIN shape (predicate appears inside a sub-SELECT, not the outer WHERE).
+2. Convert all 10 sites in one PR. Add a `_test.go` table-driven test that asserts the four variants produce equivalent EXPLAIN plans against a live DB.
+3. After the conversion, add `make grep-visibility-inline` to CI that fails on `string_to_array.*\.path` outside `visibility.go`.
+
+**Warrants its own task:** yes — security-critical; one focused PR.
+
+---
+
+#### F-3. `NoteService` is a dependency-shaped diamond
+
+🟡 future-proofing · 🟡 1-2 h · part of broader naming task
+
+**File:** `internal/services/note_service.go:23-31`
+
+```go
+type NoteService struct {
+    db          *sqlx.DB
+    projects    *ProjectService
+    appointment *AppointmentService
+}
+```
+
+`NoteService` reaches into `ProjectService.GetByID` for visibility and
+into `AppointmentService.GetByID` for personal-appointment visibility.
+That's ~6 cross-service `GetByID` calls in one file. The pattern is fine
+at this scale, but it's the canary for a "domain service" that should
+own a `CanSee(ctx, userID, parent)` method on each owning service —
+right now `NoteService` is hand-rolling the dispatch.
+
+**Fix:** add `ProjectService.CanSee(ctx, userID, projectID) (bool, error)`
+and `AppointmentService.CanSee(ctx, userID, appointmentID) (bool, error)`.
+`NoteService.ListForFrist` becomes a one-liner that asks the right
+parent service whether the user can see, then `s.list(ctx, ...)`.
+Reduces the number of full-row reads (`GetByID`) — currently every
+note-list call also pre-fetches the parent's full row.
+
+**Batch with F-4 (naming).**
+
+---
+
+### 1.2 Naming consistency
+
+The Department→PartnerUnit rename (t-paliad-070), the Akten→Projects
+data-model rename (t-paliad-024 / migration 018), and the German→English
+table rename (migration 020) all left rough edges in identifiers. The
+schema is mostly clean now (one bug — F-1 above). The Go code is
+half-renamed.
+
+#### F-4. Service layer mixes English types with German parameter and helper names
+
+🟠 friction now · 🔴 half-day+ · single-PR mechanical rename
+
+**Affected files (8):**
+
+| File | Legacy identifier | English equivalent |
+|---|---|---|
+| `note_service.go` | `CreateNotizInput`, `UpdateNotizInput`, `notizColumns`, `notizSelect`, `ListForProjekt`, `ListForFrist`, `ListForTermin`, `CreateForProjekt`, `CreateForFrist`, `CreateForTermin`, `fristProjectID` | `CreateNoteInput`, …, `noteColumns`, `noteSelect`, `ListForProject`, `ListForDeadline`, `ListForAppointment`, `CreateForProject`, …, `deadlineProjectID` |
+| `appointment_service.go` | `CreateTerminInput`, `UpdateTerminInput`, `ListForProjekt`, parameter names `terminID`, `projektID` | `CreateAppointmentInput`, `UpdateAppointmentInput`, `ListForProject`, `appointmentID`, `projectID` |
+| `deadline_service.go` | `CreateFristInput`, `UpdateFristInput`, `ListForProjekt`, `isValidFristStatus`, parameters `fristID`, `projektID` | `CreateDeadlineInput`, `UpdateDeadlineInput`, `ListForProject`, `isValidDeadlineStatus`, `deadlineID`, `projectID` |
+| `project_service.go` | `CreateProjektInput`, `UpdateProjektInput`, `validateProjektStatus` | `CreateProjectInput`, …, `validateProjectStatus` |
+| `party_service.go` | `CreateParteiInput`, `ListForProjekt` | `CreatePartyInput`, `ListForProject` |
+| `caldav_service.go` | `OnTerminCreated`, `OnTerminUpdated`, `OnTerminDeleted` | `OnAppointmentCreated`, … |
+| `caldav_ical.go` | `formatTermin` | `formatAppointment` |
+| `checklist_instance_service.go` | `ListForProjekt`, `listWithProjekt` | `ListForProject`, `listWithProject` |
+
+The mismatch is jarring because the *return types* are already English
+(`*models.Note`, `*models.Appointment`, `*models.Deadline`,
+`*models.Project`). Every reader does mental translation between
+parameter and return.
+
+This also means `note_service.go:38` reads `n.deadline_id = $1` (English
+column, correct) bound to a parameter named `fristID` (German).
+
+**Fix:** one branch via gopls "rename symbol". Order: types first
+(input structs), then methods, then parameters, then comments. Update
+the corresponding `internal/handlers/notes.go`, `appointments.go`,
+`deadlines.go` callers in the same PR — gopls handles cross-file
+symbol-rename correctly.
+
+The CLAUDE.md convention is unambiguous: "All code, table names, Go
+types, service names, URL paths, API endpoints, file names — English."
+This is just finishing the work.
+
+**Warrants its own task:** yes — one large-mechanical PR. Ship together
+with F-3 (the NoteService cleanup) since they touch the same file.
+
+---
+
+#### F-5. Stale comment in `models.go` claims escalation contact dropdown is deferred
+
+🟡 future-proofing · 🟢 ≤30 min · batch with other doc fixes
+
+**File:** `internal/models/models.go:50-52`
+
+```go
+// EscalationContactID is an optional override of the escalation channel
+// for overdue / DRINGEND mail. NULL means "fall back to global_admins".
+// The Settings UI dropdown is deferred (see CLAUDE.md); set via SQL today.
+EscalationContactID *uuid.UUID `db:"escalation_contact_id" ...`
+```
+
+The dropdown shipped on 2026-04-29 as t-paliad-066 (commit `bff2ec5`).
+The comment is now misleading.
+
+**Fix:** drop the last sentence; reference `Settings → Notifications`
+instead.
+
+---
+
+#### F-6. Go module path is still `mgit.msbls.de/m/patholo`
+
+🟡 future-proofing · 🔴 half-day+ · defer until next major touch
+
+**File:** `go.mod:1` plus 67 import statements across `internal/`,
+`cmd/`, and tests.
+
+The Gitea repo was renamed to `mAi/paliad` and auto-redirects (per
+project CLAUDE.md). So `go build` works. But `go.mod`, every `import
+"mgit.msbls.de/m/patholo/..."`, and the binary's debug info still read
+as `patholo`.
+
+**Fix:** wait. The cost is touching every Go file in the repo;
+t-paliad-018 explicitly punted on this for the same reason. Keep the
+finding on the books, do it next time someone is doing a global
+formatting pass.
+
+---
+
+#### F-7. CSS class names are still mostly German
+
+🟡 future-proofing · 🔴 half-day+ · separable from Go rename
+
+**File:** `frontend/src/styles/global.css` — 226 class definitions
+prefixed with German nouns: `.akten-detail-header`, `.akten-table`,
+`.akten-parteien-controls`, `.fristen-wizard`, `.frist-section-heading`,
+`.akten-events-empty`, etc.
+
+The TSX usages are even more confusing: `frontend/src/deadlines-detail.tsx:38`
+uses `className="akten-detail-header"` despite the page being about
+deadlines, not Akten. The CSS class is a generic "detail page header"
+shape that got named after the first user.
+
+**Fix:** rename in two passes (1: CSS only, 2: TSX usage), keep both
+class names valid (`.akten-detail-header, .detail-page-header`) for one
+deploy cycle, then drop the legacy. Conservative because there's no
+type-checker to catch missed renames in HTML.
+
+**Defer:** value-per-effort is poor — no functional impact, mostly
+churn. Worth doing next time a designer touches the stylesheet.
+
+---
+
+### 1.3 Frontend ↔ backend contract
+
+#### F-8. `i18n.ts` has 1264 keys typed as raw `string` with silent fallback
+
+🟠 friction now · 🟡 1-2 h · stand-alone task
+
+**File:** `frontend/src/client/i18n.ts:2776`
+
+```ts
+export function t(key: string): string {
+  return translations[currentLang][key] ?? translations.de[key] ?? key;
+}
+```
+
+Three problems compound:
+
+1. The key parameter is `string` — TypeScript will not catch typos.
+2. The third fallback (`?? key`) renders the literal i18n key in the
+   UX when a key is missing, so the bug is silent.
+3. There are 681 distinct `data-i18n="..."` attributes in TSX files
+   that bypass the `t()` function entirely — they're resolved by a
+   generic DOM walker (`initI18n()`) at page boot. No type-checking
+   path possible without a JSX preprocessor.
+
+The product audits already caught 4+ leaked keys this month
+(`fristen.field.project.choose`, `project_type_changed`, audit-polish
+F-04, etc.) and ship docs for t-paliad-067 mention "the i18n leak class
+keeps reappearing."
+
+**Fix (cheap step 1 — type the `t()` call):**
+
+```ts
+// Generated at build time from the keys in `translations.de`.
+export type I18nKey =
+  | "nav.home"
+  | "nav.kostenrechner"
+  | // ... 1264 more
+  | "bottomnav.badge.deadlines";
+
+export function t(key: I18nKey): string { ... }
+```
+
+Add a `frontend/build.ts` step that emits `i18n-keys.ts` from
+`translations.de`. TypeScript now catches typos in `t()` calls at
+build-time. ~501 call sites across `frontend/src/client/*.ts` get
+type-checked for free.
+
+**Fix (more thorough step 2 — typed `data-i18n`):** add a
+`bun run check-i18n-keys` CLI that greps every TSX file for
+`data-i18n="..."` and `data-i18n-placeholder="..."`, asserting the
+referenced key exists in `translations.de`. Run from the build script;
+fail the build on unknown keys. This catches the `data-i18n` class of
+leaks too.
+
+**Warrants its own task:** yes — 1 PR, two scripts, big returns.
+
+---
+
+#### F-9. i18n key namespace is split between German and English prefixes
+
+🟠 friction now · 🟡 1-2 h · stand-alone task
+
+**File:** `frontend/src/client/i18n.ts`
+
+Counts:
+
+- 444 keys with German prefixes (`fristen.`, `termine.`, `notizen.`)
+- 200 keys with German prefixes (`akten.`, `dezernat.`, `partei.`)
+- Newer keys use English prefixes (`deadlines.`, `appointments.`,
+  `notes.`, `parties.`, `partner_units.`)
+
+200+ TSX `data-i18n=` attributes still reference the German keys.
+
+A contributor adding a new "deadline edit form label" today would have
+to know whether the existing keyspace uses `fristen.field.title` or
+`deadlines.field.title`. Both shapes exist. Future drift is guaranteed.
+
+**Fix:** bulk rename all German-prefix i18n keys to their English
+equivalents; update the corresponding TSX `data-i18n=` attributes;
+verify with the F-8 build-time check. Do it as a single PR — the
+diff is mechanical and reviewable. Nothing here is user-visible.
+
+**Warrants its own task:** yes; depends on F-8 being shipped first
+(so the typed checker can validate the renames).
+
+---
+
+### 1.4 Migration management
+
+#### F-10. Migrations 004 / 005 / 006 are obsoleted by 018
+
+🟡 future-proofing · 🟢 ≤30 min · doc-only
+
+**Files:** `internal/db/migrations/004_akten.up.sql`,
+`005_akten_children.up.sql`, `006_visibility.up.sql` (~210 lines
+combined) are fully superseded by `018_projects_v2.up.sql`. Migration
+018 itself does the supersession explicitly:
+
+```sql
+-- Replaces paliad.akten with a single self-referential paliad.projects tree
+ALTER TABLE paliad.akten_events RENAME TO project_events;
+DROP TABLE paliad.akten;
+DROP FUNCTION IF EXISTS paliad.can_see_akte(uuid);
+DROP FUNCTION IF EXISTS paliad.notiz_is_visible(uuid, uuid, uuid, uuid);
+```
+
+Reading the migration history requires mentally diffing 004→018.
+First-time contributors will be confused.
+
+**Fix:** add `internal/db/migrations/SCHEMA_NOTES.md` with a one-line
+supersession map:
+
+```
+004 (akten table)             → 018 (projects table)
+005 (akten_events table)      → 018 (renamed to project_events)
+005 (notes parent FK)         → 018 + 020 (renamed columns)
+006 (can_see_akte function)   → 018 (renamed to can_see_project)
+015 (users.dezernat column)   → 027 (dropped + replaced by partner_unit_members)
+019 (seed dezernat strings)   → 027 (re-applies seed before drop)
+024 (department column rename) → 027 (further renamed to partner_unit)
+```
+
+Don't squash. The live DB has a stale `public.paliad_schema_migrations`
+artifact (per knuth's t-paliad-071 ship memory) that would block any
+re-numbering. Documentation > squashing.
+
+**Batch with F-11 + F-12.**
+
+---
+
+#### F-11. Migration 027 down silently drops the `partner_unit_events` audit table
+
+🟡 future-proofing · 🟢 ≤30 min · doc-only
+
+**File:** `internal/db/migrations/027_rename_to_partner_units.down.sql:19`
+
+```sql
+-- 1. Drop the audit table.
+DROP TABLE IF EXISTS paliad.partner_unit_events;
+```
+
+The header comment honestly admits the `users.dezernat` data loss but
+buries the audit-table drop without a warning banner. Operators
+running a rollback in 2027 would lose months of audit history without
+realising.
+
+**Fix:** prepend to the down file:
+
+```sql
+-- DATA LOSS WARNING: this rollback drops paliad.partner_unit_events with no
+-- recovery path. All audit log entries (created/updated/deleted/member_added/
+-- member_removed) are permanently lost. If you need to preserve them, dump
+-- the table before applying this rollback:
+--   pg_dump -t paliad.partner_unit_events ... > pue-backup.sql
+```
+
+**Batch with F-10 + F-12.**
+
+---
+
+#### F-12. `paliad.link_suggestions` and `paliad.link_feedback` exist in schema, are not used
+
+🟠 friction now · 🟡 1-2 h · stand-alone task
+
+**Files:**
+
+- `internal/db/migrations/011_feedback_tables.up.sql:17-38` — creates
+  `paliad.link_suggestions` and `paliad.link_feedback` (and RLS
+  policies, indexes).
+- `internal/handlers/links.go:247, 281, 290` — still POSTs to
+  `public.patholo_link_suggestions` / `public.patholo_link_feedback`
+  via PostgREST.
+
+Migration 011 (2026-04-16) flagged this explicitly:
+
+```sql
+-- A follow-on phase (P1 handler refactor) will:
+--   (1) swap handlers from PostgREST to the direct DB connection,
+--   (2) copy any meaningful rows from the public tables,
+--   (3) drop the public tables and this duplication.
+```
+
+That follow-on never landed. Today the platform:
+
+- has dead tables in its own schema,
+- still depends on a PostgREST round-trip + Supabase anon key for two
+  endpoints,
+- writes to `public.patholo_*` tables that the rest of the platform no
+  longer touches and that are invisible to the audit log
+  (`AuditService.UNION ALL` doesn't see them).
+
+**Fix:** rewrite `handleLinkSuggest` / `handleLinkFeedback` /
+`handleLinkSuggestionsPendingCount` to use `dbSvc.db.ExecContext()` /
+`QueryContext()` against `paliad.link_suggestions` /
+`paliad.link_feedback`. Drop `supabaseInsert` / `supabaseCount`
+helpers (they're only used by these three sites). Out-of-band SQL
+(not a migration) to copy any non-zero row counts from the public
+tables and drop them.
+
+After this, the only `mgit.msbls.de/m/patholo`-named or
+`patholo_*`-named runtime artifacts are: the legacy session cookie
+fallback (which has a 2026-05-18 sunset, see F-15) and the Go module
+path (F-6).
+
+**Warrants its own task:** yes — finishes a P1 from migration 011's
+own comment.
+
+---
+
+### 1.5 Tests
+
+#### F-13. Live-DB tests skip silently; no CI exists
+
+🔴 active risk · 🟡 1-2 h · stand-alone task
+
+**Files:** 7 tests across `user_service_test.go`,
+`deadline_service_test.go`, `reminder_service_test.go` (×3),
+`visibility_test.go`, `audit_service_test.go` all use the same idiom:
+
+```go
+url := os.Getenv("TEST_DATABASE_URL")
+if url == "" {
+    t.Skip("TEST_DATABASE_URL not set — skipping live DB test")
+}
+```
+
+The repo has no `.github/`, `.gitea/`, or any CI configuration. So
+in practice **these tests never run unless a developer remembers to
+set `TEST_DATABASE_URL` locally**. The t-paliad-069 reminder
+placeholder bug masked itself for ~24 h via this exact path. F-1 above
+(AdminDeleteUser SQL bug) is another instance — a passing
+`TestAdminDeleteUser` against the live DB would have failed the moment
+migration 027 was applied.
+
+**Fix (cheapest):** add `.gitea/workflows/test.yml` (Gitea Actions
+runs on this repo's host) that:
+
+1. Brings up an ephemeral Postgres 16 (`services:` block).
+2. Applies migrations.
+3. Sets `TEST_DATABASE_URL` to the ephemeral DB.
+4. Runs `go test ./...`.
+
+**Fix (safer):** also flip the skip to `t.Fatal` when `CI=true` is set,
+so a misconfigured CI workflow doesn't silently skip again.
+
+**Warrants its own task:** yes — one PR, prevents the recurrence of
+two known footguns.
+
+---
+
+#### F-14. Test coverage gaps in core services
+
+🟠 friction now · 🔴 half-day+ per service · sequence after F-13
+
+**Missing tests** (no `_test.go` exists for these services):
+
+- `caldav_crypto.go` — AES-GCM encrypt/decrypt of CalDAV passwords. Pure-Go, no DB needed. Highest priority because crypto bugs are silent and irreversible.
+- `caldav_client.go` — iCal parse / WebDAV PROPFIND. Table-driven against canned multi-status XML.
+- `caldav_ical.go` — `formatTermin`, `formatAppointment`. Pure-Go.
+- `agenda_service.go` — `annotateAgendaUrgency` is a calc routine, easily testable without DB.
+- `dashboard_service.go` — same logic class as agenda.
+- `note_service.go` — polymorphic visibility logic.
+- `appointment_service.go` — personal-vs-project visibility branching.
+- `partner_unit_service.go` — audit-emit-in-tx ordering (cronus' t-paliad-070 memory says "emit before delete" — that's an invariant worth testing).
+- `team_service.go` — team membership query shape.
+- `project_service.go` — tree-walking, ancestor/descendant logic, ltree path materialisation.
+- `party_service.go` — visibility delegation.
+
+**Effort:** non-trivial. Don't try to do all in one PR.
+
+**Suggested order (value-per-hour):**
+
+1. **`caldav_crypto_test.go`** — table-driven (plain → ciphertext → plain) with known vectors. ~30 min, irreversibly valuable.
+2. **`agenda_service_test.go`** — table-driven on `annotateAgendaUrgency`. ~30 min, no DB needed.
+3. **`partner_unit_service_test.go`** — verifies audit-emit-in-tx + ON DELETE SET NULL invariant. Live-DB test (so do this *after* F-13 to ensure CI runs it).
+4. **Everything else** — one ticket per service, low priority.
+
+**Warrants its own task:** one task per service, so head can dispatch
+in priority order.
+
+---
+
+### 1.6 Dead code & doc drift
+
+#### F-15. Legacy `patholo_session` / `patholo_refresh` cookie fallbacks
+
+🟡 future-proofing · 🟢 ≤30 min · scheduled cleanup
+
+**File:** `internal/auth/auth.go:27-28`
+
+```go
+LegacySessionCookieName = "patholo_session"
+LegacyRefreshCookieName = "patholo_refresh"
+```
+
+Per t-paliad-018 ship memory: "Remove legacy fallbacks after
+2026-05-18 (30d cookie max age)." Today is 2026-04-30. **Sunset is in
+~18 days.** All users who haven't logged out since 2026-04-18 will be
+forced to re-authenticate.
+
+**Fix:** add a calendar reminder for 2026-05-19 to delete the legacy
+constants and the fallback branch in `Middleware`. The associated
+test (`TestMiddleware_LegacyCookieAccepted`,
+`TestMiddleware_NewCookiePreferredOverLegacy`) goes away too.
+
+The middleware code that handles the upgrade path is at lines 240-256.
+
+**Warrants its own task:** yes — small one. Schedule for 2026-05-19.
+
+---
+
+#### F-16. Stale comment in `team_pages.go` references dropped API endpoint
+
+🟡 future-proofing · 🟢 ≤30 min · doc-only
+
+**File:** `internal/handlers/team_pages.go:5-7`
+
+```go
+// GET /team — directory of all Paliad users grouped by office or department.
+// Server-rendered shell; the client (assets/team.js) hydrates from /api/users
+// and /api/departments?include=members.
+```
+
+`/api/departments` was renamed to `/api/partner-units` in t-paliad-070.
+
+**Fix:** update the comment. Same for `internal/handlers/admin_users.go:123`
+("`project_teams / department_members` cleanup") which post-dates t-paliad-070.
+
+---
+
+#### F-17. `internal/db/migrations/_dev/mock_supabase_auth.sql` lives inside the embed root
+
+🟡 future-proofing · 🟢 ≤30 min · already known
+
+**File:** `internal/db/migrations/_dev/`
+
+Already noted in 2026-04-18 audit as **T-4**. Not fixed since. The
+embed.FS pattern likely filters by `NNN_*.sql` naming so it doesn't
+break, but it's one regex-loosening from a build bug.
+
+**Fix:** move to `internal/db/devtools/`.
+
+---
+
+#### F-18. Project status doc says "Audit polish-2 ~25 BATCH-level findings not yet shipped"
+
+🟡 future-proofing · 🟢 ≤30 min · doc-only
+
+**File:** `docs/project-status.md:20`
+
+The doc was last updated before today's t-paliad-073 cleanup PR shipped
+the DEFER list. Open follow-ups should be re-checked.
+
+---
+
+### 1.7 Architecture observations (no concrete fix)
+
+#### O-1. `models.go` is one 358-line file with 14+ types
+
+Already noted in 2026-04-18 as **T-11**. Not bad enough to warrant a
+ticket on its own, but next time anyone touches a struct, split by
+domain. Effort negligible.
+
+#### O-2. `frontend/build.ts` has 24 hand-maintained render-and-write pairs
+
+Already noted as **T-13**. Adding a page requires edits in 4 places
+(build.ts, handlers.go, i18n.ts, global.css). A page-manifest reduces
+to 1 place. Not urgent but would speed up new-page work.
+
+#### O-3. `RLS policies exist but never enforced`
+
+Original audit **T-1**. Status unchanged. The Go backend uses a
+service-role connection so migration 007's RLS policies never run.
+Decision still open: drop, document, or switch to per-request JWT
+connections.
+
+The 2026-04-18 audit recommended Option A (document + `SET row_security
+= off`). That's still right; one comment block in `internal/db/db.go`
+would close this out.
+
+---
+
+## 2 — Top-10 ranked by value-per-effort
+
+| Rank | ID | Description | Severity | Effort |
+|---|---|---|---|---|
+| 1 | F-1 | Fix `AdminDeleteUser` SQL — wrong table names | 🔴 | 🟢 |
+| 2 | F-13 | Add CI for live-DB integration tests | 🔴 | 🟡 |
+| 3 | F-2 | Centralise visibility predicate (10 sites → 1) | 🔴 | 🟡 |
+| 4 | F-12 | Migrate link suggestions/feedback to `paliad.link_*` | 🟠 | 🟡 |
+| 5 | F-8 | Type the i18n key + build-time `data-i18n` check | 🟠 | 🟡 |
+| 6 | F-14a | `caldav_crypto_test.go` table-driven | 🟠 | 🟢 |
+| 7 | F-14b | `agenda_service_test.go` annotateAgendaUrgency | 🟠 | 🟢 |
+| 8 | F-9 | Bulk-rename German-prefix i18n keys to English | 🟠 | 🟡 |
+| 9 | F-4 | Service layer naming sweep (German→English) | 🟠 | 🔴 |
+| 10 | F-5 + F-16 + F-18 | Comment / doc cleanup batch | 🟡 | 🟢 |
+
+Items 11+: F-10, F-11, F-15, F-17, F-3, O-1, O-2, O-3. Effort/value
+drops sharply.
+
+---
+
+## 3 — Coordination notes
+
+- **F-1 first.** It's a live bug. Ship before anything else.
+- **F-13 second.** Without CI, we'll keep building features on top of
+  silent test gates. The placeholder bug from t-paliad-069 and F-1
+  here are the second and third instances of "live-DB test would
+  have caught it." Fix the meta-problem.
+- **F-12 + F-15 are the last two `patholo_*` runtime artifacts** (after
+  the cookie sunset). Closing both makes the rebrand complete.
+- **F-4 + F-9 are mechanically large but conceptually trivial.** Good
+  AFK candidates — give to a Sonnet coder with explicit "don't change
+  semantics" framing.
+- **F-2 needs careful eyes.** It's security-adjacent. Pair with a
+  reviewer or run as a dedicated PR with explicit before/after EXPLAIN
+  plans on the affected queries.
+
+---
+
+## Appendix — items deliberately out of scope
+
+- **Performance profiling** — different audit kind. The t-paliad-058
+  `string_to_array(p.path, '.')::uuid[]` pattern is a known wart but
+  fits in F-2's centralisation work.
+- **Security review** — there's a `/security-review` skill for that.
+  Critical items C-1 through C-5 from the 2026-04-18 audit may have
+  been addressed (JWT verification was added — see `golang-jwt/jwt/v5`
+  in `go.mod`); a fresh security pass is its own task.
+- **Frontend design / accessibility** — already covered by
+  `audit-polish-*-2026-04-2*` docs.
+- **Phase H AI extraction** — explicitly deferred per CLAUDE.md.
+- **Dropping the obsoleted migrations 004-006** — schema-history
+  integrity beats tidiness; doc the supersession instead (F-10).