amadan

oficina / sheets Public

Add migrations.go: Schema, BootSchema and Models for rastrillo migration

Declares the *migrate.Set pair SKILL.md documents (Schema for
generate/check, BootSchema for what Apply runs at boot) plus the
Models list the CLI's loader reads by name from this package. rastrillo
migration generate produced internal/sheets/migrations/0001_create_sheets.sql
from the six sheets models; migration check now passes.

Verified the generated SQL by hand against the three constraints the
models carry that a generic diff could miss: cells' composite primary
key (tab_id, row_id, col_id), the unique (tab_id, position) indexes on
rows and cols, and the unique index on formats.digest. All four are in
the SQL as CREATE TABLE .. PRIMARY KEY (...) / CREATE UNIQUE INDEX —
nothing needed hand-writing via `migration new`.

Event moves from store/events.go into models.go (as sheets.Event),
with store/events.go now `type Event = sheets.Event`. This is forced,
not a tidiness call: the CLI's loader (cmd/rastrillo/migration.go,
appPackage) requires exactly one non-test package under internal/ and
reads Schema/BootSchema/Models from it by name, and package sheets
cannot import package store (store already imports sheets for the
other five models, so the reverse would cycle) — so Event's struct has
to live somewhere package sheets can see it, or `migration check` can
never catch it drifting. The alias keeps every existing store.Event
call site, production and test, compiling unchanged — same trade
b4d3e25 made for the plaintext-columns test, just no longer optional
now that a production tool needs the same reach.

