feat(memory): add new adapter - #36
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR introduces ChangesIn-Memory Database Adapter Implementation
CLI Integration for Memory Adapter
Documentation and Configuration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
packages/memory/src/repositories.ts (1)
222-236: 💤 Low valueConsider removing redundant
deletedfield.Line 233 sets
deleted: truein addition to settingdeletedAttimestamp. ThedeletedAtfield already indicates deletion (non-null means deleted). The booleandeletedfield appears redundant unless required by an external API contract.If
deletedis not consumed elsewhere, consider removing it to simplify the data model.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/memory/src/repositories.ts` around lines 222 - 236, The markDeleted implementation sets both deletedAt and a redundant boolean deleted flag; update the function markDeleted to stop writing the deleted property (only set deletedAt to indicate deletion) by removing the deleted: true entry from the object written to options.getState().customers in packages/memory/src/repositories.ts, ensure version calculation (getVersion call) and raw persistence (getPersistableRaw) remain correct, and search for any downstream consumers expecting customer.deleted and update them to check deletedAt instead (or reintroduce the boolean only if an external contract requires it).packages/memory/src/shared/schema.ts (1)
103-109: ⚖️ Poor tradeoffConsider documenting mutation risk when cloning fails.
When
structuredClonefails and returns the original value, multiple records may share references to the same default value object. Subsequent mutations would affect all records that received that default.While this is documented behavior, consider whether the caller should be notified (via warning or different return type) when cloning fails, especially for
applyTableFieldDefaultswhere shared mutable defaults could cause subtle bugs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/memory/src/shared/schema.ts` around lines 103 - 109, The cloneValue function silently returns the original value when structuredClone fails, risking shared mutable defaults (notably in applyTableFieldDefaults); change cloneValue<T>(value: T) to accept an optional logger or warn flag and, on catch, emit a clear warning (via provided logger.warn or console.warn) that structuredClone failed and the original reference is being returned, including context (function name, field/table identifier if available) so callers (e.g., applyTableFieldDefaults) can surface the risk; update applyTableFieldDefaults to call cloneValue with the warning/logger enabled so failures are reported rather than silent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/memory/src/repositories.ts`:
- Around line 135-221: Add missing bidirectional pagination tests for the
customers.list method: create at least 5 customers in the memory repository,
call customers.list with limit=2 and no cursor to get page1, then use the
returned next cursor to fetch page2 and assert that page2 has both previous and
next non-null (middle page), continue to page3 and assert boundary behavior
(previous non-null, next null), then navigate backwards using the before cursor
to page2 and assert both previous and next non-null again; ensure you assert
null/nonnull for previous/next at the correct ends to cover forward and backward
pagination paths through the list implementation.
In `@packages/memory/src/shared/schema.ts`:
- Around line 82-95: The JSDoc and implementation disagree: update
hydrateTableFieldKeys to ensure all schema-defined keys are present by
unconditionally assigning next[field.key] = record[field.key] inside the loop
(so absent keys become undefined and existing null/values are preserved),
keeping the function name hydrateTableFieldKeys and table.fields references
intact; alternatively, if you prefer the current semantics, change the JSDoc to
state that the function only copies schema-defined field values when present.
In `@packages/memory/src/state.ts`:
- Around line 667-672: The product/price seed persistence only reads raw from
product.raw and thus misses provider raw payloads stored under
Symbol.for('paymesh.raw'); update the logic in the block that calls
state.products.set (and the analogous block at lines ~705-710) so that when
persistRaw is true you prefer product.raw if present otherwise extract the
symbol-based payload via product[Symbol.for('paymesh.raw')] (falling back to
null), and store that value as raw; ensure you reference the exact setter
invocation state.products.set and the product object when making this change.
- Around line 640-673: The insertSeedProduct and insertSeedPrice flows currently
skip schema-level required-field validation; update both functions to, when
strict is true, run applyTableFieldDefaults(schema, schema.tables.products) /
applyTableFieldDefaults(schema, schema.tables.prices) on the incoming
product/price object and then call validateRequiredTableFields(schema,
schema.tables.products.name, product) / validateRequiredTableFields(schema,
schema.tables.prices.name, price) before performing validateUniqueInsert and
persisting; ensure the object used for persistence includes the defaulted fields
(and still handle createdAt/updatedAt and raw/persistRaw as before).
---
Nitpick comments:
In `@packages/memory/src/repositories.ts`:
- Around line 222-236: The markDeleted implementation sets both deletedAt and a
redundant boolean deleted flag; update the function markDeleted to stop writing
the deleted property (only set deletedAt to indicate deletion) by removing the
deleted: true entry from the object written to options.getState().customers in
packages/memory/src/repositories.ts, ensure version calculation (getVersion
call) and raw persistence (getPersistableRaw) remain correct, and search for any
downstream consumers expecting customer.deleted and update them to check
deletedAt instead (or reintroduce the boolean only if an external contract
requires it).
In `@packages/memory/src/shared/schema.ts`:
- Around line 103-109: The cloneValue function silently returns the original
value when structuredClone fails, risking shared mutable defaults (notably in
applyTableFieldDefaults); change cloneValue<T>(value: T) to accept an optional
logger or warn flag and, on catch, emit a clear warning (via provided
logger.warn or console.warn) that structuredClone failed and the original
reference is being returned, including context (function name, field/table
identifier if available) so callers (e.g., applyTableFieldDefaults) can surface
the risk; update applyTableFieldDefaults to call cloneValue with the
warning/logger enabled so failures are reported rather than silent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 08153407-c8ce-4a57-ae60-6ea337c5143c
📒 Files selected for processing (30)
.changeset/tidy-bees-jam.mdapps/web/content/docs/database/memory.mdxapps/web/content/docs/database/overview.mdxapps/web/content/docs/guides/database.mdxapps/web/content/docs/installation.mdxapps/web/content/docs/introduction.mdxlefthook.ymlpackage.jsonpackages/cli/src/commands/generate.tspackages/cli/src/commands/migrate.tspackages/cli/src/commands/status.tspackages/cli/src/lib/database.tspackages/cli/src/lib/status.tspackages/cli/src/shared/database.tspackages/cli/src/shared/generators.tspackages/cli/test/cli.test.tspackages/memory/README.mdpackages/memory/package.jsonpackages/memory/src/index.tspackages/memory/src/repositories.tspackages/memory/src/shared/cursor.tspackages/memory/src/shared/driver.tspackages/memory/src/shared/raw.tspackages/memory/src/shared/schema.tspackages/memory/src/state.tspackages/memory/src/types.tspackages/memory/test/memory.test.tspackages/memory/tsconfig.jsonpackages/memory/tsdown.config.mjspackages/paymesh/README.md
| async list(_schema, provider, sandbox, listOptions) { | ||
| const limit = listOptions?.limit ?? 20; | ||
| if (!Number.isInteger(limit) || limit <= 0) { | ||
| throw new PaymeshError({ | ||
| code: 'invalid_request', | ||
| message: 'Customer list limit must be a positive integer', | ||
| }); | ||
| } | ||
| if (listOptions?.after && listOptions?.before) { | ||
| throw new PaymeshError({ | ||
| code: 'invalid_request', | ||
| message: | ||
| 'Customer list accepts either "after" or "before", not both', | ||
| }); | ||
| } | ||
|
|
||
| const cursor = decodeCustomerCursor( | ||
| listOptions?.before ?? listOptions?.after, | ||
| listOptions?.before ? 'before' : 'after', | ||
| ); | ||
| const filtered = [...options.getState().customers.values()] | ||
| .filter( | ||
| (customer) => | ||
| customer.provider === provider && | ||
| customer.sandbox === sandbox && | ||
| customer.deletedAt === null, | ||
| ) | ||
| .sort(compareCustomerRows); | ||
|
|
||
| let pageSource = filtered; | ||
| if (cursor?.mode === 'after') { | ||
| pageSource = filtered.filter( | ||
| (customer) => | ||
| compareCustomerRowToCursor(customer, cursor.value) > 0, | ||
| ); | ||
| } else if (cursor?.mode === 'before') { | ||
| pageSource = filtered | ||
| .filter( | ||
| (customer) => | ||
| compareCustomerRowToCursor(customer, cursor.value) < 0, | ||
| ) | ||
| .sort((left, right) => compareCustomerRows(right, left)); | ||
| } | ||
|
|
||
| const hasExtra = pageSource.length > limit; | ||
| const windowRows = hasExtra ? pageSource.slice(0, limit) : pageSource; | ||
| const pageRows = | ||
| cursor?.mode === 'before' ? [...windowRows].reverse() : windowRows; | ||
| const data = pageRows.map((customer) => | ||
| mapStoredCustomer(customer, listOptions), | ||
| ); | ||
|
|
||
| return { | ||
| data, | ||
| total: filtered.length, | ||
| previous: | ||
| data.length === 0 | ||
| ? null | ||
| : cursor?.mode === 'before' | ||
| ? hasExtra | ||
| ? encodeCustomerCursor({ | ||
| createdAt: pageRows[0]!.createdAt, | ||
| providerId: pageRows[0]!.id, | ||
| }) | ||
| : null | ||
| : cursor | ||
| ? encodeCustomerCursor({ | ||
| createdAt: pageRows[0]!.createdAt, | ||
| providerId: pageRows[0]!.id, | ||
| }) | ||
| : null, | ||
| next: | ||
| data.length === 0 | ||
| ? null | ||
| : cursor?.mode === 'before' | ||
| ? encodeCustomerCursor({ | ||
| createdAt: pageRows[pageRows.length - 1]!.createdAt, | ||
| providerId: pageRows[pageRows.length - 1]!.id, | ||
| }) | ||
| : hasExtra | ||
| ? encodeCustomerCursor({ | ||
| createdAt: pageRows[pageRows.length - 1]!.createdAt, | ||
| providerId: pageRows[pageRows.length - 1]!.id, | ||
| }) | ||
| : null, | ||
| }; | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for customer list pagination test cases
rg -n -C3 'customers\.list' --glob '*test*'Repository: usepaymesh/paymesh
Length of output: 4890
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the pagination tests we already found
sed -n '200,320p' packages/paymesh/test/database.test.ts
sed -n '480,620p' packages/paymesh/test/client.test.ts
sed -n '1,200p' packages/memory/test/memory.test.tsRepository: usepaymesh/paymesh
Length of output: 10621
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n 'customers\.list\(\{[^}]*before:' --glob '*test*' .
rg -n 'customers\.list\(\{[^}]*after:' --glob '*test*' .
rg -n '\.previous' --glob '*test*' packages/paymesh/test packages/memory/test
rg -n '\.next' --glob '*test*' packages/paymesh/test packages/memory/testRepository: usepaymesh/paymesh
Length of output: 44
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n 'repositories\.customers\.list' --glob '*test*' packages
rg -n 'customers\.list\(' --glob '*test*' packages/paymesh/test packages/memory/test
rg -n '\b(before|after)\b' --glob '*test*' packages/paymesh/test packages/memory/test
rg -n '\.previous\b|\.next\b' --glob '*test*' packages/paymesh/test packages/memory/testRepository: usepaymesh/paymesh
Length of output: 4230
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n 'repositories\.customers\.list' --glob '*test*' packages
rg -n 'customers\.list\(' --glob '*test*' packages/paymesh/test packages/memory/test
rg -n '\b(before|after)\b' --glob '*test*' packages/paymesh/test packages/memory/test
rg -n '\.previous\b|\.next\b' --glob '*test*' packages/paymesh/test packages/memory/testRepository: usepaymesh/paymesh
Length of output: 4230
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n '\bbefore\s*:' packages/paymesh/test/database.test.ts
rg -n '\bafter\s*:' packages/paymesh/test/database.test.ts
rg -n 'customers\.list\(' packages/paymesh/test/database.test.ts
# Show the cursor-related test we already saw and a bit more context
sed -n '200,320p' packages/paymesh/test/database.test.ts
# If there are other pagination tests using the same list logic, inspect nearby cursor stubs/fixtures
sed -n '1080,1210p' packages/paymesh/test/database.test.tsRepository: usepaymesh/paymesh
Length of output: 7055
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the exact implementation under review
sed -n '110,260p' packages/memory/src/repositories.ts
# Also check the cursor comparison helpers used by list() pagination
rg -n 'decodeCustomerCursor|encodeCustomerCursor|compareCustomerRowToCursor|compareCustomerRows' packages/memory/src/repositories.tsRepository: usepaymesh/paymesh
Length of output: 5371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect cursor helpers
sed -n '1,220p' packages/memory/src/shared/cursor.ts
# Inspect ordering + cursor comparisons
sed -n '520,700p' packages/memory/src/repositories.tsRepository: usepaymesh/paymesh
Length of output: 4141
Extend customers.list bidirectional cursor tests (before/after).
packages/memory/src/repositories.ts computes previous/next differently for after vs before, and while existing tests cover the “first → second (after)” and “second → first (before)” transitions, they don’t cover middle-page scenarios where both cursors should be present (and the opposite end should be null). Add tests that paginate through at least 3+ pages (e.g., 5 customers with limit=2) to cover:
- forward pagination to a middle page (both
previousandnextnon-null) - backward pagination from a later page to a middle page (both
previousandnextnon-null) - boundary pages for
beforemode to ensureprevious/nextarenullat the correct ends.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/memory/src/repositories.ts` around lines 135 - 221, Add missing
bidirectional pagination tests for the customers.list method: create at least 5
customers in the memory repository, call customers.list with limit=2 and no
cursor to get page1, then use the returned next cursor to fetch page2 and assert
that page2 has both previous and next non-null (middle page), continue to page3
and assert boundary behavior (previous non-null, next null), then navigate
backwards using the before cursor to page2 and assert both previous and next
non-null again; ensure you assert null/nonnull for previous/next at the correct
ends to cover forward and backward pagination paths through the list
implementation.
| export function hydrateTableFieldKeys<TRecord extends Record<string, unknown>>( | ||
| record: TRecord, | ||
| table: ResolvedDatabaseTable, | ||
| ) { | ||
| const next: Record<string, unknown> = { ...record }; | ||
|
|
||
| for (const field of Object.values(table.fields)) { | ||
| if (record[field.key] !== undefined && record[field.key] !== null) { | ||
| next[field.key] = record[field.key]; | ||
| } | ||
| } | ||
|
|
||
| return next; | ||
| } |
There was a problem hiding this comment.
Comment does not match implementation behavior.
The JSDoc comment on line 79 states "Ensures that all schema-defined keys are present in the returned object", but the implementation only copies keys that exist and are non-null/non-undefined in the input record. Keys that are absent or null in the input will not be present in the returned object.
If the intent is to ensure all schema keys are present (e.g., initialized to undefined), the loop should unconditionally set next[field.key]. If the current behavior is correct, update the comment to reflect that it "copies schema-defined field values when present."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/memory/src/shared/schema.ts` around lines 82 - 95, The JSDoc and
implementation disagree: update hydrateTableFieldKeys to ensure all
schema-defined keys are present by unconditionally assigning next[field.key] =
record[field.key] inside the loop (so absent keys become undefined and existing
null/values are preserved), keeping the function name hydrateTableFieldKeys and
table.fields references intact; alternatively, if you prefer the current
semantics, change the JSDoc to state that the function only copies
schema-defined field values when present.
| function insertSeedProduct( | ||
| state: MemoryDatabaseState, | ||
| schema: ResolvedDatabaseSchema, | ||
| product: MemorySeedProduct, | ||
| strict: boolean, | ||
| persistRaw: boolean, | ||
| ) { | ||
| validateRequiredString(product.id, 'id', schema.tables.products.name); | ||
| validateRequiredString( | ||
| product.provider, | ||
| 'provider', | ||
| schema.tables.products.name, | ||
| ); | ||
| validateRequiredBoolean( | ||
| product.sandbox, | ||
| 'sandbox', | ||
| schema.tables.products.name, | ||
| ); | ||
|
|
||
| const key = entityKey(product.provider, product.sandbox, product.id); | ||
| validateUniqueInsert( | ||
| strict, | ||
| state.products.has(key), | ||
| schema.tables.products.name, | ||
| product.id, | ||
| ); | ||
| const now = product.createdAt ?? new Date().toISOString(); | ||
| state.products.set(key, { | ||
| ...product, | ||
| createdAt: now, | ||
| updatedAt: product.updatedAt ?? now, | ||
| raw: persistRaw ? (product.raw ?? null) : null, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Strict-mode seed validation is incomplete for products/prices.
insertSeedProduct and insertSeedPrice skip schema-level required-field validation (applyTableFieldDefaults + validateRequiredTableFields) that other seeded entities use. In strict mode, this allows invalid catalog rows to be inserted and fail later in downstream flows.
Suggested fix
function insertSeedProduct(
state: MemoryDatabaseState,
schema: ResolvedDatabaseSchema,
product: MemorySeedProduct,
strict: boolean,
persistRaw: boolean,
) {
+ const next = applyTableFieldDefaults(
+ schema.tables.products,
+ product as unknown as Record<string, unknown>,
+ );
+ validateRequiredTableFields(schema, 'products', next);
+ const productRecord = next as Record<string, unknown>;
- validateRequiredString(product.id, 'id', schema.tables.products.name);
+ validateRequiredString(productRecord.id, 'id', schema.tables.products.name);
...
}
function insertSeedPrice(
state: MemoryDatabaseState,
schema: ResolvedDatabaseSchema,
price: MemorySeedPrice,
strict: boolean,
persistRaw: boolean,
) {
+ const next = applyTableFieldDefaults(
+ schema.tables.prices,
+ price as unknown as Record<string, unknown>,
+ );
+ validateRequiredTableFields(schema, 'prices', next);
+ const priceRecord = next as Record<string, unknown>;
- validateRequiredString(price.id, 'id', schema.tables.prices.name);
+ validateRequiredString(priceRecord.id, 'id', schema.tables.prices.name);
...
}Also applies to: 675-711
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/memory/src/state.ts` around lines 640 - 673, The insertSeedProduct
and insertSeedPrice flows currently skip schema-level required-field validation;
update both functions to, when strict is true, run
applyTableFieldDefaults(schema, schema.tables.products) /
applyTableFieldDefaults(schema, schema.tables.prices) on the incoming
product/price object and then call validateRequiredTableFields(schema,
schema.tables.products.name, product) / validateRequiredTableFields(schema,
schema.tables.prices.name, price) before performing validateUniqueInsert and
persisting; ensure the object used for persistence includes the defaulted fields
(and still handle createdAt/updatedAt and raw/persistRaw as before).
| state.products.set(key, { | ||
| ...product, | ||
| createdAt: now, | ||
| updatedAt: product.updatedAt ?? now, | ||
| raw: persistRaw ? (product.raw ?? null) : null, | ||
| }); |
There was a problem hiding this comment.
Raw payload persistence is inconsistent for catalog seeds.
For product/price seeds, raw is read only from record.raw, while other entities also extract hidden raw payloads via Symbol.for('paymesh.raw'). With persistRaw: true, this drops provider raw payloads for catalog records when they are attached via symbol-based metadata.
Suggested fix
- raw: persistRaw ? (product.raw ?? null) : null,
+ raw: persistRaw
+ ? ((product as Record<string, unknown>).raw ?? getInternalRaw(product) ?? null)
+ : null,
...
- raw: persistRaw ? (price.raw ?? null) : null,
+ raw: persistRaw
+ ? ((price as Record<string, unknown>).raw ?? getInternalRaw(price) ?? null)
+ : null,Also applies to: 705-710
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/memory/src/state.ts` around lines 667 - 672, The product/price seed
persistence only reads raw from product.raw and thus misses provider raw
payloads stored under Symbol.for('paymesh.raw'); update the logic in the block
that calls state.products.set (and the analogous block at lines ~705-710) so
that when persistRaw is true you prefer product.raw if present otherwise extract
the symbol-based payload via product[Symbol.for('paymesh.raw')] (falling back to
null), and store that value as raw; ensure you reference the exact setter
invocation state.products.set and the product object when making this change.
Summary by CodeRabbit
Release Notes
New Features
@paymesh/memory, an in-memory database adapter for ephemeral data persistenceDocumentation