Skip to content

ft. ENG-24: Define ledger schema tables (accounts, journalEntries, ledgerCursors, reservations, sequenceCounter)#83

Merged
Connorbelez merged 4 commits intomainfrom
ENG-24
Mar 16, 2026
Merged

ft. ENG-24: Define ledger schema tables (accounts, journalEntries, ledgerCursors, reservations, sequenceCounter)#83
Connorbelez merged 4 commits intomainfrom
ENG-24

Conversation

@Connorbelez
Copy link
Copy Markdown
Owner

@Connorbelez Connorbelez commented Mar 16, 2026

Add ledger_reservations and ledger_sequence_counter tables, add
pendingDebits/pendingCredits fields to ledger_accounts for the
two-phase reservation pattern, expand entryType union with 3
reservation types (SHARES_RESERVED/COMMITTED/VOIDED), add
reservationId field to journal entries, and change journal entry
amount from v.int64() to v.number() per spec.

Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com

Summary by Sourcery

Introduce ledger reservation schema and normalize ledger amount/timestamp types while keeping existing lifecycle behavior intact.

New Features:

  • Add ledger_reservations table with status tracking, deal linkage, and journal entry references to support two-phase reserve/commit/void flows.
  • Add ledger_sequence_counter table for centralized ledger sequencing metadata.

Enhancements:

  • Extend ledger_accounts with pendingDebits and pendingCredits fields to track reserved balances alongside cumulative debits/credits.
  • Expand ledger_journal_entries entryType union with reservation-related types and an optional reservationId, and relax metadata typing for greater flexibility.
  • Standardize monetary amounts and timestamps on number types across ledger APIs, schema, validators, queries, and demo code while retaining BigInt for balance arithmetic internally.
  • Initialize new pending balance fields across world, treasury, and position account creation paths, including seed/demo helpers.
  • Register reservation entry types in validation wiring with placeholder validators ahead of dedicated reservation mutations.

Tests:

  • Update ledger tests and demo utilities to use numeric amounts instead of bigint literals, keeping existing behavioral coverage intact.

Summary by CodeRabbit

Release Notes

  • New Features

    • Added pending debit and credit balance tracking for accounts
    • Introduced reservation system for tracking mortgage deals and status changes
  • Tests

    • Expanded test coverage for ledger validation with numeric integrity checks and edge-case scenarios
  • Bug Fixes

    • Improved amount validation to enforce safe integer constraints and reject invalid inputs

@linear
Copy link
Copy Markdown

linear Bot commented Mar 16, 2026

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 16, 2026

Warning

Rate limit exceeded

@Connorbelez has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 19 minutes and 51 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 760b27ed-fd6d-4bd0-b250-364551adaceb

📥 Commits

Reviewing files that changed from the base of the PR and between 1367afe and 341c453.

📒 Files selected for processing (9)
  • convex/auth/__tests__/resourceChecks.test.ts
  • convex/demo/ledger.ts
  • convex/ledger/__tests__/ledger.test.ts
  • convex/ledger/internal.ts
  • convex/ledger/mutations.ts
  • convex/ledger/queries.ts
  • convex/ledger/validators.ts
  • convex/schema.ts
  • src/routes/demo/convex-ledger.tsx
📝 Walkthrough

Walkthrough

This PR refactors the ledger system to track pending balances and reservations, transitioning monetary amounts from BigInt literals to plain numbers with runtime validation, adding new schema tables for reservations and sequence counters, and updating all related mutations, queries, and validators to enforce numeric integrity constraints.

Changes

Cohort / File(s) Summary
Schema and Type Definitions
convex/schema.ts, convex/ledger/validators.ts
New tables ledger_reservations and ledger_sequence_counters introduced; existing tables extended with pendingDebits/pendingCredits fields and new entry types (SHARES_RESERVED, SHARES_COMMITTED, SHARES_VOIDED); type conversions from float64 to number and int64 to number for amounts; metadata type relaxed from record<string, any> to any.
Ledger Mutations and Internal Logic
convex/ledger/mutations.ts, convex/ledger/internal.ts
Amount input type changed to number with runtime validation (finite, integer, safe-integer checks); introduction of amountBigInt for internal BigInt-backed arithmetic; new rejectReservationViaPostEntry validator to prevent posting reservation types via postEntry; treasury and position accounts initialized with pendingDebits/pendingCredits.
Ledger Queries
convex/ledger/queries.ts
Added safeBigIntAmount helper for safe BigInt conversion; query input parameters (asOf, from, to) converted from float64 to number; internal balance updates now use safe BigInt conversion.
Demo and Seeding
convex/demo/ledger.ts
Lender amounts changed from BigInt literals to plain numbers; postSeedEntry signature updated with numeric amount type; seed data extended to initialize pendingDebits/pendingCredits for TREASURY and POSITION accounts.
Frontend Amount Parsing
src/routes/demo/convex-ledger.tsx
parseIntegerAmount return type changed from bigint to number; regex pattern updated to disallow zero; added validation for Number.isSafeInteger and overflow checking.
Test Coverage
convex/auth/__tests__/resourceChecks.test.ts, convex/ledger/__tests__/ledger.test.ts
New test helpers for ledger reservations and sequence counters; monetary literals converted from BigInt to plain numbers; new test cases for fractional amount rejection and unsafe-integer handling; expanded coverage for pending balance scenarios.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐰 Hop along, dear ledgers bright,
Numbers dance in safe-integer light,
Pending debits, credits too,
Reservations keeping track anew,
BigInts fade to numbers plain,
Validation loops to ease our pain!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 29.41% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: defining new ledger schema tables (accounts, journalEntries, ledgerCursors, reservations, sequenceCounter) and related updates.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ENG-24
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sourcery-ai
Copy link
Copy Markdown