internal/guards moved to internal/guardstest (test files only, no
behaviour change) because the CLI's appPackage requires exactly one
non-"test"-suffixed directory under internal/, and internal/guards
plus internal/sheets made two; `rastrillo migration generate .`
otherwise fails with "expected exactly one app package under
./internal, found [guards sheets]". Confirmed with `go test
./internal/guardstest/...` and full `make test`, both green.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01APtNStXDZ5H2fAc1ZuGqtS
Paul Campbell pushed by paul@keymail.dev d49ed01c9e43c949f32fda0ebb6df0cf66b81aa3 parent 8fa462a
13 files changed, +158 −28
  • internal/{guards => guardstest}/calc_isolation_test.go +0 −0
  • internal/{guards => guardstest}/excelize_confinement_test.go +0 −0
  • internal/{guards => guardstest}/forbidden_imports_test.go +0 −0
  • internal/{guards => guardstest}/linecap_test.go +0 −0
  • internal/{guards => guardstest}/repo_test.go +0 −0
  • internal/{guards => guardstest}/sealtest_containment_test.go +0 −0
  • internal/sheets/migrations.go +58 −0
  • internal/sheets/migrations/0001_create_sheets.sql +28 −0
  • internal/sheets/migrations/schema.sql +31 −0
  • internal/sheets/models.go +21 −0
  • internal/sheets/models_test.go +9 −16
  • internal/sheets/store/events.go +10 −11
  • internal/sheets/xlsx/oracle_test.go +1 −1

Intent

full ledger: core

diff --git a/internal/guards/calc_isolation_test.go b/internal/guardstest/calc_isolation_test.go
similarity index 100%
rename from internal/guards/calc_isolation_test.go
rename to internal/guardstest/calc_isolation_test.go
diff --git a/internal/guards/excelize_confinement_test.go b/internal/guardstest/excelize_confinement_test.go
similarity index 100%
rename from internal/guards/excelize_confinement_test.go
rename to internal/guardstest/excelize_confinement_test.go
diff --git a/internal/guards/forbidden_imports_test.go b/internal/guardstest/forbidden_imports_test.go
similarity index 100%
rename from internal/guards/forbidden_imports_test.go
rename to internal/guardstest/forbidden_imports_test.go
diff --git a/internal/guards/linecap_test.go b/internal/guardstest/linecap_test.go
similarity index 100%
rename from internal/guards/linecap_test.go
rename to internal/guardstest/linecap_test.go
diff --git a/internal/guards/repo_test.go b/internal/guardstest/repo_test.go
similarity index 100%
rename from internal/guards/repo_test.go
rename to internal/guardstest/repo_test.go
diff --git a/internal/guards/sealtest_containment_test.go b/internal/guardstest/sealtest_containment_test.go
similarity index 100%
rename from internal/guards/sealtest_containment_test.go
rename to internal/guardstest/sealtest_containment_test.go
diff --git a/internal/sheets/migrations.go b/internal/sheets/migrations.go
new file mode 100644
index 0000000..7bfb392
--- /dev/null
+++ b/internal/sheets/migrations.go
@@ -0,0 +1,58 @@
+package sheets
+
+import (
+ "embed"
+
+ "github.com/carlosframework/rastrillo/migrate"
+)
+
+// migrationsFS embeds internal/sheets/migrations/*.sql — this package's
+// generated migrations plus the schema.sql summary `rastrillo migration
+// generate` maintains beside them. schema.sql matches the glob but is
+// never parsed as a step (migrate.FromFS excludes it by name), so the
+// go:embed directive below needs at least one *.sql file to exist before
+// this package can compile at all; that is why an empty schema.sql was
+// committed ahead of the first `rastrillo migration generate` run rather
+// than left for it to create.
+//
+//go:embed migrations/*.sql
+var migrationsFS embed.FS
+
+// Schema is Sheets' own migration set: what `rastrillo migration
+// generate`/`check` diff Models against. Never hand-edit a file under
+// migrations/ once it has shipped — add a new one instead
+// (`rastrillo migration new <name>`).
+var Schema = migrate.MustFromFS(migrationsFS, "sheets")
+
+// BootSchema is everything migrate.Apply runs at boot, in apply order —
+// the shape SKILL.md documents as `migrate.Merge(subsystemSchemas...,
+// Schema)`. There is no framework subsystem to merge in here (this app
+// uses none of rastrillo's sessions/auth/jobs pieces), so BootSchema is
+// Schema alone; migrate.Merge is still the call, not a bare assignment,
+// so a future subsystem's schema has one obvious place to be added
+// without anyone needing to notice BootSchema didn't previously go
+// through Merge at all.
+var BootSchema = migrate.Merge(Schema)
+
+// Models lists every GORM model `rastrillo migration generate`/`check`
+// diff Schema against — Sheet, Tab, Row, Col, Cell and Format from
+// models.go, plus Event.
+//
+// Event is declared in models.go, not internal/sheets/store/events.go
+// where the rest of its behaviour (AppendInput, AppendRecalc, the Kind
+// consts) lives, and store/events.go keeps only a type alias back to it
+// (type Event = sheets.Event). That split is forced by this file, not a
+// tidiness preference: the CLI's loader (cmd/rastrillo/migration.go,
+// appPackage) requires exactly one non-test package under internal/ and
+// reads Schema, BootSchema and Models from it by name — there is no
+// second package generate/check will ever look at. package store already
+// imports package sheets for Sheet/Tab/Row/Col/Cell/Format, so the
+// reverse import sheets -> store this file would otherwise need to add
+// Event here cycles. A model check cannot see can never be caught
+// drifting, which would leave exactly one of Sheets' seven tables outside
+// the gate this file exists to build — so Event moved, and the alias
+// keeps every existing store.Event call site, production and test,
+// compiling unchanged.
+var Models = []any{
+ Sheet{}, Tab{}, Row{}, Col{}, Cell{}, Format{}, Event{},
+}
diff --git a/internal/sheets/migrations/0001_create_sheets.sql b/internal/sheets/migrations/0001_create_sheets.sql
new file mode 100644
index 0000000..b556c40
--- /dev/null
+++ b/internal/sheets/migrations/0001_create_sheets.sql
@@ -0,0 +1,28 @@
+CREATE TABLE `sheets` (`id` integer PRIMARY KEY AUTOINCREMENT,`title_sealed` blob,`created_by` integer,`created_at` datetime,`updated_at` datetime,`deleted_at` datetime);
+
+CREATE INDEX `idx_sheets_deleted_at` ON `sheets`(`deleted_at`);
+
+CREATE TABLE `tabs` (`id` integer PRIMARY KEY AUTOINCREMENT,`sheet_id` integer,`name_sealed` blob,`position` integer,`frozen_rows` integer,`frozen_cols` integer);
+
+CREATE INDEX `idx_tabs_sheet_id` ON `tabs`(`sheet_id`);
+
+CREATE TABLE `rows` (`id` integer PRIMARY KEY AUTOINCREMENT,`tab_id` integer,`position` integer,`height` integer);
+
+CREATE UNIQUE INDEX `idx_row_tab_pos` ON `rows`(`tab_id`,`position`);
+
+CREATE TABLE `cols` (`id` integer PRIMARY KEY AUTOINCREMENT,`tab_id` integer,`position` integer,`width` integer);
+
+CREATE UNIQUE INDEX `idx_col_tab_pos` ON `cols`(`tab_id`,`position`);
+
+CREATE TABLE `cells` (`tab_id` integer,`row_id` integer,`col_id` integer,`input_sealed` blob,`cached_sealed` blob,`input_version` integer,`cache_version` integer,`format_id` integer,`updated_by` integer,`updated_at` datetime,PRIMARY KEY (`tab_id`,`row_id`,`col_id`));
+
+CREATE TABLE `formats` (`id` integer PRIMARY KEY AUTOINCREMENT,`digest` text,`number_fmt` text,`bold` numeric,`italic` numeric,`fill_intent_h` real,`fill_intent_c` real,`fill_hex` text,`align` text);
+
+CREATE UNIQUE INDEX `idx_formats_digest` ON `formats`(`digest`);
+
+CREATE TABLE `events` (`id` integer PRIMARY KEY AUTOINCREMENT,`kind` text,`tab_id` integer,`actor_id` integer,`payload_sealed` blob,`trigger_id` integer,`derived_count` integer,`created_at` datetime);
+
+CREATE INDEX `idx_events_tab_id` ON `events`(`tab_id`);
+
+CREATE INDEX `idx_events_kind` ON `events`(`kind`);
+
diff --git a/internal/sheets/migrations/schema.sql b/internal/sheets/migrations/schema.sql
new file mode 100644
index 0000000..878400f
--- /dev/null
+++ b/internal/sheets/migrations/schema.sql
@@ -0,0 +1,31 @@
+-- Generated by rastrillo migration generate; DO NOT EDIT.
+-- The schema every migration in this directory adds up to.
+
+CREATE TABLE `cells` (`tab_id` integer,`row_id` integer,`col_id` integer,`input_sealed` blob,`cached_sealed` blob,`input_version` integer,`cache_version` integer,`format_id` integer,`updated_by` integer,`updated_at` datetime,PRIMARY KEY (`tab_id`,`row_id`,`col_id`));
+
+CREATE TABLE `cols` (`id` integer PRIMARY KEY AUTOINCREMENT,`tab_id` integer,`position` integer,`width` integer);
+
+CREATE TABLE `events` (`id` integer PRIMARY KEY AUTOINCREMENT,`kind` text,`tab_id` integer,`actor_id` integer,`payload_sealed` blob,`trigger_id` integer,`derived_count` integer,`created_at` datetime);
+
+CREATE TABLE `formats` (`id` integer PRIMARY KEY AUTOINCREMENT,`digest` text,`number_fmt` text,`bold` numeric,`italic` numeric,`fill_intent_h` real,`fill_intent_c` real,`fill_hex` text,`align` text);
+
+CREATE TABLE `rows` (`id` integer PRIMARY KEY AUTOINCREMENT,`tab_id` integer,`position` integer,`height` integer);
+
+CREATE TABLE `sheets` (`id` integer PRIMARY KEY AUTOINCREMENT,`title_sealed` blob,`created_by` integer,`created_at` datetime,`updated_at` datetime,`deleted_at` datetime);
+
+CREATE TABLE `tabs` (`id` integer PRIMARY KEY AUTOINCREMENT,`sheet_id` integer,`name_sealed` blob,`position` integer,`frozen_rows` integer,`frozen_cols` integer);
+
+CREATE UNIQUE INDEX `idx_col_tab_pos` ON `cols`(`tab_id`,`position`);
+
+CREATE INDEX `idx_events_kind` ON `events`(`kind`);
+
+CREATE INDEX `idx_events_tab_id` ON `events`(`tab_id`);
+
+CREATE UNIQUE INDEX `idx_formats_digest` ON `formats`(`digest`);
+
+CREATE UNIQUE INDEX `idx_row_tab_pos` ON `rows`(`tab_id`,`position`);
+
+CREATE INDEX `idx_sheets_deleted_at` ON `sheets`(`deleted_at`);
+
+CREATE INDEX `idx_tabs_sheet_id` ON `tabs`(`sheet_id`);
+
diff --git a/internal/sheets/models.go b/internal/sheets/models.go
index 0f7ec93..9a97fb4 100644
--- a/internal/sheets/models.go
+++ b/internal/sheets/models.go
@@ -80,3 +80,24 @@ type Format struct {
FillHex string // retained from XLSX import for lossless re-export
Align string
}
+
+// Event is the append-only log: live-sync fan-out, version history, undo
+// and audit. Its behaviour (AppendInput, AppendRecalc, the Kind consts)
+// lives in package store, which is where it conceptually belongs — store
+// owns persistence. The struct lives here instead, aliased back as
+// store.Event, purely because of what migrations.go needs: `rastrillo
+// migration check` diffs this package's Models against Schema, and
+// package sheets cannot import store (store already imports sheets for
+// Sheet/Tab/Row/Col/Cell/Format, so the reverse import would cycle). A
+// model the check can't see is a model it can never catch drifting, which
+// defeats the point of this file. See migrations.go and store/events.go.
+type Event struct {
+ ID uint64 `gorm:"primaryKey"`
+ Kind string `gorm:"index;size:16"`
+ TabID uint64 `gorm:"index"`
+ ActorID uint64
+ PayloadSealed []byte // seal purpose "event.payload"; empty for recalc
+ TriggerID uint64 // recalc only: the input event that caused it
+ DerivedCount int // recalc only: how many cells the pass touched
+ CreatedAt time.Time
+}
diff --git a/internal/sheets/models_test.go b/internal/sheets/models_test.go
index 7ec7c65..e79300f 100644
--- a/internal/sheets/models_test.go
+++ b/internal/sheets/models_test.go
@@ -6,7 +6,6 @@ import (
"testing"
"amadan.net/oficina/sheets/internal/sheets"
- "amadan.net/oficina/sheets/internal/sheets/store"
)
// The models must carry no column that could hold a readable cell value.
@@ -25,20 +24,14 @@ import (
//
// Add "Title string" to Sheet and this test fails; that is the point.
//
-// This lives in package sheets_test, not package sheets, specifically so
-// it can also walk store.Event: Event carries sealed user content
-// (PayloadSealed) but is declared in internal/sheets/store/events.go, not
-// models.go, so a version of this test confined to package sheets (and
-// therefore unable to import store, which itself imports sheets) could
-// never see it. Without Event in this walk, "schema lives in models.go,
-// nowhere else" was true of the file layout but not of what this gate
-// actually checked — someone could add a plaintext-shaped field straight
-// to Event and every assertion here would keep passing. Moving Event
-// into models.go instead (making sheets.Event) was the other option, but
-// it would mean renaming store.Event at every call site across store's
-// production code and tests for a schema-location tidiness win the
-// package-based fix achieves without touching Event's existing callers
-// at all.
+// Event (sealed PayloadSealed) is included via sheets.Event, not
+// store.Event: the two are the same type (store.Event is now a type
+// alias — see models.go's doc comment on Event and store/events.go),
+// but this file no longer needs to import store to reach it, so it
+// doesn't. This test still lives in package sheets_test rather than
+// package sheets — no functional need since the migrations.go work,
+// only inertia — but there is no reason to move it now that it compiles
+// either way.
func TestNoPlaintextValueColumns(t *testing.T) {
forbidden := map[string]bool{
"Title": true,
@@ -55,7 +48,7 @@ func TestNoPlaintextValueColumns(t *testing.T) {
sheets.Col{},
sheets.Cell{},
sheets.Format{},
- store.Event{},
+ sheets.Event{},
}
byteSliceType := reflect.TypeOf([]byte(nil))
diff --git a/internal/sheets/store/events.go b/internal/sheets/store/events.go
index 656f235..bce4af5 100644
--- a/internal/sheets/store/events.go
+++ b/internal/sheets/store/events.go
@@ -4,10 +4,10 @@ package store
import (
"encoding/json"
- "time"
"gorm.io/gorm"
+ "amadan.net/oficina/sheets/internal/sheets"
"amadan.net/oficina/sheets/internal/sheets/seal"
)
@@ -20,16 +20,15 @@ const (
// Event is the append-only log. It does four jobs — live-sync fan-out,
// version history, undo, and audit — which is why the storage model pays.
-type Event struct {
- ID uint64 `gorm:"primaryKey"`
- Kind string `gorm:"index;size:16"`
- TabID uint64 `gorm:"index"`
- ActorID uint64
- PayloadSealed []byte // seal purpose "event.payload"; empty for recalc
- TriggerID uint64 // recalc only: the input event that caused it
- DerivedCount int // recalc only: how many cells the pass touched
- CreatedAt time.Time
-}
+//
+// The struct itself is declared as sheets.Event, not here: see that
+// type's doc comment in models.go for why (migrations.go's Models list
+// has to see it, and package sheets cannot import this package back).
+// This alias means every existing call site — store.Event{...},
+// &store.Event{}, AutoMigrate(&store.Event{}) in tests — keeps compiling
+// unchanged; a type alias makes store.Event and sheets.Event the exact
+// same type, not merely convertible ones.
+type Event = sheets.Event
// InputEvent is a person's edit, before sealing.
type InputEvent struct {
diff --git a/internal/sheets/xlsx/oracle_test.go b/internal/sheets/xlsx/oracle_test.go
index e3cd739..7234756 100644
--- a/internal/sheets/xlsx/oracle_test.go
+++ b/internal/sheets/xlsx/oracle_test.go
@@ -22,7 +22,7 @@
//
// This lives in internal/sheets/xlsx, not internal/sheets/calc, for one
// mechanical reason and one architectural one:
-// - internal/guards/excelize_confinement_test.go fails any file outside
+// - internal/guardstest/excelize_confinement_test.go fails any file outside
// internal/sheets/xlsx (any depth) that imports excelize, with no
// exemption for _test.go files. A file in calc that imported excelize
// — even test-only — would fail that guard.