From 3e73c6f2876db199085ac9c780fa7e1fb49b2827 Mon Sep 17 00:00:00 2001 From: Serhii Zghama <20826225+serhiizghama@users.noreply.github.com> Date: Mon, 24 Aug 2026 20:33:08 +0700 Subject: [PATCH] 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. --- src/utils/filter.py | 4 +- tests/test_advanced_filters.py | 85 +++++++++++++++++++++++++++++++++- tests/utils/test_filter.py | 14 ++++++ 3 files changed, 101 insertions(+), 2 deletions(-) diff --git a/src/utils/filter.py b/src/utils/filter.py index 22d648af..78b7569f 100644 --- a/src/utils/filter.py +++ b/src/utils/filter.py @@ -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: diff --git a/tests/test_advanced_filters.py b/tests/test_advanced_filters.py index 3b187470..0ed12e20 100644 --- a/tests/test_advanced_filters.py +++ b/tests/test_advanced_filters.py @@ -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, diff --git a/tests/utils/test_filter.py b/tests/utils/test_filter.py index c90c8442..3473e827 100644 --- a/tests/utils/test_filter.py +++ b/tests/utils/test_filter.py @@ -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(