Skip to content

Commit c11d775

Browse files
committed
feat(store): ADR-009 stable uids + photos table, and a safe backup (om-hdk5)
lots.id and roll_txns.id are INTEGER PRIMARY KEY with no AUTOINCREMENT — plain rowid aliases, allocated max(rowid)+1. Delete the highest-id lot, insert the next, and the integer is silently recycled: a photo filed under the 1964 Kennedy half is adopted, with no error, by the clad quarter entered after the Kennedy was sold. Right filename, wrong coin, nothing to detect it. Export -> edit -> reimport reshuffles the same integers across every FK. So identity that outlives a row's rowid has to be its own column. This is the blocker under export (om-9cua) and photos (om-6hlp); both formally depend on it. Migration 0009 adds uid to lots + roll_txns and creates the photos table. Note the number: ADR-009 and the bead both say "0008", but they predate ADR-010, which landed branches as 0008. Same migration, next free number — ADR amended to say so. The care goes on the two ALTERed columns. A UNIQUE index does NOT imply NOT NULL: SQLite treats NULLs as mutually distinct, so any number of rows can carry a NULL uid straight through the index. The guarantee therefore lives in the Go insert path — including the one that is easy to miss, where a *partial sale* carves out a brand new lot row without the word "insert" appearing anywhere in the caller's vocabulary — and in an invariant test that pins it. Verified by mutation: drop newUID() from any insert path and the test fails. The portable schema-level fix (a 12-step table rebuild) stays rejected for the reason ADR-009 gives: PRAGMA foreign_keys=OFF is a no-op inside the transaction the migration runner wraps each file in. And ALTER COLUMN ... SET NOT NULL, which modernc accepts and which therefore looks right, is not SQLite grammar — a migration leaning on it would bind crh.db to one driver, and users open this file with other tools. photos is a NEW table, so it declares uid TEXT NOT NULL UNIQUE outright and SQLite enforces it. role is NOT NULL because a NULL role LOSES photos: the gallery filters `role NOT IN ('obverse','reverse')`, and NULL NOT IN (...) is NULL, not true — the image would sit in the database, on disk, and never render. Verified against real legacy data, not just fresh DBs: 1,224 pre-0009 rows across two v8 databases backfilled to distinct, well-formed, lowercase v4 uids — zero nulls, zero duplicates, zero cross-table collisions. randomblob() re-evaluates per row, so one UPDATE does it with no Go backfill loop. Also: `coinrollhunter backup DEST.db` via VACUUM INTO. Copying a live crh.db is the obvious move and it is wrong — recent commits may still be in the -wal sidecar, so a naive copy can miss them or catch a checkpoint mid-flight. VACUUM INTO writes one self-contained database, no sidecars, while the app stays open. It goes through store.BackupFile rather than Open+Backup because Open applies pending migrations, and a backup must not upgrade the database it is preserving — which is exactly the backup you want before an upgrade. qa/run.sh 44/44 green. All six targets still cross-compile CGO_ENABLED=0.
1 parent 368488d commit c11d775

11 files changed

Lines changed: 663 additions & 22 deletions

File tree

CLAUDE.md

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,26 @@ targets still cross-compile from Linux — which is exactly why Wails/webview_go
123123
they need CGO + a Windows toolchain and would cost us single-box releases. Still open:
124124
code-signing (SmartScreen/Gatekeeper still warn).
125125

