diff --git a/docs/v3/documentation/features/advanced/using-filters.mdx b/docs/v3/documentation/features/advanced/using-filters.mdx index 7d6caacd..4a1eefec 100644 --- a/docs/v3/documentation/features/advanced/using-filters.mdx +++ b/docs/v3/documentation/features/advanced/using-filters.mdx @@ -192,6 +192,90 @@ sessions = honcho.sessions(filters={ ``` +### Negation and Unset Fields + +A field can be unset, and negation has to account for it. `NOT` and the `ne` +comparison operator both **include** rows where the field has no value at all: a +field with no value is not the value you are excluding, so excluding that value +keeps the row. + +Positive conditions work the other way around. An unset field matches nothing, +so equality and `contains` never return those rows. To select them, filter on +`null` directly: + +| Filter | Rows where the field is unset | +| --- | --- | +| `{"field": "x"}`, `{"field": {"contains": "x"}}` | Excluded | +| `{"NOT": [{"field": "x"}]}`, `{"field": {"ne": "x"}}` | Included | +| `{"field": null}` | Only these | +| `{"field": {"ne": null}}` | Excluded — the field must have some value | + +Of the filterable fields, only a conclusion's `session_id` can be unset: a +conclusion drawn across a whole workspace belongs to no single session. Every +other field is always populated, so none of this affects filters on them. See +[Filtering Conclusions](#filtering-conclusions) for what conclusions are. + + +```python Python +# Every conclusion except the ones in this session — including +# workspace-level conclusions, which belong to no session at all +conclusions = peer.conclusions.list(filters={ + "NOT": [ + {"session_id": "session-123"} + ] +}) + +# Equivalent +conclusions = peer.conclusions.list(filters={ + "session_id": {"ne": "session-123"} +}) +``` + +```typescript TypeScript +(async () => { + // Every conclusion except the ones in this session — including + // workspace-level conclusions, which belong to no session at all + const conclusions = await peer.conclusions.list({ + filters: { NOT: [{ session_id: "session-123" }] } + }); + + // Equivalent + const same = await peer.conclusions.list({ + filters: { session_id: { ne: "session-123" } } + }); +})(); +``` + + +To exclude a value **and** require the field to be set, combine the two with +`AND`: + + +```python Python +# Excludes workspace-level conclusions and `session-123` +conclusions = peer.conclusions.list(filters={ + "AND": [ + {"session_id": {"ne": "session-123"}}, + {"session_id": {"ne": None}} + ] +}) +``` + +```typescript TypeScript +(async () => { + // Excludes workspace-level conclusions and `session-123` + const conclusions = await peer.conclusions.list({ + filters: { + AND: [ + { session_id: { ne: "session-123" } }, + { session_id: { ne: null } } + ] + } + }); +})(); +``` + + ### Combining Logical Operators Create sophisticated queries by combining different logical operators: @@ -700,6 +784,57 @@ bob_explicit = peer.conclusions_of("bob").list(filters={"level": "explicit"}) ``` +## Value Types + +A filter value has to be usable against the field it targets. Honcho validates +this before running the query and returns a `422` with an explanation when it +doesn't hold, rather than failing mid-query or quietly returning nothing. + +| Field | Accepts | +| --- | --- | +| Text — `peer_id`, `session_id`, `id`, `content` | Strings | +| Numeric — `token_count` | Numbers, or numeric strings like `"5"`. Exact for integers of any size | +| Timestamps — `created_at` | ISO 8601 strings such as `"2026-01-01"` or `"2026-01-01T12:00:00Z"` | +| Boolean — `is_active` | `true` / `false` | +| `metadata` | An object, matched by containment — bare or under `contains` | +| Fields with fixed values — `level` | One of the documented values | +| Any field | `null`, which matches rows where the field is unset | + +Three consequences worth knowing: + +- **Booleans must be real booleans.** `{"is_active": True}` filters; the string + `{"is_active": "true"}` is rejected. +- **Fixed-value fields are checked.** `{"level": "explicit"}` filters; + `{"level": "typo"}` is rejected instead of returning an empty list, so a + misspelling doesn't look like "no results". +- **`metadata` takes only the two shapes above** — bare, or under `contains`. + Comparison operators don't apply to the object as a whole, so + `{"metadata": {"ne": {...}}}` is rejected. To compare *within* metadata, put + the operator on the key — `{"metadata": {"status": {"ne": "done"}}}`. To negate + a match, wrap the whole condition in `NOT`. See + [Metadata Filtering](#metadata-filtering). + +For every field other than `metadata`, the same rules apply however the value is +wrapped — bare, under an operator, or inside an `in` list — so +`{"level": "explicit"}`, `{"level": {"ne": "explicit"}}` and +`{"level": {"in": ["explicit"]}}` all validate identically. + +An empty `in` list matches nothing: + + +```python Python +# Returns no results — an empty allowlist excludes everything +messages = session.messages(filters={"peer_id": {"in": []}}) +``` + +```typescript TypeScript +(async () => { + // Returns no results — an empty allowlist excludes everything + const messages = await session.messages({ filters: { peer_id: { in: [] } } }); +})(); +``` + + ## Scoping Recall to Sessions The [chat endpoint](/v3/documentation/features/chat) and the representation @@ -756,6 +891,7 @@ dropped filter here would widen recall instead of narrowing it. |------|----------| | Any key other than `session_id` | `422` | | A shape other than a string, a list of strings, or `{"in": [...]}` | `422` | +| An entry that isn't a well-formed session id — wildcards included | `422` | | More than 1,000 sessions | `422` | | `session_id` set alongside `filters` | The `session_id` must appear in the allowlist, else `422` | | An empty allowlist (`[]`) | Valid, and recalls nothing | diff --git a/mcp/bun.lock b/mcp/bun.lock index 29d050c2..52b157a4 100644 --- a/mcp/bun.lock +++ b/mcp/bun.lock @@ -5,7 +5,7 @@ "": { "name": "honcho-mcp-proxy", "dependencies": { - "@honcho-ai/sdk": "^2.0.0", + "@honcho-ai/sdk": "^2.2.0", "@modelcontextprotocol/sdk": "^1.26.0", "agents": "^0.4.0", "nanoid": "^5.1.7", @@ -107,7 +107,7 @@ "@esbuild/win32-x64": ["@esbuild/win32-x64@0.25.4", "", { "os": "win32", "cpu": "x64" }, "sha512-nOT2vZNw6hJ+z43oP1SPea/G/6AbN6X+bGNhNuq8NtRHy4wsMhw765IKLNmnjek7GvjWBYQ8Q5VBoYTFg9y1UQ=="], - "@honcho-ai/sdk": ["@honcho-ai/sdk@2.0.1", "", { "dependencies": { "@types/node": "^24.0.1", "zod": "4.0.0" } }, "sha512-y/Wk49C0N1miI9BZTNWFIbzdUkMZfP4Do/EJ1q4lEIK+FAOKxQgces/zET3kPKV3zF9sOUl2pXrFb/XKYayeYw=="], + "@honcho-ai/sdk": ["@honcho-ai/sdk@2.2.0", "", { "dependencies": { "zod": "4.0.0" } }, "sha512-SyygN+BrpUB2fRjhwcYmT+tcEhHrKmbj9nOZLVUFY7M5YBswJ+mZb/CeLpNbRh+QQTU8F8JWY3lQat95c6nwmA=="], "@hono/node-server": ["@hono/node-server@1.19.9", "", { "peerDependencies": { "hono": "^4" } }, "sha512-vHL6w3ecZsky+8P5MD+eFfaGTyCeOHUIFYMGpQGbrBTSmNNoxv0if69rEZ5giu36weC5saFuznL411gRX7bJDw=="], @@ -177,8 +177,6 @@ "@types/lodash": ["@types/lodash@4.17.23", "", {}, "sha512-RDvF6wTulMPjrNdCoYRC8gNR880JNGT8uB+REUpC2Ns4pRqQJhGz90wh7rgdXDPpCczF3VGktDuFGVnz8zP7HA=="], - "@types/node": ["@types/node@24.1.0", "", { "dependencies": { "undici-types": "~7.8.0" } }, "sha512-ut5FthK5moxFKH2T1CUOC6ctR67rQRvvHdFLCD2Ql6KXmMuCrjsSsRI9UsLCm9M18BMwClv4pn327UvB7eeO1w=="], - "@vercel/oidc": ["@vercel/oidc@3.1.0", "", {}, "sha512-Fw28YZpRnA3cAHHDlkt7xQHiJ0fcL+NRcIqsocZQUSmbzeIKRpwttJjik5ZGanXP+vlA4SbTg+AbA3bP363l+w=="], "accepts": ["accepts@2.0.0", "", { "dependencies": { "mime-types": "^3.0.0", "negotiator": "^1.0.0" } }, "sha512-5cvg6CtKwfgdmVqY1WIiXKc3Q1bkRqGLi+2W/6ao+6Y7gu/RCwRuAhGEzh5B4KlszSuTLgZYuqFqo5bImjNKng=="], @@ -471,8 +469,6 @@ "undici": ["undici@7.12.0", "", {}, "sha512-GrKEsc3ughskmGA9jevVlIOPMiiAHJ4OFUtaAH+NhfTUSiZ1wMPIQqQvAJUrJspFXJt3EBWgpAeoHEDVT1IBug=="], - "undici-types": ["undici-types@7.8.0", "", {}, "sha512-9UJ2xGDvQ43tYyVMpuHlsgApydB8ZKfVYTsLDhXkFL/6gfkp+U8xTGdh8pMJv1SpZna0zxG1DwsKZsreLbXBxw=="], - "unenv": ["unenv@2.0.0-rc.17", "", { "dependencies": { "defu": "^6.1.4", "exsolve": "^1.0.4", "ohash": "^2.0.11", "pathe": "^2.0.3", "ufo": "^1.6.1" } }, "sha512-B06u0wXkEd+o5gOCMl/ZHl5cfpYbDZKAT+HWTL+Hws6jWu7dCiqBBXXXzMFcFVJb8D4ytAnYmxJA83uwOQRSsg=="], "unpipe": ["unpipe@1.0.0", "", {}, "sha512-pjy2bYhSsufwWlKwPc+l3cN7+wuJlK6uz0YdJEOlQDbl6jo/YlPi4mb8agUkVC8BF7V8NuzeyPNqRksA3hztKQ=="], diff --git a/mcp/package.json b/mcp/package.json index 2311020c..900df5cf 100644 --- a/mcp/package.json +++ b/mcp/package.json @@ -15,7 +15,7 @@ "deploy:staging": "wrangler deploy --env staging" }, "dependencies": { - "@honcho-ai/sdk": "^2.1.0", + "@honcho-ai/sdk": "^2.2.0", "@modelcontextprotocol/sdk": "^1.26.0", "agents": "^0.4.0", "nanoid": "^5.1.7", diff --git a/mcp/src/tools/conclusions.ts b/mcp/src/tools/conclusions.ts index 283666b7..33674275 100644 --- a/mcp/src/tools/conclusions.ts +++ b/mcp/src/tools/conclusions.ts @@ -73,19 +73,26 @@ export function register(server: McpServer, ctx: ToolContext) { .number() .optional() .describe("Max results to return."), + filters: z + .record(z.string(), z.unknown()) + .optional() + .describe( + 'Optional: filter criteria, e.g. {"level": ["deductive", "inductive"]} to only return conclusions derived during dreaming. Levels: explicit (extracted directly from messages), deductive, inductive, contradiction. See https://honcho.dev/docs/v3/documentation/features/advanced/using-filters', + ), }, }, - async ({ workspace_id, peer_id, query, target_peer_id, top_k }) => { + async ({ workspace_id, peer_id, query, target_peer_id, top_k, filters }) => { try { const peer = await ctx.clientFor(workspace_id).peer(peer_id); const scope = target_peer_id ? peer.conclusionsOf(target_peer_id) : peer.conclusions; - const conclusions = await scope.query(query, top_k); + const conclusions = await scope.query(query, top_k, undefined, filters); return textResult( conclusions.map((c) => ({ id: c.id, content: c.content, + level: c.level, observer_id: c.observerId, observed_id: c.observedId, session_id: c.sessionId, @@ -158,6 +165,7 @@ export function register(server: McpServer, ctx: ToolContext) { { description: [ "Delete a specific conclusion by ID.", + "Use query_conclusions or list_conclusions to find the ID first.", "Use this to remove incorrect or outdated knowledge.", ].join("\n"), inputSchema: { diff --git a/mcp/src/tools/workspace.ts b/mcp/src/tools/workspace.ts index 2d626989..6eaf95a6 100644 --- a/mcp/src/tools/workspace.ts +++ b/mcp/src/tools/workspace.ts @@ -1,10 +1,12 @@ import { z } from "zod"; -import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { + BadRequestError, HonchoError, + UnprocessableEntityError, type PageResponse, type WorkspaceResponse, } from "@honcho-ai/sdk"; +import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import type { ToolContext } from "../types.js"; import { resolveWorkspaceId } from "../config.js"; import { @@ -163,11 +165,13 @@ export function register(server: McpServer, ctx: ToolContext) { "search", { description: [ - "Semantic search across messages. Scope is determined by which optional params are provided:", + "Semantic search across messages and, when peer_id is given, that peer's saved conclusions.", + "Message scope is determined by which optional params are provided:", "- No scope params: search all messages in the workspace.", "- peer_id only: search messages authored by that peer across all sessions.", "- session_id only: search messages within that session.", - "Returns an array of matching messages with their content, peer, and session info.", + "Conclusions require peer_id (self-conclusions are searched; conclusion IDs are usable with delete_conclusion).", + "Returns {messages, conclusions}.", ].join("\n"), inputSchema: { workspace_id: workspaceIdSchema(ctx), @@ -180,22 +184,95 @@ export function register(server: McpServer, ctx: ToolContext) { .string() .optional() .describe("Optional: scope search to messages in this session."), + message_limit: z + .number() + .optional() + .describe("Optional: max message results (1-100, default 10)."), + message_filters: z + .record(z.string(), z.unknown()) + .optional() + .describe( + 'Optional: filters for the message search, e.g. {"created_at": {"gte": "2026-01-01"}}. See https://honcho.dev/docs/v3/documentation/features/advanced/using-filters', + ), + conclusion_top_k: z + .number() + .optional() + .describe("Optional: max conclusion results (default 10)."), + conclusion_filters: z + .record(z.string(), z.unknown()) + .optional() + .describe( + 'Optional: filters for the conclusion search, e.g. {"level": ["deductive", "inductive"]} to only return conclusions derived during dreaming. Levels: explicit (extracted directly from messages), deductive, inductive, contradiction. The session_id param does not scope conclusions; use {"session_id": ...} here for that.', + ), }, }, - async ({ workspace_id, query, peer_id, session_id }) => { + async ({ + workspace_id, + query, + peer_id, + session_id, + message_limit, + message_filters, + conclusion_top_k, + conclusion_filters, + }) => { try { const honcho = ctx.clientFor(workspace_id); - let messages; - if (session_id) { - const session = await honcho.session(session_id); - messages = await session.search(query); - } else if (peer_id) { - const peer = await honcho.peer(peer_id); - messages = await peer.search(query); - } else { - messages = await honcho.search(query); - } - return textResult(formatMessages(messages)); + const peer = peer_id ? await honcho.peer(peer_id) : null; + const messageOptions = { + filters: message_filters, + limit: message_limit, + }; + + const searchMessages = async () => { + if (session_id) { + const session = await honcho.session(session_id); + return session.search(query, messageOptions); + } + if (peer) { + return peer.search(query, messageOptions); + } + return honcho.search(query, messageOptions); + }; + + // Conclusion search needs an (observer, observed) pair, so it only + // runs when peer_id is given. + const searchConclusions = async () => { + if (!peer) { + return []; + } + try { + return await peer.conclusions.query( + query, + conclusion_top_k, + undefined, + conclusion_filters, + ); + } catch (e) { + if ( + conclusion_filters && + (e instanceof BadRequestError || + e instanceof UnprocessableEntityError) + ) { + throw e; + } + return []; + } + }; + + const [messages, conclusions] = await Promise.all([ + searchMessages(), + searchConclusions(), + ]); + return textResult({ + messages: formatMessages(messages), + conclusions: conclusions.map((c) => ({ + id: c.id, + content: c.content, + level: c.level, + created_at: c.createdAt, + })), + }); } catch (e) { return errorResult( `Search failed: ${e instanceof Error ? e.message : String(e)}`, diff --git a/src/utils/filter.py b/src/utils/filter.py index 8df3590c..1155d408 100644 --- a/src/utils/filter.py +++ b/src/utils/filter.py @@ -1,14 +1,19 @@ import datetime +import re from collections.abc import Callable, Sequence +from decimal import Decimal from logging import getLogger -from typing import Any, TypeVar +from typing import Any, TypeVar, get_args from typing import cast as typing_cast -from sqlalchemy import ColumnElement, Select, and_, case, cast, literal, not_, or_ +from sqlalchemy import ColumnElement, Select, and_, case, cast, literal, or_ +from sqlalchemy.dialects.postgresql import JSONB from sqlalchemy.types import Numeric from ..exceptions import FilterError +from ..schemas.api import RESOURCE_NAME_PATTERN from .formatting import ILIKE_ESCAPE_CHAR, escape_ilike_pattern, parse_datetime_iso +from .types import DocumentLevel, VectorSyncState logger = getLogger(__name__) @@ -63,6 +68,157 @@ ALLOWED_EXTERNAL_TO_INTERNAL_COLUMN_MAPPING_DOCUMENTS = { MAX_SESSION_ALLOWLIST_ENTRIES = 1000 +# Columns whose values come from a closed set. Derived from the Literal types +# themselves, so adding a level (e.g. "abduction") or a sync state updates +# filter validation with no change here — an unlisted value is a 422 rather +# than a filter that silently matches nothing. +ENUM_COLUMN_VALUES: dict[str, frozenset[str]] = { + "level": frozenset(get_args(DocumentLevel)), + "sync_state": frozenset(get_args(VectorSyncState)), +} + + +def _coerce_numeric(op_value: Any) -> float | Decimal: + """Validate a numeric operand without losing precision or overflowing. + + Integers become Decimal rather than staying int. SQLAlchemy types the bind + from the operand, so a plain int renders an ``::INTEGER`` cast and anything + past int4 fails at execute time with "integer out of range" — even when the + comparison itself is meaningful. Decimal renders no cast, matching what + float() used to do, but keeps the value exact: float() rounds any int past + 2**53 and would silently compare against a different number. + + bool is narrowed first because it subclasses int; binding it as a boolean + against a numeric column produces SQL Postgres has no operator for. + + Args: + op_value: The operand to validate. + + Returns: + The operand as an exact numeric value that binds without a cast. + + Raises: + ValueError, TypeError: If the operand is not numeric. Callers convert + these to FilterError. + """ + if isinstance(op_value, bool): + return Decimal(int(op_value)) + if isinstance(op_value, float): + return op_value + if isinstance(op_value, int | Decimal): + return Decimal(op_value) + try: + return Decimal(str(op_value)) + except ArithmeticError: + raise ValueError(f"not a number: {op_value!r}") from None + + +def _column_python_type(column: Any) -> type | None: + """Return a column's Python type, or None when it doesn't declare one. + + pgvector's Vector raises NotImplementedError rather than returning a type, + so this must not be called bare. + """ + try: + return typing_cast("type | None", column.type.python_type) + except (AttributeError, NotImplementedError): + return None + + +def _coerce_operand( + column: Any, column_name: str, value: Any, operator: str = "" +) -> Any: + """Return ``value`` ready to bind against ``column``, or raise FilterError. + + SQLAlchemy types a bind from the *operand*, not the column, and the psycopg + dialect renders that type as an explicit cast. So an operand whose type + doesn't match its column compiles into valid-looking SQL and then fails at + execute time — ``operator does not exist: text = integer``. Postgres will + not implicitly bridge these, so the mismatch has to be caught here. + + Every operand passes through this one function, whatever the operator, so + ``eq``/``ne``/``gt``/``in`` cannot drift apart by construction. Callers + handle None (a null check) and ``*`` (a wildcard) before calling. + + Args: + column: SQLAlchemy column object. + column_name: Internal column name, for error messages. + value: The operand to coerce. + operator: The comparison operator, or "" for bare equality. + + Returns: + The operand, coerced where a lossless coercion exists. + + Raises: + FilterError: If the operand cannot be bound to this column. + """ + # JSONB keeps containment semantics: the operand is a JSON document, not a + # scalar to compare. `jsonb >= 5` and `jsonb @> 'text'` have no operator. + if isinstance(column.type, JSONB): + if operator in ("", "contains") and isinstance(value, dict | list): + return typing_cast("Any", value) + raise FilterError( + f"Invalid filter for column '{column_name}': a JSONB column takes an object, optionally under 'contains'" + ) + + python_type = _column_python_type(column) + if python_type is None: + raise FilterError(f"Column '{column_name}' cannot be filtered on") + + # contains/icontains build an ILIKE pattern, so the operand is stringified + # and its own type doesn't matter — but the column must be text, or + # Postgres has no `~~` operator for it. + if operator in ("contains", "icontains"): + if python_type is not str: + raise FilterError( + f"Operator '{operator}' requires a text column, but '{column_name}' is {python_type.__name__}" + ) + return value + + # bool is checked before the numeric branch: it subclasses int, so a boolean + # column would otherwise have `true` coerced to 1, which Postgres rejects + # against a boolean column ("operator does not exist: boolean <> integer"). + if python_type is bool: + if isinstance(value, bool): + return value + raise FilterError( + f"Invalid value for column '{column_name}': expected true or false, got {type(value).__name__}" + ) + + if issubclass(python_type, datetime.datetime | datetime.date): + if isinstance(value, datetime.datetime | datetime.date): + return value + if isinstance(value, str): + validated = _validate_datetime_string(value) + if validated is None: + raise FilterError(f"Invalid datetime value: {value}") + return validated + raise FilterError( + f"Invalid value for column '{column_name}': expected a datetime, got {type(value).__name__}" + ) + + if issubclass(python_type, int | float | Decimal): + try: + return _coerce_numeric(value) + except (TypeError, ValueError): + raise FilterError( + f"Invalid numeric value: {value}. Expected a number, got {type(value).__name__}" + ) from None + + if python_type is str: + if not isinstance(value, str): + raise FilterError( + f"Invalid value for column '{column_name}': expected a string, got {type(value).__name__}" + ) + allowed = ENUM_COLUMN_VALUES.get(column_name) + if allowed is not None and value not in allowed: + raise FilterError( + f"Invalid value for column '{column_name}': {value!r}. Expected one of {sorted(allowed)}" + ) + return value + + raise FilterError(f"Column '{column_name}' cannot be filtered on") + def extract_session_allowlist( filters: dict[str, Any] | None, @@ -76,6 +232,11 @@ def extract_session_allowlist( FilterError (422) rather than being silently ignored — a dropped filter on these endpoints would widen recall scope. + Entries must be well-formed session ids. Wildcards are not part of this + subset: the DSL treats ``*`` as "match everything" while the non-DSL + consumers of the allowlist treat it as a literal name, so it is rejected + rather than meaning two things at once. + Args: filters: The raw ``filters`` body, or None. must_include: A session id that must appear in the parsed allowlist — @@ -128,6 +289,17 @@ def extract_session_allowlist( for entry in entries: if not isinstance(entry, str) or not entry: raise FilterError("filters.session_id entries must be non-empty strings") + # Only names a session could actually have. The allowlist reaches + # queries three ways — direct `IN`, the filter DSL, and a Python + # membership test — and they don't agree on a value like "*", which the + # DSL reads as "drop the condition" while the others treat as a literal. + # Rejecting it here keeps the divergent value away from all three, and + # matches this endpoint's documented contract (an id, a list of ids, or + # {"in": [...]}) which never included wildcards. + if not re.fullmatch(RESOURCE_NAME_PATTERN, entry): + raise FilterError( + f"Invalid session id in filters.session_id: {entry!r}. Session ids match {RESOURCE_NAME_PATTERN}" + ) if entry not in seen: seen.add(entry) allowlist.append(entry) @@ -181,9 +353,26 @@ def apply_filter( if filters is None: return stmt - conditions = _build_filter_conditions(filters, model_class) - if conditions is not None: - stmt = stmt.where(conditions) + # Fail closed. The filter body is arbitrary client JSON, so any shape the + # DSL doesn't recognize must become a 422, not an unhandled 500 from + # somewhere deep in SQLAlchemy. The exception is still logged in full so a + # genuine bug in the builder stays visible rather than being swallowed. + try: + conditions = _build_filter_conditions(filters, model_class) + if conditions is not None: + stmt = stmt.where(conditions) + except FilterError: + raise + except Exception: + # Keys only, not the body: filter operands are client-supplied and carry + # peer/session ids and free-text `contains` values. The traceback plus + # the entry shape is what actually locates a builder bug. + logger.exception( + "Unexpected error building filter for %s; filter keys: %s", + model_class.__name__, + sorted(filters), + ) + raise FilterError("Invalid filter configuration") from None return stmt @@ -256,8 +445,15 @@ def _build_filter_conditions( _depth=_depth + 1, ) if sub_condition is not None: + # `IS NOT TRUE` rather than `NOT`: under SQL's three-valued + # logic a comparison against a NULL column is NULL, and `NOT + # NULL` is NULL, so plain negation drops rows whose column is + # unset — even though an unset column does not match what is + # being excluded. NOT [{"session_id": "abc"}] must include + # documents that have no session, since those are not "abc". + # Composes over compound sub-conditions: (a AND b) IS NOT TRUE. not_conditions.append( - not_(sub_condition) + sub_condition.is_not(True) ) # Apply NOT to each condition individually if not_conditions: conditions.append(and_(*not_conditions)) # Then AND them together @@ -327,6 +523,14 @@ def _build_field_condition( if value == "*": return None + # A null operand is a null check, not a value to compare. Every branch below + # binds the operand against the column's type, and no type accepts None, so + # this has to short-circuit or `{"col": null}` raises instead of matching the + # rows it names. Keeps bare null agreeing with `{"ne": null}` (IS NOT NULL) + # and with `NOT [{"col": null}]`. + if value is None: + return column.is_(None) + # Bare-list sugar on regular columns: {"session_id": ["a", "b"]} is # shorthand for {"session_id": {"in": ["a", "b"]}}. JSONB columns are # excluded — a bare list there keeps JSONB containment semantics. @@ -345,13 +549,21 @@ def _build_field_condition( # For JSONB fields (metadata, configuration), check if it contains nested comparison operators if column_name in JSONB_COLUMNS: return _build_nested_metadata_conditions(column, value) # pyright: ignore + elif not isinstance(column.type, JSONB): + # A dict against a scalar column compiles fine but fails in + # psycopg at execute time ("cannot adapt type 'dict'") as a 500. + # Reject unknown operator dicts here as a 422 instead. + keys = sorted(typing_cast("dict[str, Any]", value)) + raise FilterError( + f"Invalid filter for column '{key}': unsupported operator(s) {keys}. Expected one of {sorted(COMPARISON_OPERATORS)} or a scalar value." + ) else: return column == value else: if column_name in JSONB_COLUMNS: - return column.contains(value) + return column.contains(_coerce_operand(column, column_name, value)) else: - return column == value + return column == _coerce_operand(column, column_name, value) def _safe_numeric_cast( @@ -552,11 +764,6 @@ def _build_comparison_conditions( """ conditions: list[ColumnElement[bool]] = [] - # Check if this is a datetime column - is_datetime_column = hasattr(column.type, "python_type") and issubclass( - column.type.python_type, datetime.datetime - ) - for operator, op_value in comparisons.items(): # Validate that the operator is supported if operator not in COMPARISON_OPERATORS: @@ -566,29 +773,25 @@ def _build_comparison_conditions( if op_value == "*": continue + # A null operand is a null check, not a value comparison, on every + # column type. Only `ne` is meaningful: {"col": None} covers IS NULL + # via the null guard in _build_field_condition. + if op_value is None: + if operator != "ne": + raise FilterError( + f"Operator '{operator}' does not accept null. Use {{\"ne\": null}} for a not-null check, or null on its own for a null check." + ) + conditions.append(column.is_not(None)) + continue + condition = None - # For datetime columns, cast string values to timestamp - if is_datetime_column and isinstance(op_value, str): - # Validate datetime string to prevent SQL injection - validated_datetime = _validate_datetime_string(op_value) - if validated_datetime is None: - # Raise error if datetime validation fails - raise FilterError(f"Invalid datetime value: {op_value}") - - # Use the validated datetime object directly instead of string interpolation - casted_value = validated_datetime - else: - # if the operator is a numeric operator, the value must cast to a number - if operator in NUMERIC_OPERATORS: - try: - casted_value = float(op_value) - except ValueError: - raise FilterError( - f"Invalid numeric value: {op_value}. Expected a number, got {type(op_value).__name__}" - ) from None - else: - casted_value = op_value + # `in` coerces element-wise below; every other operator has one operand. + casted_value = ( + op_value + if operator == "in" + else _coerce_operand(column, column_name, op_value, operator) + ) if operator == "gte": condition = column >= casted_value @@ -599,38 +802,39 @@ def _build_comparison_conditions( elif operator == "lt": condition = column < casted_value elif operator == "ne": - condition = column != casted_value + # IS DISTINCT FROM, not <>: `NULL <> 'abc'` is NULL, so plain + # inequality drops rows whose column is unset. Identical to <> + # whenever no NULL is involved, and keeps `ne` agreeing with the + # NOT operator instead of quietly returning a different row set. + condition = column.is_distinct_from(casted_value) elif operator == "in": if hasattr(op_value, "__iter__") and not isinstance(op_value, str | bytes): # Handle wildcard in iterable - if present, matches everything, so no condition needed if "*" in op_value: continue else: - if is_datetime_column: - # Validate and cast each datetime string value - casted_values: list[str | datetime.datetime] = [] - for val in op_value: - if isinstance(val, str): - validated_datetime = _validate_datetime_string(val) - if validated_datetime is None: - raise FilterError( - f"Invalid datetime value in list: {val}" - ) - casted_values.append(validated_datetime) - else: - casted_values.append(val) - if casted_values: - condition = column.in_(casted_values) - else: - condition = column.in_(list(op_value)) + # Element-wise: one bad element poisons the whole IN, since + # its type decides the cast rendered for that parameter. + # An empty list is applied, not skipped: `in: []` must match + # nothing. Dropping the condition would widen the query to + # every row, and session scoping relies on an empty + # allowlist failing closed (see extract_session_allowlist). + condition = column.in_( + [ + _coerce_operand(column, column_name, val, operator) + for val in op_value + ] + ) else: raise FilterError( f"Invalid value for 'in' operator: {op_value}. Expected an iterable (list, tuple, set), got {type(op_value).__name__}" ) elif operator == "contains": - if column_name == "h_metadata": - # For JSONB columns, use JSONB contains - condition = column.contains(op_value) + if isinstance(column.type, JSONB): + # Keyed on the column type, not the name: internal_metadata is + # equally JSONB and was falling through to ILIKE, which + # Postgres rejects as `jsonb ~~* text`. + condition = column.contains(casted_value) else: # For text columns, use ILIKE with escaped pattern escaped_value = escape_ilike_pattern(str(op_value)) diff --git a/tests/routes/test_conclusions.py b/tests/routes/test_conclusions.py index 77cb23fb..ccd3b337 100644 --- a/tests/routes/test_conclusions.py +++ b/tests/routes/test_conclusions.py @@ -1381,3 +1381,81 @@ class TestConclusionRoutes: # Verify the conclusion has null session_id conclusion = next(c for c in data["items"] if c["id"] == created_id) assert conclusion["session_id"] is None + + @pytest.mark.asyncio + async def test_negation_includes_conclusions_with_no_session( + self, + client: TestClient, + db_session: AsyncSession, + sample_data: tuple[Workspace, Peer], + ): + """Negation must not silently drop workspace-level conclusions. + + A conclusion with no session is not "some other session", so excluding + that session has to leave it in the result. Under SQL's three-valued + logic a comparison against NULL is NULL, which would drop the row. + + This is only visible by counting returned rows — the filter builds and + executes cleanly either way. + """ + test_workspace, test_peer = sample_data + + test_peer2 = models.Peer( + name=str(generate_nanoid()), workspace_name=test_workspace.name + ) + db_session.add(test_peer2) + await db_session.flush() + + test_session = models.Session( + name=str(generate_nanoid()), workspace_name=test_workspace.name + ) + other_session = models.Session( + name=str(generate_nanoid()), workspace_name=test_workspace.name + ) + db_session.add_all([test_session, other_session]) + await db_session.commit() + + await self._create_collection( + db_session, test_workspace.name, test_peer.name, test_peer2.name + ) + + scoped = models.Document( + workspace_name=test_workspace.name, + observer=test_peer.name, + observed=test_peer2.name, + content="Scoped to a session", + session_name=test_session.name, + ) + workspace_level = models.Document( + workspace_name=test_workspace.name, + observer=test_peer.name, + observed=test_peer2.name, + content="Not scoped to any session", + session_name=None, + ) + db_session.add_all([scoped, workspace_level]) + await db_session.commit() + + def contents(filters: dict[str, object]) -> set[str]: + response = client.post( + f"/v3/workspaces/{test_workspace.name}/conclusions/list", + json={"filters": filters}, + ) + assert response.status_code == 200, response.text + return {item["content"] for item in response.json()["items"]} + + both = {"Scoped to a session", "Not scoped to any session"} + + # NOT and ne agree, and both keep the session-less conclusion. + assert contents({"NOT": [{"session_id": other_session.name}]}) == both + assert contents({"session_id": {"ne": other_session.name}}) == both + + # Requiring the field to be set is how you narrow to sessioned rows. + assert contents( + { + "AND": [ + {"session_id": {"ne": other_session.name}}, + {"session_id": {"ne": None}}, + ] + } + ) == {"Scoped to a session"} diff --git a/tests/test_session_allowlist.py b/tests/test_session_allowlist.py index aa6077cb..86c9280b 100644 --- a/tests/test_session_allowlist.py +++ b/tests/test_session_allowlist.py @@ -62,6 +62,38 @@ class TestExtractSessionAllowlist: with pytest.raises(FilterError): extract_session_allowlist({"session_id": bad}) + @pytest.mark.parametrize( + "filters", + [ + {"session_id": "*"}, + {"session_id": ["s1", "*"]}, + {"session_id": {"in": ["*"]}}, + ], + ) + def test_wildcard_rejected(self, filters: dict[str, Any]): + """A wildcard means two different things depending on which consumer + receives the allowlist: the filter DSL drops the condition entirely + (matching every session), while the direct `IN` and Python membership + paths treat "*" as a literal session name (matching none). It is not + part of this endpoint's contract, so it is rejected outright. + + The mixed list is the case that matters most — it looks narrowed. + """ + with pytest.raises(FilterError, match="Invalid session id"): + extract_session_allowlist(filters) + + @pytest.mark.parametrize("name", ["a b", "a/b", "a.b", "a%b", "s1;drop"]) + def test_malformed_session_ids_rejected(self, name: str): + with pytest.raises(FilterError, match="Invalid session id"): + extract_session_allowlist({"session_id": name}) + + def test_valid_id_characters_still_accepted(self): + """The pattern must not be stricter than the ids the API actually + issues, which include underscores and hyphens.""" + assert extract_session_allowlist({"session_id": "Valid_name-123"}) == [ + "Valid_name-123" + ] + def test_cap_enforced(self): too_many = [f"s{i}" for i in range(MAX_SESSION_ALLOWLIST_ENTRIES + 1)] with pytest.raises(FilterError, match="at most"): diff --git a/tests/utils/test_filter.py b/tests/utils/test_filter.py new file mode 100644 index 00000000..c90c8442 --- /dev/null +++ b/tests/utils/test_filter.py @@ -0,0 +1,412 @@ +"""Unit tests for filter condition building.""" + +from collections.abc import Sequence +from datetime import datetime +from decimal import Decimal +from typing import Any, cast, get_args + +import pytest +from sqlalchemy import select +from sqlalchemy.dialects import postgresql +from sqlalchemy.dialects.postgresql import JSONB +from sqlalchemy.dialects.postgresql import psycopg as psycopg_dialect + +from src.exceptions import FilterError +from src.models import Document, Message, Peer, Session +from src.utils.filter import apply_filter +from src.utils.types import DocumentLevel + + +def test_unknown_operator_dict_on_scalar_column_raises(): + """An unrecognized operator dict must 422, not reach the driver as a 500. + + Regression: {"session_id": {"operator": "null"}} compiled to + `session_name = %(param)s` with a dict bind, which psycopg rejected with + "cannot adapt type 'dict'" -> unhandled 500. + """ + with pytest.raises(FilterError): + apply_filter(select(Document), Document, {"session_id": {"operator": "null"}}) + + +def test_known_operator_dict_on_scalar_column_still_works(): + stmt = apply_filter( + select(Document), Document, {"session_id": {"in": ["s1", "s2"]}} + ) + assert "session_name IN" in str(stmt).replace("documents.", "") + + +def test_dict_on_jsonb_column_still_works(): + stmt = apply_filter(select(Document), Document, {"metadata": {"kind": "note"}}) + # Assert on the WHERE clause specifically: internal_metadata is in the + # SELECT projection either way, so checking the whole statement passes even + # when no condition was applied at all. + assert stmt.whereclause is not None + assert "internal_metadata" in str(stmt.whereclause) + compiled = stmt.compile(dialect=postgresql.dialect()) + assert {"kind": "note"} in [bind.value for bind in compiled.binds.values()] + + +def test_numeric_operand_keeps_integer_precision(): + """float() rounds anything past 2**53, silently shifting the comparison.""" + big = 2**53 + 1 + stmt = apply_filter(select(Message), Message, {"token_count": {"gt": big}}) + compiled = stmt.compile(dialect=postgresql.dialect()) + assert big in [bind.value for bind in compiled.binds.values()] + + +def test_fractional_operand_on_integer_column_is_not_truncated(): + """Coercing to the column's int type would turn `lt 5.5` into `lt 5`.""" + stmt = apply_filter(select(Message), Message, {"token_count": {"lt": 5.5}}) + compiled = stmt.compile(dialect=postgresql.dialect()) + assert 5.5 in [bind.value for bind in compiled.binds.values()] + + +def test_integer_operand_binds_without_an_integer_cast(): + """A plain int bind renders `::INTEGER`, so any value past int4 fails at + execute time with "integer out of range" even when the comparison is + meaningful. Decimal renders no cast, which is what float() used to do.""" + stmt = apply_filter(select(Message), Message, {"token_count": {"gt": 2**31}}) + assert "::INTEGER" not in str(stmt.compile(dialect=psycopg_dialect.dialect())) + + +def test_in_list_on_numeric_column_handles_out_of_range_values(): + stmt = apply_filter(select(Message), Message, {"token_count": {"in": [1, 2**31]}}) + assert "::INTEGER" not in str(stmt.compile(dialect=psycopg_dialect.dialect())) + + +def test_in_list_on_numeric_column_rejects_garbage(): + with pytest.raises(FilterError): + apply_filter(select(Message), Message, {"token_count": {"in": [1, "nope"]}}) + + +@pytest.mark.parametrize( + "filters", + [ + {"is_active": True}, + {"is_active": False}, + {"is_active": {"ne": True}}, + {"is_active": {"in": [True, False]}}, + ], +) +def test_bool_column_accepts_native_booleans(filters: dict[str, Any]): + """bool subclasses int, so a boolean column would otherwise be coerced to 1 + and rejected by Postgres as `boolean <> integer`.""" + stmt = apply_filter(select(Session), Session, filters) + assert stmt.whereclause is not None + assert "is_active" in str(stmt.whereclause) + + +@pytest.mark.parametrize( + "filters", + [ + {"is_active": "true"}, + {"is_active": "false"}, + {"is_active": 1}, + {"is_active": {"ne": "true"}}, + ], +) +def test_bool_column_rejects_non_boolean_operands(filters: dict[str, Any]): + """These bind as VARCHAR/INTEGER against a boolean column, which Postgres + rejects at execute time — a 422 is the honest answer, not a 500.""" + with pytest.raises(FilterError): + apply_filter(select(Session), Session, filters) + + +def test_numeric_string_operand_stays_exact(): + stmt = apply_filter(select(Message), Message, {"token_count": {"gt": "5"}}) + compiled = stmt.compile(dialect=postgresql.dialect()) + assert 5 in [bind.value for bind in compiled.binds.values()] + + +def test_ne_none_on_scalar_column_is_not_null(): + """Regression: float(None) raised TypeError, which the ValueError handler + missed -> unhandled 500.""" + stmt = apply_filter(select(Document), Document, {"session_id": {"ne": None}}) + assert "session_name IS NOT NULL" in str(stmt) + + +def test_ne_string_on_text_column_compares_as_string(): + """Regression: numeric operators float()-cast on every column type, so a + string inequality on a text column was rejected as a bad number.""" + stmt = apply_filter(select(Document), Document, {"session_id": {"ne": "abc"}}) + assert "session_name IS DISTINCT FROM" in str(stmt) + + +def test_numeric_operator_still_validates_on_numeric_column(): + with pytest.raises(FilterError): + apply_filter(select(Message), Message, {"token_count": {"gt": "nope"}}) + + +def test_ne_none_on_numeric_column_is_not_null(): + stmt = apply_filter(select(Message), Message, {"token_count": {"ne": None}}) + assert "token_count IS NOT NULL" in str(stmt) + + +def test_null_operand_on_non_ne_operator_raises(): + with pytest.raises(FilterError): + apply_filter(select(Message), Message, {"token_count": {"gt": None}}) + + +def test_enum_column_rejects_an_unknown_value(): + """An invalid level silently matched nothing, which reads as "no results" + rather than "you sent a value that cannot exist".""" + with pytest.raises(FilterError): + apply_filter(select(Document), Document, {"level": "banana"}) + + +@pytest.mark.parametrize("level", get_args(DocumentLevel)) +def test_enum_column_accepts_every_declared_level(level: str): + """Derived from the Literal, so a new level (e.g. "abduction") is covered + here the moment it is declared — no second list to keep in sync.""" + stmt = apply_filter(select(Document), Document, {"level": level}) + assert stmt.whereclause is not None + + +def test_empty_in_list_matches_nothing_rather_than_everything(): + """Dropping an empty IN would widen the query to every row. Session scoping + relies on an empty allowlist failing closed.""" + stmt = apply_filter(select(Document), Document, {"session_id": {"in": []}}) + assert stmt.whereclause is not None + assert "IN" in str(stmt.whereclause) + + +@pytest.mark.parametrize( + "filters", + [ + {"created_at": "2026-01-01"}, + {"created_at": {"gte": "2026-01-01"}}, + {"token_count": "5"}, + {"token_count": {"gt": "5"}}, + ], +) +def test_equality_and_comparison_paths_coerce_alike(filters: dict[str, Any]): + """The two paths had different rules: comparison operators parsed datetimes + and coerced numbers, bare equality bound the raw string and 500'd on + `timestamp with time zone = character varying`.""" + stmt = apply_filter(select(Message), Message, filters) + assert stmt.whereclause is not None + + +@pytest.mark.parametrize( + ("model", "filters"), + [ + (Message, {"token_count": {"contains": 5}}), # integer ~~* text + (Message, {"created_at": {"contains": "x"}}), # timestamptz ~~* text + (Document, {"metadata": {"gte": 5}}), # jsonb >= integer + (Document, {"metadata": {"contains": "x"}}), # jsonb ~~* text + (Document, {"source_ids": "abc"}), # jsonb = character varying + (Document, {"session_id": 5}), # text = integer + (Document, {"session_id": True}), # text = boolean + (Message, {"created_at": 5}), # timestamptz = integer + (Document, {"embedding": 5}), # no python_type at all + ], +) +def test_incompatible_operand_types_are_rejected(model: Any, filters: dict[str, Any]): + """Each of these compiled cleanly and failed in Postgres as + `operator does not exist: `.""" + with pytest.raises(FilterError): + apply_filter(select(model), model, filters) + + +def _where(model: Any, filters: dict[str, Any]) -> str: + stmt = apply_filter(select(model), model, filters) + # Without this, a dropped condition makes split("WHERE") return the whole + # statement, so a filter that silently widened could still pass the asserts. + assert stmt.whereclause is not None + return str(stmt.compile(dialect=psycopg_dialect.dialect())).split("WHERE")[-1] + + +def test_not_is_null_safe(): + """`NOT (col = v)` is NULL when col is NULL, so plain negation drops rows + whose column is unset — even though an unset column is not `v`.""" + where = _where(Document, {"NOT": [{"session_id": "abc"}]}) + assert "IS NOT true" in where + + +def test_ne_is_null_safe(): + where = _where(Document, {"session_id": {"ne": "abc"}}) + assert "IS DISTINCT FROM" in where + + +def test_not_is_null_safe_over_a_compound_condition(): + """Negation has to survive nesting, not just single comparisons.""" + where = _where( + Document, {"NOT": [{"AND": [{"session_id": "a"}, {"level": "explicit"}]}]} + ) + assert "IS NOT true" in where + assert "AND" in where + + +def test_not_is_null_safe_over_contains(): + where = _where(Document, {"NOT": [{"session_id": {"contains": "x"}}]}) + assert "ILIKE" in where + assert "IS NOT true" in where + + +def test_ne_null_still_renders_is_not_null(): + """The null operand is intercepted before the operator dispatch, so this + path is unchanged by null-safe `ne`.""" + assert "IS NOT NULL" in _where(Document, {"session_id": {"ne": None}}) + + +@pytest.mark.parametrize( + ("model", "filters"), + [ + (Document, {"session_id": None}), # text + (Message, {"token_count": None}), # numeric + (Session, {"is_active": None}), # boolean + (Message, {"created_at": None}), # datetime + (Document, {"metadata": None}), # JSONB + (Document, {"source_ids": None}), # JSONB via the raw-key fallback + ], +) +def test_bare_null_is_a_null_check(model: Any, filters: dict[str, Any]): + """Regression: every operand routes through _coerce_operand, which accepts + no None on any column type, so bare null raised FilterError instead of + matching the unset rows it names.""" + assert "IS NULL" in _where(model, filters) + + +def test_bare_null_agrees_with_negation(): + """`NOT [{col: null}]` and `{col: {ne: null}}` must select the same rows. + `x IS NULL` never evaluates to NULL, so `(x IS NULL) IS NOT true` is exactly + `x IS NOT NULL` — the three forms have to stay in agreement.""" + assert "IS NOT NULL" in _where(Document, {"session_id": {"ne": None}}) + negated = _where(Document, {"NOT": [{"session_id": None}]}) + assert "IS NULL" in negated + assert "IS NOT true" in negated + + +def test_dict_on_raw_key_jsonb_column_is_equality(): + """Document falls back to raw column names, so a JSONB column outside + JSONB_COLUMNS reaches _build_field_condition's dict branch. It compares as + equality rather than containment — unlike `metadata`.""" + where = _where(Document, {"source_ids": {"kind": "note"}}) + assert "source_ids =" in where + + +@pytest.mark.parametrize( + ("filters", "expected"), + [ + ({"session_id": "abc"}, "session_name ="), + ({"session_id": {"contains": "x"}}, "ILIKE"), + ], +) +def test_positive_predicates_are_unchanged(filters: dict[str, Any], expected: str): + """A NULL column does not equal or contain anything, so positive predicates + correctly exclude those rows and must keep their plain operators.""" + where = _where(Document, filters) + assert expected in where + assert "IS NOT true" not in where + assert "IS DISTINCT FROM" not in where + + +# --- Invariants over the whole DSL ------------------------------------------- +# +# The filter body is arbitrary client JSON. Enumerating bad shapes one at a time +# is endless, so these two tests assert the properties that make any unhandled +# shape a 422 instead of a 500, and fail on the next shape nobody thought of. + +_OPERANDS: list[Any] = [ + None, + True, + False, + 0, + -1, + 1.5, + "", + "abc", + "*", + [], + [None], + [[1]], + [{"a": 1}], + {}, + {"operator": "null"}, + {"ne": None}, + {"ne": {"a": 1}}, + {"ne": [1]}, + {"in": None}, + {"in": "abc"}, + {"in": [{"a": 1}]}, + {"in": [[1]]}, + {"gt": {}}, + {"gt": []}, + {"gt": True}, + {"contains": None}, + {"contains": {"a": 1}}, + {"lt": [1, 2]}, +] + +_COLUMNS: dict[Any, list[str]] = { + Document: ["session_id", "workspace_id", "metadata", "level", "source_ids", "id"], + Message: ["session_id", "peer_id", "token_count", "created_at", "metadata"], + Session: ["id", "is_active", "created_at", "configuration"], + Peer: ["id", "created_at", "metadata"], +} + +_MALFORMED: list[dict[str, Any]] = [ + {"AND": "notalist"}, + {"AND": [None]}, + {"AND": [[]]}, + {"AND": [1]}, + {"OR": [None]}, + {"OR": [1]}, + {"NOT": None}, + {"NOT": [None]}, + {"unknown_column": 1}, +] + + +def _filter_shapes() -> list[tuple[Any, dict[str, Any]]]: + shapes: list[tuple[Any, dict[str, Any]]] = [] + for model, columns in _COLUMNS.items(): + for column in columns: + for operand in _OPERANDS: + leaf = {column: operand} + shapes.append((model, leaf)) + shapes.append((model, {"AND": [leaf]})) + shapes.append((model, {"NOT": [leaf]})) + shapes.extend((model, bad) for bad in _MALFORMED) + return shapes + + +def test_every_filter_shape_either_compiles_or_raises_filter_error(): + """No filter body may escape as anything other than a compiled statement or + a FilterError. Anything else reaches the client as an unhandled 500.""" + escaped: list[tuple[str, dict[str, Any], str]] = [] + for model, filters in _filter_shapes(): + try: + str(apply_filter(select(model), model, filters)) + except FilterError: + pass + except Exception as exc: # pragma: no cover - failure path + escaped.append((model.__name__, filters, type(exc).__name__)) + assert not escaped, f"non-FilterError escapes: {escaped[:10]}" + + +def test_no_non_scalar_value_is_bound_to_a_scalar_column(): + """A dict or list bound to a non-JSONB parameter compiles cleanly and then + fails in psycopg at execute time — the original 500. Nothing may reach that + state, including non-scalars nested inside an `in` list.""" + offenders: list[tuple[str, dict[str, Any], str]] = [] + for model, filters in _filter_shapes(): + try: + stmt = apply_filter(select(model), model, filters) + except FilterError: + continue + compiled = stmt.compile(dialect=postgresql.dialect()) + for bind in compiled.binds.values(): + if isinstance(bind.type, JSONB): + continue + value: Any = bind.value + # An expanding IN bind holds the list itself; check its elements. + elements = cast( + "Sequence[Any]", value if isinstance(value, list | tuple) else [value] + ) + for element in elements: + if element is not None and not isinstance( + element, str | bool | int | float | Decimal | datetime + ): + offenders.append((model.__name__, filters, repr(element)[:40])) + assert not offenders, f"non-scalar bound to scalar column: {offenders[:10]}"