fix(filter): make ne on jsonb metadata keys null-safe (#1036)
* fix(filter): make ne on jsonb metadata keys null-safe * test(filter): cover null-safe ne on nested metadata keys * test(filter): count actual rows for nested-metadata ne null-safety Compile-only checks lock the operator map entry but don't catch wrong row sets under three-valued logic. Adds a live messages/list case with a message missing the key and one with empty metadata, following the scalar-column pattern in test_negation_includes_conclusions_with_no_session. * test(filter): type message_configs with a TypedDict basedpyright couldn't narrow the heterogeneous metadata dict literals, so indexing message_configs["content"] came back partially unknown and broke the sorted() calls under type checking.
This commit is contained in:
parent
ddbb90e36f
commit
3e73c6f287
|
|
@ -689,7 +689,9 @@ def _build_comparison_condition(
|
|||
"lte": lambda a, v: a <= v,
|
||||
"gt": lambda a, v: a > v,
|
||||
"lt": lambda a, v: a < v,
|
||||
"ne": lambda a, v: a != v,
|
||||
# IS DISTINCT FROM, not <>: (metadata ->> key) is NULL for an
|
||||
# absent key, and `NULL <> v` is NULL, so <> drops those rows.
|
||||
"ne": lambda a, v: a.is_distinct_from(v),
|
||||
}
|
||||
return operator_map[operator](safe_accessor, safe_value)
|
||||
except Exception as e:
|
||||
|
|
|
|||
|
|
@ -4,7 +4,7 @@ comparison operators, and wildcards across multiple models.
|
|||
"""
|
||||
|
||||
from datetime import datetime, timedelta, timezone
|
||||
from typing import Any
|
||||
from typing import Any, TypedDict
|
||||
|
||||
import pytest
|
||||
from fastapi.testclient import TestClient
|
||||
|
|
@ -13,6 +13,12 @@ from nanoid import generate as generate_nanoid
|
|||
from src.models import Peer, Workspace
|
||||
|
||||
|
||||
class MessageConfig(TypedDict):
|
||||
content: str
|
||||
peer_id: str
|
||||
metadata: dict[str, Any]
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"filter_config,expected_peer_indices,description",
|
||||
[
|
||||
|
|
@ -235,6 +241,83 @@ async def test_comparison_operators_filters(
|
|||
), f"Unexpected message '{message_config['content']}' found in results for {description}"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_nested_metadata_ne_includes_missing_and_empty_metadata(
|
||||
client: TestClient,
|
||||
sample_data: tuple[Workspace, Peer],
|
||||
):
|
||||
"""`ne` on a nested metadata key must not silently drop rows where the
|
||||
key is absent. Under SQL's three-valued logic, comparing NULL (a missing
|
||||
key or empty metadata) with `<>` yields NULL, which excludes the row —
|
||||
the filter builds and executes cleanly either way, so this has to be
|
||||
checked by counting the rows actually returned.
|
||||
"""
|
||||
test_workspace, test_peer = sample_data
|
||||
|
||||
session_id = str(generate_nanoid())
|
||||
session_response = client.post(
|
||||
f"/v3/workspaces/{test_workspace.name}/sessions",
|
||||
json={"id": session_id, "peer_names": {test_peer.name: {}}},
|
||||
)
|
||||
assert session_response.status_code == 201
|
||||
|
||||
message_configs: list[MessageConfig] = [
|
||||
{
|
||||
"content": "High priority, score 10",
|
||||
"peer_id": test_peer.name,
|
||||
"metadata": {"priority": "high", "score": 10},
|
||||
},
|
||||
{
|
||||
"content": "Low priority, score 5",
|
||||
"peer_id": test_peer.name,
|
||||
"metadata": {"priority": "low", "score": 5},
|
||||
},
|
||||
{
|
||||
"content": "Metadata present, no priority or score key",
|
||||
"peer_id": test_peer.name,
|
||||
"metadata": {"other": "value"},
|
||||
},
|
||||
{
|
||||
"content": "Empty metadata",
|
||||
"peer_id": test_peer.name,
|
||||
"metadata": {},
|
||||
},
|
||||
]
|
||||
messages_response = client.post(
|
||||
f"/v3/workspaces/{test_workspace.name}/sessions/{session_id}/messages",
|
||||
json={"messages": message_configs},
|
||||
)
|
||||
assert messages_response.status_code == 201
|
||||
|
||||
def list_contents(filter_config: dict[str, Any]) -> list[str]:
|
||||
response = client.post(
|
||||
f"/v3/workspaces/{test_workspace.name}/sessions/{session_id}/messages/list",
|
||||
json={"filters": filter_config},
|
||||
)
|
||||
assert response.status_code == 200
|
||||
return [item["content"] for item in response.json()["items"]]
|
||||
|
||||
# String value: excludes only the row where priority actually equals "high"
|
||||
string_ne_contents = list_contents({"metadata": {"priority": {"ne": "high"}}})
|
||||
assert sorted(string_ne_contents) == sorted(
|
||||
[
|
||||
message_configs[1]["content"],
|
||||
message_configs[2]["content"],
|
||||
message_configs[3]["content"],
|
||||
]
|
||||
)
|
||||
|
||||
# Numeric value: excludes only the row where score actually equals 5
|
||||
numeric_ne_contents = list_contents({"metadata": {"score": {"ne": 5}}})
|
||||
assert sorted(numeric_ne_contents) == sorted(
|
||||
[
|
||||
message_configs[0]["content"],
|
||||
message_configs[2]["content"],
|
||||
message_configs[3]["content"],
|
||||
]
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_bare_list_membership_sugar(
|
||||
client: TestClient,
|
||||
|
|
|
|||
|
|
@ -228,6 +228,20 @@ def test_ne_is_null_safe():
|
|||
assert "IS DISTINCT FROM" in where
|
||||
|
||||
|
||||
def test_nested_metadata_ne_string_is_null_safe():
|
||||
"""`ne` on a JSONB metadata key went through the operator map as plain <>,
|
||||
unlike the scalar path, so a row missing that key was silently dropped."""
|
||||
where = _where(Document, {"metadata": {"priority": {"ne": "high"}}})
|
||||
assert "IS DISTINCT FROM" in where
|
||||
assert "!=" not in where
|
||||
|
||||
|
||||
def test_nested_metadata_ne_numeric_is_null_safe():
|
||||
where = _where(Document, {"metadata": {"score": {"ne": 5}}})
|
||||
assert "IS DISTINCT FROM" in where
|
||||
assert "!=" not in where
|
||||
|
||||
|
||||
def test_not_is_null_safe_over_a_compound_condition():
|
||||
"""Negation has to survive nesting, not just single comparisons."""
|
||||
where = _where(
|
||||
|
|
|
|||
Loading…
Reference in New Issue