fix: WS5 hazards batch - CORS, scheduler auth, reranker dedupe, wiring, write txns
- CORS: drop allow_credentials (wildcard origin + credentials told
browsers to attach credentials for any site); origins configurable via
CORS_ALLOW_ORIGINS (default * is safe without credentials). Verified
live: preflight no longer advertises access-control-allow-credentials.
- Scheduler tasks: auth moved from a plain Authorization header (which
the Scheduler's rest_api_executor does NOT env-substitute) to its
auth {type: bearer, token: ${LIBRARY_API_KEY}} block, substituted from
the Scheduler's own environment at execution time. The registrar no
longer resolves the real key client-side, so it can never be persisted
into the scheduled_tasks.config JSONB column. Also fixed: JSON bodies
moved from the ignored "body" key to "payload" (the executor only
reads config["payload"], so the tasks would have POSTed empty bodies
and failed required-user validation).
- Reranker: parsed ranking indices are deduplicated preserving first
occurrence (an LLM answer like "3,3,1" duplicated a result).
- HybridRAG wiring consolidated into dependencies.get_hybrid_rag_service
(now including volatile_service); the inline copies in /query/hybrid
and /wiki/pages/smart-create are gone - smart-create previously ran
without the volatile leg, and the singleton was unused.
- Remaining Neo4j writes (GraphService ingestion/deletes/purges/entity
mentions, webhook rename+delete cleanup, document-sync _index_graph,
consolidation mark-processed/add-entity) moved from auto-commit
execute_query to execute_write managed transactions with retry.
Verified end-to-end on the local dev server as llm_tester: /query/hybrid
200 with all five legs ok (volatile now active), background persistence
landed as one transaction.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QbFZyDvYksazX6nYQYZ67L
This commit is contained in:
@@ -18,6 +18,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
- **Weekly quality report** — `POST /maintenance/quality-report {user}` runs the duplicate scan, flags stale pages (not updated in N days AND ≤ M SearchQuery hits from the graph data), lists pages missing tags/description, folds in the latest integrity-check results (Redis-cached or run inline), and writes a dated report page to `users/{user}/system/quality-reports/YYYY-MM-DD` (same-day reruns update the same page — the page id is remembered in Redis because the Wiki.js listing lags page creation). Response returns the full report content + page path. Verified end-to-end against the local dev server as `llm_tester`.
|
||||
- **Nightly integrity check** — `POST /maintenance/integrity-check {user}` (read-only: reports, never auto-fixes) reports per tenant: wiki pages with ZERO vectors in Qdrant (silent-skip reindex victims), orphaned vectors whose wiki page no longer exists, unexpected Qdrant collections (test-tenant residue and unknown namespaces flagged; other services' collections counted as foreign), Neo4j Document nodes without wiki counterparts, plus counts and duration. The latest report is cached in Redis (30 days) so the weekly quality report can fold it in.
|
||||
|
||||
### Fixed (hazards batch)
|
||||
|
||||
- **CORS wildcard + credentials removed** - `allow_origins=["*"]` combined with `allow_credentials=True` told browsers to attach credentials for any site. Credentials are now disabled (all real callers are server-to-server and use the `Authorization` header, which wildcard-origin CORS without credentials still permits) and the origin list is configurable via `CORS_ALLOW_ORIGINS` (comma-separated, default `*`).
|
||||
- **Scheduler task auth no longer stores raw tokens** - the Phase C Scheduler task definitions put `Authorization: Bearer ${LIBRARY_API_KEY}` in plain `headers` and the registrar substituted the REAL key client-side on `--execute`, which would persist it in the Scheduler's `scheduled_tasks.config` JSONB column — and the Scheduler's `rest_api_executor` does not substitute env vars in plain headers anyway (only in `url`/`payload`/`auth`). The definitions now use the executor's `auth: {type: bearer, token: "${LIBRARY_API_KEY}"}` block, substituted from the SCHEDULER's environment at execution time; the registrar sends the placeholder verbatim and no longer needs (or accepts) the key. Also fixed: the JSON body moved from the ignored `body` key to `payload` (the executor only reads `config["payload"]`, so the tasks would have POSTed empty bodies and failed Phase B user validation).
|
||||
- **Reranker index parser dedupes** - an LLM ranking answer like `3,3,1` inserted the same result into the final ranking twice; parsed indices are now deduplicated preserving first occurrence.
|
||||
- **Single HybridRAG wiring point** - three separate constructions existed: an unused `dependencies.get_hybrid_rag_service` singleton lacking `volatile_service`, an inline per-request copy in the `/query/hybrid` router, and another inline copy in `/wiki/pages/smart-create` also lacking `volatile_service`. All callers now use the dependencies singleton, which includes `volatile_service` (smart-create research can now hit the volatile cache leg).
|
||||
- **Hot-path Neo4j writes use write transactions** - remaining graph writes ran as auto-commit `execute_query` calls (no retry, no transaction-function semantics): `GraphService` ingestion (`update_from_page` document + entity queries), `delete_page`, `create_entity_mentions`, document/paperless/collection node deletion, orphan-entity purge, stale-document purge, `cleanup_broken_relationships`; the webhook rename/delete cleanup writes; document-sync `_index_graph`; and consolidation's `_mark_search_processed`/`_add_entity_to_graph`. All now go through `execute_write` (managed transaction with driver retry). Read paths are unchanged.
|
||||
|
||||
### Changed (performance)
|
||||
|
||||
- **Content extractor hardened** - `ContentExtractor` now downloads pages with `httpx.AsyncClient` under real connect (3s) and read timeouts on the event loop; only the CPU-bound Trafilatura parse runs in the thread pool. Previously `trafilatura.fetch_url` ran inside the worker thread with no caller-side timeout control, so an `asyncio.wait_for` timeout abandoned the thread while it kept downloading for up to ~30s. Trafilatura now parses each document ONCE via `bare_extraction` (text + metadata together) — the old code ran `extract()` twice (the XML pass was computed and discarded) plus `bare_extraction`, three full parses per page. `extract_batch` caps full-page extractions per call (default 8; overflow URLs return unsuccessful so the web leg falls back to the search snippet), responses are capped at 5MB before parsing, and thread-pool queue depth is logged for backpressure visibility.
|
||||
|
||||
+27
-14
@@ -12,7 +12,6 @@ SCHEDULER_URL=http://<scheduler-host>:8090 \
|
||||
|
||||
# Actually register/update the tasks (deploy checklist step):
|
||||
SCHEDULER_URL=http://<scheduler-host>:8090 \
|
||||
LIBRARY_API_KEY=<the library-desk API key> \
|
||||
.venv/bin/python scripts/register_scheduler_tasks.py --execute
|
||||
```
|
||||
|
||||
@@ -21,8 +20,15 @@ Conventions:
|
||||
- All tasks call the **production** library-desk container
|
||||
(`http://library-desk:8089`) with the explicit production tenant
|
||||
`user=jpmschweitzer` (there is no default tenant — Phase B).
|
||||
- `${LIBRARY_API_KEY}` is a placeholder for the library-desk API key
|
||||
(`LIBRARY_API_KEY` in the container env). Never commit the real value.
|
||||
- `${LIBRARY_API_KEY}` is a literal placeholder stored in the task's
|
||||
`auth.token` field. The Scheduler's `rest_api_executor` substitutes
|
||||
`${ENV_VAR}` placeholders from **its own environment at execution
|
||||
time** (it substitutes `url`/`payload`/`auth` — NOT plain `headers`),
|
||||
so the raw key is never stored in the `scheduled_tasks.config` JSONB
|
||||
column. The **Scheduler container** must have `LIBRARY_API_KEY` in its
|
||||
environment. Never commit or register the real value.
|
||||
- The JSON body goes in `config.payload` (the executor ignores a `body`
|
||||
key).
|
||||
- Schedule fields use the Scheduler's convention: `-1` = every,
|
||||
`day_of_week`: `0` = Monday … `6` = Sunday.
|
||||
|
||||
@@ -53,11 +59,14 @@ in Redis for the weekly quality report.
|
||||
"method": "POST",
|
||||
"url": "http://library-desk:8089/maintenance/integrity-check",
|
||||
"headers": {
|
||||
"Content-Type": "application/json",
|
||||
"Authorization": "Bearer ${LIBRARY_API_KEY}"
|
||||
"Content-Type": "application/json"
|
||||
},
|
||||
"body": {
|
||||
"payload": {
|
||||
"user": "jpmschweitzer"
|
||||
},
|
||||
"auth": {
|
||||
"type": "bearer",
|
||||
"token": "${LIBRARY_API_KEY}"
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -88,14 +97,17 @@ latest integrity results, and writes the dated report page to
|
||||
"method": "POST",
|
||||
"url": "http://library-desk:8089/maintenance/quality-report",
|
||||
"headers": {
|
||||
"Content-Type": "application/json",
|
||||
"Authorization": "Bearer ${LIBRARY_API_KEY}"
|
||||
"Content-Type": "application/json"
|
||||
},
|
||||
"body": {
|
||||
"payload": {
|
||||
"user": "jpmschweitzer",
|
||||
"stale_days": 30,
|
||||
"dedup_threshold": 0.9,
|
||||
"write_page": true
|
||||
},
|
||||
"auth": {
|
||||
"type": "bearer",
|
||||
"token": "${LIBRARY_API_KEY}"
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -103,7 +115,7 @@ latest integrity results, and writes the dated report page to
|
||||
|
||||
## 3. Daily Paperless orphan cleanup — 05:00
|
||||
|
||||
Hits the **existing** cleanup endpoint (query parameters, empty body).
|
||||
Hits the **existing** cleanup endpoint (query parameters, empty payload).
|
||||
`dry_run=false` deletes vectors/graph nodes for documents that were
|
||||
removed from Paperless-ngx.
|
||||
|
||||
@@ -125,10 +137,11 @@ removed from Paperless-ngx.
|
||||
"config": {
|
||||
"method": "POST",
|
||||
"url": "http://library-desk:8089/maintenance/cleanup/paperless?user=jpmschweitzer&dry_run=false",
|
||||
"headers": {
|
||||
"Authorization": "Bearer ${LIBRARY_API_KEY}"
|
||||
},
|
||||
"body": {}
|
||||
"payload": {},
|
||||
"auth": {
|
||||
"type": "bearer",
|
||||
"token": "${LIBRARY_API_KEY}"
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
@@ -17,10 +17,13 @@ SAFETY MODEL
|
||||
if present, and disable test_example_task.
|
||||
- The Scheduler API location comes from the environment (SCHEDULER_URL);
|
||||
there is no hardcoded production default.
|
||||
- The library-desk API key is embedded as the literal placeholder
|
||||
``${LIBRARY_API_KEY}`` in dry-run output. On --execute it is replaced
|
||||
with the LIBRARY_API_KEY environment variable (required then, never
|
||||
printed).
|
||||
- NO SECRET IS EVER STORED: the library-desk API key is referenced as the
|
||||
literal placeholder ``${LIBRARY_API_KEY}`` inside the task's
|
||||
``auth.token`` field. The Scheduler's rest_api_executor substitutes
|
||||
``${ENV_VAR}`` placeholders from ITS OWN environment at execution time
|
||||
(it substitutes url/payload/auth — NOT plain headers), so the raw token
|
||||
never lands in the scheduled_tasks.config JSONB column. The Scheduler
|
||||
container must therefore have LIBRARY_API_KEY in its environment.
|
||||
|
||||
Usage:
|
||||
# Preview (default)
|
||||
@@ -28,12 +31,11 @@ Usage:
|
||||
python scripts/register_scheduler_tasks.py
|
||||
|
||||
# Register for real (deploy checklist step)
|
||||
SCHEDULER_URL=http://scheduler-host:8090 LIBRARY_API_KEY=... \
|
||||
SCHEDULER_URL=http://scheduler-host:8090 \
|
||||
python scripts/register_scheduler_tasks.py --execute
|
||||
"""
|
||||
|
||||
import argparse
|
||||
import copy
|
||||
import json
|
||||
import os
|
||||
import sys
|
||||
@@ -66,11 +68,11 @@ TASKS = [
|
||||
"config": {
|
||||
"method": "POST",
|
||||
"url": f"{LIBRARY_BASE_URL}/maintenance/integrity-check",
|
||||
"headers": {
|
||||
"Content-Type": "application/json",
|
||||
"Authorization": f"Bearer {API_KEY_PLACEHOLDER}",
|
||||
},
|
||||
"body": {"user": PRODUCTION_TENANT},
|
||||
"headers": {"Content-Type": "application/json"},
|
||||
# rest_api_executor sends config["payload"] as the JSON body and
|
||||
# substitutes ${ENV_VAR} in auth.token from the Scheduler's env.
|
||||
"payload": {"user": PRODUCTION_TENANT},
|
||||
"auth": {"type": "bearer", "token": API_KEY_PLACEHOLDER},
|
||||
},
|
||||
},
|
||||
{
|
||||
@@ -93,16 +95,14 @@ TASKS = [
|
||||
"config": {
|
||||
"method": "POST",
|
||||
"url": f"{LIBRARY_BASE_URL}/maintenance/quality-report",
|
||||
"headers": {
|
||||
"Content-Type": "application/json",
|
||||
"Authorization": f"Bearer {API_KEY_PLACEHOLDER}",
|
||||
},
|
||||
"body": {
|
||||
"headers": {"Content-Type": "application/json"},
|
||||
"payload": {
|
||||
"user": PRODUCTION_TENANT,
|
||||
"stale_days": 30,
|
||||
"dedup_threshold": 0.9,
|
||||
"write_page": True,
|
||||
},
|
||||
"auth": {"type": "bearer", "token": API_KEY_PLACEHOLDER},
|
||||
},
|
||||
},
|
||||
{
|
||||
@@ -128,8 +128,8 @@ TASKS = [
|
||||
f"{LIBRARY_BASE_URL}/maintenance/cleanup/paperless"
|
||||
f"?user={PRODUCTION_TENANT}&dry_run=false"
|
||||
),
|
||||
"headers": {"Authorization": f"Bearer {API_KEY_PLACEHOLDER}"},
|
||||
"body": {},
|
||||
"payload": {},
|
||||
"auth": {"type": "bearer", "token": API_KEY_PLACEHOLDER},
|
||||
},
|
||||
},
|
||||
]
|
||||
@@ -140,16 +140,6 @@ TASK_UPDATES = [
|
||||
]
|
||||
|
||||
|
||||
def substitute_api_key(task: dict, api_key: str) -> dict:
|
||||
"""Return a deep copy of the task with the API-key placeholder filled in."""
|
||||
resolved = copy.deepcopy(task)
|
||||
headers = resolved.get("config", {}).get("headers", {})
|
||||
for name, value in headers.items():
|
||||
if API_KEY_PLACEHOLDER in value:
|
||||
headers[name] = value.replace(API_KEY_PLACEHOLDER, api_key)
|
||||
return resolved
|
||||
|
||||
|
||||
def dry_run(scheduler_url: str) -> None:
|
||||
print("=" * 72)
|
||||
print("DRY RUN - nothing will be sent. Re-run with --execute to register.")
|
||||
@@ -166,7 +156,7 @@ def dry_run(scheduler_url: str) -> None:
|
||||
f"{len(TASKS)} task definition(s), {len(TASK_UPDATES)} update(s).")
|
||||
|
||||
|
||||
def execute(scheduler_url: str, api_key: str) -> int:
|
||||
def execute(scheduler_url: str) -> int:
|
||||
failures = 0
|
||||
with httpx.Client(base_url=scheduler_url, timeout=30.0) as client:
|
||||
health = client.get("/health")
|
||||
@@ -176,7 +166,9 @@ def execute(scheduler_url: str, api_key: str) -> int:
|
||||
|
||||
for task in TASKS:
|
||||
name = task["task_name"]
|
||||
payload = substitute_api_key(task, api_key)
|
||||
# Sent verbatim: the ${LIBRARY_API_KEY} placeholder is resolved
|
||||
# by the Scheduler at execution time, never stored as a raw key.
|
||||
payload = task
|
||||
exists = client.get(f"/tasks/{name}").status_code == 200
|
||||
if exists:
|
||||
resp = client.put(f"/tasks/{name}", json=payload)
|
||||
@@ -231,13 +223,8 @@ def main() -> int:
|
||||
if not scheduler_url:
|
||||
print("ERROR: SCHEDULER_URL must be set for --execute")
|
||||
return 1
|
||||
api_key = os.environ.get("LIBRARY_API_KEY", "")
|
||||
if not api_key:
|
||||
print("ERROR: LIBRARY_API_KEY must be set for --execute "
|
||||
"(it fills the Authorization header placeholder)")
|
||||
return 1
|
||||
|
||||
return execute(scheduler_url, api_key)
|
||||
return execute(scheduler_url)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
|
||||
@@ -95,6 +95,18 @@ class Settings(BaseSettings):
|
||||
app_version: str = Field(default=__version__, description="Application version")
|
||||
debug: bool = Field(default=False, description="Debug mode")
|
||||
|
||||
# CORS: comma-separated list of allowed browser origins. The default "*"
|
||||
# is only acceptable because allow_credentials is disabled (see main.py).
|
||||
cors_allow_origins: str = Field(
|
||||
default="*",
|
||||
description="Comma-separated CORS allowed origins (credentials are never allowed)"
|
||||
)
|
||||
|
||||
@property
|
||||
def cors_allow_origins_list(self) -> list[str]:
|
||||
"""cors_allow_origins parsed into a list for CORSMiddleware."""
|
||||
return [o.strip() for o in self.cors_allow_origins.split(",") if o.strip()]
|
||||
|
||||
# RAG Search Configuration
|
||||
search_cache_ttl: int = Field(default=300, ge=0, le=3600, description="Search cache TTL in seconds")
|
||||
search_timeout: int = Field(default=10, ge=1, le=60, description="SearXNG timeout in seconds")
|
||||
|
||||
@@ -718,7 +718,14 @@ def get_ingestion_service() -> "IngestionService":
|
||||
|
||||
@lru_cache
|
||||
def get_hybrid_rag_service() -> "HybridRAGService":
|
||||
"""Get HybridRAGService singleton."""
|
||||
"""
|
||||
Get HybridRAGService singleton.
|
||||
|
||||
The single wiring point for HybridRAG — routers must depend on this
|
||||
instead of constructing their own instance (previous inline copies in
|
||||
the /query/hybrid and /wiki/smart-create routers diverged on
|
||||
volatile_service).
|
||||
"""
|
||||
from src.services.hybrid_rag_service import HybridRAGService
|
||||
return HybridRAGService(
|
||||
vector_service=get_vector_service(),
|
||||
@@ -726,7 +733,8 @@ def get_hybrid_rag_service() -> "HybridRAGService":
|
||||
searxng_client=get_searxng_client(),
|
||||
ollama_client=get_ollama_client(),
|
||||
content_extractor=get_content_extractor(),
|
||||
settings=get_settings()
|
||||
settings=get_settings(),
|
||||
volatile_service=get_volatile_cache_service()
|
||||
)
|
||||
|
||||
|
||||
|
||||
+10
-3
@@ -39,11 +39,18 @@ app = FastAPI(
|
||||
redoc_url="/redoc",
|
||||
)
|
||||
|
||||
# CORS middleware
|
||||
# CORS middleware.
|
||||
# allow_credentials is deliberately False: combined with a wildcard origin it
|
||||
# would tell browsers to attach cookies/credentials for ANY site, which is the
|
||||
# classic CORS misconfiguration. All real callers (tatlock, the Scheduler) are
|
||||
# server-to-server and use the Authorization header, which wildcard-origin
|
||||
# CORS without credentials still permits. Origins can be restricted via the
|
||||
# CORS_ALLOW_ORIGINS env (comma-separated) once a cross-origin browser UI
|
||||
# exists; the bundled static UI is served same-origin and needs no CORS.
|
||||
app.add_middleware(
|
||||
CORSMiddleware,
|
||||
allow_origins=["*"], # Configure appropriately for production
|
||||
allow_credentials=True,
|
||||
allow_origins=get_settings().cors_allow_origins_list,
|
||||
allow_credentials=False,
|
||||
allow_methods=["*"],
|
||||
allow_headers=["*"],
|
||||
)
|
||||
|
||||
@@ -11,49 +11,14 @@ import logging
|
||||
from src.models.hybrid_rag import HybridRAGRequest, HybridRAGResponse
|
||||
from src.services.hybrid_rag_service import HybridRAGService
|
||||
from src.core.dependencies import (
|
||||
Neo4jDep, WikiJSDep, QdrantDep, OllamaDep,
|
||||
SearXNGDep, ContentExtractorDep, verify_api_key, get_settings,
|
||||
RequiredUserQuery
|
||||
verify_api_key, get_hybrid_rag_service, RequiredUserQuery
|
||||
)
|
||||
from src.config import Settings
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
router = APIRouter(prefix="/query", tags=["HybridRAG"])
|
||||
|
||||
|
||||
# Dependency to get HybridRAG service
|
||||
def get_hybrid_rag_service(
|
||||
neo4j_client: Neo4jDep,
|
||||
wiki_client: WikiJSDep,
|
||||
qdrant_client: QdrantDep,
|
||||
ollama_client: OllamaDep,
|
||||
searxng_client: SearXNGDep,
|
||||
content_extractor: ContentExtractorDep,
|
||||
settings: Settings = Depends(get_settings)
|
||||
) -> HybridRAGService:
|
||||
"""Get HybridRAG service instance with all dependencies."""
|
||||
from src.services.vector_service import VectorService
|
||||
from src.services.graph_service import GraphService
|
||||
from src.services.volatile_service import VolatileCacheService
|
||||
|
||||
# Create component services
|
||||
vector_service = VectorService(qdrant_client, wiki_client, ollama_client)
|
||||
graph_service = GraphService(neo4j_client, wiki_client)
|
||||
volatile_service = VolatileCacheService(qdrant_client, ollama_client, settings)
|
||||
|
||||
# Create HybridRAG service
|
||||
return HybridRAGService(
|
||||
vector_service=vector_service,
|
||||
graph_service=graph_service,
|
||||
searxng_client=searxng_client,
|
||||
ollama_client=ollama_client,
|
||||
content_extractor=content_extractor,
|
||||
settings=settings,
|
||||
volatile_service=volatile_service
|
||||
)
|
||||
|
||||
|
||||
@router.post("/hybrid", response_model=HybridRAGResponse)
|
||||
async def hybrid_search(
|
||||
request: HybridRAGRequest,
|
||||
|
||||
@@ -320,7 +320,7 @@ async def process_page_rename(
|
||||
"""
|
||||
|
||||
try:
|
||||
await neo4j.execute_query(update_query, {
|
||||
await neo4j.execute_write(update_query, {
|
||||
"page_id": page_id,
|
||||
"new_path": new_path,
|
||||
"new_title": new_title
|
||||
@@ -441,7 +441,7 @@ async def cleanup_deleted_page(
|
||||
"""
|
||||
|
||||
try:
|
||||
result = await neo4j.execute_query(delete_doc_query, {"page_id": page_id})
|
||||
result = await neo4j.execute_write(delete_doc_query, {"page_id": page_id})
|
||||
deleted_count = result[0]["deleted_count"] if result else 0
|
||||
logger.info(f"Deleted {deleted_count} Document node(s) for page {page_id}")
|
||||
except Exception as e:
|
||||
@@ -462,7 +462,7 @@ async def cleanup_deleted_page(
|
||||
"""
|
||||
|
||||
try:
|
||||
result = await neo4j.execute_query(delete_entity_query, {"entity_id": entity_id})
|
||||
result = await neo4j.execute_write(delete_entity_query, {"entity_id": entity_id})
|
||||
deleted = result[0]["deleted_count"] if result else 0
|
||||
if deleted > 0:
|
||||
logger.info(f"Deleted orphaned entity: {entity_name}")
|
||||
@@ -478,7 +478,7 @@ async def cleanup_deleted_page(
|
||||
"""
|
||||
|
||||
try:
|
||||
result = await neo4j.execute_query(cleanup_search_query, {})
|
||||
result = await neo4j.execute_write(cleanup_search_query, {})
|
||||
cleaned = result[0]["cleaned_count"] if result else 0
|
||||
if cleaned > 0:
|
||||
logger.info(f"Cleaned up {cleaned} broken SearchQuery relationships")
|
||||
|
||||
+5
-13
@@ -23,11 +23,10 @@ from src.clients.neo4j_client import Neo4jClient
|
||||
from src.clients.qdrant_client import QdrantClientWrapper
|
||||
from src.clients.ollama_client import OllamaClient
|
||||
from src.core.dependencies import (
|
||||
WikiJSDep, Neo4jDep, QdrantDep, OllamaDep, SearXNGDep, ContentExtractorDep,
|
||||
WikiJSDep, Neo4jDep, QdrantDep, OllamaDep,
|
||||
verify_api_key, get_settings, get_hybrid_rag_service, get_ingestion_service,
|
||||
RequiredUserQuery
|
||||
)
|
||||
from src.services.hybrid_rag_service import HybridRAGService
|
||||
from src.services.wiki_page_writer import WikiPageWriter
|
||||
from src.services.entity_linking_utils import apply_bidirectional_entity_linking
|
||||
from src.config import Settings
|
||||
@@ -180,8 +179,6 @@ async def smart_create_page(
|
||||
neo4j_client: Neo4jDep,
|
||||
qdrant_client: QdrantDep,
|
||||
ollama_client: OllamaDep,
|
||||
searxng_client: SearXNGDep,
|
||||
content_extractor: ContentExtractorDep,
|
||||
settings: Settings = Depends(get_settings),
|
||||
api_key: str = Depends(verify_api_key)
|
||||
):
|
||||
@@ -218,18 +215,13 @@ async def smart_create_page(
|
||||
try:
|
||||
user = request.user
|
||||
|
||||
# Build services
|
||||
# Build services. HybridRAG comes from the single wiring point in
|
||||
# dependencies so it includes volatile_service (a previous inline
|
||||
# copy here lacked it).
|
||||
wiki_service = WikiService(wiki_client)
|
||||
vector_service = VectorService(qdrant_client, wiki_client, ollama_client)
|
||||
graph_service = GraphService(neo4j_client, wiki_client)
|
||||
hybrid_rag_service = HybridRAGService(
|
||||
vector_service=vector_service,
|
||||
graph_service=graph_service,
|
||||
searxng_client=searxng_client,
|
||||
ollama_client=ollama_client,
|
||||
content_extractor=content_extractor,
|
||||
settings=settings
|
||||
)
|
||||
hybrid_rag_service = get_hybrid_rag_service()
|
||||
wiki_page_writer = WikiPageWriter(ollama_client=ollama_client, settings=settings)
|
||||
|
||||
# Step 1-5: Research + Generate + Create page
|
||||
|
||||
@@ -613,7 +613,7 @@ JSON:"""
|
||||
"""
|
||||
|
||||
try:
|
||||
await self.neo4j.execute_query(query, {"search_id": search_id})
|
||||
await self.neo4j.execute_write(query, {"search_id": search_id})
|
||||
logger.debug(f"Marked search {search_id} as processed")
|
||||
except Exception as e:
|
||||
logger.error(f"Failed to mark search as processed: {e}")
|
||||
@@ -1006,7 +1006,7 @@ JSON:"""
|
||||
"""
|
||||
|
||||
try:
|
||||
await self.neo4j.execute_query(query, {
|
||||
await self.neo4j.execute_write(query, {
|
||||
"name": entity_name,
|
||||
"description": description,
|
||||
"search_id": source_search_id
|
||||
|
||||
@@ -281,7 +281,7 @@ class DocumentSyncService:
|
||||
d.updated_at = datetime()
|
||||
RETURN d
|
||||
"""
|
||||
await self.neo4j.execute_query(
|
||||
await self.neo4j.execute_write(
|
||||
query,
|
||||
{
|
||||
"paperless_id": document_id,
|
||||
|
||||
@@ -469,7 +469,7 @@ class GraphService:
|
||||
RETURN d
|
||||
"""
|
||||
|
||||
await self.neo4j.execute_query(doc_query, {
|
||||
await self.neo4j.execute_write(doc_query, {
|
||||
"page_id": page_id,
|
||||
"title": page.get("title"),
|
||||
"path": page.get("path"),
|
||||
@@ -493,7 +493,7 @@ class GraphService:
|
||||
RETURN e, r
|
||||
"""
|
||||
|
||||
result = await self.neo4j.execute_query(entity_query, {
|
||||
result = await self.neo4j.execute_write(entity_query, {
|
||||
"name": entity.text,
|
||||
"page_id": page_id,
|
||||
"confidence": entity.confidence
|
||||
@@ -557,7 +557,7 @@ class GraphService:
|
||||
"""
|
||||
|
||||
try:
|
||||
result = await self.neo4j.execute_query(
|
||||
result = await self.neo4j.execute_write(
|
||||
delete_query,
|
||||
{"page_id": page_id}
|
||||
)
|
||||
@@ -1353,7 +1353,7 @@ Feel free to expand it with more details!
|
||||
"""
|
||||
|
||||
try:
|
||||
results = await self.neo4j.execute_query(
|
||||
results = await self.neo4j.execute_write(
|
||||
query,
|
||||
{"page_id": page_id, "entity_names": names}
|
||||
)
|
||||
@@ -1392,7 +1392,7 @@ Feel free to expand it with more details!
|
||||
"""
|
||||
|
||||
try:
|
||||
result = await self.neo4j.execute_query(
|
||||
result = await self.neo4j.execute_write(
|
||||
delete_query,
|
||||
{"document_id": document_id}
|
||||
)
|
||||
@@ -1434,7 +1434,7 @@ Feel free to expand it with more details!
|
||||
"""
|
||||
|
||||
try:
|
||||
result = await self.neo4j.execute_query(
|
||||
result = await self.neo4j.execute_write(
|
||||
delete_query,
|
||||
{"paperless_id": paperless_id}
|
||||
)
|
||||
@@ -1478,7 +1478,7 @@ Feel free to expand it with more details!
|
||||
"""
|
||||
|
||||
try:
|
||||
result = await self.neo4j.execute_query(
|
||||
result = await self.neo4j.execute_write(
|
||||
delete_query,
|
||||
{"collection_id": collection_id}
|
||||
)
|
||||
@@ -1568,7 +1568,7 @@ Feel free to expand it with more details!
|
||||
"""
|
||||
|
||||
try:
|
||||
results = await self.neo4j.execute_query(query, {})
|
||||
results = await self.neo4j.execute_write(query, {})
|
||||
purged_count = results[0]["purged_count"] if results else 0
|
||||
|
||||
logger.info(f"Purged {purged_count} orphan entities for user {user}")
|
||||
@@ -1651,7 +1651,7 @@ Feel free to expand it with more details!
|
||||
DETACH DELETE d
|
||||
RETURN count(d) as purged_count
|
||||
"""
|
||||
results = await self.neo4j.execute_query(query, {"page_ids": page_ids})
|
||||
results = await self.neo4j.execute_write(query, {"page_ids": page_ids})
|
||||
count = results[0]["purged_count"] if results else 0
|
||||
total_purged += count
|
||||
logger.info(f"Purged {count} wiki Document nodes")
|
||||
@@ -1664,7 +1664,7 @@ Feel free to expand it with more details!
|
||||
DETACH DELETE d
|
||||
RETURN count(d) as purged_count
|
||||
"""
|
||||
results = await self.neo4j.execute_query(query, {"document_ids": document_ids})
|
||||
results = await self.neo4j.execute_write(query, {"document_ids": document_ids})
|
||||
count = results[0]["purged_count"] if results else 0
|
||||
total_purged += count
|
||||
logger.info(f"Purged {count} Document Store Document nodes")
|
||||
@@ -1702,7 +1702,7 @@ Feel free to expand it with more details!
|
||||
"""
|
||||
|
||||
try:
|
||||
results = await self.neo4j.execute_query(query, {})
|
||||
results = await self.neo4j.execute_write(query, {})
|
||||
cleaned_count = results[0]["cleaned_count"] if results else 0
|
||||
|
||||
if cleaned_count > 0:
|
||||
|
||||
@@ -864,9 +864,15 @@ Ranking:"""
|
||||
timeout=LLM_CALL_TIMEOUT_SECONDS
|
||||
)
|
||||
|
||||
# Parse response: "3,1,5,2,4" → [2, 0, 4, 1, 3] (0-indexed)
|
||||
# Parse response: "3,1,5,2,4" → [2, 0, 4, 1, 3] (0-indexed).
|
||||
# Deduplicated preserving first occurrence: an LLM answer like
|
||||
# "3,3,1" must not put the same result in the ranking twice.
|
||||
indices_str = response.strip().split('\n')[0] # Take first line
|
||||
indices = [int(x.strip()) - 1 for x in indices_str.split(",") if x.strip().isdigit()]
|
||||
indices = list(dict.fromkeys(
|
||||
int(x.strip()) - 1
|
||||
for x in indices_str.split(",")
|
||||
if x.strip().isdigit()
|
||||
))
|
||||
|
||||
# Reorder results according to LLM ranking
|
||||
reranked = []
|
||||
|
||||
@@ -47,6 +47,7 @@ def mock_neo4j():
|
||||
"""Mock Neo4j client."""
|
||||
mock = AsyncMock()
|
||||
mock.execute_query = AsyncMock()
|
||||
mock.execute_write = AsyncMock()
|
||||
return mock
|
||||
|
||||
|
||||
@@ -520,8 +521,8 @@ async def test_mark_search_processed(consolidation_service, mock_neo4j):
|
||||
"""Test marking search as processed."""
|
||||
await consolidation_service._mark_search_processed(TEST_SEARCH_ID)
|
||||
|
||||
mock_neo4j.execute_query.assert_called_once()
|
||||
call_args = mock_neo4j.execute_query.call_args
|
||||
mock_neo4j.execute_write.assert_called_once()
|
||||
call_args = mock_neo4j.execute_write.call_args
|
||||
assert TEST_SEARCH_ID in str(call_args)
|
||||
|
||||
|
||||
|
||||
@@ -67,6 +67,7 @@ def _make_service(searches, ollama_response):
|
||||
return []
|
||||
|
||||
neo4j.execute_query = AsyncMock(side_effect=fake_query)
|
||||
neo4j.execute_write = AsyncMock(side_effect=fake_query)
|
||||
|
||||
ollama = AsyncMock()
|
||||
ollama.generate_text = AsyncMock(return_value=ollama_response)
|
||||
|
||||
@@ -42,6 +42,7 @@ def mock_ollama():
|
||||
def mock_neo4j():
|
||||
neo4j = MagicMock()
|
||||
neo4j.execute_query = AsyncMock(return_value=[])
|
||||
neo4j.execute_write = AsyncMock(return_value=[])
|
||||
return neo4j
|
||||
|
||||
|
||||
|
||||
@@ -27,6 +27,7 @@ def mock_neo4j():
|
||||
"""Mock Neo4j client."""
|
||||
mock = AsyncMock()
|
||||
mock.execute_query = AsyncMock(return_value=[{"d": {"page_id": TEST_PAGE_ID}}])
|
||||
mock.execute_write = AsyncMock(return_value=[{"d": {"page_id": TEST_PAGE_ID}}])
|
||||
return mock
|
||||
|
||||
|
||||
@@ -89,12 +90,12 @@ class TestDocumentNodeCreation:
|
||||
user=TEST_USER
|
||||
)
|
||||
|
||||
# Verify execute_query was called
|
||||
assert mock_neo4j.execute_query.called
|
||||
# Verify the write transaction was used
|
||||
assert mock_neo4j.execute_write.called
|
||||
assert result.success is True
|
||||
|
||||
# Find the document creation query
|
||||
calls = mock_neo4j.execute_query.call_args_list
|
||||
calls = mock_neo4j.execute_write.call_args_list
|
||||
doc_creation_call = None
|
||||
for call in calls:
|
||||
query = call[0][0] if call[0] else ""
|
||||
@@ -128,7 +129,7 @@ class TestDocumentNodeCreation:
|
||||
assert result.success is True
|
||||
|
||||
# Find the document creation query
|
||||
calls = mock_neo4j.execute_query.call_args_list
|
||||
calls = mock_neo4j.execute_write.call_args_list
|
||||
doc_creation_call = None
|
||||
for call in calls:
|
||||
query = call[0][0] if call[0] else ""
|
||||
@@ -171,6 +172,7 @@ class TestEntityStubSkipping:
|
||||
assert result.success is True
|
||||
# Neo4j should NOT be called for entity-stub pages
|
||||
assert mock_neo4j.execute_query.call_count == 0
|
||||
assert mock_neo4j.execute_write.call_count == 0
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_skip_auto_generated_pages(
|
||||
@@ -196,6 +198,7 @@ class TestEntityStubSkipping:
|
||||
|
||||
assert result.success is True
|
||||
assert mock_neo4j.execute_query.call_count == 0
|
||||
assert mock_neo4j.execute_write.call_count == 0
|
||||
|
||||
|
||||
class TestPageNotFound:
|
||||
@@ -242,7 +245,7 @@ class TestEntityExtraction:
|
||||
|
||||
assert result.success is True
|
||||
# Should have called neo4j at least once (for document node)
|
||||
assert mock_neo4j.execute_query.called
|
||||
assert mock_neo4j.execute_write.called
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
|
||||
+20
-11
@@ -83,11 +83,20 @@ class TestSchedulerTaskDefinitions:
|
||||
mod = self._load_module()
|
||||
for task in mod.TASKS:
|
||||
config = task["config"]
|
||||
auth = config["headers"]["Authorization"]
|
||||
assert auth == f"Bearer {mod.API_KEY_PLACEHOLDER}"
|
||||
# Explicit production tenant in body or query string (Phase B)
|
||||
body_user = config.get("body", {}).get("user")
|
||||
assert body_user == "jpmschweitzer" or "user=jpmschweitzer" in config["url"]
|
||||
# Auth goes through the executor's auth block so the Scheduler
|
||||
# substitutes ${LIBRARY_API_KEY} from ITS environment at
|
||||
# execution time (plain headers are NOT substituted).
|
||||
assert config["auth"] == {
|
||||
"type": "bearer",
|
||||
"token": mod.API_KEY_PLACEHOLDER,
|
||||
}
|
||||
assert "Authorization" not in config.get("headers", {})
|
||||
# The executor sends config["payload"] as the JSON body ("body"
|
||||
# would be silently ignored)
|
||||
assert "body" not in config
|
||||
# Explicit production tenant in payload or query string (Phase B)
|
||||
payload_user = config.get("payload", {}).get("user")
|
||||
assert payload_user == "jpmschweitzer" or "user=jpmschweitzer" in config["url"]
|
||||
|
||||
def test_paperless_task_hits_existing_endpoint(self):
|
||||
mod = self._load_module()
|
||||
@@ -96,13 +105,13 @@ class TestSchedulerTaskDefinitions:
|
||||
assert "/maintenance/cleanup/paperless" in task["config"]["url"]
|
||||
assert "dry_run=false" in task["config"]["url"]
|
||||
|
||||
def test_substitute_api_key_replaces_placeholder_without_mutating(self):
|
||||
def test_no_client_side_key_substitution(self):
|
||||
"""The raw API key must never be resolved client-side — that would
|
||||
store it hardcoded in the Scheduler's scheduled_tasks.config."""
|
||||
mod = self._load_module()
|
||||
original = mod.TASKS[0]
|
||||
resolved = mod.substitute_api_key(original, "sekret")
|
||||
assert resolved["config"]["headers"]["Authorization"] == "Bearer sekret"
|
||||
# The module-level definition keeps the placeholder
|
||||
assert mod.API_KEY_PLACEHOLDER in original["config"]["headers"]["Authorization"]
|
||||
assert not hasattr(mod, "substitute_api_key")
|
||||
for task in mod.TASKS:
|
||||
assert mod.API_KEY_PLACEHOLDER in task["config"]["auth"]["token"]
|
||||
|
||||
def test_dry_run_is_default_and_sends_nothing(self, capsys, monkeypatch):
|
||||
mod = self._load_module()
|
||||
|
||||
@@ -110,9 +110,11 @@ def hybrid_service(vector_service, graph_service, volatile_service, mock_ollama)
|
||||
|
||||
|
||||
def _all_cypher(mock_neo4j) -> str:
|
||||
"""Concatenate all Cypher sent to the mocked Neo4j client."""
|
||||
"""Concatenate all Cypher sent to the mocked Neo4j client (reads + writes)."""
|
||||
return "\n".join(
|
||||
str(call.args[0]) for call in mock_neo4j.execute_query.await_args_list
|
||||
str(call.args[0])
|
||||
for mock in (mock_neo4j.execute_query, mock_neo4j.execute_write)
|
||||
for call in mock.await_args_list
|
||||
)
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user