Yusuf Karadag

{
  "command": "pwd && ls",
  "description": "Change to track dir and list"
}

/Users/warrenthompson/Source/LoginID/05-data-ops

{
  "file_path": "/Users/warrenthompson/Source/LoginID/05-data-ops/PLAN.md"
}

1	# Track 05 — Data Ops: Planning Phase Plan
2	
3	Lead: Priya Nandakumar (opus/high for this phase). Track directory: `05-data-ops/`. Org plan: `../PLAN.md`. Board row: `../PLANNING.md`. Rules, palette, patterns, hire procedure: `../CASTING.md`. Coordination approach: `../02-ai-security-architecture/planning-approach.md`.
4	
5	## Context
6	
7	All three of the assignment's questions sit on top of one schema and one DAO contract, and this track owns both. That is the whole reason 05 is on the first-wave critical path: 03 cannot write the DAO until the contract is stable, and 04 cannot plan migration execution until the migration approach is named.
8	
9	The first-wave design work already exists here as prose — `research-data-ops-best-practices.md` (sourced research), `multi-db-strategy.md` (interface contract and schema shape), `pii-governance.md` (retention and non-conflation rules). What does not exist is the thing 03 actually needs. `planning-approach.md` records my own commitment on this and Marcus adopted it project-wide: a hand-off is implementable only if the receiver can act on it **without re-deriving the sender's reasoning**. `multi-db-strategy.md` today describes `Search(ctx, query) (results []UserProfile, total int, error)` in a sentence. It does not say what `query` is, which fields are nullable, what `Update` does when the row is gone, or whether `total` is the filtered count or the page count. Renata would have to invent all of that, and inventing it is a design decision that belongs to this track, not hers.
10	
11	So the deliverable this phase is aimed at is narrow and specific: turn the prose contract into a Go-shaped contract — interface signatures, field-level schema with nullability and types, error semantics, and explicit out-of-scope notes telling 03 what it must not redesign. Everything else in this plan either feeds that artifact or unblocks 04.
12	
13	The planning-phase rule holds throughout: this is a Go-*shaped* contract written inside a markdown file. No `.go`, no `.sql`, no `go.mod`, until Warren's explicit go-ahead. A fenced Go block inside `multi-db-strategy.md` is the design; a file the compiler would accept is code.
14	
15	## 1. Deliverables and tasks
16	
17	Six deliverables, each sized to one Jira Story under the track's Epic in project `LT`. Stories are listed in dependency order; S1 is the first Phase 3 deliverable and the one everything downstream waits on. Acceptance criterion for every story is the hand-off standard above.
18	
19	### S1 — Go-shaped contract in `multi-db-strategy.md` (the critical-path deliverable)
20	
21	Extend `multi-db-strategy.md` in place — it stays the single source of truth, no new parallel doc. Adds, as fenced Go-shaped blocks and tables rather than prose:
22	
23	- **Interface signatures** for `ProfileRepository`, `CredentialRepository`, `AuthMethodRepository` — exact method names, parameter and return types, and the `ProfileQuery` / `ProfileSearchResult` types `Search` takes and returns (shape ruled in S2).
24	- **Error semantics**, which today are one sentence about `ErrNotFound`. Needs the full sentinel set and when each is returned: `ErrNotFound`, `ErrDuplicateUsername` (unique-constraint violation on `user_credential.username`, which both backends must translate out of their driver-specific error into the shared sentinel), `ErrInvalidMethod` (FK violation on `method_id`), and the rule that everything else is wrapped with `%w` and passed through. Also: does `Update` on a missing row return `ErrNotFound` or upsert-create? (My position going in: `Update` is a portable upsert per the existing doc, so it creates — which means the doc must say so loudly, because "Update creates" is exactly the surprise that produces a bug in 03.)
25	- **Field-level schema table per entity** — column, Go type, SQL type per backend family, nullable yes/no, constraint, and a one-line note. Nullability is where this contract is most likely to be wrong and most expensive to fix, so it gets its own reviewer (S3).
26	- **Context and cancellation**: every method takes `ctx` first and is expected to honour cancellation; no method holds a transaction across calls. If the service layer needs a multi-statement transaction, that is a follow-up on the contract, not something 03 improvises — stated explicitly.
27	- **Explicitly out of scope for 03** — a numbered list so there is no ambiguity about what 03 must not redesign: the PII/credential separation (governance control, `pii-governance.md`), the UUID-everywhere PK strategy, the `auth_method` lookup table rather than a native enum, the `ON CONFLICT DO UPDATE` upsert form, the documented SQLite fuzzy-search parity gap, and the two-package (`postgres` serving Postgres + CockroachDB, `sqlite` separate) split. Each with a one-line "why, and who to flag if it doesn't fit."
28	- **Alignment note to 03's published factory**: `../PLANNING.md` service boundaries names `dao.New(driver, dsn string) (dao.Repository, error)` — a single `Repository` return type, while this track specifies three interfaces. Resolve it in the contract (my expected resolution: `dao.Repository` is a small composite that exposes `Profiles()`, `Credentials()`, `Methods()`, keeping Renata's factory signature stable exactly as she declared it) rather than leaving two tracks each thinking the other will bend.
29	
30	Consumes: S2 (query shape), S3 (nullability review), and the S4 ruling. Produces the 05→03 hand-off. Story closes when S6's cold read comes back clean.
31	
32	### S2 — `Search()` query shape decision
33	
34	The one genuinely open design question in the contract, and the one with the most ways to be quietly wrong. Decides: the `ProfileQuery` field set and which fields are optional; match semantics per field (name partial/fuzzy, phone exact-or-prefix on the E.164-normalized value, region/country exact); how empty and multiple filters combine (AND, and whether a wholly empty query is a legal "list everything" or an error); pagination as limit/offset versus a keyset cursor, with a maximum page size; sort order and whether it is stable enough for offset pagination to be meaningful at all; and what `total` means — total matching rows, or rows on this page. Also settles how the documented SQLite parity gap surfaces to a caller: identical results with worse performance, or ranked-vs-unranked differences a caller could observe.
35	
36	Pattern: **adversarial pair** (`../CASTING.md` §5 names this decision by name). Artifact: `decisions/search-query-shape.md`, folded into S1.
37	
38	### S3 — Multi-DB abstraction ruling
39	
40	`multi-db-strategy.md` already recommends two packages behind one interface and rejects dialect branching. That recommendation was reached by one agent reading sources. The catalog names this decision for **three hats**, and it is worth the three turns because it is the structural choice 03 builds the entire DAO around — codegen (sqlc) is a real third option that was never actually argued, only mentioned. Possible outcomes: the existing recommendation is confirmed (most likely, and the confirmation is then a documented ruling rather than an assumption), or codegen displaces hand-written SQL, which changes what S1's contract has to pin down.
41	
42	Artifact: `decisions/multi-db-abstraction.md` with the three ≤300-word positions, my synthesis, and the pattern footer (model per seat, hire turns consumed) that 02 added to every pattern.
43	
44	### S4 — Migration approach hand-off for 04
45	
46	`../PLANNING.md` lists 04 as blocked on "05's migration tool/approach". `multi-db-strategy.md` has a migration section, but in Theo's terms it is a preference, not a mechanism. Produces `migration-approach.md` — a new file, because its receiver is 04, not 03, and folding an infra hand-off into the DAO contract would make both harder to read:
47	
48	- Tool: goose (already reasoned in `multi-db-strategy.md`; golang-migrate acceptable if 03 has tooling preferences — one rotating-devil's-advocate objection turn, not a debate).
49	- Directory layout and naming for the shared series plus the named per-backend override set (currently exactly one override: the `pg_trgm` GIN index, omitted on SQLite).
50	- What 04 must provide: when migrations run relative to service start, whether the service fails closed if the schema is behind, migration-lock behaviour under multiple replicas, and the DB privileges the migration runner needs versus the runtime user.
51	- Seed data: the `auth_method` `"password"` row is a seeded row, not a migration constant — how it is applied and what makes it idempotent.
52	- Explicitly not owned here: where the databases run, how the pipeline invokes goose, environment topology. Named so 04 does not read a boundary into this doc that isn't there.
53	
54	### S5 — Retention windows and the deletion-job contract
55	
56	`pii-governance.md` states the policy shape and stops at "a product/legal decision this track doesn't own outright." That is true and it is also not actionable — 04 cannot build a retention sweep against it. This story puts numbers in a table, marked as **proposed defaults for a POC pending a product/legal ruling**, and specifies the mechanism the numbers plug into: what a sweep selects on (`source`, `updated_at`), whether deletion is hard delete or tombstone, what cascades to `user_credential` (the FK is already cascade-delete, so deleting a profile silently destroys its credentials — deliberate and worth stating), and what a subject-deletion request touches versus a bulk retention sweep. Separate proposed windows for `source = 'direct'` and `source = 'idp_cache'`, with the `idp_cache` window shorter and the reasoning stated.
57	
58	Owner seat: the Spreadsheet hire, who will not accept "a reasonable window" as an answer. Artifact: `pii-governance.md` extended with a retention table; policy shape only, execution stays 04's.
59	
60	### S6 — Cold-read legibility pass on the 05→03 contract
61	
62	The catalog calls the **newcomer's question** pass the highest-ROI pattern in the project and names "every 05→03 hand-off" as its use. A hire who has not watched this contract being written reads S1 cold and lists what they could not act on without asking me. I fix every item or record why not. This is the acceptance test for S1 and for the hand-off standard I asked the project to adopt; failing to run it on my own output would be the most embarrassing available outcome.
63	
64	Artifact: `decisions/handoff-legibility-05-03.md`. Runs after S1 drafts, gates S1 closing.
65	
66	## 2. Hire roster — 4 hires
67	
68	Matches the suggested cast in `../CASTING.md` §3 exactly. Variance check against the fourteen hires already registered: Systems Cartographer, Storyteller, Detail Hawk and Spreadsheet each currently sit at 1× org-wide, so all four land at 2× — at the cap, none over it, and none repeated within this team. Rule 2 (same archetype, different function) holds in every case, stated per hire below. Rule 3 (lead's temperament opposite) is Saoirse. Rule 5: this roster spans serious↔playful and cautious↔bold both. Model: **sonnet** for all four; none of these seats makes a foundational call alone — I hold those at opus, which is the entire reason this session was relaunched.
69	
70	Two seats in my debate plan have no archetype left in the palette (Designated Skeptic and Newcomer are both at 2× org-wide). Rather than burn a hire on a near-duplicate, I substitute within this roster and say so explicitly in §3 — the pattern protocols are about the role played, not the label worn.
71	
72	### Hire 1 — Anders Vogel · Systems Cartographer · serious, bold
73	
74	Function: multi-DB abstraction — drafts the interface contract's structural shape and the Proposer seat on `Search()`. Distinct from Ines Dabrowski's Systems Cartographer function in 03 (Go module layout and service topology): Anders draws the boundary *inside* the DAO between interface and per-backend implementation; Ines draws boundaries between packages and binaries. Different artifacts, no overlap.
75	
76	> Persona paragraph for `../CASTING.md` §4 (127 words):
77	>
78	> Calm and unusually direct for someone who thinks in diagrams; opens by drawing the boundary before anyone argues about what sits on either side of it. Decides by mapping the interaction surface first — what crosses this line, in which direction, how often — then choosing the abstraction that makes the smallest surface. Pushes back hard on any interface whose implementations must know about each other, and on abstractions justified by a backend nobody has committed to adding. Six years on a payments platform's storage layer where a leaky repository interface bled dialect assumptions into three services, then four years at a data-infrastructure vendor building connectors across five engines. Blind spot: abstracts past the concrete — he will design for the fourth backend before the second one works. Concedes cleanly, in writing, and moves on.
79	
80	### Hire 2 — Saoirse Byrne · Storyteller · playful, bold (my temperament opposite, rule 3)
81	
82	Function: consumer's-eye reader and cold-read seat — argues the contract from the position of whoever has to use it, and runs the newcomer's-question pass on S1. Distinct from Imogen Hale's Storyteller function in 01 (product framing and narrative thesis): Saoirse's subject is a schema and an interface, not a market; her output is "here is where this hand-off stops being followable," not a README thesis.
83	
84	> Persona paragraph for `../CASTING.md` §4 (133 words):
85	>
86	> Warm, quick, and cheerfully unbothered by being the least formal person in a schema review; narrates a design as the journey of one record — created here, read there, deleted when. Decides by walking the path end to end and noticing where the story skips a step, which is usually where the bug is. Pushes back on documents that are correct but unfollowable, and on any governance rule stated so abstractly that nobody could tell whether they had broken it. Five years writing developer documentation and SDK guides at an API company, three as a solutions engineer who watched customers misread her own docs in real time. Reads SQL comfortably and writes Go badly enough to be an honest proxy for a confused implementer. Blind spot: charming beats correct — the clearer story is not always the true one. Disagrees by retelling it.
87	
88	### Hire 3 — Yusuf Karadag · Detail Hawk · serious, cautious
89	
90	Function: DDL and nullability review, plus the numbered-objection seat on `Search()`. Distinct from Oren Castellan's Detail Hawk function in 03 (Go code review — error semantics, off-by-one): Yusuf reviews the schema and contract *before* code exists; Oren reviews code after. `../CASTING.md` §2 names this exact pair as the allowed second use.
91	
92	> Persona paragraph for `../CASTING.md` §4 (129 words):
93	>
94	> Quiet, exacting, and slower to speak than everyone else in the room because he is still reading the column list. Decides one field at a time: type, nullability, constraint, default, and what happens on the day the constraint is violated in production. Pushes back on any nullable column whose null has no stated meaning, on "we'll validate it in the application layer," and on error handling that collapses three distinct database failures into one generic message. Nine years as a database engineer at a healthcare data processor under audit conditions, then three on a platform team migrating a large schema across engines. Deep in Postgres constraint semantics and the places SQLite quietly disagrees. Blind spot: cannot see the forest — he will hold a release over a defaulted column. Objects in numbered lists and expects numbered answers.
95	
96	### Hire 4 — Beatriz Achterberg · Spreadsheet · playful, cautious
97	
98	Function: retention windows and the deletion-job contract (S5), plus the pragmatist hat in the three-hats run. Distinct from Desmond Okafor's Spreadsheet function in 01 (auditing evidence behind claims): Beatriz sets numbers that become policy rather than checking numbers someone else asserted.
99	
100	> Persona paragraph for `../CASTING.md` §4 (131 words):
101	>
102	> Dry, genial, and faintly delighted when a question she was told was qualitative turns out to have a number hiding in it. Decides by building the smallest table that forces the choice into the open — one row per case, one column per thing that must be true — and refuses to move until every cell is filled or explicitly marked unknown. Pushes back on "reasonable," "appropriate," and "as needed" wherever those words stand in for a duration, and on retention policies with no named mechanism to enforce them. Seven years in data governance at an insurance group writing the retention schedules the auditors read, then three on a privacy-engineering team building deletion pipelines. Blind spot: she measures only what is measurable, and will quietly drop what resists a column. Disagrees by showing you the empty cell.
103	
104	## 3. Debate plan
105	
106	Patterns are `../CASTING.md` §5 protocols; every run ends in a file under `05-data-ops/decisions/` with the ⟨02⟩ footer recording model per seat and hire turns consumed.
107	
108	**Three hats — the multi-DB abstraction (S3).** Three parallel ≤300-word positions on per-driver implementations vs. dialect branches vs. codegen (sqlc):
109	- *Optimist* — Anders: the case for the cleanest structural answer, codegen included.
110	- *Pessimist* — Yusuf, substituting for the Veteran seat (both Veterans are 03's; Yusuf's failure-mode instinct fits): where each option breaks at the boundary, generated-code review burden, dialect drift.
111	- *Pragmatist* — Beatriz: what each costs in files, review surface and time for a take-home whose SQLite backend is explicitly not a production peer.
112	
113	I synthesize and rule. 3 hire turns + my synthesis. Justified because it is the structural choice 03 builds against and codegen has never actually been argued here, only name-checked.
114	
115	**Adversarial pair — the `Search()` query shape (S2).** Anders proposes the `ProfileQuery` / result shape; Yusuf writes numbered objections, each ending with the "what would change my mind" line ⟨02⟩ requires; Anders answers every objection in writing and may decline to revise, but never silently; I rule. Yusuf holds the Designated Skeptic seat, which is at 2× org-wide and unavailable — his numbered-objection habit is the part of that archetype this pattern actually needs. 3 hire turns + my ruling.
116	
117	**Newcomer's question — the 05→03 contract (S6).** Saoirse reads the drafted S1 cold and lists what she could not act on without asking me; I fix or record why not. The Newcomer archetype is at 2× org-wide (Wesley Okonkwo, 04, currently on loan to 01), so Saoirse plays the seat — with one contingency: if her pass comes back thin, I will ask team-lead for a short Wesley loan for a second cold read, since the value of this pattern comes from genuine unfamiliarity and a hire who has read this track's docs is only a partial proxy. 1–2 hire turns.
118	
119	**Rotating devil's advocate — two single-turn objections.** Beatriz objects to the migration approach (S4) before it goes to 04; Saoirse objects to the retention windows (S5) before they go into `pii-governance.md`. Cheapest pattern in the catalog; the seat rotates so neither becomes "the negative one."
120	
121	**Explicitly not debated — and why:**
122	- *DAO CRUD semantics beyond `Search`* — `Get`/`Create`/`Delete` are settled by the assignment and by the existing doc; the catalog names DAO CRUD as red/blue waste and it is three-hats waste too.
123	- *No red team / blue team in this track at all.* The catalog reserves it for the connector token lifecycle, which is 02's. Attack-tree work against this schema would duplicate `../02-ai-security-architecture/threat-model.md` and cross an ownership line.
124	- *The address field names* (`street_address`, `locality`, `region`, `postal_code`, `country`) — fixe
… truncated (20 KB cap)

{
  "file_path": "/Users/warrenthompson/Source/LoginID/05-data-ops/profiles/yusuf-karadag/yusuf-karadag.md"
}

1	---
2	schema_version: 1
3	name: "Yusuf Karadag"
4	slug: "yusuf-karadag"
5	image: "profiles/yusuf-karadag/yusuf-karadag.png"
6	
7	nsfw: false
8	display:
9	  image: true
10	  name: true
11	  autostart: false
12	generation:
13	  backend: "comfyui"
14	  model: "comfyui-workflow.json"
15	  prompt: "Yusuf Karadag, a quiet exacting man in his mid-40s with dark greying close-cropped hair, thin metal-framed glasses, wearing a plain slate-gray button-down shirt, attentive unhurried expression, head-and-shoulders portrait, centered, neutral light gray studio background, professional corporate headshot photography, soft studio lighting, sharp focus, photorealistic"
16	  negative_prompt: blurry, low quality, distorted, extra limbs, watermark, text, nsfw, nude, nudity, explicit, sexual content
17	  seed: null
18	  gif_mode: null
19	  created_at: "2026-09-12T00:00:00Z"
20	---
21	
22	# Yusuf Karadag
23	
24	![Yusuf Karadag](profiles/yusuf-karadag/yusuf-karadag.png)
25	
26	
27	## Personality
28	
29	Quiet, exacting, and slower to speak than everyone else in the room because he is still reading the column list. Decides one field at a time: type, nullability, constraint, default, and what happens on the day the constraint is violated in production. Pushes back on any nullable column whose null has no stated meaning, on "we'll validate it in the application layer," and on error handling that collapses three distinct database failures into one generic message. Nine years as a database engineer at a healthcare data processor under audit conditions, then three on a platform team migrating a large schema across engines. Deep in Postgres constraint semantics and the places SQLite quietly disagrees. Blind spot: cannot see the forest — he will hold a release over a defaulted column. Objects in numbered lists and expects numbered answers.
30	
31	

{
  "file_path": "/Users/warrenthompson/Source/LoginID/05-data-ops/multi-db-strategy.md"
}

1	# Multi-Database Abstraction Strategy
2	
3	Owner: 05-data-ops. Implemented by: `03-engineering-delivery`, from the prose contract below — this project is still in planning phase, so the interface and schema exist here as design, not as `.go`/`.sql` files, until there's an explicit go-ahead to write code. Full research trail: `research-data-ops-best-practices.md`.
4	
5	## The interface contract (prose, not code)
6	
7	Three repository interfaces, one per entity, each implemented by two backend packages (below) and never containing SQL themselves — the service layer depends only on these shapes:
8	
9	- **`ProfileRepository`** — `Get(ctx, id) (*UserProfile, error)`; `Search(ctx, query) (results []UserProfile, total int, error)` where `query` filters by name (partial/fuzzy), phone (exact/prefix), region, country, plus limit/offset for pagination; `Create(ctx, *UserProfile) (*UserProfile, error)`; `Update(ctx, *UserProfile) (*UserProfile, error)` implemented as a portable upsert; `Delete(ctx, id) error`.
10	- **`CredentialRepository`** — `GetByUsername(ctx, username) (*UserCredential, error)` as the primary auth lookup path; `ListByUserID(ctx, userID) ([]UserCredential, error)`; `Create`, `Update`, `Delete` mirroring `ProfileRepository`'s shape. Never accepts or returns `UserProfile` fields.
11	- **`AuthMethodRepository`** — `Get(ctx, id)`, `GetByName(ctx, name)`, `List(ctx, activeOnly bool)`. Read-heavy; writes to this table are an operational action (adding a supported method), not part of the normal request path.
12	
13	Domain types implied by the above: `UserProfile` (`ID`, `Name`, `Phone`, `StreetAddress`, `Locality`, `Region`, `PostalCode`, `Country`, `Source` — `"direct"` or `"idp_cache"` — `CreatedAt`, `UpdatedAt`); `UserCredential` (`ID`, `UserID` FK, `Username` unique, `MethodID` FK, `Secret` as bytes/nil, `HashAlgo`, `HashCost`, `CreatedAt`, `UpdatedAt`); `AuthMethod` (`ID`, `Name`, `RequiresSecret` bool, `IsActive` bool). A shared `ErrNotFound` sentinel is returned by every backend implementation for a missing row, so the service layer never has to distinguish `sql.ErrNoRows` from a SQLite-specific miss.
14	
15	Two concrete implementations satisfy all three interfaces:
16	
17	- `postgres` package — serves both PostgreSQL and CockroachDB from one implementation. They share enough of the wire protocol and SQL surface (see the compatibility table below) that a second implementation would be duplicated effort, not genuine portability work.
18	- `sqlite` package — a separate implementation. SQLite diverges enough (no native UUID/BOOLEAN, no trigram index, app-side ID generation) that forcing it through the Postgres implementation would mean littering that code with backend conditionals, which is the exact failure mode a repository-per-backend split is meant to avoid.
19	
20	The service layer depends only on the interfaces and is constructed with whichever concrete implementation the startup config names. Backend choice is a wiring decision, never a runtime branch inside business logic.
21	
22	## Why not one implementation with dialect branches
23	
24	A single implementation with `if backend == "sqlite"` scattered through query construction was considered and rejected: it couples unrelated dialect concerns into one code path, makes it easy to add a Postgres-only feature (trigram search) without noticing SQLite silently falls back to nothing, and is harder to test in isolation per backend. Two implementations behind one interface is the standard Go shape for this problem (see sources in the research doc) and is what the interface contract above is designed against.
25	
26	## Dialect differences that survive into the DAO layer
27	
28	| Concern | Postgres / CockroachDB | SQLite | Handling |
29	|---|---|---|---|
30	| Primary key | `UUID DEFAULT gen_random_uuid()`, server-generated | `TEXT`, app-generated before insert | DAO layer generates the UUID in Go for SQLite; server-generates for Postgres/CockroachDB. Callers never see the difference — `Create()` returns the populated `ID` either way. |
31	| Placeholders | `$1, $2, ...` | `?` | Handled by the driver/query builder, not hand-rolled per query. |
32	| Upsert | `INSERT ... ON CONFLICT (id) DO UPDATE SET ...` | Same syntax, supported since SQLite 3.24 | One query shape used everywhere. CockroachDB's `UPSERT` shorthand is deliberately not used, to avoid a second query dialect. |
33	| Fuzzy name search | `pg_trgm` GIN index, native on both Postgres and CockroachDB | No native equivalent | `Search()` behaves identically from the caller's side; the SQLite implementation falls back to a `LIKE '%term%'` scan. This is a documented, deliberate parity gap, not an oversight — SQLite is the local/dev/demo backend, not a production peer. |
34	| JSON | `JSONB`, indexable, has query operators | `TEXT`/`BLOB` with a differently-formatted internal `JSONB` (SQLite 3.45+) that is not disk-compatible with Postgres's | Not currently used in the reference schema. If a raw IDP `/identity` payload is ever cached, it is stored as opaque `TEXT` at the DAO boundary — no JSON-operator filtering pushed into either implementation, so the column stays portable. |
35	| Boolean | Native `BOOLEAN` | `INTEGER` 0/1 | Mapped at the DAO boundary; Go `bool` on both sides of the interface. |
36	
37	## Schema shape (prose, not DDL)
38	
39	`auth_method` — lookup table, not a native enum: `id` (UUID), `name` (unique — `"password"`, `"passkey"`, `"otp"`, `"oidc"`, ...), `requires_secret` (bool, false for passkey/WebAuthn-style methods), `is_active` (bool), `created_at`. Seeded with one `"password"` row; new methods are inserted as data, never a migration.
40	
41	`user_profile` — pure PII, no secrets: `id` (UUID PK), `name`, `phone` (normalized E.164 at write time), `street_address`, `locality`, `region`, `postal_code`, `country` (ISO 3166-1 alpha-2), `source` (`"direct"` or `"idp_cache"`, constrained), `created_at`, `updated_at`.
42	
43	`user_credential` — this system's own login credentials, no PII: `id` (UUID PK), `user_id` (FK → `user_profile.id`, cascade delete), `username` (unique), `method_id` (FK → `auth_method.id`), `secret` (nullable, salted hash only, never plaintext), `hash_algo`, `hash_cost`, `created_at`, `updated_at`.
44	
45	Indexes: B-Tree on `user_profile.phone`; trigram GIN on `user_profile.name` (Postgres/CockroachDB only); unique on `user_credential.username`; FK indexes on `user_credential.user_id` and `.method_id`.
46	
47	Primary keys are UUID everywhere — server-generated (`gen_random_uuid()`) on Postgres/CockroachDB, app-generated in Go before insert on SQLite (which has no native UUID type). One PK strategy across backends, no per-backend branch in the DAO.
48	
49	## Migration approach
50	
51	- One shared SQL migration series covers the base schema (`CREATE TABLE`, standard column types, the portable upsert-compatible constraints) since it's close enough across all three backends for this schema's scope.
52	- A small, explicitly-named set of per-backend override files covers what genuinely can't be shared — currently exactly one: the `pg_trgm` GIN index on `user_profile.name`, which has no SQLite equivalent and is simply omitted there rather than faked.
53	- Recommended tool: **goose** — plain-SQL-first, minimal dependencies, and its style fits "mostly shared, occasionally per-backend" files more naturally than golang-migrate's stricter up/down pairing. golang-migrate is an equally defensible choice if the engineering track already has tooling preferences; Atlas is worth naming as the more sophisticated schema-as-state alternative but is more than this take-home's scope needs.
54	- Migration files live under version control alongside the DAO code (`03-engineering-delivery` owns the pipeline that runs them — see `../04-infra-devops/CLAUDE.md` for where/how they're actually executed in CI/CD).
55	
56	## What the engineering track should take from this
57	
58	1. Implement `ProfileRepository`, `CredentialRepository`, `AuthMethodRepository` per the interface contract above, once there's a go-ahead to write code — don't redesign the contract, flag back to this track if it doesn't fit.
59	2. Two packages: `postgres` (serves Postgres + CockroachDB) and `sqlite`. No third implementation needed unless a fourth backend is added to the assignment.
60	3. Use the schema shape above as the starting migration content for both backend families.
61	4. Never let `UserCredential` and `UserProfile` fields mix in either implementation, even as an internal convenience struct — that boundary is a governance control (see `pii-governance.md`), not just a modeling choice.
62	

{
  "file_path": "/Users/warrenthompson/Source/LoginID/05-data-ops/pii-governance.md"
}

1	# PII Data Governance Note
2	
3	Owner: 05-data-ops. Covers `user_profile` retention, IDP-cached PII (question 3's `/identity` response), and how retention interacts with `user_credential`. Threat modeling and access control for this data are `02-ai-security-architecture`'s call, not restated here — see `../02-ai-security-architecture/CLAUDE.md`.
4	
5	## Data minimization
6	
7	The schema collects only the fields the assignment names: name, phone, and the specific address subfields the `/identity` endpoint returns (`street_address`, `locality`, `region`, `postal_code`, `country`). No speculative fields (e.g., date of birth, government ID) were added. If a future requirement needs more PII, that's a deliberate schema change and a deliberate governance decision, not a default to expand into.
8	
9	## Retention
10	
11	`user_profile` rows are not retained indefinitely by default. Two clocks apply, distinguished by the `source` column:
12	
13	- **`source = 'direct'`** — profile created through this system's own registration flow. Retention window is a product/legal decision this track doesn't own outright, but the schema supports enforcing whatever window is chosen: `created_at`/`updated_at` plus a scheduled deletion job keyed on those columns.
14	- **`source = 'idp_cache'`** — profile hydrated from a third-party IDP connector's `/identity` response. This data is subject to the **same retention obligation as directly-collected PII** — caching it does not reset or relax that obligation. If anything it adds a vendor-processor relationship on top: this system is now downstream custodian of another party's collected PII, which should be documented as a data-flow (third-party collection → this system → disposal) rather than left implicit.
15	
16	Tagging cache rows distinctly by `source` is what makes a targeted deletion request or a bulk retention sweep possible without touching directly-collected profiles by accident.
17	
18	## What must never happen, structurally enforced
19	
20	- **`user_credential` never carries PII.** No name, phone, or address column exists on that table, and the `UserCredential` type described in `multi-db-strategy.md` has no fields that could hold it. A credential-store breach exposes usernames and hashes, not addresses.
21	- **`user_profile` never carries a secret.** No password hash or raw secret column exists on that table. A profile-store breach exposes PII, not authentication material.
22	- **The third-party IDP's own `/auth` credentials (the `username`/`password` this system POSTs to the vendor's `/auth` endpoint) are never persisted in `user_credential` or anywhere else in this schema.** That table is for this system's own login credentials. Storing a vendor's auth secret here would mean this system is custodian of a third party's credential material with no assignment requirement to do so — a clear scope and governance violation, not just a modeling mistake.
23	
24	This separation is treated as a governance control, not merely a normalization convenience: it's the reason a breach of one table doesn't cascade into exposing the other class of data.
25	
26	## Handoff notes
27	
28	- `02-ai-security-architecture` should build credential hashing (algorithm, cost factor, salt handling) on top of the `secret`/`hash_algo`/`hash_cost` columns already reserved in the schema — flagged directly to Marcus once this schema landed.
29	- `04-infra-devops` owns where retention sweeps and deletion jobs actually run (cron, scheduled job, pipeline step) — this note specifies the policy shape, not the execution mechanism.
30	

{
  "command": "python3 /Users/warrenthompson/.claude/skills/profile-gen/scripts/show_profile.py --profile /Users/warrenthompson/Source/LoginID/05-data-ops/profiles/yusuf-karadag/yusuf-karadag.md --root /Users/warrenthompson/Source/LoginID/05-data-ops 2>&1 | tail -5",
  "description": "Display persona portrait in pane"
}

{"name": "Yusuf Karadag", "mode": "hud", "hud": true, "badge": false, "background": false, "inline": false}

{
  "to": "priya-nandakumar",
  "subject": "Yusuf onboarded — seats confirmed, two nullability suspects",
  "body": "Onboarded. Read root CLAUDE.md, track CLAUDE.md, PLAN.md, my persona, multi-db-strategy.md, pii-governance.md. Portrait is up.\n\nSeats confirmed:\n1. S3 three-hats — pessimist, substituting for Veteran. Where each abstraction option breaks at the boundary.\n2. S2 adversarial pair — Designated Skeptic on `Search()`. Numbered objections, each closing with a one-line \"what would change my mind.\" Understood as non-optional; I'd write them that way regardless.\n3. S1 — field-level schema table reviewer, nullability specifically.\n\nTwo columns I expect to be wrong, instinct only:\n\n1. `user_credential.secret` — the doc says \"nullable\" with no stated meaning for the null. At least three distinct states collapse into it: method requires no secret (passkey), secret not yet set, secret revoked. And `hash_algo`/`hash_cost` nullability is unstated entirely, so nothing forbids a row with a secret and no algorithm.\n\n2. `user_profile.postal_code`, standing in for the whole address block — nullability is unstated for all five address columns, and `/identity` will return partial addresses. Null-versus-empty-string also behaves differently in the two backends under a unique or search predicate.\n\nReady for S2 or S3 whenever you are."
}

<tool_use_error>Error: No such tool available: SendMessage. SendMessage is disabled for this session, in subagents as well as here.</tool_use_error>

{
  "command": "grep -n -i \"sqlc\\|codegen\\|cockroach\\|retry\\|serializ\" research-data-ops-best-practices.md | head -40",
  "description": "Search research doc for codegen and Cockroach notes"
}

56:## 2. Multi-database abstraction strategy (PostgreSQL / CockroachDB / SQLite)
61:- Each backend (Postgres/CockroachDB — which can typically *share* one implementation given CockroachDB's wire-protocol and much of its SQL compatibility with Postgres — and SQLite) gets its own concrete struct implementing that interface, isolated in its own package, selected at startup via config/driver name.
62:- The service layer only ever depends on the interface; swapping backends is "import a different driver, construct a different concrete repository," per the threedots.tech and pawelgrzybek.com writeups on this exact pattern in Go (https://threedots.tech/post/repository-pattern-in-go/, https://pawelgrzybek.com/repository-pattern-in-go-service/). `database/sql` plus per-driver SQL string sets (or a query builder) is enough — no ORM is required, though one (e.g. sqlc, squirrel, GORM) is a reasonable implementation choice for the engineering track to justify.
67:| Concern | PostgreSQL | CockroachDB | SQLite |
69:| Primary key type | `SERIAL`/`BIGSERIAL` or `UUID` both fine | **UUID strongly preferred** — sequential/serial keys create write hotspots on a single range/node in a distributed cluster; `SERIAL` is offered only for Postgres-compatibility, and CockroachDB's own docs and engineers recommend `gen_random_uuid()` for real workloads (https://www.cockroachlabs.com/blog/how-to-choose-a-primary-key/, https://adhdecode.com/articles/cockroachdb/cockroachdb-uuid-vs-serial-primary-key/, https://github.com/cockroachdb/cockroach/issues/41258) | No native autoincrement concept the same way; `INTEGER PRIMARY KEY` is a rowid alias, or generate UUIDs at the app layer |
70:| **Decision for this schema:** use `UUID` as the primary key type universally across all three backends. It's the one choice that is simultaneously correct/required for CockroachDB, fully supported in Postgres, and easy to generate app-side for SQLite — avoiding a per-backend PK strategy branch in the DAO. |
73:| Upsert syntax | `INSERT ... ON CONFLICT (...) DO UPDATE SET ...` | Supports the same `ON CONFLICT` syntax (Postgres-compatible) *and* its own `UPSERT` shorthand, which skips the read that `ON CONFLICT DO UPDATE` performs and is faster when there's no secondary index involved (https://docs.cockroachlabs.com/docs/stable/upsert, https://www.cockroachlabs.com/blog/sql-upsert/) | Supports `INSERT ... ON CONFLICT (...) DO UPDATE SET ...` since SQLite 3.24 — same syntax family |
74:| **Implication:** `ON CONFLICT (...) DO UPDATE` is the one upsert syntax all three understand — use it as the portable default in the DAO's SQL, rather than CockroachDB's `UPSERT` shorthand, to keep one query string (modulo placeholder syntax) working across backends. |
76:| General caveat | — | Despite wire-protocol and much syntax compatibility with Postgres, CockroachDB is **not a drop-in Postgres replacement**: distributed transaction semantics, lack of some Postgres extensions, and performance characteristics (e.g., the PK hotspot issue above) differ meaningfully. The official compatibility page is the authoritative diff (https://www.cockroachlabs.com/docs/stable/postgresql-compatibility). | SQLite has no concurrent-writer story, no network protocol, no `ROLE`/`GRANT` model — it's viable for local/dev/test but the DAO's SQLite implementation is realistically a "does it work for local dev and the take-home demo" backend, not a production peer of the other two. Worth stating explicitly rather than pretending it's an equal third option. |
79:- https://www.cockroachlabs.com/blog/how-to-choose-a-primary-key/
80:- https://github.com/cockroachdb/cockroach/issues/41258
81:- https://adhdecode.com/articles/cockroachdb/cockroachdb-uuid-vs-serial-primary-key/
82:- https://docs.cockroachlabs.com/docs/stable/upsert
83:- https://www.cockroachlabs.com/blog/sql-upsert/
84:- https://www.cockroachlabs.com/docs/stable/postgresql-compatibility
97:- **golang-migrate** — the most established/widely used; plain up/down `.sql` files per version, supports many backends including Postgres and SQLite (CockroachDB via its Postgres driver, given wire compatibility). Simple, direct, no schema-diffing intelligence.
101:**Structuring migrations across three backends:** Given the schema is intentionally kept portable (UUID PKs everywhere, `ON CONFLICT` upsert syntax common to all three, JSON treated as opaque text), the great majority of DDL can be **one shared migration file set**, since standard `CREATE TABLE`/column-type SQL (aside from minor type-name differences) is close enough between Postgres, CockroachDB, and SQLite for this schema's scope. Recommend:
103:- A small number of **per-backend override files** only where genuinely necessary — e.g., a Postgres/CockroachDB-only migration adding a `pg_trgm`/trigram GIN index for name search (§4), which has no SQLite equivalent and would need a different SQLite index strategy or to be skipped there.
118:- **Name lookup:** name search is usually partial/fuzzy ("contains" or "starts with"), which a plain B-Tree index doesn't serve well for `LIKE '%term%'` queries. PostgreSQL's `pg_trgm` extension builds trigram-based GIN indexes that make arbitrary substring/fuzzy `LIKE` and similarity searches fast, and this is the standard PostgreSQL pattern for name search (https://medium.com/@saritasa/how-to-optimize-name-search-in-postgresql-with-trigram-and-btree-indexes-02c57eb27687). **CockroachDB also supports trigram indexes natively** (https://docs.cockroachlabs.com/docs/stable/trigram-indexes, https://www.cockroachlabs.com/blog/use-cases-trigram-indexes/), so this index strategy is portable between the two production-grade backends. SQLite has no native trigram/GIN equivalent — for the SQLite backend, either accept a full-scan `LIKE` for name search (acceptable for a local/dev/demo backend, consistent with §2's framing of SQLite as non-production-parity) or use SQLite's FTS5 extension for a rough parity if search-quality parity is important.
119:- **Case-insensitivity:** if usernames or names need case-insensitive equality (not fuzzy) matching, `CITEXT` (Postgres/CockroachDB) paired with a plain B-Tree index is the efficient pattern for that specific case — separate from the trigram index used for fuzzy/partial name search (https://medium.com/codex/case-insensitive-text-search-in-postgresql-whats-fast-and-what-fails-f836024c4590).
124:- `user_profile`: B-Tree on `phone`; trigram GIN on `name` (Postgres/CockroachDB only — SQLite falls back to full scan or FTS5)
129:- https://docs.cockroachlabs.com/docs/stable/trigram-indexes
130:- https://www.cockroachlabs.com/blog/use-cases-trigram-indexes/
188:- One `ProfileRepository` and one `CredentialRepository` Go interface at the service layer; concrete implementations per backend package (a shared Postgres/CockroachDB implementation given their wire/SQL compatibility, plus a separate SQLite implementation), selected via config at startup — no ORM required, `database/sql` plus per-driver placeholder handling is sufficient.
189:- UUID primary keys everywhere (required for CockroachDB hotspot avoidance, portable to the other two) rather than branching PK strategy per backend.
190:- `INSERT ... ON CONFLICT (...) DO UPDATE` as the one upsert syntax supported by all three backends — avoid CockroachDB's `UPSERT` shorthand in shared SQL to keep queries portable.
192:- Migrations: mostly one shared SQL migration series (golang-migrate or goose) using cross-compatible types, with a small number of explicitly-named per-backend override files only where required (e.g., a Postgres/CockroachDB-only trigram index on `user_profile.name`, absent in SQLite).
193:- Indexes: B-Tree on `phone`; trigram GIN on `name` (Postgres/CockroachDB; SQLite falls back to `LIKE` scan or FTS5); unique index on `username`; FK indexes on `user_id` and `method_id`.

{
  "command": "grep -n \"Malformed identifiers\" -A 40 multi-db-strategy.md | head -60",
  "description": "Read the A4 amendment subsection"
}

386:### Malformed identifiers — `ErrInvalidArgument`, validated above the backends (amendment A4)
387-
388-A syntactically invalid `id` (`"does-not-exist"`, not a UUID) behaves differently per backend and the contract did not say what it should mean. Postgres/CockroachDB type `id` as `UUID`, so the driver raises `22P02` (`invalid_text_representation`) *before any row lookup*; SQLite types it as `TEXT`, so the same string is an ordinary lookup that finds nothing and returns `ErrNotFound`. Found by LT-38's conformance suite — which is the suite doing precisely the job it was required for.
389-
390-**Ruling: a malformed identifier is `ErrInvalidArgument`, not `ErrNotFound`.** The alternative — treating it as "not found", on the reasoning that an invalid id can never name an existing row — is defensible in isolation but inconsistent with the rest of this contract, and consistency decides it. Every other malformed input here already maps to an argument error rather than to a data outcome: a non-empty `ID` passed to `Create`, a `Country` that is not two characters, an out-of-range `Limit` or `Offset`. Making a malformed UUID the single exception, where bad input is silently reinterpreted as a legitimate business answer, would be the one inconsistency a caller could not predict from the rest of the document.
391-
392-It is also the more useful answer operationally: a caller sending malformed identifiers has a bug, and `ErrNotFound` hides it forever behind a plausible 404 while `ErrInvalidArgument` surfaces it on the first call. Note for `../02-ai-security-architecture/`: this distinction leaks nothing about existence, because it is decided purely by *syntax* before any lookup occurs — a well-formed id for a row that does not exist still returns `ErrNotFound`, so the two cases cannot be used to probe which identifiers are real.
393-
394-**Enforcement site — and this is the part that matters more than the ruling.** The validation happens **once, above the backends**, in shared DAO code, not as two per-backend error translations that must agree. Every method taking an `id` validates UUID syntax before touching a backend. Parity is then *by construction*: Postgres never reaches the query and never emits `22P02` on this path, and SQLite never silently misses. Two translation tables that must be kept in agreement is the failure mode this contract has spent three amendments removing; one validator above both is a single site that cannot drift.
395-
396-`22P02` is still mapped to `ErrInvalidArgument` in the backend translation as a backstop, for any path where a malformed identifier reaches SQL another way. That mapping is defence in depth, not the enforcement site.
397-
398-**Conformance assertion (§7):** a malformed identifier returns `ErrInvalidArgument` on every backend, and a well-formed identifier naming no row returns `ErrNotFound` on every backend. Both, not just the first — the pair is what proves the distinction is syntactic rather than existential.
399-
400-### The write-path retry seam — a decision, not an observation
401-
402-CockroachDB defaults to SERIALIZABLE and returns client-retryable `40001` in situations PostgreSQL at READ COMMITTED never produces, and its internal retry cannot help once results have begun streaming. `Upsert`'s `ON CONFLICT DO UPDATE` under contention is directly exposed to this.
403-
404-**Ruling: every write goes through a `crdb.ExecuteTx`-style retry wrapper, and the CockroachDB backend stays at its default SERIALIZABLE.** The alternative — running CockroachDB at READ COMMITTED to drop the requirement — was considered and rejected: CockroachDB's READ COMMITTED is documented as *stronger* than PostgreSQL's, so choosing it to "simplify" would introduce a second engine-behaviour divergence hiding behind a matching level name. That is the same error, one layer down, that this phase has spent its time removing. The wrapper is near-zero cost on PostgreSQL and load-bearing on CockroachDB.
405-
406-`Search` is read-only, two independent statements, no explicit transaction — it never encounters `40001` and needs no wrapper.
407-
408-**Where the wrapper sits, and how it knows the engine.** The ruling above said *that* writes retry and not *where* the retry lives — a cold read pointed out that three implementers would produce three different codebases from it, all faithful. Settled:
409-
410-- The wrapper is an **unexported helper inside the `postgres` package**, e.g. `withRetry(ctx, func(tx) error)`, and every write method in that package runs its statement inside it. It is not in the composite from §1 and not above the interface: putting it there would wrap reads too and would push engine-specific behaviour above the boundary the interface exists to draw.
411-- The **`sqlite` package has no wrapper at all** — not a no-op shim. SQLite has no retryable-serialization error class, so a no-op would be ceremony implying a shared abstraction that does not exist.
412-- The `postgres` implementation struct carries an unexported `engine` field set by `New` from the driver string — `"postgres"` or `"cockroachdb"`. `withRetry` retries only when `engine == cockroachdb`; on `"postgres"` it calls the function once and returns. **That field is the entire reason the two driver strings are distinct** despite selecting the same package, which §1 asserted without saying how the distinction was carried.
413-
414-## 5. Field-level schema
415-
416-Nullability is the most expensive thing here to get wrong, which is why it carries its own column and its own reviewer.
417-
418-### `user_profile`
419-
420-| Column | Go type | Postgres / CockroachDB | SQLite | Null? | Constraint / note |
421-|---|---|---|---|---|---|
422-| `id` | `string` | `UUID DEFAULT gen_random_uuid()` | `TEXT` | NOT NULL | PK. App-generated in Go before insert on SQLite; server-generated elsewhere. Callers never see the difference. |
423-| `name` | `string` | `TEXT COLLATE "C"` | `TEXT COLLATE BINARY` | NOT NULL | `CHECK (length(trim(name)) > 0)`. Collation named explicitly — see §6.3. |
424-| `phone` | `*string` | `TEXT` | `TEXT` | NULL ok | E.164-normalized at write. named `ck_user_profile_phone_e164`. Postgres/CockroachDB: `CHECK (phone IS NULL OR phone ~ '^\+[1-9][0-9]{1,14}$')`. SQLite has no regex, so: `CHECK (phone IS NULL OR (phone GLOB '+[1-9]*' AND length(phone) BETWEEN 8 AND 16 AND NOT phone GLOB '*[^+0-9]*'))`. Given rather than left as "a GLOB equivalent" — a cold read correctly noted that inventing it would be a design decision, not an implementation detail. |
425-| `street_address` | `*string` | `TEXT` | `TEXT` | NULL ok | `CHECK (street_address IS NULL OR length(trim(street_address)) > 0)`, named `ck_user_profile_street_address_nonempty` |
426-| `locality` | `*string` | `TEXT` | `TEXT` | NULL ok | `CHECK (locality IS NULL OR length(trim(locality)) > 0)`, named `ck_user_profile_locality_nonempty` |
--
563:| A4 | 2026-09-12 | §4, §7 | Malformed identifiers return `ErrInvalidArgument`, validated once above the backends rather than translated per-backend | LT-38's conformance suite found `Get` with a malformed id diverging: Postgres/CockroachDB raise `22P02` before any lookup (`id` is `UUID`-typed), SQLite returns `ErrNotFound` (`id` is `TEXT`). §3a said only that `Get` returns `ErrNotFound`, and §4's "same sentinel for the same condition" did not define what "the same condition" means when column types make a malformed id a different *kind* of failure per engine. Ruled for consistency with every other malformed input in this contract, which already maps to an argument error rather than a data outcome | Authored: **Priya Nandakumar**. Surfaced by: **Renata Cole** (03) via the LT-38 conformance suite — the suite finding a genuine divergence is the per-driver ruling's condition being met. Objection turn: **Yusuf Karadag** (05, Detail Hawk — error semantics) |
564-| A3 | 2026-09-12 | §6.2, §7 | The SQLite enforcement site for the cascade guarantee is the FK **plus a connection-level `foreign_keys` pragma**, set in the DSN; §7 gains cascade and FK-violation assertions | §6.2 named `ON DELETE CASCADE` as the enforcement site, which is true on Postgres/CockroachDB and **false on SQLite**, where foreign keys are off unless enabled per connection. The failure is silent — no error, just a cascade that does not happen — and it would have taken a governance control (§6.2), a retention guarantee (`pii-governance.md`) and an error sentinel (`ErrInvalidMethod`) with it. §7 had no assertion covering any of the three, so no test would have caught it. Found reviewing LT-37 | Authored: **Priya Nandakumar**. Surfaced by: **Renata Cole**'s LT-37 SQLite migration and driver — the defect is in my contract's enforcement site, not in her code, which compensates correctly for today's configuration |
565-| A2 | 2026-09-12 | §7 | Split "conformance" into signature conformance (mechanical, LT-35) and behavioural conformance (the cross-backend suite) | 03 flagged during implementation that the two were being conflated — LT-35's test is signature/type conformance, a different thing from the cross-backend behavioural suite. §7 named only the second while its title implied both, so a signature diff could have been used to tick a requirement it does not satisfy. That would have left the per-driver ruling in `decisions/multi-db-abstraction.md` resting on nothing | Authored: **Priya Nandakumar**. Originated by: **Renata Cole** (03 lead) — the distinction is hers; I only wrote it into the contract. Implementation-side propagation to QA: Renata → Oren Castellan |
566-| A1 | 2026-09-12 | §1a (new) | Package placement, the leaf criterion, and how to read the signatures | LT-35's "byte-for-byte" acceptance criterion is unmeetable as literally stated: 03's layout puts domain structs in `internal/model`, so `*UserProfile` does not compile inside `internal/dao`. Flagged pre-emptively before implementation rather than at review | Authored: **Priya Nandakumar**. Reviewed: **Anders Vogel** — confirmed the ruling, replaced the usage-based placement reason with the leaf/dependency-direction test, closed `ProfileQuery` rather than deferring it, and found both drift paths. No separate objection seat; the review carried the objections |
567-
568-**Standing rule for anything landing in git or Jira from this track:** commits are made from a worktree (never a checkout in the shared tree), authored as `Priya Nandakumar (05 Data Ops lead, Claude Opus 5)`, and carry `Story:` and `Work-By:` trailers naming every persona who contributed to that change — author, reviewers, and objection seats alike, not only whoever typed it.

{
  "command": "grep -n \"ErrInvalidArgument\\|ErrNotFound\\|DeleteExpired\\|DeleteProfile\\|GetByUsername\\|ListByUserID\\|func\\|Upsert\" multi-db-strategy.md | sed -n '1,80p'",
  "description": "Find sentinel and method references"
}

7:> **This section's interface listing has been removed, not corrected.** It restated the repository methods, the domain types and the error sentinel, and every one of those restatements had drifted from the authoritative version: `Update` was described as a portable upsert (it never creates — `Upsert` is a separate method), phone search as exact/prefix (exact only, per finding F9 with 01), pagination as bare limit/offset (there are `Limit` 1..100 and `Offset` 0..10 000 ceilings, enforced with `ErrInvalidQuery`), and `ErrNotFound` as the only sentinel (there are seven).
34:| Upsert | `INSERT ... ON CONFLICT (id) DO UPDATE SET ...` | Same syntax, supported since SQLite 3.24 | One query shape used everywhere. CockroachDB's `UPSERT` shorthand is deliberately not used, to avoid a second query dialect. |
90:func New(driver, dsn string) (Repository, error)   // driver: "postgres" | "cockroachdb" | "sqlite"
103:    DeleteExpired(ctx context.Context, class RetentionClass, olderThan time.Time, maxRows int) (SweepResult, error)
104:    DeleteProfile(ctx context.Context, id string, externalRef *string) error
125:| Error sentinels (§4) | `internal/dao` | `ErrNotFound` | unchanged |
131:**`ProfileQuery` stays in `dao`, and this is now settled rather than left open.** I had flagged it to 03 as theirs to overrule, on the worry that `api` importing `dao` to construct a query is an interface whose callers must know its implementation package. Review closed it: that objection is about callers knowing `postgres` or `sqlite`, not about knowing `dao`. `api` already imports `dao` to name `dao.Repository` and to compare `dao.ErrNotFound`, so no new dependency edge is created and moving the type would remove nothing. The leaf test agrees — `ProfileQuery` is named across one existing edge and has no meaning without the method that consumes it, so putting it in the leaf would make a package that depends on nothing carry a type defined entirely by something above it. `SweepResult` and `RetentionClass` follow the same reasoning.
184:- On `Create`, the caller leaves `ID` as `""`. The DAO generates the UUID (in Go for SQLite, server-side elsewhere) and returns the populated struct. **A non-empty `ID` passed to `Create` is a caller error and returns `ErrInvalidArgument`** — it is not a request to use that ID. This removes the contradiction a cold read found between "callers never see the difference" and `Create` returning `ErrAlreadyExists` "if the ID is taken": for profiles, the ID cannot be taken, because the caller never chooses it.
185:- `CreatedAt` and `UpdatedAt` are set by the DAO on every write and ignored on input. `UpdatedAt` is touched by `Create`, `Update`, and `Upsert` — including IDP re-hydration, which is load-bearing for retention (`pii-governance.md`).
197:    Upsert(ctx context.Context, p *UserProfile) (*UserProfile, error)
202:    GetByUsername(ctx context.Context, username string) (*UserCredential, error)
203:    ListByUserID(ctx context.Context, userID string) ([]UserCredential, error)
244:**`Create` / `Update` / `Upsert` are three distinct methods, and this changes the earlier prose.** The previous version of this document described `Update` as "implemented as a portable upsert", which would mean `Update` on a missing row silently *creates* it. That is exactly the kind of surprise that produces a bug in a caller, so it is ruled out:
247:- `Update` — update an existing row by ID. **`ErrNotFound` if the row does not exist.** It never creates.
248:- `Upsert` — the portable `INSERT ... ON CONFLICT (id) DO UPDATE` form. For a caller that **already holds the `id`** and does not know whether the row is still present — re-hydrating a known profile. Any other caller uses `Create` or `Update`. (This bullet previously justified `Upsert` by the IDP hydration path, a justification retracted three paragraphs below and left standing here; caught in the same sweep as F20.)
250:**`Update` and `Upsert` are FULL REPLACEMENTS, not partial patches.** This is the single most consequential thing in this document and it was missing from the first draft. Every column is written from the struct you pass. **A nil `Phone` sets the stored phone to NULL; it does not leave the existing value alone.** A caller wanting patch semantics must read, modify, and write back. Stated this loudly because both readings are reasonable and the wrong guess silently destroys data.
252:**`Upsert` conflicts on `id` only, and the caller must already hold that `id`.** It exists for re-hydrating a row already known to exist — not for resolving an unknown one. A cold read caught the circularity in the original justification: the IDP path "does not know whether the row exists" and therefore does not know its `id` either, so `Upsert` could not have served the caller it was justified with. **Resolving an `/identity` payload to an existing profile row is not the DAO's problem** — there is deliberately no unique key on `phone` or `name` to match on, because neither is unique in reality. It belongs to **`api-service`**, which is the party that both decides identity and holds a DAO handle: 03's layout gives `idp-connector` no database access at all (`../03-engineering-delivery/decisions/go-layout-debate.md`), so the connector fetches the payload and its caller decides what the payload *is* and then calls `Create` or `Update`. An earlier version of this sentence said "the connector" and was wrong about which process can reach the database (finding F22).
254:**`Delete` on a missing row returns `ErrNotFound`**, in both repositories — not a silent nil. Consistent with `Update`. Callers wanting idempotent delete check `errors.Is(err, ErrNotFound)` and treat it as success.
256:**`GetByUsername` lower-cases its input inside the DAO.** Callers pass whatever the user typed. This matches the `UNIQUE (lower(username))` index in §5; requiring callers to fold case themselves would put the guarantee outside its enforcement site.
273:- Errors: any of `Credentials().Create`'s errors, plus `ErrInvalidArgument`. On any failure **neither row exists**.
297:**`DeleteExpired(ctx, class, olderThan, maxRows)`** — one class per call, never a mixed-class predicate, because a single predicate spanning two clocks is how the wrong window gets applied to the wrong rows.
299:**Bounded by `maxRows`, and one transaction per batch — not one per sweep.** A class-wide delete with no chunking holds a single transaction over an unbounded row count, which is the failure mode 03 already identified for credential writes in their `context-propagation.md`. `maxRows` is required, must be in `1..10_000`, and returns `ErrInvalidArgument` outside it. The caller loops until `Drained` is true. Each batch is its own transaction, so an interrupted sweep leaves a consistent database with fewer rows deleted — never a half-written batch.
301:**`DeleteExpired` must not be called with a request-scoped context.** It is a background operation governed by the job's own timeout; a sweep cancelled halfway because an inbound HTTP request went away is a retention policy that silently depends on request lifetimes. Constructing that context is 03's layer, not the DAO's — stated here as a requirement on the caller, with its enforcement site named as 03's sweep-invocation code plus a conformance test asserting an already-cancelled context returns before deleting anything. The clock column is the class's own: `updated_at` for `direct` and `idp_cache`, **`created_at` for `idp_cache_orphan`** — a clock the read path touches would let an orphan reset its own expiry and never die. Each deleted profile cascades to its credentials and writes one `deletion_log` row with reason `retention_sweep`, in the same transaction as the delete.
305:**`DeleteProfile(ctx, id, externalRef)`** — subject-deletion. Writes the `deletion_log` row in the same transaction, with **`reason` set internally to `subject_request`; it is not a parameter.**
307:That is a deliberate narrowing from the first draft, which took `reason` as a caller-supplied string. A free string lets a caller write `retention_sweep` onto a subject-deletion row by copy-paste, which silently destroys the one property the audit trail exists to have: that its reason codes are trustworthy. **The reason a row exists is a fact about which method was called, so the method should assert it — not the caller.** `DeleteExpired` writes `retention_sweep` the same way. The two reason codes in `ck_deletion_log_reason` are therefore exhaustive by construction rather than by convention, and a third value is a contract change that adds a method, not a new string a caller may pass.
319:| `Profiles().Get` | yes (lookup key) | — | — | `ErrNotFound` |
321:| `Profiles().Create` | **no** — must be `""`, DAO generates | DAO sets both | — | `ErrInvalidArgument` (non-empty ID, or CHECK violation); `ErrAlreadyExists` — see note below |
322:| `Profiles().Update` | yes — identifies the row | DAO sets `UpdatedAt` | **full replace**; nil NULLs the column | `ErrNotFound`, `ErrInvalidArgument` |
323:| `Profiles().Upsert` | **yes** — conflict target; caller must already hold it | DAO sets `UpdatedAt`; sets `CreatedAt` on insert only — **the `DO UPDATE SET` list must exclude `created_at`** | **full replace** | `ErrInvalidArgument` |
324:| `Profiles().Delete` | yes | — | — | `ErrNotFound`; **cascades to credentials, see §6.2** |
325:| `Credentials().GetByUsername` | — | — | — | `ErrNotFound`; DAO lower-cases input |
326:| `Credentials().ListByUserID` | — | — | — | empty slice, not `ErrNotFound`, when the user has none |
327:| `Credentials().Create` | **no** — DAO generates | DAO sets both | — | `ErrDuplicateUsername`, `ErrInvalidMethod`, `ErrInvalidCredential`, `ErrInvalidArgument` |
328:| `Credentials().Update` | yes | DAO sets `UpdatedAt` | **full replace** | `ErrNotFound`, `ErrDuplicateUsername`, `ErrInvalidCredential` |
329:| `Credentials().Delete` | yes | — | — | `ErrNotFound`; does **not** delete the profile (§6.2) |
330:| `Methods().Get` / `GetByName` | yes / name | — | — | `ErrNotFound` |
333:`DeleteExpired` and `DeleteProfile` (§3c) are not in the table above: both are composite-level, both write `deletion_log` in the same transaction as the delete with the reason code asserted by the method rather than the caller, `DeleteExpired` returns counts rather than rows and is batched by a required `maxRows`, and both return `ErrInvalidArgument` on a bad class or batch bound. `CreateProfileWithCredential` (§3b) is likewise not in it because it spans two rows: IDs DAO-generated for both, `c.UserID` ignored on input and set from the created profile, both timestamps DAO-set, and on failure neither row exists.
343:3. **`Upsert` must not overwrite `created_at`.** Its `ON CONFLICT (id) DO UPDATE SET` list excludes `created_at` explicitly. Without that, every IDP re-hydration would silently reset the creation timestamp — on a column `pii-governance.md` keys retention to. This was an omission that would have produced a silent, retention-affecting bug, and it is now stated at both the method table and here.
349:    ErrNotFound          = errors.New("dao: not found")
354:    ErrInvalidArgument   = errors.New("dao: invalid argument")
377:| any CHECK violation (`source`, `name`, `phone`, `country`, address non-empty) | `23514` | `SQLITE_CONSTRAINT_CHECK` | `ErrInvalidArgument` |
380:**`ErrInvalidArgument` closes a real hole**: the commonest caller mistake is a write that violates a CHECK constraint, and in the first draft that arrived at the service layer as a raw, engine-specific driver error. `ErrInvalidQuery` protected reads and nothing protected writes.
386:### Malformed identifiers — `ErrInvalidArgument`, validated above the backends (amendment A4)
388:A syntactically invalid `id` (`"does-not-exist"`, not a UUID) behaves differently per backend and the contract did not say what it should mean. Postgres/CockroachDB type `id` as `UUID`, so the driver raises `22P02` (`invalid_text_representation`) *before any row lookup*; SQLite types it as `TEXT`, so the same string is an ordinary lookup that finds nothing and returns `ErrNotFound`. Found by LT-38's conformance suite — which is the suite doing precisely the job it was required for.
390:**Ruling: a malformed identifier is `ErrInvalidArgument`, not `ErrNotFound`.** The alternative — treating it as "not found", on the reasoning that an invalid id can never name an existing row — is defensible in isolation but inconsistent with the rest of this contract, and consistency decides it. Every other malformed input here already maps to an argument error rather than to a data outcome: a non-empty `ID` passed to `Create`, a `Country` that is not two characters, an out-of-range `Limit` or `Offset`. Making a malformed UUID the single exception, where bad input is silently reinterpreted as a legitimate business answer, would be the one inconsistency a caller could not predict from the rest of the document.
392:It is also the more useful answer operationally: a caller sending malformed identifiers has a bug, and `ErrNotFound` hides it forever behind a plausible 404 while `ErrInvalidArgument` surfaces it on the first call. Note for `../02-ai-security-architecture/`: this distinction leaks nothing about existence, because it is decided purely by *syntax* before any lookup occurs — a well-formed id for a row that does not exist still returns `ErrNotFound`, so the two cases cannot be used to probe which identifiers are real.
396:`22P02` is still mapped to `ErrInvalidArgument` in the backend translation as a backstop, for any path where a malformed identifier reaches SQL another way. That mapping is defence in depth, not the enforcement site.
398:**Conformance assertion (§7):** a malformed identifier returns `ErrInvalidArgument` on every backend, and a well-formed identifier naming no row returns `ErrNotFound` on every backend. Both, not just the first — the pair is what proves the distinction is syntactic rather than existential.
402:CockroachDB defaults to SERIALIZABLE and returns client-retryable `40001` in situations PostgreSQL at READ COMMITTED never produces, and its internal retry cannot help once results have begun streaming. `Upsert`'s `ON CONFLICT DO UPDATE` under contention is directly exposed to this.
410:- The wrapper is an **unexported helper inside the `postgres` package**, e.g. `withRetry(ctx, func(tx) error)`, and every write method in that package runs its statement inside it. It is not in the composite from §1 and not above the interface: putting it there would wrap reads too and would push engine-specific behaviour above the boundary the interface exists to draw.
412:- The `postgres` implementation struct carries an unexported `engine` field set by `New` from the driver string — `"postgres"` or `"cockroachdb"`. `withRetry` retries only when `engine == cockroachdb`; on `"postgres"` it calls the function once and returns. **That field is the entire reason the two driver strings are distinct** despite selecting the same package, which §1 asserted without saying how the distinction was carried.
496:| B-Tree on `user_credential(user_id)` | all | `ListByUserID`, cascade delete |
512:   Why this matters beyond tidiness: with foreign keys off, deleting a profile leaves its credentials orphaned, `DeleteExpired`'s cascade silently retains credential rows past their profile's retention window, and `ErrInvalidMethod` can never be produced because the `method_id` FK does not fire. A governance control, a retention guarantee and an error sentinel all rest on this one pragma.
518:8. **The `ON CONFLICT ... DO UPDATE` upsert form.** The one syntax all three backends accept; CockroachDB's `UPSERT` shorthand is deliberately unused to avoid a second dialect. *Enforcement: `Upsert`'s single query shape.*
541:- a zero-value `RetentionClass` (`""`) returns `ErrInvalidArgument` — a caller-reachable invalid input, unlike `ErrAlreadyExists`, and therefore owed an explicit assertion rather than only prose;
542:- `maxRows` outside `1..10_000` returns `ErrInvalidArgument`;
563:| A4 | 2026-09-12 | §4, §7 | Malformed identifiers return `ErrInvalidArgument`, validated once above the backends rather than translated per-backend | LT-38's conformance suite found `Get` with a malformed id diverging: Postgres/CockroachDB raise `22P02` before any lookup (`id` is `UUID`-typed), SQLite returns `ErrNotFound` (`id` is `TEXT`). §3a said only that `Get` returns `ErrNotFound`, and §4's "same sentinel for the same condition" did not define what "the same condition" means when column types make a malformed id a different *kind* of failure per engine. Ruled for consistency with every other malformed input in this contract, which already maps to an argument error rather than a data outcome | Authored: **Priya Nandakumar**. Surfaced by: **Renata Cole** (03) via the LT-38 conformance suite — the suite finding a genuine divergence is the per-driver ruling's condition being met. Objection turn: **Yusuf Karadag** (05, Detail Hawk — error semantics) |

{
  "command": "python3 /Users/warrenthompson/.claude/skills/profile-gen/scripts/show_profile.py --profile /Users/warrenthompson/Source/LoginID/05-data-ops/profiles/yusuf-karadag/yusuf-karadag.md --root /Users/warrenthompson/Source/LoginID/05-data-ops",
  "description": "Re-enable persona overlay"
}

{"name": "Yusuf Karadag", "mode": "hud", "hud": true, "badge": false, "background": false, "inline": false}