Merge origin/main into abigail/dev-2371
Keep workspace-scoped MCP clients while taking main's conclusion search and filter support. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
ajspig
commit
9236f57a10
|
|
@ -192,6 +192,90 @@ sessions = honcho.sessions(filters={
|
|||
```
|
||||
</CodeGroup>
|
||||
|
||||
### 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.
|
||||
|
||||
<CodeGroup>
|
||||
```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" } }
|
||||
});
|
||||
})();
|
||||
```
|
||||
</CodeGroup>
|
||||
|
||||
To exclude a value **and** require the field to be set, combine the two with
|
||||
`AND`:
|
||||
|
||||
<CodeGroup>
|
||||
```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 } }
|
||||
]
|
||||
}
|
||||
});
|
||||
})();
|
||||
```
|
||||
</CodeGroup>
|
||||
|
||||
### 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"})
|
|||
```
|
||||
</CodeGroup>
|
||||
|
||||
## 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:
|
||||
|
||||
<CodeGroup>
|
||||
```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: [] } } });
|
||||
})();
|
||||
```
|
||||
</CodeGroup>
|
||||
|
||||
## 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 |
|
||||
|
|
|
|||
|
|
@ -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=="],
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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: {
|
||||
|
|
|
|||
|
|
@ -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)}`,
|
||||
|
|
|
|||
|
|
@ -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))
|
||||
|
|
|
|||
|
|
@ -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"}
|
||||
|
|
|
|||
|
|
@ -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"):
|
||||
|
|
|
|||
|
|
@ -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: <coltype> <op> <operandtype>`."""
|
||||
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]}"
|
||||
Loading…
Reference in New Issue