* fix(filter): reject unknown operator dicts on scalar columns
An unrecognized operator dict on a non-JSONB column (e.g.
{"session_id": {"operator": "null"}}) fell through to `column == value`,
binding a dict to a VARCHAR parameter. That compiles, then fails in the
driver at execute time with "cannot adapt type 'dict'" — an unhandled
500 for what is invalid input.
Raise FilterError (422) instead. The guard lives in the shared
_build_field_condition, so every route through apply_filter is covered.
It keys on the actual column type rather than the JSONB_COLUMNS name
list, so dict equality still works on JSONB columns reachable through
Document's raw-key fallback (e.g. source_ids), where the driver adapts
dicts fine.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* chore(filter): name psycopg explicitly in comment
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(filter): handle null operands and non-numeric columns in comparisons
Two defects in _build_comparison_conditions, both reachable from any
route that accepts filters:
1. A null operand hit float(None), raising TypeError where only
ValueError was caught — an unhandled 500. A null operand is a null
check, not a value comparison, so {"ne": null} now compiles to
IS NOT NULL and the other operators reject null with a 422. Equality
against null already produced IS NULL via _build_field_condition.
2. Numeric operators float()-cast on every column type, so a string
inequality on a text column ({"session_id": {"ne": "abc"}}) was
rejected as an invalid number. Coercion is now gated on the column
actually being numeric; text columns compare as text. Numeric columns
still validate, and TypeError is caught alongside ValueError.
Existing ne coverage only exercised the JSONB metadata path, which uses
_safe_numeric_cast and handles strings — the scalar column path was
untested. Adds cases for both.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(filter): fail closed on unrecognized filter shapes
The filter body is arbitrary client JSON with no schema, so validation
was emergent: any shape the DSL didn't recognize surfaced as an
unhandled 500 from somewhere in SQLAlchemy or psycopg. Fixing individual
shapes doesn't converge — a fuzz over the DSL found five more families
beyond the three already fixed here:
{"AND": [None]} TypeError, non-dict in a logical list
{"AND": [[]]} AttributeError on .items()
{"session_id": {"gte": true}} SQLAlchemy ArgumentError
{"embedding": []} NotImplementedError, no python_type
{"session_id": {"ne": {...}}} execute-time "cannot adapt type 'dict'"
Two generic guards instead:
1. Any operand bound to a non-JSONB column must be a scalar, checked
element-wise for `in`. A dict or list bound to a scalar column
compiles cleanly and only fails in the driver at execute time, so it
has to be rejected during construction. JSONB columns are exempt —
a dict there is a containment match.
2. apply_filter fails closed: FilterError propagates, anything else is
logged with logger.exception (filter shape included) and re-raised as
FilterError. Unknown filter failures become 422s while staying fully
visible as errors rather than being swallowed.
Adds two invariant tests over a generated matrix of filter shapes: every
shape either compiles or raises FilterError, and no non-scalar is ever
bound to a scalar column. Both fail without the guards above. They cover
shapes nobody enumerated, so the next unimagined body fails in CI rather
than in production.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* chore: clear the two remaining basedpyright warnings
`uv run basedpyright tests/ src/` reported two warnings in files that
predate this change. The pre-commit hook is file-scoped, so neither was
visible unless the owning file was touched.
- src/vector_store/__init__.py: join the lancedb error message with
explicit `+` instead of adjacent literals (reportImplicitStringConcatenation).
- tests/test_cache_redaction.py: the test covers a private helper
deliberately, so annotate the import (reportPrivateUsage).
No behavior change; whole-tree check is now clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(filter): keep numeric operands exact instead of coercing to float
float() rounds any integer past 2**53 and flattens a Decimal, so
{"token_count": {"gt": 9007199254740993}} silently compared against
9007199254740992 — a different row set than the client asked for.
_coerce_numeric passes already-numeric operands through untouched and
only parses strings, trying int() before float() so "5" stays exact
while "5.5" still parses. bool narrows to int: it is an int subclass,
but binding it as a boolean against a numeric column produces SQL
Postgres has no operator for.
Not coerced to the column's own type: int(5.5) would turn
{"token_count": {"lt": 5.5}} into `lt 5`, changing which rows match.
Also fixes a vacuous assertion in test_dict_on_jsonb_column_still_works.
It checked for "internal_metadata" in the whole statement, but that name
is in the SELECT projection either way, so the test passed even when no
WHERE clause was applied. Now asserts on stmt.whereclause and that the
filter payload is actually bound.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(filter): bind boolean columns as booleans, numerics without a cast
Boolean columns were treated as numeric because bool subclasses int, so
`{"is_active": {"ne": true}}` coerced true to 1 and Postgres rejected
`boolean <> integer` at execute time. Confirmed against a live database:
every form except bare equality with a native boolean was a 500.
Boolean columns now get their own branch: native true/false bind as
booleans, and any other operand is a FilterError. SQLAlchemy types the
bind from the operand rather than the column, so "true" renders
`is_active = %(param)s::VARCHAR` and Postgres has no such operator — a
422 is the honest answer. String booleans have never worked, are absent
from the docs (every documented boolean is inside metadata, which is
JSONB containment and unaffected), and produced no Sentry events in 90
days, so nothing can depend on the current behavior.
Also corrects the previous commit. Coercing operands to exact ints made
SQLAlchemy render an ::INTEGER cast, so any value past int4 — not 2**53
— started failing with "integer out of range" where float() had silently
compared as a double. Decimal keeps the value exact and renders no cast,
matching what float() did. The `in` branch never went through coercion
at all, so {"token_count": {"in": [1, 2147483648]}} was a 500 before
this PR too; it now takes the same path.
Verified end to end against the live database, not just at compile time.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(filter): coerce every operand against its column's type in one place
The DSL had two operand paths with different rules. Comparison operators
parsed datetimes and coerced numbers; bare equality bound whatever it was
handed. SQLAlchemy types a bind from the operand rather than the column
and the psycopg dialect renders that type as an explicit cast, so a
mismatch compiled into valid-looking SQL and failed at execute time:
"operator does not exist: timestamp with time zone = character varying".
A matrix of column type x operand type x operator against a live
database found 462 combinations, of which 55 built cleanly and then
failed. The most plausible was a filter someone would write first try:
{"created_at": "2026-01-01"} 500
{"created_at": {"gte": "2026-01-01"}} worked
_coerce_operand now handles every operand, whatever the operator, keyed
on the column's real type: JSONB takes an object, boolean takes only
true/false, datetime parses strings, numeric goes through _coerce_numeric,
text requires a string, and a column with no python_type (pgvector) is
not filterable. eq/ne/gt/in cannot drift apart because they share the
one call; `in` coerces element-wise, since a single element's type
decides the cast rendered for that parameter. The matrix is now clean.
This is a net deletion: the separate datetime, numeric, in-datetime and
boolean branches, plus _require_bindable_operand, all collapse into it.
Two more execute-time failures fixed on the way. `contains` was keyed on
column_name == "h_metadata", so Document's equally-JSONB
internal_metadata fell through to ILIKE and produced `jsonb ~~* text`;
it now keys on the column type. And {"source_ids": "abc"} was
`jsonb = character varying`.
Closed-set columns are validated against the Literal that defines them,
so declaring a new level or sync state updates filter validation with no
change here. {"level": "banana"} was silently matching nothing.
Empty IN is now always applied rather than skipped. Unifying the branches
inherited a guard that had only ever wrapped the datetime path, which
dropped the condition entirely and widened the query to every row —
fail-open on an empty allowlist, which session scoping relies on to fail
closed (see extract_session_allowlist). Caught by an existing test that
asserts returned rows; the fuzz and the type matrix only check for
errors, so neither would have seen it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(filter): make NOT and ne include rows where the field is unset
NOT (col = v) and col <> v are NULL when col is NULL, so negation
dropped rows whose column is unset — a conclusion with no session is
not "some other session", but was excluded anyway. IS NOT TRUE and
IS DISTINCT FROM behave identically when no NULL is involved.
Adds a row-count test, since this class of bug builds and executes
cleanly, and documents {"ne": null} for excluding unset fields.
* fix(filter): reject session ids that can't name a real session
extract_session_allowlist accepted any non-empty string, so "*" reached
the three consumers of the allowlist — direct IN, the filter DSL, and a
Python membership test — which disagree about it. The DSL reads "*" as
"drop the condition" and matches every session; the others treat it as a
literal name and match none. One /chat request could have some recall
sources unscoped and others scoped to nothing.
Entries are now validated against RESOURCE_NAME_PATTERN, the same pattern
the API requires of session ids, so no session could ever be named "*"
anyway. Wildcards were never part of this endpoint's documented contract
(an id, a list of ids, or {"in": [...]}), and a wildcard alongside a
top-level session_id already 422'd via must_include.
* fix(filter): treat a bare null operand as a null check
Routing every operand through _coerce_operand made bare `None` a type
error rather than a null check, so {"session_id": null} raised FilterError
where it previously built IS NULL: _build_field_condition used to end in
`column == value`, which SQLAlchemy renders as IS NULL. Confirmed 422 on
all five column families (text, numeric, boolean, datetime, JSONB).
Nothing caught it. The docs added in this branch promise
`{"session_id": None}` matches unset rows, the comment in
_build_comparison_conditions claimed the equality path already covered it,
and the DSL-wide invariant test accepts "compiles OR raises FilterError",
so a 422 passed. _coerce_operand's docstring already stated the contract
its caller wasn't honoring — "Callers handle None (a null check) and `*`
(a wildcard) before calling" — so the guard restores that rather than
adding a new rule.
The three null forms now agree: {"col": null} is IS NULL, {"col": {"ne":
null}} is IS NOT NULL, NOT [{"col": null}] is (IS NULL) IS NOT true.
Also from review of #947:
- Log filter keys, not the body. That log line is new in this branch and
operands carry peer/session ids and free-text `contains` values; the
traceback plus the entry shape is what locates a builder bug.
- Assert whereclause in the _where test helper, so a dropped condition
fails instead of returning the whole statement to substring-match.
- Cover the raw-key JSONB path via source_ids, a JSONB column outside
JSONB_COLUMNS reachable through Document's raw-key fallback.
- Drop the orphaned comment left above ENUM_COLUMN_VALUES when
_coerce_operand replaced SCALAR_OPERAND_TYPES.
- Document that a JSONB column takes an object bare or under `contains`
and nothing else. Bare {"metadata": X} is containment, so the `ne` this
branch removed was never its inverse: a row with {"status":"done","x":1}
satisfied both it and {"metadata": {"status":"done"}}. Per-key operators
and NOT cover the real intents.
- Rewrite "Negation and Unset Fields" to lead with the operator rule and a
truth table, forward-linking to Filtering Conclusions instead of using
conclusions ~525 lines before they are introduced. A conclusion's
session_id is the only nullable documented filterable field, verified
across Message/Document/Session/Peer/Workspace.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
c9f836c7f6