F-07 + F-10 (Medium): Self-describing ledger — currency, scale, schema version#7
Conversation
The database was not self-describing: its integers are meaningless
without knowing the scale (and currency) they denominate, and there was
no schema version — the documented migration story for incompatible
changes was "delete the database", which is not a migration story for
financial records. Bumping SCALE would have silently reinterpreted
every stored amount 10x smaller, and USD/EUR postings routed into the
same ledger would "balance" undetectably.
- New pluts_ledger_meta table (key/value)
- migrate() stamps scale and schema_version (SCHEMA_VERSION const), and
refuses to open a ledger stamped at a different scale — demanding an
explicit rescale migration instead of silent reinterpretation
- migrate(sql, { currency }) optionally stamps the ledger currency and
refuses to re-open under a different one, catching cross-currency
routing bugs at provision time
- getLedgerMeta(sql) reads it all back; exported with SCHEMA_VERSION
- Databases provisioned before the meta table are treated as version 0
and stamped on next migrate (all DDL is idempotent IF NOT EXISTS)
Audit findings F-07 and F-10 (Medium).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 69937eefbd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Two review findings on the metadata stamping in migrate(): - schema_version was INSERT OR IGNOREd without ever comparing the stored value, so a build rolled back below a later release's stamp would run against a schema it has never seen. migrate now throws when the stored version is newer than SCHEMA_VERSION, and upserts (rather than ignores) when advancing an older version. - A whitespace-only opts.currency passed the truthy check and stamped a trimmed empty string — a blank denomination the mismatch guard can never catch, silently preserved by INSERT OR IGNORE forever after. The trim now happens before the presence check, so whitespace-only reads as absent. Addresses PR #7 review (2x P2). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ea493495af
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The guard rejected newer stored versions only after the DDL loop had already executed every v1 statement — an old build could throw raw SQLite errors or create obsolete objects against a v2-shaped database before it refused. migrate now creates only the meta table (stable by contract), reads schema_version, and rejects newer versions before touching the rest of SCHEMA_STATEMENTS. Test provisions a v2-stamped database and asserts the refusal leaves no other tables behind. Addresses PR #7 review (P2, guard placement). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d9c43c60bd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
An unstamped ledger predates the meta table, which shipped while SCALE was 2 — its integers denominate scale 2 regardless of this build's SCALE. The scale guard only fired when a scale row existed, so a raised-SCALE build opening such a ledger stamped the new scale over historic amounts, silently reinterpreting them. migrate now refuses unstamped ledgers that already hold amounts unless SCALE still equals the legacy scale; empty unstamped ledgers stamp the current scale as before. Tests mock SCALE=3 to reach the refusal branch. Also merges origin/main and reconciles its frozen-arrays/aliasing test. Addresses PR #7 review (P2, unstamped scale). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 90fe263e14
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
4377d23 to
2b5696b
Compare
|
@codex Sorry man, I fucked up and killed this branch and PR. If you can, revert back to last known good state and then carry on fixing the merge conflicts. |
|
Summary
Testing
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4377d23033
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…spec stub Two injuries from the hand merge of main: - Main's payload_hash legacy migration (F-05) landed inside the if (currency) block, so databases migrated without a configured currency never got the column. Moved back to top level — it must run on every migrate. - migrate.spec.ts's SqlStorage stub dropped bind parameters, so this branch's parameterized meta stamps inserted NULLs and failed the NOT NULL constraint. The stub now forwards binds like the other specs. Full suite, typecheck, and lint pass; reviewed the merged diff top to bottom — the meta/scale/version guard logic and orderings are intact. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Both sides rewrote migrate(); the merged ordering preserves each intent: the meta table + schema-version rollback guard run first (refuse before any DDL), then main's legacy negative-rowid repair (must precede trigger creation), then the DDL loop, then the scale/version/currency stamps and the payload_hash backfill. SCHEMA_STATEMENTS keeps main's F-04/F-14 triggers plus this branch's meta table DDL. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 840f82227d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // existing amounts would silently reinterpret them; refuse until an | ||
| // explicit rescale migration runs. Empty ledgers hold no amounts to | ||
| // misread and stamp the current scale directly. | ||
| if (stampedScale.length === 0 && SCALE !== LEGACY_UNSTAMPED_SCALE) { |
There was a problem hiding this comment.
Fresh evidence is the final placement: when a build with SCALE !== LEGACY_UNSTAMPED_SCALE opens an unstamped ledger that already has rows in pluts_amounts, this guard throws only after the SCHEMA_STATEMENTS loop above has installed append-only triggers such as pluts_amounts_no_update. The database is left refusing amount updates, so the rescale migration required by the error cannot adjust stored minor units without first knowing to drop internal triggers; check for legacy amounts and refuse before applying the rest of the DDL.
Useful? React with 👍 / 👎.
1 participant
Findings
F-07 (Medium): no currency dimension exists anywhere — amounts are bare integers, and the database carries no record of what they denominate. Answering the audit's question directly: can entries in different currencies balance against each other? Yes, trivially — 100 USD-cents credits against 100 EUR-cents debits passes every check, and nothing in the data can even prove which rows were which.
F-10 (Medium): no schema version table; the README's documented procedure for incompatible schema changes was deleting DO storage. Worse, the promised
SCALE-raise migration path had no mechanism and no record of which scale existing integers were written at — an upgraded build would silently reinterpret every historical $10.00 (1000minor units) as $1.000, with the trial balance still zero.Fix
New
pluts_ledger_metakey/value table;migrate()stamps and verifies:scale— the decimal scale stored minor units were written at. A ledger stamped at scale 2 opened by a build compiled at scale 3 now throwsRepositoryErrordemanding an explicit rescale migration, instead of silently misvaluing every amount.schema_version— the new exportedSCHEMA_VERSIONconstant. Pre-meta databases are treated as version 0 and stamped on next migrate (all DDL is idempotent, so 0→1 needs no data steps). Future incompatible changes bump the constant and add explicit upgrade steps — never "reset the database".currency(opt-in, backwards compatible) —migrate(sql, { currency: "NZD" })records the denomination; re-opening under a different currency throws, so a routing bug sending EUR postings into a USD ledger fails at provision time.getLedgerMeta(sql)reads it back.This is deliberately the first step on multi-currency: it makes the single-currency-per-ledger model explicit and enforceable in the data. Per-line currency tagging / FX-pair entries can layer on later without breaking stamped ledgers.
Testing
New
ledger-meta.spec.ts(written first, watched fail) running realmigrate()against SQLite vianode:sqlite: fresh stamp, currency recording, stamping an existing un-stamped ledger, currency-mismatch refusal, scale-mismatch refusal, idempotent re-runs. README Migrations section rewritten to match.Full suite: 84 passed.
tsc --noEmitandbiome checkclean.Note: touches
migrate()like #5 does — whichever merges second will need a trivial conflict resolution (both append independent post-DDL steps).🤖 Generated with Claude Code