126+
Added 2026-07-12 (ADR-009 stable uids + backup, om-hdk5): `lots.id` / `roll_txns.id` are
127+
bare rowid aliases — SQLite hands out `max(rowid)+1`, so a delete+insert **recycles** the
128+
integer and a photo filed under it is silently adopted by a different coin. **Migration
129+
0009** (`0009_stable_uids_photos.sql` — ADR-009 says "0008", but ADR-010/branches took that
130+
number first) adds `uid` to `lots` + `roll_txns`, backfills them in pure SQL with the
131+
`randomblob` UUIDv4 recipe 0008 proved (non-deterministic, re-evaluates **per row**), and
132+
creates the **`photos`** table (arbitrary N per owner, `role`+`seq`, owned by a lot *or* a
133+
roll_txn, path `photos/<owner_uid>/<photo_uid>.<ext>` — nothing mutable in the path).
134+
**A UNIQUE index does not imply NOT NULL in SQLite**, so on the two ALTERed columns the
135+
guarantee lives in Go (`newUID()` on all three insert paths — including the easy-to-miss
136+
one where a *partial sale* carves out a new lot) plus the invariant tests in
137+
`uid_test.go`. Don't "fix" this with `ALTER COLUMN … SET NOT NULL`: modernc accepts it but
138+
it isn't SQLite grammar and would bind `crh.db` to one driver. `model.Holding.UID` /
139+
`model.RollTxn.UID` are scanned as `NullString` and exposed on the API — export (om-9cua)
140+
and photos (om-6hlp) are now unblocked. Also **`coinrollhunter backup DEST.db`** via
141+
`VACUUM INTO`: one consistent self-contained file, safe on a *live* database (a plain `cp`
142+
of `crh.db` can miss commits still in the `-wal` sidecar). It uses `store.BackupFile`, not
143+
`Open`+`Backup`, because `Open` applies pending migrations — a backup must not upgrade the
144+
thing it is preserving.
145+
126146
Build notes: `make build` (UI then Go). In this container, Go needs a writable cache —
127147
`go env -w GOCACHE=/go/cache`. The UI build needs Node 22 + npm registry access. `web/dist`
128148
is a git-ignored build artifact (only its `.gitkeep` is committed, so `go:embed all:dist`

README.md

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,25 @@ Your data is saved to a single SQLite file in your user data directory:
6464
| **macOS** | `~/Library/Application Support/CoinRollHunter/crh.db` |
6565
| **Linux** | `~/.local/share/coinrollhunter/crh.db` |
6666

67-
Back it up by copying that file. (If you already have a `crh.db` sitting next to the
68-
binary — how earlier versions worked — that one keeps being used, so upgrading never
69-
loses your holdings.)
67+
(If you already have a `crh.db` sitting next to the binary — how earlier versions
68+
worked — that one keeps being used, so upgrading never loses your holdings.)
69+
70+
### Backing up
71+
72+
Don't copy `crh.db` by hand while the app is open. SQLite keeps recent changes in a
73+
`-wal` sidecar file, so a plain copy can miss your latest edits or catch a write
74+
half-finished. Use:
75+
76+
```bash
77+
./coinrollhunter backup my-coins-2026-07-12.db
78+
```
79+
80+
That writes one complete, self-contained database — no sidecars, nothing else needed —
81+
and it's safe to run with the app still open. Copy the result anywhere: a USB stick, a
82+
sync folder, another machine. Open it later with `./coinrollhunter serve --db my-coins-2026-07-12.db`.
83+
84+
It won't overwrite an existing backup, and it never modifies the database it's reading —
85+
so it's also the right thing to run *before* upgrading to a new version.
7086

7187
> **Unpack it first.** Windows will happily run an `.exe` straight from inside the zip
7288
> preview, but it does that by unpacking to a temporary folder that gets cleaned up later.

cmd/coinrollhunter/main.go

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,13 @@ func main() {
5757
fmt.Fprintln(os.Stderr, "demo:", err)
5858
os.Exit(1)
5959
}
60+
case "backup":
61+
// No "backup:" prefix here — store.Backup's errors already carry one, and
62+
// the user does not need to be told twice.
63+
if err := runBackup(os.Args[2:]); err != nil {
64+
fmt.Fprintln(os.Stderr, err)
65+
os.Exit(1)
66+
}
6067
case "version", "-v", "--version":
6168
fmt.Printf("coinrollhunter %s\n", version)
6269
case "-h", "--help", "help":
@@ -84,6 +91,10 @@ usage:
8491
Seed a separate database with ~15 months of fictional data and serve it —
8592
a full dashboard to explore before entering your own. Your real data is
8693
untouched; --reset regenerates the demo from scratch.
94+
coinrollhunter backup [--db crh.db] DEST.db
95+
Write a consistent snapshot of your data to a single file — safe to run
96+
while the app is open, and safe to copy anywhere. Copying crh.db by hand
97+
is not: recent changes may still live in its -wal sidecar.
8798
coinrollhunter version
8899
Print the build version.
89100
`)
@@ -161,6 +172,49 @@ func runServe(args []string) error {
161172
})
162173
}
163174

175+
// runBackup writes a consistent snapshot of the database to a single file, while
176+
// the app is running. The point is that the result is safe to copy anywhere — onto
177+
// a USB stick, into a sync folder, across to another machine — which a live crh.db
178+
// is not: its recent commits may still be sitting in a -wal sidecar.
179+
func runBackup(args []string) error {
180+
fs := flag.NewFlagSet("backup", flag.ExitOnError)
181+
dbPath := fs.String("db", "", "path to the SQLite database (default: the same one the app uses)")
182+
if err := fs.Parse(args); err != nil {
183+
return err
184+
}
185+
if fs.NArg() != 1 {
186+
return fmt.Errorf("usage: coinrollhunter backup [--db crh.db] DEST.db")
187+
}
188+
dest := fs.Arg(0)
189+
190+
// Default to whatever the app itself would open, so `backup` finds the user's
191+
// real data without them having to know where it lives.
192+
src := *dbPath
193+
if src == "" {
194+
var err error
195+
if src, err = defaultDBPath(); err != nil {
196+
return err
197+
}
198+
}
199+
if _, err := os.Stat(src); err != nil {
200+
return fmt.Errorf("no database at %s", src)
201+
}
202+
203+
// BackupFile, not Open+Backup: Open applies pending migrations, so backing up
204+
// through it would upgrade the database you were trying to snapshot *before*
205+
// upgrading it — exactly the backup you'd want if an upgrade went wrong.
206+
if err := store.BackupFile(src, dest); err != nil {
207+
return err
208+
}
209+
fi, err := os.Stat(dest)
210+
if err != nil {
211+
return err
212+
}
213+
fmt.Printf("Backed up %s -> %s (%.1f MB)\n", src, dest, float64(fi.Size())/(1<<20))
214+
fmt.Println("This is a complete, self-contained database — copy it anywhere.")
215+
return nil
216+
}
217+
164218
// runDemo seeds a separate demo database with the fictional dataset (only when
165219
// it's empty, so demo edits survive restarts) and serves it. --reset deletes
166220
// and regenerates. The user's real database is never touched.

docs/ADR-009-stable-export-identifier.md

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,13 @@ Export therefore becomes a bundle: per-table CSVs (including `photos.csv`), a `p
139139
tree, and a `manifest.json` carrying schema version and file list — rather than loose
140140
CSVs.
141141

142-
### (c) Migration 0008 is additive; on the *altered* tables, `NOT NULL` lives in Go
142+
### (c) The migration is additive; on the *altered* tables, `NOT NULL` lives in Go
143+
144+
> **Numbering, as shipped.** This section was written as "migration 0008". ADR-010
145+
> (branches) landed first and took 0008, so the uid + photos migration shipped as
146+
> **`0009_stable_uids_photos.sql`**. Same SQL, next free number. ADR-010 (c) already
147+
> anticipated this: it generated `branches.uid` with the same `randomblob` recipe and
148+
> noted it "unblocks ADR-009's om-hdk5 lots/roll_txns uid backfill".
143149
144150
```sql
145151
-- Backfill expression, used once per altered table:

internal/model/model.go

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,12 @@ type ItemType struct {
2828
// Holding is a specimen you own — a quantity of some ItemType, with acquisition,
2929
// custody, and disposal data. ADR-003.
3030
type Holding struct {
31-
ID int64 `json:"id"`
31+
ID int64 `json:"id"`
32+
// UID is this specimen's permanent identity (ADR-009): opaque, generated by the
33+
// store at insert, never recycled by a delete and never renumbered by a reimport.
34+
// ID is a rowid alias and cannot do that job — it is the internal join key only.
35+
// The export row key and the photo path stem are this.
36+
UID string `json:"uid"`
3237
ItemTypeID int64 `json:"item_type_id"`
3338
RollTxnID int64 `json:"roll_txn_id,omitempty"` // box this find came from (0 = none)
3439
Activity string `json:"activity"` // "bullion" | "crh"
@@ -129,7 +134,11 @@ func Resolve(h Holding, t ItemType) Lot {
129134
// dollars. The entry unit (box/roll/face/coin) is preserved for display, but
130135
// FaceUSD is the source of truth (see ADR-001 R7).
131136
type RollTxn struct {
132-
ID int64 `json:"id"`
137+
ID int64 `json:"id"`
138+
// UID is the purchase record's permanent identity (ADR-009). Box/roll records
139+
// carry photos too — the box end with its bank stamps, the wrapper, the receipt —
140+
// so they need a stable path stem exactly as specimens do.
141+
UID string `json:"uid"`
133142
Date string `json:"date"`
134143
// Bank is the branch's canonical name — a resolved display/entry value, not a
135144
// stored column since migration 0008 (ADR-010): writes find-or-create a branch

internal/store/backup_test.go

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
package store
2+
3+
import (
4+
"database/sql"
5+
"os"
6+
"path/filepath"
7+
"testing"
8+
9+
"github.com/tompscanlan/coinrollhunter/internal/model"
10+
)
11+
12+
// sqlOpen reaches past Store to the raw driver, so a test can stand up a database
13+
// at an arbitrary schema version without Open() migrating it out from under us.
14+
func sqlOpen(path string) (*sql.DB, error) { return sql.Open("sqlite", path) }
15+
16+
// The claim Backup makes is specifically that it works on a LIVE database — the app
17+
// is open, the server is running, you did not stop anything. So test it that way:
18+
// write, back up without closing, and check the snapshot is complete and readable
19+
// on its own.
20+
func TestBackupSnapshotsALiveDatabase(t *testing.T) {
21+
dir := t.TempDir()
22+
src := filepath.Join(dir, "crh.db")
23+
24+
s, err := Open(src)
25+
if err != nil {
26+
t.Fatal(err)
27+
}
28+
defer s.Close()
29+
30+
typeID, err := s.InsertItemType(model.ItemType{Kind: "coin", Name: "Mercury Dime", Metal: "silver", FineOzEach: 0.0723})
31+
if err != nil {
32+
t.Fatal(err)
33+
}
34+
for i := 0; i < 50; i++ {
35+
if _, err := s.InsertHolding(model.Holding{
36+
ItemTypeID: typeID, Activity: "crh", Qty: 1, BasisUSD: 0.10, Acquired: "2026-07-01",
37+
}); err != nil {
38+
t.Fatal(err)
39+
}
40+
}
41+
42+
dest := filepath.Join(dir, "backup.db")
43+
if err := s.Backup(dest); err != nil {
44+
t.Fatal(err)
45+
}
46+
47+
// One file, no sidecars. That is the property that makes it copyable: a -wal or
48+
// -shm left beside it would mean the snapshot is incomplete on its own.
49+
for _, sidecar := range []string{dest + "-wal", dest + "-shm"} {
50+
if _, err := os.Stat(sidecar); err == nil {
51+
t.Errorf("backup left a %s sidecar — the file is not self-contained", filepath.Base(sidecar))
52+
}
53+
}
54+
55+
// The source is still open and usable; a backup must not disturb it.
56+
if _, err := s.InsertHolding(model.Holding{ItemTypeID: typeID, Activity: "crh", Qty: 1, BasisUSD: 0.10, Acquired: "2026-07-02"}); err != nil {
57+
t.Fatalf("source database broken after backup: %v", err)
58+
}
59+
60+
// The snapshot opens standalone and carries every row committed before it ran —
61+
// and not the one committed after.
62+
b, err := Open(dest)
63+
if err != nil {
64+
t.Fatalf("backup will not open: %v", err)
65+
}
66+
defer b.Close()
67+
68+
var n int
69+
if err := b.db.QueryRow(`SELECT count(*) FROM lots`).Scan(&n); err != nil {
70+
t.Fatal(err)
71+
}
72+
if n != 50 {
73+
t.Errorf("backup has %d lots, want the 50 committed before it ran", n)
74+
}
75+
everyRowHasAUID(t, b)
76+
77+
var ver int
78+
if err := b.db.QueryRow(`PRAGMA user_version`).Scan(&ver); err != nil {
79+
t.Fatal(err)
80+
}
81+
if srcVer, _ := s.Version(); ver != srcVer {
82+
t.Errorf("backup schema version %d != source %d", ver, srcVer)
83+
}
84+
}
85+
86+
// The backup you most want is the one taken *before* an upgrade — so BackupFile
87+
// must not migrate the source on its way through. Open() does, which is why the CLI
88+
// does not go through it.
89+
func TestBackupFileDoesNotMigrateTheSource(t *testing.T) {
90+
dir := t.TempDir()
91+
src := filepath.Join(dir, "old.db")
92+
93+
// A database from an older release: schema version 0, none of our tables.
94+
db, err := sqlOpen(src)
95+
if err != nil {
96+
t.Fatal(err)
97+
}
98+
if _, err := db.Exec(`CREATE TABLE lots (id INTEGER PRIMARY KEY); PRAGMA user_version = 4`); err != nil {
99+
t.Fatal(err)
100+
}
101+
db.Close()
102+
103+
dest := filepath.Join(dir, "snap.db")
104+
if err := BackupFile(src, dest); err != nil {
105+
t.Fatal(err)
106+
}
107+
108+
for name, path := range map[string]string{"source": src, "snapshot": dest} {
109+
d, err := sqlOpen(path)
110+
if err != nil {
111+
t.Fatal(err)
112+
}
113+
var v int
114+
if err := d.QueryRow(`PRAGMA user_version`).Scan(&v); err != nil {
115+
t.Fatal(err)
116+
}
117+
d.Close()
118+
if v != 4 {
119+
t.Errorf("%s is at schema version %d, want 4 — backup migrated it", name, v)
120+
}
121+
}
122+
}
123+
124+
// A backup command that can silently overwrite the previous backup is a footgun in
125+
// the one place you least want one.
126+
func TestBackupRefusesToOverwrite(t *testing.T) {
127+
dir := t.TempDir()
128+
s, err := Open(filepath.Join(dir, "crh.db"))
129+
if err != nil {
130+
t.Fatal(err)
131+
}
132+
defer s.Close()
133+
134+
dest := filepath.Join(dir, "backup.db")
135+
if err := os.WriteFile(dest, []byte("an earlier backup"), 0o644); err != nil {
136+
t.Fatal(err)
137+
}
138+
if err := s.Backup(dest); err == nil {
139+
t.Fatal("Backup overwrote an existing file")
140+
}
141+
got, err := os.ReadFile(dest)
142+
if err != nil {
143+
t.Fatal(err)
144+
}
145+
if string(got) != "an earlier backup" {
146+
t.Error("the existing backup was clobbered")
147+
}
148+
}

internal/store/crud.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ func (s *Store) DeleteItemType(id int64) error { return s.deleteByID("item_type"
6262

6363
func (s *Store) ListHoldings() ([]model.Holding, error) {
6464
rows, err := s.db.Query(
65-
`SELECT id, item_type_id, roll_txn_id, activity, qty, gross_weight, purity, weight_unit, basis_usd,
65+
`SELECT id, uid, item_type_id, roll_txn_id, activity, qty, gross_weight, purity, weight_unit, basis_usd,
6666
premium_usd, face_value_usd, acquired, source, location, insured_value, attributes,
6767
notes, category, subcategory, trophy, disposed, disposed_usd
6868
FROM lots ORDER BY id`)
@@ -74,13 +74,17 @@ func (s *Store) ListHoldings() ([]model.Holding, error) {
7474
for rows.Next() {
7575
var h model.Holding
7676
var rtid sql.NullInt64
77-
var wu, src, loc, attr, notes, cat, subcat, disp sql.NullString
77+
var uid, wu, src, loc, attr, notes, cat, subcat, disp sql.NullString
7878
var trophy int64
79-
if err := rows.Scan(&h.ID, &h.ItemTypeID, &rtid, &h.Activity, &h.Qty, &h.GrossWeight, &h.Purity,
79+
if err := rows.Scan(&h.ID, &uid, &h.ItemTypeID, &rtid, &h.Activity, &h.Qty, &h.GrossWeight, &h.Purity,
8080
&wu, &h.BasisUSD, &h.PremiumUSD, &h.FaceValueUSD, &h.Acquired, &src, &loc,
8181
&h.InsuredValue, &attr, &notes, &cat, &subcat, &trophy, &disp, &h.DisposedUSD); err != nil {
8282
return nil, err
8383
}
84+
// NullString, not string: lots.uid has no schema-level NOT NULL (ADR-009 (c)),
85+
// so a row written by some other tool could still read back NULL. Scanning it
86+
// as a plain string would error out the whole list rather than surfacing it.
87+
h.UID = uid.String
8488
h.RollTxnID = rtid.Int64
8589
h.WeightUnit, h.Source, h.Location = wu.String, src.String, loc.String
8690
h.Attributes, h.Notes, h.Disposed = attr.String, notes.String, disp.String

0 commit comments

Comments
 (0)