fix(postgres): scrub every byte class Postgres refuses, at every bind site (#411)

[jphein]

What

backends/postgres.py now scrubs both byte classes Postgres refuses, at all four bind sites, over nested metadata and ids — not just top-level NULs in documents.

Why

backends/pgvector.py has stripped NUL (upstream MemPalace#1829) and lone UTF-16 surrogates (upstream MemPalace#1833) before binding since those landed, because one stray byte anywhere in a mined corpus aborts the whole batch. postgres.py only ever grew the NUL half (#417), and only over documents and top-level metadata values. Three ways a single transcript still took down a mine survived:

1. Lone surrogates were never handled at all. psycopg cannot UTF-8-encode the parameter, and json.dumps escapes it to a \udXXX sequence that the ::jsonb cast rejects as an unsupported Unicode escape.
2. Nested metadata was never walked. _replace_nul_bytes looked at top-level str values only, so a NUL one level down inside a list, a sub-dict, or a dict key serialized to an escape and aborted the batch exactly as an unscrubbed top-level one would.
3. ids were never scrubbed, though they bind into a text column as the ON CONFLICT key.

The measured incident

The issue was filed off a carried local patch: mining arbitrary conversation transcripts reliably encounters both byte classes, and on the postgres backend a single occurrence raises and takes down the whole batch — so one bad byte blocks ingestion until the offending source is found by hand, where the pgvector backend would have absorbed it. The 21 new tests reproduce every surviving route in-memory; each was watched failing against main before the fix went in (the get/delete pair was re-verified red by reverting just that call).

How

_replace_nul_bytes is widened into _scrub_unstorable(documents=, ids=, metadatas=):

• fix(postgres): replace NUL bytes at the write boundary instead of crashing the mine #417's contract is kept exactly — U+FFFD substitution plus a counted nul_bytes_replaced in the drawer's metadata, so the substitution stays provenanced and never silent — and the symmetric lone_surrogates_replaced is added. Counts describe the document, which is the verbatim payload; metadata is scrubbed too but is bookkeeping, not the user's words.
• Both substitutions are one character wide, so the scrub is length-preserving and the surrogate count falls out of a zip-diff rather than a second regex.
• It runs at all four bind sites, not only add/upsert: update() serializes metadata into its own ::jsonb cast, and get()/delete() bind ids into text comparisons. Scrubbing writes alone would have filed a drawer under an id its own caller could no longer look up — a new unreachable-drawer state, so the read seams are part of the fix rather than scope creep.

On "reuse the helper so the two backends can't drift again"

The issue asks for a shared helper. pgvector.py is byte-identical to upstream/develop (git diff upstream/develop -- mempalace/backends/pgvector.py is empty) and postgres.py is fork-only — so importing a shared helper into pgvector would trade this drift risk for a guaranteed conflict on every future upstream sync of a file we currently carry clean.

The drift is caught instead by a parity test driven by pgvector's own sanitizers: for eight hostile strings, _strip_nul and strip_lone_surrogates must both find nothing left to do after our scrub. That fails if either backend grows a class the other lacks, in either direction, and costs no upstream divergence. The substitution policies differ on purpose (pgvector deletes the byte; this fork replaces and counts it, per #417), so the compared invariant is storability, not equality.

Tests

tests/test_postgres_unstorable_bytes.py — 21 cases:

• lone surrogate in a document, replaced and counted
• lone surrogate in a metadata value
• valid astral surrogate pairs left intact (emoji must not be mangled)
• ids scrubbed for both classes
• nested metadata: list items, sub-dict values, and dict keys
• non-string JSON scalars pass through with types intact
• the fix(postgres): replace NUL bytes at the write boundary instead of crashing the mine #417 NUL contract preserved verbatim
• clean input gains no provenance keys
• the four bind sites actually call it (add, update, get, delete, asserted on the bound parameters through a fake connection)
• 8-case pgvector parity sweep

tests/test_postgres_nul_bytes.py updated to the new helper name; #417's two cases kept as-is.

Full suite: 6815 passed, 82 skipped, 115 deselected. (5 pre-existing test_init_filters_sys_path_from_leaked_pythonpath failures reproduce on the unmodified branch in any git worktree — filed separately, unrelated to this change.)

ruff check and ruff format --check clean; scripts/check-docs.sh green after all three renderers.

Part of #411

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[coderabbitai]

Warning

Review limit reached

Next included review available in 57 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 3b4fb79d-a207-4ade-9ea5-589660e0a15e

📥 Commits

Reviewing files that changed from the base of the PR and between 1a61cec and 251c3bd.

📒 Files selected for processing (8)

• CLAUDE.md
• FORK_CHANGELOG.md
• README.md
• docs/fork-changes.yaml
• mempalace/backends/postgres.py
• tests/test_postgres_nul_bytes.py
• tests/test_postgres_unstorable_bytes.py
• website/public/llms-full.txt

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}