mirror of
https://github.com/pewdiepie-archdaemon/odysseus.git
synced 2026-08-05 02:45:28 +00:00
feat(memory): add Memory Graph API — GET /api/memory/graph + manual links
New routes/memory/memory_graph_routes.py, mounted in app.py:
- GET /api/memory/graph — owner-scoped node/edge graph (require_user,
no new privilege beyond existing GET /api/memory).
- GET /api/memory/graph/{id}/neighbors — lazy single-node expansion for
graphs beyond the response limit.
- POST /api/memory/{id}/links, DELETE /api/memory/{id}/links/{target_id}
— manual relationship editing, gated by can_manage_memory like other
memory mutations, reusing the existing 404-on-owner-mismatch pattern.
Must be included before memory_router in app.py: memory_routes.py's
GET/PUT/DELETE /api/memory/{memory_id} wildcard would otherwise swallow
GET /api/memory/graph, since Starlette matches routes in registration
order, not by specificity. A dedicated TestClient-based regression test
(test_memory_graph_route_ordering.py) locks this in — the repo's usual
"call the endpoint function directly" test style can't catch this class
of bug since it looks up routes by exact path string, not by simulating
real request matching.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
177ac60678
commit
92327b60ad
4 changed files with 424 additions and 0 deletions
7
app.py
7
app.py
|
|
@ -666,6 +666,13 @@ app.include_router(setup_session_routes(
|
|||
from routes.admin_wipe.admin_wipe_routes import setup_admin_wipe_routes
|
||||
app.include_router(setup_admin_wipe_routes(session_manager))
|
||||
|
||||
# Memory Graph View (beta) — MUST be included before memory_router below.
|
||||
# memory_routes.py's GET/PUT/DELETE /api/memory/{memory_id} wildcard would
|
||||
# otherwise swallow GET /api/memory/graph, since Starlette matches routes in
|
||||
# registration order across the whole app, not by specificity.
|
||||
from routes.memory.memory_graph_routes import setup_memory_graph_routes
|
||||
app.include_router(setup_memory_graph_routes(memory_manager, memory_vector=memory_vector))
|
||||
|
||||
# Memory
|
||||
from routes.memory.memory_routes import setup_memory_routes
|
||||
memory_router = setup_memory_routes(memory_manager, session_manager, memory_vector=memory_vector)
|
||||
|
|
|
|||
148
routes/memory/memory_graph_routes.py
Normal file
148
routes/memory/memory_graph_routes.py
Normal file
|
|
@ -0,0 +1,148 @@
|
|||
# routes/memory/memory_graph_routes.py
|
||||
"""Memory Graph View endpoints: read-only graph derivation plus manual
|
||||
relationship (link) editing between a user's own memories.
|
||||
|
||||
Kept as a separate router (not folded into memory_routes.py's wildcard-heavy
|
||||
router) but MUST be included in app.py before that router — see the comment
|
||||
at the include_router call site. `GET /api/memory/graph` would otherwise be
|
||||
swallowed by memory_routes.py's `GET /api/memory/{memory_id}` wildcard if
|
||||
that router's routes were checked first.
|
||||
"""
|
||||
from typing import Dict, List, Optional
|
||||
import logging
|
||||
|
||||
from fastapi import APIRouter, HTTPException, Query, Request
|
||||
|
||||
from services.memory import MemoryManager
|
||||
from src.auth_helpers import get_current_user, require_privilege, require_user
|
||||
from src.memory_graph import (
|
||||
DEFAULT_MAX_EDGES_PER_NODE,
|
||||
DEFAULT_MIN_SIMILARITY,
|
||||
build_graph,
|
||||
)
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
def setup_memory_graph_routes(memory_manager: MemoryManager, memory_vector=None):
|
||||
"""Set up Memory Graph View routes."""
|
||||
router = APIRouter(prefix="/api/memory", tags=["memory-graph"])
|
||||
|
||||
def _owner(request: Request) -> Optional[str]:
|
||||
return get_current_user(request)
|
||||
|
||||
def _verify_memory_owner(memory: dict, user: Optional[str]):
|
||||
"""Raise 404 if user doesn't own this memory. Mirrors
|
||||
memory_routes.py's _verify_memory_owner: strict ownership so a
|
||||
legacy/null-owner memory never leaks across accounts."""
|
||||
if user is None:
|
||||
return # Auth disabled
|
||||
if memory.get("owner") != user:
|
||||
raise HTTPException(404, "Memory not found")
|
||||
|
||||
@router.get("/graph")
|
||||
def get_memory_graph(
|
||||
request: Request,
|
||||
category: Optional[List[str]] = Query(None),
|
||||
min_similarity: float = Query(DEFAULT_MIN_SIMILARITY, ge=0.0, le=1.0),
|
||||
max_edges_per_node: int = Query(DEFAULT_MAX_EDGES_PER_NODE, ge=1, le=50),
|
||||
include_session_edges: bool = Query(True),
|
||||
include_manual_edges: bool = Query(True),
|
||||
limit: int = Query(1000, ge=1, le=5000),
|
||||
):
|
||||
"""Return the caller's own memories as a derived node/edge graph."""
|
||||
user = require_user(request)
|
||||
memories = memory_manager.load(owner=user)
|
||||
return build_graph(
|
||||
memories,
|
||||
memory_vector,
|
||||
categories=category,
|
||||
min_similarity=min_similarity,
|
||||
max_edges_per_node=max_edges_per_node,
|
||||
include_session_edges=include_session_edges,
|
||||
include_manual_edges=include_manual_edges,
|
||||
limit=limit,
|
||||
)
|
||||
|
||||
@router.get("/graph/{memory_id}/neighbors")
|
||||
def get_memory_graph_neighbors(
|
||||
request: Request,
|
||||
memory_id: str,
|
||||
min_similarity: float = Query(DEFAULT_MIN_SIMILARITY, ge=0.0, le=1.0),
|
||||
max_edges_per_node: int = Query(DEFAULT_MAX_EDGES_PER_NODE, ge=1, le=50),
|
||||
):
|
||||
"""Lazy drill-down: one node plus its immediate derived neighbors.
|
||||
|
||||
For graphs too large to render whole (see build_graph's `limit`/
|
||||
`truncated`), the frontend can expand a single node on demand instead
|
||||
of the server ever needing to compute/return the entire graph.
|
||||
"""
|
||||
user = require_user(request)
|
||||
memories = memory_manager.load(owner=user)
|
||||
target = next((m for m in memories if m.get("id") == memory_id), None)
|
||||
if target is None:
|
||||
raise HTTPException(404, "Memory not found")
|
||||
_verify_memory_owner(target, user)
|
||||
|
||||
full = build_graph(
|
||||
memories,
|
||||
memory_vector,
|
||||
min_similarity=min_similarity,
|
||||
max_edges_per_node=max_edges_per_node,
|
||||
limit=len(memories) or 1,
|
||||
)
|
||||
neighbor_ids = {
|
||||
(e["target"] if e["source"] == memory_id else e["source"])
|
||||
for e in full["edges"]
|
||||
if memory_id in (e["source"], e["target"])
|
||||
}
|
||||
neighbor_ids.add(memory_id)
|
||||
nodes = [n for n in full["nodes"] if n["id"] in neighbor_ids]
|
||||
edges = [e for e in full["edges"] if e["source"] in neighbor_ids and e["target"] in neighbor_ids]
|
||||
return {"nodes": nodes, "edges": edges, "meta": {"node_count": len(nodes), "edge_count": len(edges)}}
|
||||
|
||||
@router.post("/{memory_id}/links")
|
||||
def add_memory_link(request: Request, memory_id: str, target_id: str = Query(...)):
|
||||
"""Create an explicit manual relationship between two of the caller's
|
||||
own memories (the Memory Graph View's "draw a link" affordance)."""
|
||||
user = require_privilege(request, "can_manage_memory")
|
||||
if target_id == memory_id:
|
||||
raise HTTPException(400, "A memory cannot link to itself")
|
||||
|
||||
all_mem = memory_manager.load_all()
|
||||
source = next((m for m in all_mem if m.get("id") == memory_id), None)
|
||||
if source is None:
|
||||
raise HTTPException(404, "Memory not found")
|
||||
_verify_memory_owner(source, user)
|
||||
target = next((m for m in all_mem if m.get("id") == target_id), None)
|
||||
if target is None:
|
||||
raise HTTPException(404, "Target memory not found")
|
||||
_verify_memory_owner(target, user)
|
||||
|
||||
links = list(source.get("links") or [])
|
||||
if target_id not in links:
|
||||
links.append(target_id)
|
||||
source["links"] = links
|
||||
memory_manager.save(all_mem)
|
||||
return {"ok": True, "links": links}
|
||||
|
||||
@router.delete("/{memory_id}/links/{target_id}")
|
||||
def remove_memory_link(request: Request, memory_id: str, target_id: str):
|
||||
"""Remove a manual relationship. Idempotent — removing a link that
|
||||
doesn't exist is not an error, matching how memory delete/pin already
|
||||
treat repeat calls as harmless in this codebase."""
|
||||
user = require_privilege(request, "can_manage_memory")
|
||||
all_mem = memory_manager.load_all()
|
||||
source = next((m for m in all_mem if m.get("id") == memory_id), None)
|
||||
if source is None:
|
||||
raise HTTPException(404, "Memory not found")
|
||||
_verify_memory_owner(source, user)
|
||||
|
||||
links = list(source.get("links") or [])
|
||||
if target_id in links:
|
||||
links = [l for l in links if l != target_id]
|
||||
source["links"] = links
|
||||
memory_manager.save(all_mem)
|
||||
return {"ok": True, "links": links}
|
||||
|
||||
return router
|
||||
59
tests/test_memory_graph_route_ordering.py
Normal file
59
tests/test_memory_graph_route_ordering.py
Normal file
|
|
@ -0,0 +1,59 @@
|
|||
"""Regression guard for the one real collision risk in this feature.
|
||||
|
||||
routes/memory/memory_routes.py registers `GET/PUT/DELETE /api/memory/{memory_id}`
|
||||
as a single-segment wildcard. Starlette matches routes in registration order
|
||||
across the whole app, not by specificity, so `GET /api/memory/graph` would be
|
||||
silently swallowed by that wildcard (memory_id="graph") if memory_router were
|
||||
ever included before memory_graph_router. app.py documents and enforces the
|
||||
required order; this test builds a minimal app the same way and proves a real
|
||||
HTTP request resolves to the graph handler, not the wildcard 404 path — a
|
||||
plain "call the endpoint function directly" test (the repo's usual route-test
|
||||
style) can't catch this class of bug because it looks up routes by exact
|
||||
path-string equality, not by simulating Starlette's request matching.
|
||||
"""
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
from fastapi import FastAPI
|
||||
from fastapi.testclient import TestClient
|
||||
from starlette.requests import Request
|
||||
|
||||
from routes.memory.memory_graph_routes import setup_memory_graph_routes
|
||||
from routes.memory.memory_routes import setup_memory_routes
|
||||
|
||||
|
||||
def _build_app():
|
||||
app = FastAPI()
|
||||
|
||||
memory_manager = MagicMock()
|
||||
memory_manager.load.return_value = []
|
||||
session_manager = MagicMock()
|
||||
|
||||
# Mirrors app.py: memory_graph_router included BEFORE memory_router.
|
||||
app.include_router(setup_memory_graph_routes(memory_manager, memory_vector=None))
|
||||
app.include_router(setup_memory_routes(memory_manager, session_manager, memory_vector=None))
|
||||
|
||||
@app.middleware("http")
|
||||
async def _fake_auth(request: Request, call_next):
|
||||
request.state.current_user = "alice"
|
||||
request.state.api_token = False
|
||||
return await call_next(request)
|
||||
|
||||
return app
|
||||
|
||||
|
||||
def test_graph_route_is_not_swallowed_by_memory_id_wildcard():
|
||||
client = TestClient(_build_app())
|
||||
resp = client.get("/api/memory/graph")
|
||||
assert resp.status_code == 200
|
||||
body = resp.json()
|
||||
assert "nodes" in body and "edges" in body
|
||||
|
||||
|
||||
def test_memory_id_wildcard_still_works_for_real_ids():
|
||||
client = TestClient(_build_app())
|
||||
resp = client.get("/api/memory/some-real-id")
|
||||
# Not found (empty memory store), but resolved by the wildcard handler,
|
||||
# not a 422/other error — proves the wildcard route still works normally
|
||||
# once the graph route (checked first) doesn't match.
|
||||
assert resp.status_code == 404
|
||||
assert resp.json()["detail"] == "Memory not found"
|
||||
210
tests/test_memory_graph_routes.py
Normal file
210
tests/test_memory_graph_routes.py
Normal file
|
|
@ -0,0 +1,210 @@
|
|||
"""Memory Graph View route tests.
|
||||
|
||||
Follows the repo's established convention (see
|
||||
tests/test_memory_routes_session_owner.py): build the router via its
|
||||
setup_*_routes factory directly, monkeypatch auth helpers, look up the
|
||||
target endpoint by path, and call it directly with a hand-built Request
|
||||
stand-in. No TestClient/ASGI app — except in
|
||||
test_memory_graph_route_ordering.py, which specifically needs real Starlette
|
||||
path matching.
|
||||
"""
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import pytest
|
||||
from fastapi import HTTPException
|
||||
|
||||
import routes.memory.memory_graph_routes as mgr
|
||||
|
||||
|
||||
def _route(router, path, method):
|
||||
for r in router.routes:
|
||||
if r.path == path and method in getattr(r, "methods", set()):
|
||||
return r.endpoint
|
||||
raise AssertionError(path)
|
||||
|
||||
|
||||
def _request(user):
|
||||
return SimpleNamespace(
|
||||
state=SimpleNamespace(current_user=user, api_token=False),
|
||||
app=SimpleNamespace(state=SimpleNamespace(auth_manager=None)),
|
||||
client=SimpleNamespace(host="127.0.0.1"),
|
||||
)
|
||||
|
||||
|
||||
def _allow_memory_management(monkeypatch, caller):
|
||||
monkeypatch.setattr(mgr, "require_privilege", lambda request, key: caller)
|
||||
|
||||
|
||||
def test_graph_only_returns_callers_own_memories(monkeypatch):
|
||||
monkeypatch.setattr(mgr, "get_current_user", lambda request: "alice", raising=False)
|
||||
monkeypatch.setattr(mgr, "require_user", lambda request: "alice", raising=False)
|
||||
memory_manager = MagicMock()
|
||||
memory_manager.load.side_effect = lambda owner=None: (
|
||||
[{"id": "m1", "text": "alice's note", "owner": "alice", "category": "fact", "uses": 0, "timestamp": 1}]
|
||||
if owner == "alice" else []
|
||||
)
|
||||
router = mgr.setup_memory_graph_routes(memory_manager, memory_vector=None)
|
||||
get_graph = _route(router, "/api/memory/graph", "GET")
|
||||
|
||||
out = get_graph(request=_request("alice"), category=None, min_similarity=0.75,
|
||||
max_edges_per_node=5, include_session_edges=True,
|
||||
include_manual_edges=True, limit=1000)
|
||||
|
||||
assert [n["id"] for n in out["nodes"]] == ["m1"]
|
||||
memory_manager.load.assert_called_with(owner="alice")
|
||||
|
||||
|
||||
def test_graph_neighbors_rejects_foreign_owned_memory(monkeypatch):
|
||||
monkeypatch.setattr(mgr, "get_current_user", lambda request: "bob", raising=False)
|
||||
monkeypatch.setattr(mgr, "require_user", lambda request: "bob", raising=False)
|
||||
memory_manager = MagicMock()
|
||||
memory_manager.load.return_value = [
|
||||
{"id": "victim-mem", "text": "alice secret", "owner": "alice", "category": "fact", "uses": 0, "timestamp": 1},
|
||||
]
|
||||
router = mgr.setup_memory_graph_routes(memory_manager, memory_vector=None)
|
||||
neighbors = _route(router, "/api/memory/graph/{memory_id}/neighbors", "GET")
|
||||
|
||||
with pytest.raises(HTTPException) as exc:
|
||||
neighbors(request=_request("bob"), memory_id="victim-mem", min_similarity=0.75, max_edges_per_node=5)
|
||||
assert exc.value.status_code == 404
|
||||
|
||||
|
||||
def test_graph_neighbors_returns_404_for_unknown_id(monkeypatch):
|
||||
monkeypatch.setattr(mgr, "get_current_user", lambda request: "alice", raising=False)
|
||||
monkeypatch.setattr(mgr, "require_user", lambda request: "alice", raising=False)
|
||||
memory_manager = MagicMock()
|
||||
memory_manager.load.return_value = []
|
||||
router = mgr.setup_memory_graph_routes(memory_manager, memory_vector=None)
|
||||
neighbors = _route(router, "/api/memory/graph/{memory_id}/neighbors", "GET")
|
||||
|
||||
with pytest.raises(HTTPException) as exc:
|
||||
neighbors(request=_request("alice"), memory_id="nope", min_similarity=0.75, max_edges_per_node=5)
|
||||
assert exc.value.status_code == 404
|
||||
|
||||
|
||||
def test_graph_neighbors_scopes_to_connected_subgraph(monkeypatch):
|
||||
monkeypatch.setattr(mgr, "get_current_user", lambda request: "alice", raising=False)
|
||||
monkeypatch.setattr(mgr, "require_user", lambda request: "alice", raising=False)
|
||||
memory_manager = MagicMock()
|
||||
memory_manager.load.return_value = [
|
||||
{"id": "a", "text": "t", "owner": "alice", "category": "fact", "uses": 0, "timestamp": 1, "session_id": "s1"},
|
||||
{"id": "b", "text": "t", "owner": "alice", "category": "fact", "uses": 0, "timestamp": 1, "session_id": "s1"},
|
||||
{"id": "c", "text": "t", "owner": "alice", "category": "fact", "uses": 0, "timestamp": 1},
|
||||
]
|
||||
router = mgr.setup_memory_graph_routes(memory_manager, memory_vector=None)
|
||||
neighbors = _route(router, "/api/memory/graph/{memory_id}/neighbors", "GET")
|
||||
|
||||
out = neighbors(request=_request("alice"), memory_id="a", min_similarity=0.75, max_edges_per_node=5)
|
||||
|
||||
assert {n["id"] for n in out["nodes"]} == {"a", "b"}
|
||||
|
||||
|
||||
def test_add_link_requires_can_manage_memory_privilege(monkeypatch):
|
||||
monkeypatch.setattr(mgr, "get_current_user", lambda request: "alice", raising=False)
|
||||
|
||||
def deny(request, key):
|
||||
raise HTTPException(403, "nope")
|
||||
monkeypatch.setattr(mgr, "require_privilege", deny)
|
||||
|
||||
memory_manager = MagicMock()
|
||||
router = mgr.setup_memory_graph_routes(memory_manager, memory_vector=None)
|
||||
add_link = _route(router, "/api/memory/{memory_id}/links", "POST")
|
||||
|
||||
with pytest.raises(HTTPException) as exc:
|
||||
add_link(request=_request("alice"), memory_id="a", target_id="b")
|
||||
assert exc.value.status_code == 403
|
||||
memory_manager.save.assert_not_called()
|
||||
|
||||
|
||||
def test_add_link_rejects_foreign_owned_target(monkeypatch):
|
||||
monkeypatch.setattr(mgr, "get_current_user", lambda request: "bob", raising=False)
|
||||
_allow_memory_management(monkeypatch, "bob")
|
||||
memory_manager = MagicMock()
|
||||
memory_manager.load_all.return_value = [
|
||||
{"id": "bob-mem", "text": "t", "owner": "bob"},
|
||||
{"id": "alice-mem", "text": "t", "owner": "alice"},
|
||||
]
|
||||
router = mgr.setup_memory_graph_routes(memory_manager, memory_vector=None)
|
||||
add_link = _route(router, "/api/memory/{memory_id}/links", "POST")
|
||||
|
||||
with pytest.raises(HTTPException) as exc:
|
||||
add_link(request=_request("bob"), memory_id="bob-mem", target_id="alice-mem")
|
||||
assert exc.value.status_code == 404
|
||||
memory_manager.save.assert_not_called()
|
||||
|
||||
|
||||
def test_add_link_rejects_self_link(monkeypatch):
|
||||
monkeypatch.setattr(mgr, "get_current_user", lambda request: "alice", raising=False)
|
||||
_allow_memory_management(monkeypatch, "alice")
|
||||
memory_manager = MagicMock()
|
||||
router = mgr.setup_memory_graph_routes(memory_manager, memory_vector=None)
|
||||
add_link = _route(router, "/api/memory/{memory_id}/links", "POST")
|
||||
|
||||
with pytest.raises(HTTPException) as exc:
|
||||
add_link(request=_request("alice"), memory_id="a", target_id="a")
|
||||
assert exc.value.status_code == 400
|
||||
memory_manager.save.assert_not_called()
|
||||
|
||||
|
||||
def test_add_link_persists_bidirectionally_addressable_link(monkeypatch):
|
||||
monkeypatch.setattr(mgr, "get_current_user", lambda request: "alice", raising=False)
|
||||
_allow_memory_management(monkeypatch, "alice")
|
||||
memory_manager = MagicMock()
|
||||
entries = [
|
||||
{"id": "a", "text": "t", "owner": "alice"},
|
||||
{"id": "b", "text": "t", "owner": "alice"},
|
||||
]
|
||||
memory_manager.load_all.return_value = entries
|
||||
router = mgr.setup_memory_graph_routes(memory_manager, memory_vector=None)
|
||||
add_link = _route(router, "/api/memory/{memory_id}/links", "POST")
|
||||
|
||||
out = add_link(request=_request("alice"), memory_id="a", target_id="b")
|
||||
|
||||
assert out == {"ok": True, "links": ["b"]}
|
||||
memory_manager.save.assert_called_once_with(entries)
|
||||
assert entries[0]["links"] == ["b"]
|
||||
|
||||
|
||||
def test_add_link_is_idempotent_when_link_already_present(monkeypatch):
|
||||
monkeypatch.setattr(mgr, "get_current_user", lambda request: "alice", raising=False)
|
||||
_allow_memory_management(monkeypatch, "alice")
|
||||
memory_manager = MagicMock()
|
||||
entries = [
|
||||
{"id": "a", "text": "t", "owner": "alice", "links": ["b"]},
|
||||
{"id": "b", "text": "t", "owner": "alice"},
|
||||
]
|
||||
memory_manager.load_all.return_value = entries
|
||||
router = mgr.setup_memory_graph_routes(memory_manager, memory_vector=None)
|
||||
add_link = _route(router, "/api/memory/{memory_id}/links", "POST")
|
||||
|
||||
out = add_link(request=_request("alice"), memory_id="a", target_id="b")
|
||||
|
||||
assert out["links"] == ["b"]
|
||||
|
||||
|
||||
def test_remove_link_is_idempotent_when_link_absent(monkeypatch):
|
||||
monkeypatch.setattr(mgr, "get_current_user", lambda request: "alice", raising=False)
|
||||
_allow_memory_management(monkeypatch, "alice")
|
||||
memory_manager = MagicMock()
|
||||
memory_manager.load_all.return_value = [{"id": "a", "text": "t", "owner": "alice"}]
|
||||
router = mgr.setup_memory_graph_routes(memory_manager, memory_vector=None)
|
||||
remove_link = _route(router, "/api/memory/{memory_id}/links/{target_id}", "DELETE")
|
||||
|
||||
out = remove_link(request=_request("alice"), memory_id="a", target_id="never-linked")
|
||||
|
||||
assert out == {"ok": True, "links": []}
|
||||
memory_manager.save.assert_not_called()
|
||||
|
||||
|
||||
def test_remove_link_rejects_foreign_owned_source(monkeypatch):
|
||||
monkeypatch.setattr(mgr, "get_current_user", lambda request: "bob", raising=False)
|
||||
_allow_memory_management(monkeypatch, "bob")
|
||||
memory_manager = MagicMock()
|
||||
memory_manager.load_all.return_value = [{"id": "alice-mem", "text": "t", "owner": "alice", "links": ["x"]}]
|
||||
router = mgr.setup_memory_graph_routes(memory_manager, memory_vector=None)
|
||||
remove_link = _route(router, "/api/memory/{memory_id}/links/{target_id}", "DELETE")
|
||||
|
||||
with pytest.raises(HTTPException) as exc:
|
||||
remove_link(request=_request("bob"), memory_id="alice-mem", target_id="x")
|
||||
assert exc.value.status_code == 404
|
||||
Loading…
Add table
Reference in a new issue