GeezerVison is showing github.com — fix(filter): gate numeric JSON casts on the stored value's type by deshmukh-viraj · Pull Request #1932 · MemMachine/MemMachine
Conversation
What does this PR do?
A numeric comparison over a raw JSON metadata key, such as m.size > 3, cast the extracted text with CAST(... AS INTEGER) on PostgreSQL. When any episode in the project held a non-numeric value for that key (for example "large"), the CAST failed for the whole query and the search answered 500. On SQLite the same filter answered, since SQLite casts non-numeric text to 0.
compile_sql_filter now accepts an optional dialect name. With it, a numeric JSON comparison casts the extracted text per backend: on PostgreSQL the cast runs inside a CASE guarded by a numeric regex pattern, so only text that parses reaches the cast; on SQLite the extracted text is cast as NUMERIC (SQLite casts never raise). JSON numbers and numeric strings both match, unparseable text (e.g. "large") answers no match instead of the 500, and the query always answers.
Numeric literals also match numeric strings, not just JSON numbers: the REST API stores every metadata value as a string (MemoryMessage.metadata: dict[str, str]), and the API's own documented example filter metadata.user_id=123 (Examples.FILTER_MEM) must still find a stored "123" on PostgreSQL. Non-numeric casts (string equality, boolean) are unchanged.
Related Issue
Fixes #1917
Type of Change
- Bug fix (non-breaking change that fixes an issue)
Changes Made
packages/server/src/memmachine_server/common/filter/sql_filter_util.py: optional dialect_name parameter on compile_sql_filter (threaded through the tree); numeric JSON comparisons in _compile_json_leaf (both Comparison and In) cast through _numeric_json_clause, which matches JSON numbers and numeric strings per dialectpackages/server/src/memmachine_server/common/episode_store/episode_sqlalchemy_store.py: passes its engine's dialect namepackages/server/src/memmachine_server/semantic_memory/storage/sqlalchemy_pgvector_semantic.py: passes "postgresql" at both filter sitespackages/server/server_tests/memmachine_server/common/filter/test_sql_filter_util.py: mixed-type fixture (text, int, int, numeric text) and a string-only fixture shaped like REST-written metadata (user_id "123" / "45" / "abc", the documented example), queried through both the sqlite and pg engine fixtures, plus compile-time assertions that the PostgreSQL cast is CASE-guarded and that unknown dialects stay unguarded
The segment store and the SQLite vector stores keep their current behavior: the segment store's property filter uses the type-tagged properties_json encoding, which already gates per leaf, and the SQLite vector stores use properties_json too.
How to Test
uv run pytest packages/server/server_tests/memmachine_server/common/filter/test_sql_filter_util.py -v - 60 passed locally on SQLite; the mixed-type and string-only fixtures also run against the PostgreSQL testcontainer in the integration lane (not runnable in this environment, no docker)uv run pytest packages/server/server_tests -q --ignore=packages/server/server_tests/memmachine_server/common/vector_store/vector_search_engine/test_hnswlib_engine.py - 2072 passed, 16 skipped, excluding a pre-existing hnswlib collection gap (ModuleNotFoundError: No module named 'hnswlib') that reproduces on unmodified mainuv run ruff check and uv run ruff format --check on touched files - clean
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
OpenAI and Cohere clients were built without a timeout, so the SDK
defaults applied (ten minutes for OpenAI). One slow or hung provider
held searches and stalled semantic extraction for every project.
Add a request_timeout_seconds field to the OpenAI responses, OpenAI
chat completions, and Cohere reranker configs (defaults 120s and 30s)
and pass it to the client constructors.
A numeric comparison over a raw JSON metadata key, such as m.size > 3,
cast the extracted text on PostgreSQL. When any episode in the project
held a non-numeric value for that key, the CAST failed and the whole
query answered 500. SQLite casts non-numeric text to 0, so the same
filter answered there.
compile_sql_filter now takes an optional dialect name. With it, numeric
(and only numeric) JSON comparisons are gated on the backend's JSON
type: jsonb_typeof = 'number' on PostgreSQL and json_type IN
('integer', 'real') on SQLite. Rows whose stored value is not a number
no longer match a numeric comparison, both backends agree, and the
query answers instead of erroring. Numeric strings such as "5" stored
as JSON text no longer match numeric comparisons, which is the
behavior the issue's suggested gate implies.
The episode store passes its engine's dialect; the pgvector semantic
store passes postgresql. Callers without a known dialect keep the
ungated behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this gate, the filter example the API documents finds nothing in the PostgreSQL episode store, which /memories/list reads, for metadata written through the REST API. Examples.FILTER_MEM is metadata.user_id=123 AND metadata.session_id=abc (doc.py#L625), and SpecDoc.FILTER_MEM gives 'metadata.user_id=123' (#L228). _parse_value makes the unquoted 123 an int (filter_parser.py#L320-L321), while the REST API stores every metadata value as a string (MemoryMessage.metadata: dict[str, str], spec.py#L483). _json_numeric_gate adds jsonb_typeof(...) = 'number' for an int literal. A stored "123" fails it, so metadata.user_id=123 no longer finds an episode whose user_id is "123"; today the cast to integer finds it.
This is the question #1917 left open, "whether a numeric string compares as a number is part of the fix", answered no, applied to the usage the docs show. Read from the source at 377510d and this PR's head (75df7fa); not run.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
The stored-type gate excluded the REST API's string metadata from
numeric comparisons, so the documented example metadata.user_id=123
found nothing for REST-written data. Numeric literals now match JSON
numbers and numeric strings alike: PostgreSQL casts inside a CASE
guarded by a numeric pattern so no unparseable text reaches the cast,
and SQLite casts the extracted text as NUMERIC. Unparseable text is
excluded instead of erroring, as MemMachine#1917 expects.
|
Agreed — the stored-type gate broke the documented usage. Both references check out: Examples.FILTER_MEM documents metadata.user_id=123 while the REST API persists metadata as dict[str, str], so every REST-written value is a JSON string and jsonb_typeof(...) = 'number' excluded all of it. #1917's own repro also counts a stored "5" matching m.size > 3 as baseline behavior, so excluding numeric strings was the wrong call.
Pushed eb2fefb: numeric literals now compare against both JSON numbers and numeric strings.
Unparseable text (e.g. "large") still answers no match instead of a 500, and string/bool comparisons are unchanged.
Tests: added a string-only metadata fixture shaped like REST writes (user_id "123"/"45"/"abc") covering m.user_id = 123, m.user_id > 100, and exclusion without error; the mixed-type expectations now include the numeric-string row (m.size > 3 matches "3.5"); compile assertions updated to the CASE guard. 60 passed on SQLite locally; the PostgreSQL testcontainer lane still can't run in this environment (no docker), so the PostgreSQL path is read-verified against the compiled SQL, like your review.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This branch has not been deployed
No deployments
Reviewers
At least 2 approving reviews are required to merge this pull request.
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
|