diff --git a/backend/README.md b/backend/README.md index 435a645488..800a227721 100644 --- a/backend/README.md +++ b/backend/README.md @@ -130,6 +130,10 @@ to the last returned sequence to page forward through a bounded history range. FastAPI application providing REST endpoints for frontend integration: +Integer metadata filters match exact JSON integers, including signed-64-bit +boundaries. Stored integers outside that range are ignored rather than rounded +to a boundary (SQLite) or causing the search to fail (PostgreSQL). + | Route | Purpose | |-------|---------| | `GET /api/models` | List available LLM models | diff --git a/backend/packages/harness/deerflow/persistence/AGENTS.md b/backend/packages/harness/deerflow/persistence/AGENTS.md index b6efe8d090..13aec4f279 100644 --- a/backend/packages/harness/deerflow/persistence/AGENTS.md +++ b/backend/packages/harness/deerflow/persistence/AGENTS.md @@ -10,6 +10,15 @@ Alembic stamp/upgrade workers started inside `bootstrap_schema()` remain owned b On SQLite, `BEGIN IMMEDIATE` takes the database-wide write lock, so every unrelated writer (run status, thread metadata, the scheduler) waits for the transaction and fails with `database is locked` after `busy_timeout` (30s). Do slow work such as document conversion before opening a locked transaction; lock only to revalidate and publish, as `ProjectDocumentRepository.publish_under_live_lock` does. `tests/test_project_document_tools.py::TestConversionSerialization` pins this. +## JSON integer filters + +Stored JSON integers are not bounded by the signed-64-bit filter input contract. +SQLite predicates must check the extracted SQL value's `typeof`, not only JSON +`json_type`, to exclude oversized integers decoded as REAL. PostgreSQL predicates +compare integer text (including `-0` for zero) without casting arbitrary stored +numbers to BIGINT or NUMERIC. Preserve integer/float/boolean/string distinctions. +`tests/test_json_integer_matching.py` exercises both dialects; PostgreSQL opts in +with `DEERFLOW_TEST_POSTGRES_URL` and uses connection-local temporary tables. ## Scheduled-task lifecycle Scheduled-task lifecycle rules live in one place each. `ScheduledTaskRepository.update` is the single reactivation choke point: any `status -> enabled` from a non-enabled row re-checks `end_condition_reached` after applying the same request's renewal fields and raises `ScheduledTaskLimitsExhausted` (HTTP/tool `limits_exhausted`). The same method moves `unmet_streak_after_seq` when `goal_objective`, `prompt` or `stop_condition` actually changes (`append_standing_note` too; Resume never), and `_unmet_streak` skips both rows at/below that boundary and goal-check failures (`finalization.CHECK_FAILURE_CODES`). A host pause marker (`is_host_pause_marker`) survives a manual trial on a paused task. A task is finished (`completed` by `max_runs`/`end_at`) only by `finalize_occurrence` or `finish_task_at_end_condition` (both in `scheduled_task_runs/finalization.py`), which report `task_stopped`/`task_paused`/`task_finished` to the finalization observer on a live -> paused/finished transition only, inside the caller's transaction; `test_no_inline_end_condition_completion` fails on any other `status = "completed"` in the two repositories. The scheduler's observer (`ScheduledTaskService._on_finalization`) is always installed and writes `scheduled_task_events` rows for the originating chat in that transaction; its lookups fall back on bad stored data and only database errors propagate, because it runs inside batch recovery. Pausing a terminal task returns `"finished"`. `stop_condition` has its own column; only `ScheduledTaskService` composes it into the launched message (`scheduler/stop_rule.launch_prompt`), so never write it into `prompt`. REST and capability validation share `app/gateway/scheduled_task_validation.py`, and every error raised there, in `routers/scheduled_tasks.py` and in `scheduled_task_access.py` uses `scheduler_error(status, code, message, **params)` with a code listed in `contracts/scheduled_task_errors_contract.json` (request models declare types only, so value rules never become FastAPI list details). diff --git a/backend/packages/harness/deerflow/persistence/json_compat.py b/backend/packages/harness/deerflow/persistence/json_compat.py index c59ef7921a..6cd57b8656 100644 --- a/backend/packages/harness/deerflow/persistence/json_compat.py +++ b/backend/packages/harness/deerflow/persistence/json_compat.py @@ -20,8 +20,8 @@ _KEY_CHARSET_RE = re.compile(r"^[A-Za-z0-9_\-]+$") # Allowed value types for metadata filter values (same set accepted by JsonMatch). ALLOWED_FILTER_VALUE_TYPES: tuple[type, ...] = (type(None), bool, int, float, str) -# SQLite raises an overflow when binding values outside signed 64-bit range; -# PostgreSQL overflows during BIGINT cast. Reject at validation time instead. +# Keep filter inputs portable: SQLite cannot bind integers outside signed +# 64-bit range. Stored JSON numbers may still be larger on either backend. _INT64_MIN = -(2**63) _INT64_MAX = 2**63 - 1 @@ -47,8 +47,8 @@ def validate_metadata_filter_value(value: object) -> bool: SQLAlchemy's ``inherit_cache`` invariant when ``value`` is unhashable. Integer values are additionally restricted to the signed 64-bit range - ``[-2**63, 2**63 - 1]``: SQLite overflows when binding larger values - and PostgreSQL overflows during the ``BIGINT`` cast. + ``[-2**63, 2**63 - 1]`` so filters remain portable to SQLite, which + cannot bind larger integer values. """ if not isinstance(value, ALLOWED_FILTER_VALUE_TYPES): return False @@ -135,11 +135,9 @@ class _Dialect: num_types: tuple[str, ...] num_cast: str int_types: tuple[str, ...] - int_cast: str - # None for SQLite where json_type already returns 'integer'/'real'; - # regex literal for PostgreSQL where json_typeof returns 'number' for - # both ints and floats, so an extra guard prevents CAST errors on floats. - int_guard: str | None + # PostgreSQL ->> returns the JSON number spelling; SQLite json_extract + # returns a native integer or a (possibly lossy) real for large integers. + int_as_text: bool string_type: str bool_type: str | None @@ -149,8 +147,7 @@ _SQLITE = _Dialect( num_types=("integer", "real"), num_cast="REAL", int_types=("integer",), - int_cast="INTEGER", - int_guard=None, + int_as_text=False, string_type="text", bool_type=None, ) @@ -160,8 +157,7 @@ _PG = _Dialect( num_types=("number",), num_cast="DOUBLE PRECISION", int_types=("number",), - int_cast="BIGINT", - int_guard="'^-?[0-9]+$'", + int_as_text=True, string_type="string", bool_type="boolean", ) @@ -189,11 +185,19 @@ def _build_clause(compiler: SQLCompiler, typeof: str, extract: str, value: objec return f"{typeof} = '{bool_str}'" return f"({typeof} = '{dialect.bool_type}' AND {extract} = '{bool_str}')" if isinstance(value, int): - bp = _bind(compiler, value, BigInteger(), **kw) - if dialect.int_guard: - # CASE prevents CAST error when json_typeof = 'number' also matches floats - return f"(CASE WHEN {_type_check(typeof, dialect.int_types)} AND {extract} ~ {dialect.int_guard} THEN CAST({extract} AS {dialect.int_cast}) END = {bp})" - return f"({_type_check(typeof, dialect.int_types)} AND CAST({extract} AS {dialect.int_cast}) = {bp})" + if dialect.int_as_text: + # JSON integer spellings are canonical except for -0. Comparing + # text avoids overflowing BIGINT (or even NUMERIC) on stored values + # and still excludes decimal/exponent spellings from int filters. + bp = _bind(compiler, str(value), String(), **kw) + comparison = f"{extract} IN ({bp}, '-0')" if value == 0 else f"{extract} = {bp}" + else: + bp = _bind(compiler, value, BigInteger(), **kw) + # json_type reports 'integer' for oversized JSON integers too, but + # json_extract returns REAL. Casting that REAL to INTEGER clamps it + # to an int64 boundary and would create a false positive. + comparison = f"typeof({extract}) = 'integer' AND {extract} = {bp}" + return f"({_type_check(typeof, dialect.int_types)} AND {comparison})" if isinstance(value, float): bp = _bind(compiler, value, Float(), **kw) return f"({_type_check(typeof, dialect.num_types)} AND CAST({extract} AS {dialect.num_cast}) = {bp})" diff --git a/backend/tests/test_json_integer_matching.py b/backend/tests/test_json_integer_matching.py new file mode 100644 index 0000000000..c50677f4d4 --- /dev/null +++ b/backend/tests/test_json_integer_matching.py @@ -0,0 +1,64 @@ +"""Execute integer predicates against JSON values, including oversized numbers.""" + +import json +import os + +import pytest +from sqlalchemy import JSON, Column, MetaData, String, Table, select, text +from sqlalchemy.ext.asyncio import create_async_engine + +from deerflow.persistence.json_compat import json_match + + +@pytest.fixture(params=["sqlite", "postgresql"]) +async def json_table(request): + if request.param == "postgresql": + url = os.getenv("DEERFLOW_TEST_POSTGRES_URL") + if not url: + pytest.skip("set DEERFLOW_TEST_POSTGRES_URL to exercise real PostgreSQL integer matching") + if url.startswith("postgresql://"): + url = url.replace("postgresql://", "postgresql+asyncpg://", 1) + else: + url = "sqlite+aiosqlite:///:memory:" + + engine = create_async_engine(url) + # A connection-local table keeps opt-in PostgreSQL runs isolated from any + # application schema and is discarded even when an assertion fails. + table = Table("json_integer_matching", MetaData(), Column("id", String), Column("data", JSON), prefixes=["TEMPORARY"]) + try: + async with engine.connect() as connection: + async with connection.begin(): + await connection.run_sync(table.create) + yield connection, table + finally: + await engine.dispose() + + +@pytest.mark.anyio +@pytest.mark.parametrize("expected", [-(2**63), -(2**63) + 1, -42, -1, 0, 1, 42, 2**63 - 2, 2**63 - 1]) +async def test_integer_filter_ignores_unrepresentable_stored_numbers(json_table, expected): + connection, table = json_table + integers = [-(2**63), -(2**63) + 1, -42, -1, 0, 1, 42, 2**63 - 2, 2**63 - 1] + rows = [{"id": str(value), "data": json.dumps({"x": value})} for value in integers] + rows += [ + {"id": "below-min", "data": json.dumps({"x": -(2**63) - 1})}, + {"id": "above-max", "data": json.dumps({"x": 2**63})}, + {"id": "large-negative", "data": json.dumps({"x": -(10**100)})}, + {"id": "large-positive", "data": json.dumps({"x": 10**100})}, + # PostgreSQL JSON accepts numbers beyond even NUMERIC's precision. + {"id": "beyond-numeric", "data": '{"x": ' + "9" * 131073 + "}"}, + {"id": "negative-zero", "data": '{"x": -0}'}, + {"id": "float", "data": json.dumps({"x": float(expected)})}, + {"id": "exponent", "data": '{"x": 42e0}'}, + {"id": "string", "data": json.dumps({"x": str(expected)})}, + {"id": "boolean", "data": '{"x": true}'}, + {"id": "null", "data": '{"x": null}'}, + {"id": "array", "data": '{"x": [42]}'}, + {"id": "object", "data": '{"x": {}}'}, + {"id": "missing", "data": "{}"}, + ] + # Insert raw JSON to retain -0 and exponent spellings on both backends. + await connection.execute(text("INSERT INTO json_integer_matching (id, data) VALUES (:id, :data)"), rows) + result = await connection.execute(select(table.c.id).where(json_match(table.c.data, "x", expected))) + expected_ids = {str(expected)} | ({"negative-zero"} if expected == 0 else set()) + assert set(result.scalars()) == expected_ids diff --git a/backend/tests/test_thread_meta_repo.py b/backend/tests/test_thread_meta_repo.py index 6edd327e6d..2e619dd3cc 100644 --- a/backend/tests/test_thread_meta_repo.py +++ b/backend/tests/test_thread_meta_repo.py @@ -511,6 +511,16 @@ class TestThreadMetaRepository: hits = {r["thread_id"] for r in await repo.search(metadata={"id": large})} assert hits == {"t1"} + @pytest.mark.anyio + @pytest.mark.parametrize("boundary, outside", [(2**63 - 1, 2**63), (-(2**63), -(2**63) - 1)]) + async def test_search_integer_boundary_does_not_match_out_of_range_metadata(self, repo, boundary, outside): + await repo.create("exact", metadata={"id": boundary}) + await repo.create("outside", metadata={"id": outside}) + await repo.create("huge", metadata={"id": outside * 10**100}) + + hits = {record["thread_id"] for record in await repo.search(metadata={"id": boundary})} + assert hits == {"exact"} + @pytest.mark.anyio async def test_membership_exposed_via_reserved_metadata_key(self, repo): from deerflow.persistence.thread_meta import THREAD_PROJECT_METADATA_KEY @@ -754,13 +764,13 @@ class TestJsonMatchCompilation: sql = expr.compile(dialect=engine.dialect, compile_kwargs={"literal_binds": True}) assert str(sql) == expected_fragment, f"value={value!r}: {sql}" - # int: uses INTEGER cast for precision, type-check narrows to 'integer' only + # int: require a native SQL integer, not an oversized integer decoded as REAL int_expr = json_match(t.c.data, "k", 42) sql = str(int_expr.compile(dialect=engine.dialect, compile_kwargs={"literal_binds": True})) assert "json_type" in sql assert "= 'integer'" in sql - assert "INTEGER" in sql - assert "CAST" in sql + assert "typeof(json_extract" in sql + assert "CAST" not in sql # float: uses REAL cast, type-check spans 'integer' and 'real' float_expr = json_match(t.c.data, "k", 3.14) @@ -795,14 +805,13 @@ class TestJsonMatchCompilation: sql = expr.compile(dialect=dialect, compile_kwargs={"literal_binds": True}) assert str(sql) == expected_fragment, f"value={value!r}: {sql}" - # int: CASE guard prevents CAST error when 'number' also matches floats + # int: compare number spellings without casting unbounded stored values int_expr = json_match(t.c.data, "k", 42) sql = str(int_expr.compile(dialect=dialect, compile_kwargs={"literal_binds": True})) assert "json_typeof" in sql assert "'number'" in sql - assert "BIGINT" in sql - assert "CASE WHEN" in sql - assert "'^-?[0-9]+$'" in sql + assert "CAST" not in sql + assert "(t.data ->> 'k') = '42'" in sql # float: uses DOUBLE PRECISION cast float_expr = json_match(t.c.data, "k", 3.14)