fix(search): reject non-positive limit and survive highlight overflow (#898)

* fix(search): reject non-positive limit and survive highlight overflow

- SearchRequest.limit now has ge=1 so 0/negative values are rejected with
  422 instead of flowing into SurrealDB as LIMIT -1/0 (500 or empty set) (#863)
- text_search() catches SurrealDB's "position overflow" highlight error and
  falls back to vector_search() so large/multi-byte chunks no longer 500 (#648)
- add tests covering limit validation and the overflow fallback path

* fix(search): surface error when vector fallback also fails

Address cubic review on #898:
- text_search() no longer returns [] when the vector-search fallback also
  fails; it logs the traceback and raises DatabaseOperationError so a total
  search outage is not masked as a legitimate empty result set
- document the fallback + failure behavior in domain/CLAUDE.md
- update the test to assert the double-failure path raises

CHANGELOG.md

@@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 - `POST /sources/{id}/retry` no longer returns `400 "Source is not associated with any notebooks"` for every source; it now queries the `reference` graph edge by its `in`/`out` columns instead of a non-existent `source` column (#861)
+- Text search no longer returns a 500 when SurrealDB's `search::highlight` hits a "position overflow" on large or multi-byte document chunks; it now falls back to vector search and returns results (#648)
+- `POST /api/search` now rejects a non-positive `limit` with a `422` instead of passing `LIMIT -1`/`LIMIT 0` to SurrealDB (which caused a 500 or a silently empty result set) (#863)
 
 ### Performance
 - Notebook source list no longer re-renders every `SourceCard` on unrelated state changes (layout toggles, context selection), and completed sources no longer each open a status-polling query. Both scaled with the number of sources and caused UI lag on large notebooks (#503)

api/models.py

@@ -32,7 +32,7 @@ class NotebookResponse(BaseModel):
 class SearchRequest(BaseModel):
     query: str = Field(..., description="Search query")
     type: Literal["text", "vector"] = Field("text", description="Search type")
-    limit: int = Field(100, description="Maximum number of results", le=1000)
+    limit: int = Field(100, description="Maximum number of results", ge=1, le=1000)
     search_sources: bool = Field(True, description="Include sources in search")
     search_notes: bool = Field(True, description="Include notes in search")
     minimum_score: float = Field(

open_notebook/domain/CLAUDE.md

@@ -43,7 +43,7 @@ Two base classes support different persistence patterns: **ObjectModel** (mutabl
 - **Asset**: File/URL reference helper
 
 - **Search functions**:
-  - `text_search()`: Full-text keyword search
+  - `text_search()`: Full-text keyword search. On a SurrealDB `search::highlight` "position overflow" (large/multi-byte chunks) it falls back to `vector_search()`; if that also fails it raises `DatabaseOperationError` (never silently returns an empty list)
   - `vector_search()`: Semantic search via embeddings (default minimum_score=0.2)
 
 ### content_settings.py

open_notebook/domain/notebook.py

@@ -707,6 +707,28 @@ async def text_search(
             {"keyword": keyword, "results": results, "source": source, "note": note},
         )
         return search_results
+    except RuntimeError as e:
+        # SurrealDB's search::highlight can compute a byte position that exceeds the
+        # stored string length on large or multi-byte chunks, aborting the whole query
+        # ("position overflow"). Fall back to vector search so the user still gets
+        # results instead of a 500. See issue #648.
+        if "position overflow" in str(e):
+            logger.warning(
+                f"Highlight position overflow, falling back to vector search: {str(e)}"
+            )
+            try:
+                return await vector_search(keyword, results, source, note)
+            except Exception as ve:
+                # Both search paths failed (e.g. no embedding model configured).
+                # Surface the failure instead of returning [] — an empty list would
+                # be indistinguishable from a legitimate "no matches" and mask a
+                # total search outage from callers.
+                logger.error(f"Vector search fallback also failed: {str(ve)}")
+                logger.exception(ve)
+                raise DatabaseOperationError(ve)
+        logger.error(f"Error performing text search: {str(e)}")
+        logger.exception(e)
+        raise DatabaseOperationError(e)
     except Exception as e:
         logger.error(f"Error performing text search: {str(e)}")
         logger.exception(e)

tests/test_search_api.py

@@ -0,0 +1,104 @@
+from unittest.mock import AsyncMock, patch
+
+import pytest
+from fastapi.testclient import TestClient
+
+
+@pytest.fixture
+def client():
+    """Create test client after environment variables have been cleared by conftest."""
+    from api.main import app
+
+    return TestClient(app)
+
+
+class TestSearchLimitValidation:
+    """SearchRequest.limit must reject non-positive values (#863)."""
+
+    @pytest.mark.parametrize("bad_limit", [0, -1, -100])
+    def test_non_positive_limit_returns_422(self, bad_limit, client):
+        response = client.post(
+            "/api/search",
+            json={"query": "x", "type": "text", "limit": bad_limit},
+        )
+        assert response.status_code == 422
+
+    def test_limit_above_max_returns_422(self, client):
+        response = client.post(
+            "/api/search",
+            json={"query": "x", "type": "text", "limit": 1001},
+        )
+        assert response.status_code == 422
+
+    @patch("api.routers.search.text_search", new_callable=AsyncMock)
+    def test_valid_limit_returns_200(self, mock_text_search, client):
+        mock_text_search.return_value = []
+        response = client.post(
+            "/api/search",
+            json={"query": "x", "type": "text", "limit": 10},
+        )
+        assert response.status_code == 200
+        mock_text_search.assert_awaited_once()
+
+
+class TestTextSearchHighlightOverflowFallback:
+    """text_search() must fall back to vector search on a highlight position overflow (#648)."""
+
+    @pytest.mark.asyncio
+    async def test_position_overflow_falls_back_to_vector_search(self):
+        from open_notebook.domain import notebook as notebook_module
+
+        overflow = RuntimeError(
+            "A value can't be highlighted: position overflow: 2545 - len: 1965"
+        )
+        with (
+            patch.object(
+                notebook_module, "repo_query", new_callable=AsyncMock, side_effect=overflow
+            ),
+            patch.object(
+                notebook_module,
+                "vector_search",
+                new_callable=AsyncMock,
+                return_value=[{"id": "source:1"}],
+            ) as mock_vector,
+        ):
+            result = await notebook_module.text_search("hello", 10)
+
+        assert result == [{"id": "source:1"}]
+        mock_vector.assert_awaited_once_with("hello", 10, True, True)
+
+    @pytest.mark.asyncio
+    async def test_position_overflow_raises_when_vector_also_fails(self):
+        from open_notebook.domain import notebook as notebook_module
+        from open_notebook.exceptions import DatabaseOperationError
+
+        overflow = RuntimeError("position overflow: 1 - len: 0")
+        with (
+            patch.object(
+                notebook_module, "repo_query", new_callable=AsyncMock, side_effect=overflow
+            ),
+            patch.object(
+                notebook_module,
+                "vector_search",
+                new_callable=AsyncMock,
+                side_effect=Exception("no embedding model"),
+            ),
+        ):
+            # When both search paths fail, surface the error rather than masking it
+            # as an empty result set.
+            with pytest.raises(DatabaseOperationError):
+                await notebook_module.text_search("hello", 10)
+
+    @pytest.mark.asyncio
+    async def test_other_runtime_errors_still_raise(self):
+        from open_notebook.domain import notebook as notebook_module
+        from open_notebook.exceptions import DatabaseOperationError
+
+        with patch.object(
+            notebook_module,
+            "repo_query",
+            new_callable=AsyncMock,
+            side_effect=RuntimeError("some other db failure"),
+        ):
+            with pytest.raises(DatabaseOperationError):
+                await notebook_module.text_search("hello", 10)