F-08 (Medium): Journal sequence numbers and deterministic ordering#8
Conversation
Entries were keyed by random UUID with no sequence: same-date entries
returned in nondeterministic order, an account's amounts were ordered
by UUID (meaningless), and nothing could prove completeness. A journal
is by definition a chronological, numbered record — sequential
numbering is what lets an auditor assert "no entries are missing" and
cite a posting ("JE 142").
- pluts_entries gains seq INTEGER with a unique index; assigned as
MAX(seq)+1 inside the posting transaction (safe under the DO
single-writer model); Entry/EntryDTO carry it
- migrate() upgrades legacy databases via a table_info probe: ALTER,
then backfill by rowid (insertion order = posting order; the library
never deletes), idempotently
- Deterministic ordering everywhere: allEntries and entriesForAccount
order by (date, seq); amountsForAccount now joins entries and orders
by the owning entry's (date, seq) instead of random UUID
- Ledger.verifyJournalComplete(): MAX(seq) === COUNT(*) — the
auditor's completeness assertion as a one-call check, backed by
Repository.entrySequenceStats()
Audit finding F-08 (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: 5ec6a7c85e
ℹ️ 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 journal sequencing: - The seq backfill only ran when the column was just added, so a deploy interrupted between ALTER TABLE and the backfill left NULL rows behind a 'column exists' guard forever. The backfill now runs on every migrate, numbering unnumbered rows after the highest existing seq in rowid order — continuing from MAX(seq) also avoids unique-index collisions with rows posted after such an interruption (plain seq = rowid could collide). - MAX(seq) === COUNT(*) detects entries missing between surviving rows but not a contiguous tail truncation, which shifts both together. Renamed verifyJournalComplete to verifyNoSequenceGaps and re-documented it (and README) to claim exactly what it proves; a true completeness assertion would need a high-water mark persisted outside the entry rows. Addresses PR #8 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: c224241462
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9da2b50301
ℹ️ 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".
Fixes the biome formatting failure from the last push. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The fake's exec(query) dropped bind arguments, so the seq backfill's parameterized UPDATE ran with unbound placeholders — silently a no-op on this node:sqlite (NULL binds) and an outright failure on versions that reject missing values. The fake now forwards ...binds like the journal-sequence spec's fake. Addresses PR #8 review (P2). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
migrate() now composes all three features in dependency order: meta table + schema-version rollback guard first (refuse before any DDL), main's legacy negative-rowid repair (before the triggers exist), this branch's seq column ALTER (before the DDL loop creates the seq unique index), the DDL loop, the seq backfill, then the scale/version/currency stamps and payload_hash backfill. pluts_entries keeps both the seq column and main's calendar-date CHECK. 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: d9678096f2
ℹ️ 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".
| description TEXT NOT NULL, | ||
| date TEXT NOT NULL, | ||
| posted_at TEXT NOT NULL, | ||
| seq INTEGER, |
There was a problem hiding this comment.
When consumer code or a maintenance script uses the raw ctx.storage.sql handle, this new audit field is still freely updatable because the existing pluts_entries_no_update trigger only guards id, description, date, and posted_at. That means existing entries can be renumbered, e.g. swapped through temporary sequence values, while preserving uniqueness and MAX(seq) = COUNT(*), so JE identities and statement ordering can be rewritten without detection; add a migration-safe trigger/check that allows the initial NULL-to-number backfill but blocks later seq changes.
Useful? React with 👍 / 👎.
1 participant
Finding
F-08 (Medium) from the ledger audit: entries were keyed by random UUID with no monotonic sequence.
allEntriesordered bydateonly — same-day entries returned in nondeterministic order;amountsForAccountordered byentry_id ASC, i.e. by random UUID, a meaningless sequence for a statement view. There was no gap-detectable numbering: nothing could show "no entries have been removed."Why it matters: a journal is by definition a chronological, numbered record. Sequential numbering is what lets an auditor assert completeness ("the sequence has no gaps") and gives each posting a citable identity ("JE-142"). Two runs of the same journal report listing the same day's entries differently reads as a control deficiency.
Fix
pluts_entries.seq(INTEGER, unique index): assigned asMAX(seq)+1inside the existing posting transaction — safe under the Durable Object single-writer model. Carried onEntryandEntryDTO.migrate():table_infoprobe →ALTER TABLE→ backfill byrowid(insertion order = posting order; the library never deletes). Idempotent; re-runs never renumber. Runs before the DDL loop so the unique index applies cleanly.allEntriesandentriesForAccountorder by(date, seq);amountsForAccountnow joinspluts_entriesand orders an account's activity by the owning entry's(date, seq)— a proper statement view. The in-memory test repository mirrors all of it.Ledger.verifyJournalComplete():MAX(seq) === COUNT(*)— the auditor's completeness assertion as a one-call check (backed by the newRepository.entrySequenceStats()). Combined with the append-only triggers in F-04 (High) + F-14: Append-only triggers and row-validity checks #4, deletion becomes both impossible and detectable.Compatibility note with #4: its
pluts_entriesUPDATE trigger is deliberately column-scoped to the financial fields, so this PR's seq backfill works on databases that already have the triggers, in either merge order.Testing
Written first and watched fail: increasing seq assignment; same-date entries ordered by posting sequence in both directions; date-then-seq ordering;
verifyJournalCompleteon populated and empty ledgers; and real-DDL migration tests vianode:sqlite— fresh databases get the column + unique index, legacy databases are backfilled1..Nin insertion order and re-migrating never renumbers.Full suite: 85 passed.
tsc --noEmitandbiome checkclean.🤖 Generated with Claude Code