sourcery-ai Bot commented Mar 16, 2026

Reviewer's Guide

Refactors the ledger domain to use schema-compliant numeric amounts and timestamps while introducing reservation-ready schema (accounts pending fields, reservations table, sequence counter) and wiring journal/validation logic to accept new reservation entry types without changing existing lifecycle behavior.

ER diagram for updated ledger tables with reservations and sequence counter

erDiagram
  ledger_accounts {
    string _id
    string mortgageId
    string lenderId
    int64 cumulativeDebits
    int64 cumulativeCredits
    int64 pendingDebits
    int64 pendingCredits
    number createdAt
    json metadata
  }

  ledger_journal_entries {
    string _id
    string entryType
    string reservationId
    string mortgageId
    number timestamp
    number amount
    string effectiveDate
    string debitAccountId
    string creditAccountId
    string idempotencyKey
    json source
    string causedBy
    string reason
    json metadata
  }

  ledger_cursors {
    string _id
    string consumerId
    int64 lastProcessedSequence
    number lastProcessedAt
  }

  ledger_reservations {
    string _id
    string mortgageId
    string sellerAccountId
    string buyerAccountId
    number amount
    string status
    string dealId
    string reserveJournalEntryId
    string commitJournalEntryId
    string voidJournalEntryId
    number createdAt
    number resolvedAt
  }

  ledger_sequence_counter {
    string _id
    string name
    int64 value
  }

  ledger_accounts ||--o{ ledger_journal_entries : debitAccount
  ledger_accounts ||--o{ ledger_journal_entries : creditAccount

  ledger_accounts ||--o{ ledger_reservations : sellerAccount
  ledger_accounts ||--o{ ledger_reservations : buyerAccount

  ledger_journal_entries ||--o{ ledger_reservations : reserveEntry
  ledger_journal_entries ||--o{ ledger_reservations : commitEntry
  ledger_journal_entries ||--o{ ledger_reservations : voidEntry

  ledger_journal_entries ||--o{ ledger_cursors : consumption

  ledger_sequence_counter ||--o{ ledger_journal_entries : sequenceSource
Loading

Flow diagram for ledger reservation lifecycle and journal entry types

flowchart TD
  A[Create ledger_reservations row<br/>status pending<br/>reserveJournalEntryId set] --> B[Post journal entry<br/>entryType SHARES_RESERVED<br/>reservationId references reservation]

  B --> C{Reservation outcome}

  C --> D[Commit reservation<br/>update ledger_reservations<br/>status committed<br/>commitJournalEntryId set<br/>resolvedAt set]
  D --> E[Post journal entry<br/>entryType SHARES_COMMITTED<br/>reservationId references reservation]

  C --> F[Void reservation<br/>update ledger_reservations<br/>status voided<br/>voidJournalEntryId set<br/>resolvedAt set]
  F --> G[Post journal entry<br/>entryType SHARES_VOIDED<br/>reservationId references reservation]

  classDef pending fill:#fff3cd,stroke:#d39e00,color:#000
  classDef terminal fill:#d4edda,stroke:#155724,color:#000
  class A,B pending
  class D,E,F,G terminal
Loading

File-Level Changes

Change Details Files
Switch ledger amounts from bigint/int64 to number while preserving bigint-based balance arithmetic and validation logic.
  • Change PostEntryInput.amount, mutation validators, and public mutation args (issue/transfer/redeem) from bigint/int64 to number
  • Convert numeric input amounts to BigInt at mutation boundaries (e.g., amountBigInt = BigInt(args.amount)) for all cumulative balance and invariant checks
  • Update queries, history, and demo helpers to cast journal entry numeric amounts back to BigInt when computing balances
  • Migrate unit tests and demo seed data from bigint literals (e.g., 5_000n) to numeric literals (e.g., 5000)
convex/ledger/mutations.ts
convex/ledger/validators.ts
convex/ledger/queries.ts
convex/demo/ledger.ts
src/routes/demo/convex-ledger.tsx
convex/ledger/__tests__/ledger.test.ts
Extend ledger schema with reservation-related structures and sequencing support, and add pending balance fields to accounts.
  • Add pendingDebits and pendingCredits int64 fields to ledger_accounts and populate them on all account creation paths
  • Introduce ledger_reservations table with status, dealId, and journal-entry linkage plus indices by mortgage, seller, and deal
  • Add ledger_sequence_counter table for a named ledger_sequence counter
  • Wire reservation-oriented fields into journal entries: add SHARES_RESERVED/SHARES_COMMITTED/SHARES_VOIDED entryTypes and an optional reservationId field
convex/schema.ts
convex/ledger/internal.ts
convex/ledger/mutations.ts
convex/auth/__tests__/resourceChecks.test.ts
Normalize timestamps and metadata types across ledger tables to align with the spec and future consumers.
  • Change createdAt/timestamp/lastProcessedAt fields in ledger tables and related queries from float64 to number
  • Relax metadata fields in journal entries and ledger mutations from record<string, any> to arbitrary JSON (v.any())
  • Update validators and query inputs to expect numeric timestamps instead of float64
convex/schema.ts
convex/ledger/validators.ts
convex/ledger/queries.ts
Update validation plumbing to work with numeric amounts and support new reservation entry types without enforcing business rules yet.
  • Extend ValidationContext with a pre-converted amountBigInt field used for all invariant checks instead of args.amount
  • Adjust all validators (mint, issue, transfer, redeem, burn, correction) to compare against amountBigInt and update error messages accordingly
  • Register SHARES_RESERVED, SHARES_COMMITTED, and SHARES_VOIDED in the VALIDATORS map using a noop validator to defer their business rules to future mutations
convex/ledger/mutations.ts
Align demo seed, cleanup, and UI helper logic with the new schema and amount types.
  • Generalize demo postSeedEntry to use the schema entryType union and numeric amount, converting to BigInt only when updating cumulative balances
  • Ensure all account creation in demo seed includes pendingDebits/pendingCredits and uses Number(UNITS_PER_MORTGAGE) when posting mint/burn entries
  • Update demo cleanup logic to sum remaining debits/credits using BigInt(entry.amount) after the journal amount switch to number
  • Change interactive UI parseIntegerAmount helper to return number rather than bigint
convex/demo/ledger.ts
src/routes/demo/convex-ledger.tsx

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Copy Markdown
Owner Author

@Connorbelez Connorbelez marked this pull request as ready for review March 16, 2026 23:05
Copilot AI review requested due to automatic review settings March 16, 2026 23:05
Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 5 issues, and left some high level feedback:

  • Now that amount is a number everywhere, consider enforcing that it is a safe positive integer (e.g., via Number.isSafeInteger or a shared validator) before converting to BigInt, to avoid subtle precision or NaN issues leaking into ledger arithmetic.
  • You’re converting bigint constants like UNITS_PER_MORTGAGE to number with Number(...); if these ever grow beyond Number.MAX_SAFE_INTEGER, this will silently lose precision, so it may be worth centralizing this conversion with an explicit range check or documenting an upper bound on allowed units.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Now that `amount` is a `number` everywhere, consider enforcing that it is a safe positive integer (e.g., via `Number.isSafeInteger` or a shared validator) before converting to `BigInt`, to avoid subtle precision or NaN issues leaking into ledger arithmetic.
- You’re converting bigint constants like `UNITS_PER_MORTGAGE` to `number` with `Number(...)`; if these ever grow beyond `Number.MAX_SAFE_INTEGER`, this will silently lose precision, so it may be worth centralizing this conversion with an explicit range check or documenting an upper bound on allowed units.

## Individual Comments

### Comment 1
<location path="convex/ledger/mutations.ts" line_range="80" />
<code_context>
 	});

 	// Update cumulative balances
+	const amountBigInt = BigInt(args.amount);
 	const debitAccount = await ctx.db.get(args.debitAccountId);
 	const creditAccount = await ctx.db.get(args.creditAccountId);
</code_context>
<issue_to_address>
**issue (bug_risk):** Consider validating `args.amount` as a safe integer before converting to `BigInt` to avoid runtime errors and precision issues.

Because `amount` is a `number`, `BigInt(args.amount)` will throw for non-integers (e.g., `1.5`, `NaN`, `Infinity`) or values outside the safe integer range. Given this is a core ledger boundary, it’s better to enforce a finite, safe integer and return a clear error than to let these surface as uncaught runtime exceptions.
</issue_to_address>

### Comment 2
<location path="src/routes/demo/convex-ledger.tsx" line_range="32" />
<code_context>
 const POSITIVE_INTEGER_RE = /^\d+$/;

-function parseIntegerAmount(value: string): bigint {
+function parseIntegerAmount(value: string): number {
 	const trimmed = value.trim();
 	if (!POSITIVE_INTEGER_RE.test(trimmed)) {
</code_context>
<issue_to_address>
**issue (bug_risk):** `parseIntegerAmount` should guard against values outside the safe integer range when returning `number`.

Previously this returned `bigint`, so large integers were represented exactly. With `number`, an input like `"9007199254740993000"` will parse but lose precision via `Number(trimmed)`. Since this is used for ledger amounts, please validate that the result is within `Number.MIN_SAFE_INTEGER``Number.MAX_SAFE_INTEGER` and reject values outside that range to avoid silent precision errors.
</issue_to_address>

### Comment 3
<location path="convex/schema.ts" line_range="873" />
<code_context>
 		debitAccountId: v.id("ledger_accounts"),
 		creditAccountId: v.id("ledger_accounts"),
-		amount: v.int64(),
+		amount: v.number(),
 		idempotencyKey: v.string(),
 		causedBy: v.optional(v.id("ledger_journal_entries")),
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider constraining or documenting the expected numeric shape of `amount` now that it’s a plain `number`.

This change weakens validation: `v.number()` will accept floats, `NaN`, and `Infinity`, but the rest of the code assumes whole-number arithmetic and converts `amount` to `bigint`. Consider adding a stricter validation layer (e.g., in higher-level mutation validators) that enforces finite integer amounts within a safe range so the data model matches how `amount` is used.

Suggested implementation:

```typescript
		timestamp: v.number(),
		debitAccountId: v.id("ledger_accounts"),
		creditAccountId: v.id("ledger_accounts"),
		// NOTE: Amount is expected to be a finite integer in the smallest currency unit
		// (e.g. cents), suitable for conversion to bigint. Higher-level mutations should
		// enforce: Number.isFinite(amount), Number.isInteger(amount), and a safe range.
		amount: v.number(),
		idempotencyKey: v.string(),

```

To fully implement the stricter validation you described, you should also:

1. In the mutation(s) that create ledger journal entries (where `amount` is received from the client), add runtime validation before writing to the table, for example:
   - `Number.isFinite(amount)`
   - `Number.isInteger(amount)`
   - `Math.abs(amount) <= Number.MAX_SAFE_INTEGER` (or a domain-specific max)
   - Reject or throw an error if these conditions fail.

2. If you have a shared validator/util layer (e.g. `validateLedgerAmount(amount: number)`), implement it there and call it from all mutations that accept or transform `amount`, so the invariant is enforced consistently.

3. If any existing code path already writes floats or very large numbers into `amount`, update or guard those paths to comply with the new invariant before deploying this change.
</issue_to_address>

### Comment 4
<location path="convex/ledger/__tests__/ledger.test.ts" line_range="308" />
<code_context>
-		await mintAndIssue(t, "m1", "seller", 5_000n);
+		await mintAndIssue(t, "m1", "seller", 5_000);

 		await expect(
 			auth.mutation(api.ledger.mutations.transferShares, {
 				mortgageId: "m1",
</code_context>
<issue_to_address>
**suggestion (testing):** Extend amount validation tests to cover non-integer and large numeric values now that `amount` is a `number`.

Now that `amount` is a `number` and converted with `BigInt(args.amount)`, values like `0.5` or numbers beyond `Number.MAX_SAFE_INTEGER`/`MIN_SAFE_INTEGER` will fail differently than before.

Please add tests that:
- Invoke a representative mutation (e.g. `postEntry` via `issueShares`) with a fractional amount (e.g. `0.5`) and assert a clear, deterministic validation error rather than an unhandled `RangeError` from `BigInt()`.
- Use an amount just above `Number.MAX_SAFE_INTEGER` and assert either rejection with a clear message or whatever explicit range policy you choose.

Grouping these next to the existing "zero amount" and rejection tests will keep all amount-validation behavior in one place.

Suggested implementation:

```typescript
	it("T-047: transferShares rejects insufficient seller balance", async () => {
		const t = createTestHarness();
		const auth = asLedgerUser(t);
		await mintAndIssue(t, "m1", "seller", 5_000);


				lenderId: "lender-a",
				amount: 10_000,
				effectiveDate: "2026-01-01",
				idempotencyKey: "issue-a",
				source: SYS_SOURCE,
				mortgageId: "m1",
				sellerLenderId: "lender-a",
				buyerLenderId: "lender-b",
	});

	it("T-048: transferShares rejects fractional amount", async () => {
		const t = createTestHarness();
		const auth = asLedgerUser(t);

		// Give the seller some balance so we only exercise amount validation,
		// not insufficient-balance handling.
		await mintAndIssue(t, "m-fractional", "seller", 10_000);

		await expect(
			auth.mutation(api.ledger.mutations.transferShares, {
				mortgageId: "m-fractional",
				sellerLenderId: "seller",
				buyerLenderId: "buyer",
				amount: 0.5,
				effectiveDate: "2026-01-03",
				idempotencyKey: "fractional-amount",
				source: SYS_SOURCE,
			}),
		).rejects.toThrow(/amount.*integer/i);
	});

	it("T-049: transferShares rejects amount above Number.MAX_SAFE_INTEGER", async () => {
		const t = createTestHarness();
		const auth = asLedgerUser(t);

		await mintAndIssue(t, "m-max-safe", "seller", 10_000);

		const tooLarge = Number.MAX_SAFE_INTEGER + 1;

		await expect(
			auth.mutation(api.ledger.mutations.transferShares, {
				mortgageId: "m-max-safe",
				sellerLenderId: "seller",
				buyerLenderId: "buyer",
				amount: tooLarge,
				effectiveDate: "2026-01-04",
				idempotencyKey: "too-large-amount",
				source: SYS_SOURCE,
			}),
		).rejects.toThrow(/amount.*safe integer/i);

```

Because I only see a fragment of `ledger.test.ts`, you’ll likely need to adjust a few details to fit the surrounding code:

1. **Placement of the new tests**  
   - Ensure the closing `});` I added for `T-047` is consistent with the actual end of that test.
   - If `T-047` already has its own closing brace, instead insert `T-048` and `T-049` immediately after that test block (still within the same `describe` group that holds the amount-related tests).

2. **Arguments for `transferShares`**  
   - Confirm the exact parameter names for `transferShares` (e.g. `sellerLenderId`, `buyerLenderId`, `mortgageId`, `effectiveDate`, `idempotencyKey`, `source`) and adjust them if your API differs.
   - If your existing tests use specific lender IDs (e.g. `"lender-a"` / `"lender-b"` instead of `"seller"` / `"buyer"`), align the new tests to those conventions.

3. **Error message expectations**  
   - The new tests currently assert on regexes:
     - Fractional amount: `/amount.*integer/i`
     - Too-large amount: `/amount.*safe integer/i`
   - Update the regex to match your chosen validation messages (for example, `/amount must be an integer/` or `/amount must be a safe integer/`).
   - In the implementation that converts `amount` with `BigInt(args.amount)`, add explicit guards such as:
     - `Number.isInteger(args.amount)` to reject fractional values with a deterministic error.
     - `Number.isSafeInteger(args.amount)` or a custom range check to reject values beyond the safe integer range.
   - Ensure the implementation throws your own domain error (or a typed error) rather than allowing a raw `RangeError` from `BigInt` to surface.

4. **Optional: use `issueShares` / `postEntry` instead**  
   - If you prefer to exercise `postEntry` via `issueShares`, mirror the same two tests but using `api.ledger.mutations.issueShares` and the argument shape already used elsewhere in this file.
   - Place them next to existing `issueShares` tests that cover "zero amount" or similar validation behavior.

Once these adjustments are made, the test suite will explicitly cover:
- Fractional numeric `amount` values.
- Numeric `amount` values above `Number.MAX_SAFE_INTEGER`.
And assert clear, deterministic validation failures instead of relying on `BigInt` runtime errors.
</issue_to_address>

### Comment 5
<location path="convex/auth/__tests__/resourceChecks.test.ts" line_range="171-172" />
<code_context>
 		lenderId,
 		cumulativeDebits: balance,
 		cumulativeCredits: 0n,
+		pendingDebits: 0n,
+		pendingCredits: 0n,
 		createdAt: NOW,
 	});
</code_context>
<issue_to_address>
**suggestion (testing):** Update auth resource-check tests to include the new ledger tables and pending fields where applicable.

This helper now seeds `pendingDebits`/`pendingCredits`, and the schema adds `ledger_reservations` and `ledger_sequence_counter`. Please:

- Add/extend auth/resource tests to cover read/write access to `ledger_reservations` and `ledger_sequence_counter` for both unauthorized and authorized users.
- Update any field-exposure tests for ledger accounts to include `pendingDebits` and `pendingCredits`, so access-control regressions on these new fields are caught by tests.

Suggested implementation:

```typescript
		lenderId,
		cumulativeDebits: balance,
		cumulativeCredits: 0n,
		pendingDebits: 0n,
		pendingCredits: 0n,
		createdAt: NOW,
	});
}

// -----------------------------------------------------------------------------
// Ledger auth/resource tests for new tables & pending fields
// -----------------------------------------------------------------------------

// NOTE: These tests should mirror the existing ledger_accounts auth checks.
// If your test utilities or query/mutation names differ, adjust accordingly.

describe("auth/resource checks for ledger tables", () => {
	// Replace these with your actual Convex test client helpers / auth contexts.
	// For example, you might have `unauthenticated`, `asLender`, `asAdmin`, etc.
	const unauthenticatedClient = makeTestClient(); // placeholder
	const authorizedClient = makeTestClient({ role: "lender" }); // placeholder

	describe("ledger_reservations", () => {
		it("denies unauthenticated read access to ledger_reservations", async () => {
			await expect(
				unauthenticatedClient.query("ledger_reservations:list", {}),
			).rejects.toThrow();
		});

		it("denies unauthenticated write access to ledger_reservations", async () => {
			await expect(
				unauthenticatedClient.mutation("ledger_reservations:create", {
					lenderId: "lender_1",
					accountId: "account_1",
					amount: 1n,
				}),
			).rejects.toThrow();
		});

		it("allows authorized read access to ledger_reservations", async () => {
			const result = await authorizedClient.query(
				"ledger_reservations:list",
				{},
			);

			// Basic shape check; refine to match your schema if needed.
			expect(Array.isArray(result)).toBe(true);
		});

		it("allows authorized write access to ledger_reservations", async () => {
			const reservation = await authorizedClient.mutation(
				"ledger_reservations:create",
				{
					lenderId: "lender_1",
					accountId: "account_1",
					amount: 1n,
				},
			);

			expect(reservation).toMatchObject({
				lenderId: "lender_1",
				accountId: "account_1",
				amount: 1n,
			});
		});
	});

	describe("ledger_sequence_counter", () => {
		it("denies unauthenticated read access to ledger_sequence_counter", async () => {
			await expect(
				unauthenticatedClient.query("ledger_sequence_counter:get", {}),
			).rejects.toThrow();
		});

		it("denies unauthenticated write access to ledger_sequence_counter", async () => {
			await expect(
				unauthenticatedClient.mutation(
					"ledger_sequence_counter:increment",
					{},
				),
			).rejects.toThrow();
		});

		it("allows authorized read access to ledger_sequence_counter", async () => {
			const counter = await authorizedClient.query(
				"ledger_sequence_counter:get",
				{},
			);

			// Check that a sequence number is exposed but not writeable directly.
			expect(counter).toEqual(
				expect.objectContaining({
					sequence: expect.any(Number),
				}),
			);
		});

		it("allows authorized increment of ledger_sequence_counter", async () => {
			const before = await authorizedClient.query(
				"ledger_sequence_counter:get",
				{},
			);

			const after = await authorizedClient.mutation(
				"ledger_sequence_counter:increment",
				{},
			);

			expect(after.sequence).toBe(before.sequence + 1);
		});
	});
});

// -----------------------------------------------------------------------------
// Field-exposure tests for ledger_accounts including pendingDebits/Credits
// -----------------------------------------------------------------------------

describe("ledger_accounts field exposure", () => {
	const authorizedClient = makeTestClient({ role: "lender" }); // placeholder

	it("exposes pendingDebits and pendingCredits for authorized ledger account reads", async () => {
		// Seed or fetch an account via the same helper that uses NOW and balance.
		const account = await authorizedClient.query("ledger_accounts:getOne", {
			lenderId: "lender_1",
		});

		expect(account).toEqual(
			expect.objectContaining({
				lenderId: "lender_1",
				cumulativeDebits: expect.any(BigInt),
				cumulativeCredits: expect.any(BigInt),
				pendingDebits: expect.any(BigInt),
				pendingCredits: expect.any(BigInt),
			}),
		);
	});

	it("does not expose ledger account fields to unauthorized callers", async () => {
		const unauthenticatedClient = makeTestClient(); // placeholder

		await expect(
			unauthenticatedClient.query("ledger_accounts:getOne", {
				lenderId: "lender_1",
			}),
		).rejects.toThrow();
	});
});

```

The above edits assume several things that you will need to align with your actual test harness and Convex functions:

1. Replace `makeTestClient`, `ledger_reservations:list`, `ledger_reservations:create`, `ledger_sequence_counter:get`, `ledger_sequence_counter:increment`, and `ledger_accounts:getOne` with the actual helper(s) and query/mutation names used in your codebase.
2. If you already have `describe` blocks for ledger auth/resource checks (for example, for `ledger_accounts`), instead of introducing new top-level `describe`s, nest the new `ledger_reservations` and `ledger_sequence_counter` tests alongside the existing ones to keep structure consistent.
3. If you already maintain centralized lists/arrays of readable/writable tables per role (e.g. `READABLE_TABLES_FOR_LENDER`, `WRITABLE_TABLES_FOR_LENDER`), also update those lists to include `ledger_reservations` and `ledger_sequence_counter`, and extend any generic tests that iterate those lists to assert access.
4. If you have existing “field exposure” snapshot tests or shared helpers for asserting which fields are visible on `ledger_accounts`, update those snapshots/helpers to include `pendingDebits` and `pendingCredits` rather than adding the new `describe("ledger_accounts field exposure", ...)` block verbatim.
5. Ensure that any type assertions (e.g. using `BigInt`) match how your test framework and Convex client serialize BigInt values (string vs native `bigint`), and adjust the expectations accordingly (`expect.any(String)` vs `expect.any(BigInt)`).
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread convex/ledger/mutations.ts
Comment thread src/routes/demo/convex-ledger.tsx
Comment thread convex/schema.ts
Comment thread convex/ledger/__tests__/ledger.test.ts
Comment thread convex/auth/__tests__/resourceChecks.test.ts
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Mar 16, 2026

Greptile Summary

This PR introduces the two-phase reservation pattern for the ledger by adding ledger_reservations and ledger_sequence_counter tables, augmenting ledger_accounts with pendingDebits/pendingCredits, expanding the entryType union with SHARES_RESERVED/COMMITTED/VOIDED, and migrating all public-facing amount fields from v.int64() (bigint) to v.number().

Key issues found:

  • Unhandled RangeError on non-integer amounts (convex/ledger/mutations.ts): v.number() accepts floats, but BigInt(1000.5) throws a cryptic RangeError at runtime. A Number.isInteger() guard is missing before every BigInt(args.amount) call (also in convex/demo/ledger.ts).
  • noopValidator exposes reservation entry types without any validation (convex/ledger/mutations.ts): SHARES_RESERVED/COMMITTED/VOIDED are wired to a no-op in the VALIDATORS map, meaning callers of the public postEntry mutation can post these entry types against arbitrary accounts with no balance checks and without updating the new pendingDebits/pendingCredits fields, potentially silently corrupting ledger state.
  • ledger_sequence_counter is defined but unused, and has no unique index (convex/schema.ts): nextSequenceNumber still derives the sequence from ledger_journal_entries, leaving the new table orphaned and susceptible to duplicate rows when it is eventually wired up.
  • reservationId uses v.string() instead of v.id("ledger_reservations") (convex/schema.ts): the reference loses type safety and Convex's referential integrity even though the target table is added in this same PR.

Confidence Score: 2/5

  • Not safe to merge as-is — reservation entry types can be posted through the public API without any validation, which can corrupt pending-balance fields.
  • Two logic issues require attention before merging: (1) the missing integer guard before BigInt() conversions will produce opaque runtime errors for any non-integer input, and (2) the noopValidator approach silently accepts SHARES_RESERVED/COMMITTED/VOIDED through the existing public postEntry endpoint without updating pendingDebits/pendingCredits, creating a data-integrity gap that existing tests do not cover. The schema additions themselves are well-structured, and the bigint→number migration is consistently applied.
  • convex/ledger/mutations.ts (noop validator + missing integer check) and convex/schema.ts (reservationId type, ledger_sequence_counter missing unique index)

Important Files Changed

Filename Overview
convex/schema.ts Adds pendingDebits/pendingCredits to ledger_accounts, three new reservation entry types to ledger_journal_entries, reservationId (typed as plain string instead of id), ledger_reservations table, and ledger_sequence_counter table. The counter table lacks a unique index and reservationId loses type safety.
convex/ledger/mutations.ts Changes amount from bigint to number throughout, adds BigInt() conversions for arithmetic, and introduces noopValidator for reservation entry types. Two critical issues: no integer guard before BigInt(args.amount) (float inputs cause unhandled RangeError), and noopValidator exposes reservation entry types through the public postEntry with zero business-rule validation.
convex/ledger/validators.ts Migrates all amount fields from v.int64() to v.number(), adds three new entry type literals, and relaxes metadata from v.record(v.string(), v.any()) to v.any(). Changes are straightforward and consistent with the PR intent.
convex/ledger/queries.ts Adds BigInt() wrappers around entry.amount in point-in-time balance calculations and changes timestamp inputs from v.float64() to v.number(). Logic is correct; the returned bigint balance values are consistent with the pre-existing query return types.
convex/ledger/internal.ts Adds pendingDebits: 0n and pendingCredits: 0n to all account creation paths (WORLD, POSITION). Changes are mechanical and correct.
convex/demo/ledger.ts Migrates demo amounts from bigint literals to plain numbers, adds pendingDebits/pendingCredits to account insertions, and uses BigInt() for cumulative balance arithmetic. Minor: the unguarded BigInt(args.amount) in postSeedEntry shares the float-input risk noted in mutations.ts.
convex/ledger/tests/ledger.test.ts Mechanical replacement of bigint literals (10_000n) with plain numbers (10_000) to match updated validator signatures. No logic changes.
convex/auth/tests/resourceChecks.test.ts Adds pendingDebits: 0n and pendingCredits: 0n to the insertLedgerPosition test helper to satisfy the updated schema. No logic changes.
convex/seed/seedHelpers.ts Minor formatting cleanup only — inlines a multi-line index query into a single line. No functional changes.
src/routes/demo/convex-ledger.tsx Updates parseIntegerAmount to return number instead of bigint (BigInt(trimmed)Number(trimmed)). Correct given the validator change; the regex guard ensures only digit strings reach Number() so no precision concern at these UI-input scales.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant postEntry (public)
    participant postEntryInternal
    participant VALIDATORS
    participant DB

    Caller->>postEntry (public): entryType: "SHARES_RESERVED", amount: number
    postEntry (public)->>postEntryInternal: args
    postEntryInternal->>postEntryInternal: amount <= 0 check
    postEntryInternal->>postEntryInternal: BigInt(args.amount) ⚠️ RangeError if float
    postEntryInternal->>VALIDATORS: validateEntryType()
    VALIDATORS->>VALIDATORS: noopValidator() → no-op ⚠️
    VALIDATORS-->>postEntryInternal: (no error)
    postEntryInternal->>DB: insert ledger_journal_entries
    postEntryInternal->>DB: patch cumulativeDebits / cumulativeCredits
    note over postEntryInternal,DB: pendingDebits/pendingCredits NOT updated ⚠️
    postEntryInternal-->>Caller: journal entry

    note over Caller,DB: Future path (not yet implemented)
    Caller->>reserveShares (future): amount, seller, buyer
    reserveShares (future)->>DB: insert ledger_reservations {status: pending}
    reserveShares (future)->>DB: patch pendingDebits + pendingCredits
    reserveShares (future)->>DB: insert SHARES_RESERVED journal entry
Loading

Last reviewed commit: 126e083

Copy link
Copy Markdown
Owner Author

@coderabbitai please review

Comment thread convex/ledger/mutations.ts
Comment thread convex/ledger/mutations.ts Outdated
Comment thread convex/schema.ts Outdated
Comment thread convex/schema.ts Outdated
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 16, 2026

@Connorbelez Sure! I'll kick off a review of PR #83 right away.


🧠 Learnings used
Learnt from: Connorbelez
Repo: Connorbelez/tanstackTemplate PR: 5
File: specs/governed-transitions/chunks/chunk-01-schema-machine/context.md:209-213
Timestamp: 2026-03-13T03:17:24.805Z
Learning: In the Connorbelez/tanstackTemplate repository, do not flag .md files for markdown formatting/linting issues (e.g., MD040 fenced code block language). Only reference .md files (specs, PRDs, design docs) when they reveal architectural drift or implementation inconsistencies against actual code.

Learnt from: CR
Repo: Connorbelez/tanstackTemplate PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-16T16:49:07.648Z
Learning: After completing a major unit of work, run `coderabbit review --plain` to get a code review summary and check for potential issues or improvements

Learnt from: CR
Repo: Connorbelez/tanstackTemplate PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-16T06:17:10.192Z
Learning: After completing a major unit of work, run `coderabbit review --plain` to get a code review summary and check for potential issues or improvements

Learnt from: CR
Repo: Connorbelez/tanstackTemplate PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T16:48:37.308Z
Learning: After completing a major unit of work, run `coderabbit review --plain` to get a code review summary
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Defines and updates the mortgage ownership ledger schema and related Convex ledger code to support reservation workflows and numeric (JS number) amounts per the spec.

Changes:

  • Adds ledger_reservations and ledger_sequence_counter tables, and extends ledger_journal_entries.entryType with reservation-related events.
  • Migrates journal entry amount (and some timestamps/metadata) from int64/float64 to number, updating validators, queries, mutations, demo code, and tests accordingly.
  • Adds pendingDebits / pendingCredits fields to ledger_accounts for a two-phase reservation pattern.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
src/routes/demo/convex-ledger.tsx Updates demo UI amount parsing to return number for ledger mutations.
convex/seed/seedHelpers.ts Minor formatting change to an indexed query.
convex/schema.ts Updates ledger tables (new pending fields, reservation types/fields, new reservations & sequence counter tables, number timestamps/metadata).
convex/ledger/validators.ts Updates entry type union and amount/metadata validators to number/any.
convex/ledger/queries.ts Updates point-in-time/history query inputs to number timestamps; converts entry amounts to BigInt for balance math.
convex/ledger/mutations.ts Switches mutation inputs/entry inserts to number amounts while keeping bigint balance arithmetic via conversion.
convex/ledger/internal.ts Ensures newly-created ledger accounts populate pending fields.
convex/ledger/tests/ledger.test.ts Updates ledger tests to use numeric amounts and validate new schema behavior.
convex/demo/ledger.ts Updates demo seeding and demo journal/state queries to use numeric amounts and new account fields.
convex/auth/tests/resourceChecks.test.ts Updates ledger position test helper inserts to include pending fields.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread convex/schema.ts Outdated
Comment thread src/routes/demo/convex-ledger.tsx Outdated
Comment thread convex/ledger/mutations.ts
Comment thread convex/ledger/mutations.ts Outdated
Comment thread convex/ledger/queries.ts
Comment thread convex/schema.ts Outdated
Connorbelez added a commit that referenced this pull request Mar 16, 2026
- Document amount field as finite integer in smallest currency unit
- Change reservationId from v.string() to v.id("ledger_reservations")
- Add .index("by_name", ["name"]) to ledger_sequence_counter

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Comment thread convex/ledger/mutations.ts Outdated
Connorbelez and others added 4 commits March 16, 2026 19:30
…dgerCursors, reservations, sequenceCounter)

Add ledger_reservations and ledger_sequence_counter tables, add
pendingDebits/pendingCredits fields to ledger_accounts for the
two-phase reservation pattern, expand entryType union with 3
reservation types (SHARES_RESERVED/COMMITTED/VOIDED), add
reservationId field to journal entries, and change journal entry
amount from v.int64() to v.number() per spec.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…types via postEntry

Address PR review feedback:
- Add Number.isFinite, Number.isInteger, Number.isSafeInteger checks before
  BigInt() conversion to prevent RangeError on non-integers, NaN, Infinity,
  and unsafe values (in both mutations.ts and demo/ledger.ts).
- Replace noopValidator for SHARES_RESERVED, SHARES_COMMITTED, SHARES_VOIDED
  with validators that throw clear errors directing callers to use dedicated
  reserveShares/commitReservation/voidReservation mutations.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Document amount field as finite integer in smallest currency unit
- Change reservationId from v.string() to v.id("ledger_reservations")
- Add .index("by_name", ["name"]) to ledger_sequence_counter

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ixes, tests

- Add Number.isFinite/isInteger/isSafeInteger guards before all BigInt()
  conversions in mutations, queries, and demo code
- Replace noopValidator for reservation entry types with explicit rejection
  directing callers to dedicated mutations
- Change reservationId to v.id("ledger_reservations") for type safety
- Add unique index on ledger_sequence_counter.name
- Document amount field as finite integer in smallest currency unit
- Fix parseIntegerAmount to enforce positive integers and safe range
- Add defensive safeBigIntAmount helper in queries
- Add tests for fractional and MAX_SAFE_INTEGER overflow amounts
- Add auth resource-check tests for new ledger tables and pending fields
- Fix fluent.ts typecheck error (remove unnecessary runMutation constraint)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Connorbelez Connorbelez merged commit 8fed55a into main Mar 16, 2026
0 of 2 checks passed
@coderabbitai coderabbitai Bot mentioned this pull request Mar 17, 2026
Connorbelez added a commit that referenced this pull request Apr 20, 2026
- Document amount field as finite integer in smallest currency unit
- Change reservationId from v.string() to v.id("ledger_reservations")
- Add .index("by_name", ["name"]) to ledger_sequence_counter

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Connorbelez added a commit that referenced this pull request Apr 20, 2026
…ixes, tests

- Add Number.isFinite/isInteger/isSafeInteger guards before all BigInt()
  conversions in mutations, queries, and demo code
- Replace noopValidator for reservation entry types with explicit rejection
  directing callers to dedicated mutations
- Change reservationId to v.id("ledger_reservations") for type safety
- Add unique index on ledger_sequence_counter.name
- Document amount field as finite integer in smallest currency unit
- Fix parseIntegerAmount to enforce positive integers and safe range
- Add defensive safeBigIntAmount helper in queries
- Add tests for fractional and MAX_SAFE_INTEGER overflow amounts
- Add auth resource-check tests for new ledger tables and pending fields
- Fix fluent.ts typecheck error (remove unnecessary runMutation constraint)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Connorbelez added a commit that referenced this pull request Apr 20, 2026
…dgerCursors, reservations, sequenceCounter) (#83)

Add ledger_reservations and ledger_sequence_counter tables, add
pendingDebits/pendingCredits fields to ledger_accounts for the
two-phase reservation pattern, expand entryType union with 3
reservation types (SHARES_RESERVED/COMMITTED/VOIDED), add
reservationId field to journal entries, and change journal entry
amount from v.int64() to v.number() per spec.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

## Summary by Sourcery

Introduce ledger reservation schema and normalize ledger amount/timestamp types while keeping existing lifecycle behavior intact.

New Features:
- Add ledger_reservations table with status tracking, deal linkage, and journal entry references to support two-phase reserve/commit/void flows.
- Add ledger_sequence_counter table for centralized ledger sequencing metadata.

Enhancements:
- Extend ledger_accounts with pendingDebits and pendingCredits fields to track reserved balances alongside cumulative debits/credits.
- Expand ledger_journal_entries entryType union with reservation-related types and an optional reservationId, and relax metadata typing for greater flexibility.
- Standardize monetary amounts and timestamps on number types across ledger APIs, schema, validators, queries, and demo code while retaining BigInt for balance arithmetic internally.
- Initialize new pending balance fields across world, treasury, and position account creation paths, including seed/demo helpers.
- Register reservation entry types in validation wiring with placeholder validators ahead of dedicated reservation mutations.

Tests:
- Update ledger tests and demo utilities to use numeric amounts instead of bigint literals, keeping existing behavioral coverage intact.

<!-- This is an auto-generated comment: release notes by coderabbit.ai -->

## Summary by CodeRabbit

* **New Features**
  * Added support for share reservations with status tracking (pending, committed, voided).
  * Introduced new reservation management system to track mortgage share deals and transactions.

* **Chores**
  * Standardized numeric type handling throughout ledger operations for consistency.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants