Skip to content

fix: accept string IDs at the Rust/TS boundary for BYODB BigInt#477

Merged
rpkruse merged 1 commit intomainfrom
ts-rust-id-binding-fix
May 6, 2026
Merged

fix: accept string IDs at the Rust/TS boundary for BYODB BigInt#477
rpkruse merged 1 commit intomainfrom
ts-rust-id-binding-fix

Conversation

@rpkruse
Copy link
Copy Markdown
Contributor

@rpkruse rpkruse commented May 6, 2026

Problem

CockroachDB uses BIGINT primary keys that exceed JavaScript's Number.MAX_SAFE_INTEGER
(e.g. 1172888091194032000). Our PostgreSQL/CockroachDB storage drivers were recently fixed
to return these IDs as strings instead of casting them to Number, which silently lost
precision.

However, the Rust N-API boundary hadn't been updated to match. When the TypeScript SDK passed
a stringified ID back through the retrieve path, serde tried to deserialize the JSON string
"1172888091194032000" into a strict i64 field and panicked:

[Error: invalid type: string "1", expected i64]

Solution

Widen the ID types at every layer of the stack — Rust structs, TypeScript interfaces, and the
index.d.ts declaration — so the engine acts as a universal pass-through that accepts both
integer IDs (SQLite, MySQL) and string IDs (PostgreSQL, CockroachDB).

Rust (core/bindings/node/src/)

types.rsNapiRecallObject and NapiRecallSummary:

  • id: i64id: Either<i64, String>, consistent with the existing pattern already used by
    NapiEmbeddingRow and NapiCandidateFactRow
  • entity_fact_id / fact_id: Option<i64>Option<Either<i64, String>>
  • Dropped #[derive(Serialize, Deserialize)] from both structs since they no longer go through
    a serde round-trip

engine.rsretrieve method:

  • Replaced the lossy serde_json::to_value(&results)serde_json::from_value(...) round-trip
    with a direct into_iter().map(...) that explicitly matches FactId::Int(n) → Either::A(n)
    and FactId::String(s) → Either::B(s)
  • Added a fact_id_from_json helper to safely extract Option<Either<i64, String>> from the
    opaque serde_json::Value summary blobs
  • Added RankedFact to the storage import

TypeScript (memori-ts/src/)

types/api.tsRecallObject, NapiRecallRow, RecallSummary:

  • id, entity_fact_id, fact_id, entityFactId, factId widened from number to
    number | string

core/engine.ts:

  • Updated stale as number casts on s.entityFactId / s.factId to as number | string

native/index.d.ts (auto-generated, updated ahead of next napi build):

  • NapiRecallObject.id and NapiRecallSummary.entityFactId / factId widened to
    number | string

utils/utils.ts:

  • Map<number, RecallSummary[]>Map<number | string, RecallSummary[]> in the cloud-path
    summary grouping function

Compatibility

  • SQLite / MySQL — integer IDs still flow through as Either::A(i64) / number. No
    behavior change.
  • PostgreSQL / CockroachDB — string IDs now flow through as Either::B(String) / string
    without truncation or panic.
  • No public API surface change; id accepting number | string is a non-breaking widening for
    all callers.

@rpkruse rpkruse requested a review from devwdave May 6, 2026 17:06
@rpkruse rpkruse merged commit b2121ba into main May 6, 2026
47 of 48 checks passed
@rpkruse rpkruse deleted the ts-rust-id-binding-fix branch May 6, 2026 17:39
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