mirror of
https://github.com/pewdiepie-archdaemon/odysseus.git
synced 2026-10-18 04:42:20 +02:00
Compare commits
3
Commits
bda7f4068c
...
76ff5cbe0b
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
76ff5cbe0b | ||
|
|
ec0805acc4 | ||
|
|
8f0eaf837d |
@@ -663,7 +663,7 @@ app.include_router(setup_session_routes(
|
||||
))
|
||||
|
||||
# Admin Danger Zone wipes (Settings → System → Danger Zone)
|
||||
from routes.admin_wipe_routes import setup_admin_wipe_routes
|
||||
from routes.admin_wipe.admin_wipe_routes import setup_admin_wipe_routes
|
||||
app.include_router(setup_admin_wipe_routes(session_manager))
|
||||
|
||||
# Memory
|
||||
@@ -704,7 +704,7 @@ from routes.diagnostics_routes import setup_diagnostics_routes
|
||||
app.include_router(setup_diagnostics_routes(rag_manager, rag_available, research_handler, memory_vector))
|
||||
|
||||
# Cleanup
|
||||
from routes.cleanup_routes import setup_cleanup_routes
|
||||
from routes.cleanup.cleanup_routes import setup_cleanup_routes
|
||||
app.include_router(setup_cleanup_routes(session_manager))
|
||||
|
||||
# Personal docs
|
||||
@@ -787,7 +787,7 @@ from routes.hwfit_routes import setup_hwfit_routes
|
||||
app.include_router(setup_hwfit_routes())
|
||||
|
||||
# Model A/B Comparison
|
||||
from routes.compare_routes import setup_compare_routes
|
||||
from routes.compare.compare_routes import setup_compare_routes
|
||||
app.include_router(setup_compare_routes(session_manager))
|
||||
|
||||
# User Preferences
|
||||
|
||||
@@ -0,0 +1,5 @@
|
||||
"""Admin wipe route domain package (slice 2h, #4082/#4071).
|
||||
|
||||
Contains admin_wipe_routes.py, migrated from the flat routes/ directory.
|
||||
Backward-compat shim at routes/admin_wipe_routes.py re-exports from here.
|
||||
"""
|
||||
@@ -0,0 +1,176 @@
|
||||
"""Admin Danger Zone — per-category wipes.
|
||||
|
||||
Each endpoint is admin-only and truncates exactly one domain so the
|
||||
user can selectively reset memory / skills / notes / etc. without
|
||||
nuking everything. The catch-all `chats` endpoint mirrors the
|
||||
existing /api/sessions/all so the Danger Zone speaks one URL pattern.
|
||||
|
||||
URL shape: DELETE /api/admin/wipe/{kind}
|
||||
Kinds: chats, memory, skills, notes, tasks, documents, gallery, calendar.
|
||||
"""
|
||||
|
||||
import json
|
||||
import logging
|
||||
import os
|
||||
import shutil
|
||||
from fastapi import APIRouter, HTTPException, Request
|
||||
|
||||
from core.middleware import require_admin
|
||||
from core.database import (
|
||||
SessionLocal,
|
||||
Session as DbSession,
|
||||
ChatMessage as DbChatMessage,
|
||||
Memory,
|
||||
Note,
|
||||
ScheduledTask,
|
||||
TaskRun,
|
||||
Document,
|
||||
DocumentVersion,
|
||||
GalleryImage,
|
||||
GalleryAlbum,
|
||||
CalendarEvent,
|
||||
CalendarCal,
|
||||
)
|
||||
from src.constants import DATA_DIR, SKILLS_DIR, SKILLS_FILE, GALLERY_DIR, GALLERY_UPLOADS_DIR
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
def _wipe_memory_files():
|
||||
"""Blank memory.json + drop the per-owner tidy-state sidecar so the
|
||||
next audit doesn't try to diff against gone memories."""
|
||||
for name in ("memory.json", "memory_tidy_state.json"):
|
||||
p = os.path.join(DATA_DIR, name)
|
||||
if not os.path.exists(p):
|
||||
continue
|
||||
try:
|
||||
if name == "memory.json":
|
||||
with open(p, "w", encoding="utf-8") as f:
|
||||
json.dump([], f)
|
||||
else:
|
||||
os.remove(p)
|
||||
except OSError as e:
|
||||
logger.warning(f"Could not reset {name}: {e}")
|
||||
|
||||
|
||||
def _rmtree_quiet(path: str):
|
||||
"""rmtree that doesn't crash if the path doesn't exist."""
|
||||
if os.path.isdir(path):
|
||||
try:
|
||||
shutil.rmtree(path)
|
||||
except OSError as e:
|
||||
logger.warning(f"Could not remove {path}: {e}")
|
||||
|
||||
|
||||
def setup_admin_wipe_routes(session_manager):
|
||||
"""The session_manager is passed in so we can also clear its
|
||||
in-memory cache when wiping chats — without it the DB is empty
|
||||
but the next /api/sessions returns stale entries."""
|
||||
router = APIRouter(prefix="/api/admin")
|
||||
|
||||
@router.delete("/wipe/{kind}")
|
||||
def wipe(kind: str, request: Request):
|
||||
require_admin(request)
|
||||
kind = (kind or "").strip().lower()
|
||||
|
||||
db = SessionLocal()
|
||||
try:
|
||||
if kind == "chats":
|
||||
count = db.query(DbSession).count()
|
||||
db.query(DbChatMessage).delete()
|
||||
db.query(DbSession).delete()
|
||||
db.commit()
|
||||
try:
|
||||
session_manager.sessions.clear()
|
||||
except Exception:
|
||||
pass
|
||||
return {"status": "deleted", "kind": kind, "count": count}
|
||||
|
||||
if kind == "memory":
|
||||
count = db.query(Memory).count()
|
||||
db.query(Memory).delete()
|
||||
db.commit()
|
||||
_wipe_memory_files()
|
||||
# Drop the vector store too so semantic search doesn't
|
||||
# return ghosts. Lazy import — chromadb may not be
|
||||
# initialised in every deployment.
|
||||
try:
|
||||
from src.memory_vector import get_memory_vector_store
|
||||
mv = get_memory_vector_store()
|
||||
if mv and hasattr(mv, "clear"):
|
||||
mv.clear()
|
||||
except Exception as e:
|
||||
logger.info(f"Memory vector clear skipped: {e}")
|
||||
return {"status": "deleted", "kind": kind, "count": count}
|
||||
|
||||
if kind == "skills":
|
||||
# Skills live as SKILL.md files under data/skills/. Drop
|
||||
# the entire directory; the SkillsManager re-creates the
|
||||
# tree on next write.
|
||||
skills_dir = SKILLS_DIR
|
||||
count = 0
|
||||
if os.path.isdir(skills_dir):
|
||||
# Count SKILL.md files for the response — quick walk.
|
||||
for _, _, files in os.walk(skills_dir):
|
||||
count += sum(1 for f in files if f == "SKILL.md")
|
||||
_rmtree_quiet(skills_dir)
|
||||
# Legacy fallback file
|
||||
legacy = SKILLS_FILE
|
||||
if os.path.exists(legacy):
|
||||
try:
|
||||
os.remove(legacy)
|
||||
except OSError:
|
||||
pass
|
||||
return {"status": "deleted", "kind": kind, "count": count}
|
||||
|
||||
if kind == "notes":
|
||||
count = db.query(Note).count()
|
||||
db.query(Note).delete()
|
||||
db.commit()
|
||||
return {"status": "deleted", "kind": kind, "count": count}
|
||||
|
||||
if kind == "tasks":
|
||||
# TaskRun rows reference tasks via FK — clear them first.
|
||||
db.query(TaskRun).delete()
|
||||
count = db.query(ScheduledTask).count()
|
||||
db.query(ScheduledTask).delete()
|
||||
db.commit()
|
||||
return {"status": "deleted", "kind": kind, "count": count}
|
||||
|
||||
if kind == "documents":
|
||||
# DocumentVersion FKs Document — clear children first.
|
||||
db.query(DocumentVersion).delete()
|
||||
count = db.query(Document).count()
|
||||
db.query(Document).delete()
|
||||
db.commit()
|
||||
return {"status": "deleted", "kind": kind, "count": count}
|
||||
|
||||
if kind == "gallery":
|
||||
count = db.query(GalleryImage).count() + db.query(GalleryAlbum).count()
|
||||
db.query(GalleryImage).delete()
|
||||
db.query(GalleryAlbum).delete()
|
||||
db.commit()
|
||||
# Also drop the upload dir so disk doesn't keep orphans.
|
||||
_rmtree_quiet(GALLERY_DIR)
|
||||
_rmtree_quiet(GALLERY_UPLOADS_DIR)
|
||||
return {"status": "deleted", "kind": kind, "count": count}
|
||||
|
||||
if kind == "calendar":
|
||||
# Events FK calendars — clear children first, then both.
|
||||
db.query(CalendarEvent).delete()
|
||||
count = db.query(CalendarCal).count()
|
||||
db.query(CalendarCal).delete()
|
||||
db.commit()
|
||||
return {"status": "deleted", "kind": kind, "count": count}
|
||||
|
||||
raise HTTPException(400, f"Unknown wipe kind: {kind!r}")
|
||||
except HTTPException:
|
||||
raise
|
||||
except Exception as e:
|
||||
db.rollback()
|
||||
logger.exception(f"Wipe {kind} failed")
|
||||
raise HTTPException(500, f"Wipe {kind} failed: {e}")
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
return router
|
||||
+12
-171
@@ -1,176 +1,17 @@
|
||||
"""Admin Danger Zone — per-category wipes.
|
||||
"""Backward-compat shim — canonical location is routes/admin_wipe/admin_wipe_routes.py.
|
||||
|
||||
Each endpoint is admin-only and truncates exactly one domain so the
|
||||
user can selectively reset memory / skills / notes / etc. without
|
||||
nuking everything. The catch-all `chats` endpoint mirrors the
|
||||
existing /api/sessions/all so the Danger Zone speaks one URL pattern.
|
||||
|
||||
URL shape: DELETE /api/admin/wipe/{kind}
|
||||
Kinds: chats, memory, skills, notes, tasks, documents, gallery, calendar.
|
||||
This module is replaced in ``sys.modules`` by the canonical module object so
|
||||
that ``import routes.admin_wipe_routes``, ``from routes.admin_wipe_routes
|
||||
import X``, ``importlib.import_module("routes.admin_wipe_routes")``, and the
|
||||
``import ... as admin_wipe_routes`` + ``monkeypatch.setattr(admin_wipe_routes,
|
||||
"SessionLocal", ...)`` / ``"require_admin"`` pattern used by
|
||||
test_admin_wipe_gallery.py all operate on the *same* object the application
|
||||
actually uses. Keeps existing import paths working after slice 2h
|
||||
(#4082/#4071).
|
||||
"""
|
||||
|
||||
import json
|
||||
import logging
|
||||
import os
|
||||
import shutil
|
||||
from fastapi import APIRouter, HTTPException, Request
|
||||
import sys as _sys
|
||||
|
||||
from core.middleware import require_admin
|
||||
from core.database import (
|
||||
SessionLocal,
|
||||
Session as DbSession,
|
||||
ChatMessage as DbChatMessage,
|
||||
Memory,
|
||||
Note,
|
||||
ScheduledTask,
|
||||
TaskRun,
|
||||
Document,
|
||||
DocumentVersion,
|
||||
GalleryImage,
|
||||
GalleryAlbum,
|
||||
CalendarEvent,
|
||||
CalendarCal,
|
||||
)
|
||||
from src.constants import DATA_DIR, SKILLS_DIR, SKILLS_FILE, GALLERY_DIR, GALLERY_UPLOADS_DIR
|
||||
from routes.admin_wipe import admin_wipe_routes as _canonical # noqa: F401
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
def _wipe_memory_files():
|
||||
"""Blank memory.json + drop the per-owner tidy-state sidecar so the
|
||||
next audit doesn't try to diff against gone memories."""
|
||||
for name in ("memory.json", "memory_tidy_state.json"):
|
||||
p = os.path.join(DATA_DIR, name)
|
||||
if not os.path.exists(p):
|
||||
continue
|
||||
try:
|
||||
if name == "memory.json":
|
||||
with open(p, "w", encoding="utf-8") as f:
|
||||
json.dump([], f)
|
||||
else:
|
||||
os.remove(p)
|
||||
except OSError as e:
|
||||
logger.warning(f"Could not reset {name}: {e}")
|
||||
|
||||
|
||||
def _rmtree_quiet(path: str):
|
||||
"""rmtree that doesn't crash if the path doesn't exist."""
|
||||
if os.path.isdir(path):
|
||||
try:
|
||||
shutil.rmtree(path)
|
||||
except OSError as e:
|
||||
logger.warning(f"Could not remove {path}: {e}")
|
||||
|
||||
|
||||
def setup_admin_wipe_routes(session_manager):
|
||||
"""The session_manager is passed in so we can also clear its
|
||||
in-memory cache when wiping chats — without it the DB is empty
|
||||
but the next /api/sessions returns stale entries."""
|
||||
router = APIRouter(prefix="/api/admin")
|
||||
|
||||
@router.delete("/wipe/{kind}")
|
||||
def wipe(kind: str, request: Request):
|
||||
require_admin(request)
|
||||
kind = (kind or "").strip().lower()
|
||||
|
||||
db = SessionLocal()
|
||||
try:
|
||||
if kind == "chats":
|
||||
count = db.query(DbSession).count()
|
||||
db.query(DbChatMessage).delete()
|
||||
db.query(DbSession).delete()
|
||||
db.commit()
|
||||
try:
|
||||
session_manager.sessions.clear()
|
||||
except Exception:
|
||||
pass
|
||||
return {"status": "deleted", "kind": kind, "count": count}
|
||||
|
||||
if kind == "memory":
|
||||
count = db.query(Memory).count()
|
||||
db.query(Memory).delete()
|
||||
db.commit()
|
||||
_wipe_memory_files()
|
||||
# Drop the vector store too so semantic search doesn't
|
||||
# return ghosts. Lazy import — chromadb may not be
|
||||
# initialised in every deployment.
|
||||
try:
|
||||
from src.memory_vector import get_memory_vector_store
|
||||
mv = get_memory_vector_store()
|
||||
if mv and hasattr(mv, "clear"):
|
||||
mv.clear()
|
||||
except Exception as e:
|
||||
logger.info(f"Memory vector clear skipped: {e}")
|
||||
return {"status": "deleted", "kind": kind, "count": count}
|
||||
|
||||
if kind == "skills":
|
||||
# Skills live as SKILL.md files under data/skills/. Drop
|
||||
# the entire directory; the SkillsManager re-creates the
|
||||
# tree on next write.
|
||||
skills_dir = SKILLS_DIR
|
||||
count = 0
|
||||
if os.path.isdir(skills_dir):
|
||||
# Count SKILL.md files for the response — quick walk.
|
||||
for _, _, files in os.walk(skills_dir):
|
||||
count += sum(1 for f in files if f == "SKILL.md")
|
||||
_rmtree_quiet(skills_dir)
|
||||
# Legacy fallback file
|
||||
legacy = SKILLS_FILE
|
||||
if os.path.exists(legacy):
|
||||
try:
|
||||
os.remove(legacy)
|
||||
except OSError:
|
||||
pass
|
||||
return {"status": "deleted", "kind": kind, "count": count}
|
||||
|
||||
if kind == "notes":
|
||||
count = db.query(Note).count()
|
||||
db.query(Note).delete()
|
||||
db.commit()
|
||||
return {"status": "deleted", "kind": kind, "count": count}
|
||||
|
||||
if kind == "tasks":
|
||||
# TaskRun rows reference tasks via FK — clear them first.
|
||||
db.query(TaskRun).delete()
|
||||
count = db.query(ScheduledTask).count()
|
||||
db.query(ScheduledTask).delete()
|
||||
db.commit()
|
||||
return {"status": "deleted", "kind": kind, "count": count}
|
||||
|
||||
if kind == "documents":
|
||||
# DocumentVersion FKs Document — clear children first.
|
||||
db.query(DocumentVersion).delete()
|
||||
count = db.query(Document).count()
|
||||
db.query(Document).delete()
|
||||
db.commit()
|
||||
return {"status": "deleted", "kind": kind, "count": count}
|
||||
|
||||
if kind == "gallery":
|
||||
count = db.query(GalleryImage).count() + db.query(GalleryAlbum).count()
|
||||
db.query(GalleryImage).delete()
|
||||
db.query(GalleryAlbum).delete()
|
||||
db.commit()
|
||||
# Also drop the upload dir so disk doesn't keep orphans.
|
||||
_rmtree_quiet(GALLERY_DIR)
|
||||
_rmtree_quiet(GALLERY_UPLOADS_DIR)
|
||||
return {"status": "deleted", "kind": kind, "count": count}
|
||||
|
||||
if kind == "calendar":
|
||||
# Events FK calendars — clear children first, then both.
|
||||
db.query(CalendarEvent).delete()
|
||||
count = db.query(CalendarCal).count()
|
||||
db.query(CalendarCal).delete()
|
||||
db.commit()
|
||||
return {"status": "deleted", "kind": kind, "count": count}
|
||||
|
||||
raise HTTPException(400, f"Unknown wipe kind: {kind!r}")
|
||||
except HTTPException:
|
||||
raise
|
||||
except Exception as e:
|
||||
db.rollback()
|
||||
logger.exception(f"Wipe {kind} failed")
|
||||
raise HTTPException(500, f"Wipe {kind} failed: {e}")
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
return router
|
||||
_sys.modules[__name__] = _canonical
|
||||
|
||||
@@ -0,0 +1,5 @@
|
||||
"""Cleanup route domain package (slice 2g, #4082/#4071).
|
||||
|
||||
Contains cleanup_routes.py, migrated from the flat routes/ directory.
|
||||
Backward-compat shim at routes/cleanup_routes.py re-exports from here.
|
||||
"""
|
||||
@@ -0,0 +1,60 @@
|
||||
# routes/cleanup_routes.py
|
||||
"""Routes for cleanup operations."""
|
||||
import logging
|
||||
from fastapi import APIRouter, HTTPException, Request
|
||||
from src.cleanup_service import get_cleanup_preview, cleanup_sessions
|
||||
from src.auth_helpers import get_current_user
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
def setup_cleanup_routes(session_manager):
|
||||
"""
|
||||
Setup cleanup-related routes.
|
||||
|
||||
Args:
|
||||
session_manager: SessionManager instance
|
||||
|
||||
Returns:
|
||||
APIRouter instance with cleanup routes
|
||||
"""
|
||||
router = APIRouter(prefix="/api/cleanup")
|
||||
|
||||
@router.get("/preview")
|
||||
async def cleanup_preview(request: Request):
|
||||
"""
|
||||
Preview what would be cleaned up without making any changes.
|
||||
|
||||
Returns:
|
||||
JSON response with lists of sessions that would be archived/deleted and estimated space savings
|
||||
"""
|
||||
user = get_current_user(request)
|
||||
try:
|
||||
preview = await get_cleanup_preview(owner=user)
|
||||
return preview
|
||||
except Exception as e:
|
||||
logger.error(f"Cleanup preview failed: {e}")
|
||||
raise HTTPException(500, "Cleanup preview generation failed")
|
||||
|
||||
@router.post("")
|
||||
async def cleanup_endpoint(request: Request):
|
||||
"""
|
||||
Perform cleanup operations:
|
||||
1. Archive inactive sessions (not accessed for 7 days)
|
||||
2. Delete old sessions (archived, not important, not accessed for 14+ days, with fewer than 10 messages)
|
||||
|
||||
Returns:
|
||||
JSON response with counts of deleted and archived sessions, and space freed
|
||||
"""
|
||||
user = get_current_user(request)
|
||||
try:
|
||||
archived_count, deleted_count, space_freed_mb = await cleanup_sessions(session_manager, owner=user)
|
||||
return {
|
||||
"archived_count": archived_count,
|
||||
"deleted_count": deleted_count,
|
||||
"space_freed_mb": round(space_freed_mb, 2)
|
||||
}
|
||||
except Exception as e:
|
||||
logger.error(f"Cleanup failed: {e}")
|
||||
raise HTTPException(500, "Cleanup operation failed")
|
||||
|
||||
return router
|
||||
+13
-56
@@ -1,60 +1,17 @@
|
||||
# routes/cleanup_routes.py
|
||||
"""Routes for cleanup operations."""
|
||||
import logging
|
||||
from fastapi import APIRouter, HTTPException, Request
|
||||
from src.cleanup_service import get_cleanup_preview, cleanup_sessions
|
||||
from src.auth_helpers import get_current_user
|
||||
"""Backward-compat shim — canonical location is routes/cleanup/cleanup_routes.py.
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
This module is replaced in ``sys.modules`` by the canonical module object so
|
||||
that ``import routes.cleanup_routes``, ``from routes.cleanup_routes import X``,
|
||||
``importlib.import_module("routes.cleanup_routes")``, and the string-targeted
|
||||
``monkeypatch.setattr("routes.cleanup_routes.get_cleanup_preview", ...)`` /
|
||||
``"routes.cleanup_routes.get_current_user"`` / ``"routes.cleanup_routes.
|
||||
cleanup_sessions"`` pattern used by test_cleanup_owner_scope.py all operate
|
||||
on the *same* object the application actually uses. Keeps existing import
|
||||
paths working after slice 2g (#4082/#4071).
|
||||
"""
|
||||
|
||||
def setup_cleanup_routes(session_manager):
|
||||
"""
|
||||
Setup cleanup-related routes.
|
||||
import sys as _sys
|
||||
|
||||
Args:
|
||||
session_manager: SessionManager instance
|
||||
from routes.cleanup import cleanup_routes as _canonical # noqa: F401
|
||||
|
||||
Returns:
|
||||
APIRouter instance with cleanup routes
|
||||
"""
|
||||
router = APIRouter(prefix="/api/cleanup")
|
||||
|
||||
@router.get("/preview")
|
||||
async def cleanup_preview(request: Request):
|
||||
"""
|
||||
Preview what would be cleaned up without making any changes.
|
||||
|
||||
Returns:
|
||||
JSON response with lists of sessions that would be archived/deleted and estimated space savings
|
||||
"""
|
||||
user = get_current_user(request)
|
||||
try:
|
||||
preview = await get_cleanup_preview(owner=user)
|
||||
return preview
|
||||
except Exception as e:
|
||||
logger.error(f"Cleanup preview failed: {e}")
|
||||
raise HTTPException(500, "Cleanup preview generation failed")
|
||||
|
||||
@router.post("")
|
||||
async def cleanup_endpoint(request: Request):
|
||||
"""
|
||||
Perform cleanup operations:
|
||||
1. Archive inactive sessions (not accessed for 7 days)
|
||||
2. Delete old sessions (archived, not important, not accessed for 14+ days, with fewer than 10 messages)
|
||||
|
||||
Returns:
|
||||
JSON response with counts of deleted and archived sessions, and space freed
|
||||
"""
|
||||
user = get_current_user(request)
|
||||
try:
|
||||
archived_count, deleted_count, space_freed_mb = await cleanup_sessions(session_manager, owner=user)
|
||||
return {
|
||||
"archived_count": archived_count,
|
||||
"deleted_count": deleted_count,
|
||||
"space_freed_mb": round(space_freed_mb, 2)
|
||||
}
|
||||
except Exception as e:
|
||||
logger.error(f"Cleanup failed: {e}")
|
||||
raise HTTPException(500, "Cleanup operation failed")
|
||||
|
||||
return router
|
||||
_sys.modules[__name__] = _canonical
|
||||
|
||||
@@ -0,0 +1,5 @@
|
||||
"""Compare route domain package (slice 2i, #4082/#4071).
|
||||
|
||||
Contains compare_routes.py, migrated from the flat routes/ directory.
|
||||
Backward-compat shim at routes/compare_routes.py re-exports from here.
|
||||
"""
|
||||
@@ -0,0 +1,365 @@
|
||||
# routes/compare_routes.py
|
||||
"""Model A/B comparison routes."""
|
||||
import json
|
||||
import uuid
|
||||
import random
|
||||
from datetime import datetime
|
||||
from fastapi import APIRouter, Form, HTTPException, Request
|
||||
from typing import List
|
||||
from pydantic import BaseModel
|
||||
import logging
|
||||
|
||||
from core.database import Comparison, SessionLocal
|
||||
from core.session_manager import SessionManager
|
||||
from src.auth_helpers import get_current_user
|
||||
from routes.session_routes import _reject_raw_endpoint_url_for_non_admin
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
router = APIRouter(prefix="/api/compare", tags=["compare"])
|
||||
|
||||
|
||||
def _owned_endpoint_by_url(db, base_url, owner):
|
||||
"""ModelEndpoint whose base_url == `base_url` and is VISIBLE to `owner`
|
||||
(their own rows + legacy null-owner "shared" rows); None otherwise.
|
||||
|
||||
Owner-scoped on purpose. ModelEndpoint is per-user (core/database.py: non-null
|
||||
owner = private, "the model picker only shows the endpoint to that user") and
|
||||
holds a decrypted `api_key`. start_comparison copies the matched row's api_key
|
||||
into the caller-owned [CMP] session's headers, which then drives that session's
|
||||
/api/chat_stream calls — so an UNSCOPED base_url match would let a user mint a
|
||||
comparison bound to ANOTHER user's private endpoint and spend that owner's
|
||||
api_key / reach whatever base_url they configured. Mirrors
|
||||
session_routes._owned_endpoint. A null/empty owner is a no-op (single-user /
|
||||
legacy mode).
|
||||
"""
|
||||
from core.database import ModelEndpoint
|
||||
from src.auth_helpers import owner_filter
|
||||
q = db.query(ModelEndpoint).filter(ModelEndpoint.base_url == base_url)
|
||||
return owner_filter(q, ModelEndpoint, owner).first()
|
||||
|
||||
|
||||
def _owned_endpoint_by_id(db, endpoint_id, owner):
|
||||
"""ModelEndpoint whose id == `endpoint_id` and is VISIBLE to `owner` (their
|
||||
own rows + legacy null-owner "shared" rows); None otherwise.
|
||||
|
||||
Preferred over _owned_endpoint_by_url for credential resolution: two visible
|
||||
endpoints can share the same base_url but hold DIFFERENT api_keys (e.g. two
|
||||
accounts on the same provider). A base_url-only match returns whichever row
|
||||
sorts first, so it can copy the WRONG owner-scoped key into the [CMP] session.
|
||||
An id pins the exact registered endpoint, so /api/compare/start prefers it and
|
||||
only falls back to URL matching for legacy / admin raw-URL callers. Owner
|
||||
scoping is identical to _owned_endpoint_by_url (a null/empty owner is a no-op).
|
||||
"""
|
||||
from core.database import ModelEndpoint
|
||||
from src.auth_helpers import owner_filter
|
||||
q = db.query(ModelEndpoint).filter(ModelEndpoint.id == endpoint_id)
|
||||
return owner_filter(q, ModelEndpoint, owner).first()
|
||||
|
||||
|
||||
class RecordVoteRequest(BaseModel):
|
||||
prompt: str
|
||||
models: List[str]
|
||||
winner: str # model name or "tie"
|
||||
is_blind: bool = True
|
||||
|
||||
|
||||
def setup_compare_routes(session_manager: SessionManager):
|
||||
"""Setup comparison routes."""
|
||||
|
||||
@router.post("/start")
|
||||
def start_comparison(
|
||||
request: Request,
|
||||
prompt: str = Form(...),
|
||||
model_a: str = Form(...),
|
||||
model_b: str = Form(...),
|
||||
endpoint_a: str = Form(""),
|
||||
endpoint_b: str = Form(""),
|
||||
endpoint_a_id: str = Form(""),
|
||||
endpoint_b_id: str = Form(""),
|
||||
is_blind: str = Form("true"),
|
||||
):
|
||||
"""Create two ephemeral sessions and a comparison record.
|
||||
|
||||
Returns the comparison ID and the two session IDs so the client
|
||||
can fire two independent SSE streams to /api/chat_stream.
|
||||
"""
|
||||
user = getattr(request.state, 'current_user', None)
|
||||
comp_id = str(uuid.uuid4())
|
||||
sid_a = str(uuid.uuid4())
|
||||
sid_b = str(uuid.uuid4())
|
||||
|
||||
# Blind mapping: randomly assign left/right
|
||||
blind = str(is_blind).lower() == "true"
|
||||
if blind:
|
||||
mapping = {"left": "a", "right": "b"}
|
||||
if random.random() > 0.5:
|
||||
mapping = {"left": "b", "right": "a"}
|
||||
else:
|
||||
mapping = {"left": "a", "right": "b"}
|
||||
|
||||
# Map session IDs to left/right based on blind mapping
|
||||
session_left = sid_a if mapping["left"] == "a" else sid_b
|
||||
session_right = sid_a if mapping["right"] == "a" else sid_b
|
||||
|
||||
# In blind mode, name the helper sessions by their neutral slot
|
||||
# ("Model A" / "Model B") instead of the real model. Otherwise the
|
||||
# session name leaks the model in the sidebar and GET /api/sessions,
|
||||
# de-anonymizing the comparison before the user votes (issue #1285).
|
||||
slot_name = {session_left: "Model A", session_right: "Model B"}
|
||||
|
||||
# SECURITY: resolve and validate BOTH endpoints before creating any
|
||||
# session. Compare copies a registered endpoint's Authorization header
|
||||
# into the [CMP] session, so validating one endpoint while creating its
|
||||
# session, then rejecting the other, would leave a partial compare
|
||||
# session behind with that header attached. Doing all the owner-scope
|
||||
# resolution + raw-URL rejection up front means a 403 on either endpoint
|
||||
# aborts the whole request with nothing created and no header copied.
|
||||
from src.endpoint_resolver import build_chat_url, build_headers, normalize_base
|
||||
resolved = []
|
||||
db = SessionLocal()
|
||||
try:
|
||||
for sid, model, endpoint, endpoint_id in [
|
||||
(sid_a, model_a, endpoint_a, endpoint_a_id),
|
||||
(sid_b, model_b, endpoint_b, endpoint_b_id),
|
||||
]:
|
||||
# Prefer an explicit endpoint id: it pins the EXACT registered
|
||||
# endpoint (and its api_key), even when two endpoints visible to
|
||||
# the caller share a base_url with different keys — a URL-only
|
||||
# match would copy whichever row sorts first, i.e. possibly the
|
||||
# wrong key. Fall back to URL resolution only for legacy / admin
|
||||
# raw-URL callers that don't send an id.
|
||||
eid = endpoint_id.strip() if isinstance(endpoint_id, str) else ""
|
||||
if eid:
|
||||
ep = _owned_endpoint_by_id(db, eid, user)
|
||||
if ep is None:
|
||||
# An id the caller can't see (wrong owner / deleted) must
|
||||
# NOT silently fall back to a same-URL row with a different
|
||||
# key — that's exactly the mix-up ids exist to prevent.
|
||||
raise HTTPException(404, "Model endpoint not found")
|
||||
# The id already resolved the endpoint; ignore any raw URL the
|
||||
# caller also sent and dial the stored config instead.
|
||||
endpoint = ep.base_url
|
||||
elif not endpoint:
|
||||
raise HTTPException(
|
||||
422, "endpoint_a/endpoint_b or endpoint_a_id/endpoint_b_id is required"
|
||||
)
|
||||
else:
|
||||
# Resolve the supplied URL to a ModelEndpoint the caller owns
|
||||
# (their own rows + legacy null-owner shared rows), scoped so a
|
||||
# comparison can't borrow another user's private endpoint key.
|
||||
base = normalize_base(endpoint)
|
||||
ep = _owned_endpoint_by_url(db, base, user)
|
||||
# Reject *unregistered* raw URLs for signed-in non-admins; a
|
||||
# matched registered endpoint supplies an id so the caller can
|
||||
# still compare endpoints they own. Blanket-rejecting here (the
|
||||
# earlier `endpoint_id=None` call) locked non-admins out of
|
||||
# compare entirely, since compare resolves endpoints by URL with
|
||||
# no endpoint_id. Mirrors the gallery inpaint/harmonize checks.
|
||||
# Raised here (phase 1), before any session exists.
|
||||
_reject_raw_endpoint_url_for_non_admin(
|
||||
request, user, str(ep.id) if ep is not None else None, endpoint
|
||||
)
|
||||
# Bind the [CMP] session to the RESOLVED endpoint, not the raw
|
||||
# caller-supplied string. When the URL matches a registered
|
||||
# endpoint visible to the caller, use that row's own normalized
|
||||
# base URL (the same value owner scoping + endpoint validation
|
||||
# already vetted) so the session dials exactly where the stored
|
||||
# config points. The raw `endpoint` only survives for callers
|
||||
# allowed to pass one — admins / single-user mode, where
|
||||
# `_reject_raw_endpoint_url_for_non_admin` is a no-op and `ep`
|
||||
# is None. Mirrors the registered-endpoint path in session_routes.
|
||||
session_endpoint_url = (
|
||||
build_chat_url(normalize_base(ep.base_url)) if ep is not None else endpoint
|
||||
)
|
||||
# Headers come only from a matched endpoint's key; None when
|
||||
# `ep` is None (raw admin URL or no match), so a comparison can
|
||||
# never inherit another user's key/headers.
|
||||
headers = build_headers(ep.api_key, ep.base_url) if (ep and ep.api_key) else None
|
||||
resolved.append((sid, model, session_endpoint_url, headers))
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
# Both endpoints validated — only now create the ephemeral [CMP]
|
||||
# sessions and copy any resolved headers.
|
||||
for sid, model, session_endpoint_url, headers in resolved:
|
||||
name = f"[CMP] {slot_name[sid]}" if blind else f"[CMP] {model.split('/')[-1]}"
|
||||
session_manager.create_session(
|
||||
session_id=sid,
|
||||
name=name,
|
||||
endpoint_url=session_endpoint_url,
|
||||
model=model,
|
||||
rag=False,
|
||||
owner=user,
|
||||
)
|
||||
if headers:
|
||||
s = session_manager.sessions.get(sid)
|
||||
if s:
|
||||
s.headers = headers
|
||||
|
||||
# Store comparison record
|
||||
db = SessionLocal()
|
||||
try:
|
||||
comp = Comparison(
|
||||
id=comp_id,
|
||||
prompt=prompt,
|
||||
model_a=model_a,
|
||||
model_b=model_b,
|
||||
# Record the URL the session actually dials. For URL callers this
|
||||
# is their raw input; for id-only callers (empty endpoint_a/_b)
|
||||
# fall back to the resolved endpoint URL so the column stays
|
||||
# meaningful and non-null. resolved is in [a, b] order.
|
||||
endpoint_a=endpoint_a or resolved[0][2],
|
||||
endpoint_b=endpoint_b or resolved[1][2],
|
||||
is_blind=blind,
|
||||
blind_mapping=json.dumps(mapping),
|
||||
owner=user,
|
||||
)
|
||||
db.add(comp)
|
||||
db.commit()
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
# In blind mode, withhold the model identities AND the left/right
|
||||
# mapping from the response. The client already knows model_a/model_b
|
||||
# (it sent them), so returning either would defeat blind mode. They are
|
||||
# revealed by POST /api/compare/{id}/vote once the user has voted (#1285).
|
||||
return {
|
||||
"id": comp_id,
|
||||
"session_left": session_left,
|
||||
"session_right": session_right,
|
||||
"model_left": None if blind else (model_a if mapping["left"] == "a" else model_b),
|
||||
"model_right": None if blind else (model_a if mapping["right"] == "a" else model_b),
|
||||
"is_blind": blind,
|
||||
"mapping": None if blind else mapping,
|
||||
}
|
||||
|
||||
@router.post("/{comp_id}/vote")
|
||||
def vote_comparison(
|
||||
request: Request,
|
||||
comp_id: str,
|
||||
winner: str = Form(...), # "left", "right", or "tie"
|
||||
):
|
||||
"""Record the user's vote and reveal model names if blind."""
|
||||
user = get_current_user(request)
|
||||
db = SessionLocal()
|
||||
try:
|
||||
comp = db.query(Comparison).filter(Comparison.id == comp_id).first()
|
||||
if not comp:
|
||||
raise HTTPException(404, "Comparison not found")
|
||||
# SECURITY: strict ownership — null-owner Comparisons were
|
||||
# accessible to every user.
|
||||
if user and comp.owner != user:
|
||||
raise HTTPException(404, "Comparison not found")
|
||||
if comp.winner:
|
||||
raise HTTPException(400, "Already voted")
|
||||
|
||||
mapping = json.loads(comp.blind_mapping) if comp.blind_mapping else {"left": "a", "right": "b"}
|
||||
|
||||
if winner == "tie":
|
||||
comp.winner = "tie"
|
||||
elif winner == "left":
|
||||
comp.winner = mapping["left"]
|
||||
elif winner == "right":
|
||||
comp.winner = mapping["right"]
|
||||
else:
|
||||
raise HTTPException(400, "winner must be 'left', 'right', or 'tie'")
|
||||
|
||||
comp.voted_at = datetime.utcnow()
|
||||
db.commit()
|
||||
|
||||
return {
|
||||
"winner": comp.winner,
|
||||
"model_a": comp.model_a,
|
||||
"model_b": comp.model_b,
|
||||
"revealed": {
|
||||
"left": comp.model_a if mapping["left"] == "a" else comp.model_b,
|
||||
"right": comp.model_a if mapping["right"] == "a" else comp.model_b,
|
||||
},
|
||||
}
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
@router.post("/record")
|
||||
def record_comparison(request: Request, body: RecordVoteRequest):
|
||||
"""Lightweight endpoint to record a comparison vote from the frontend."""
|
||||
user = get_current_user(request)
|
||||
comp_id = str(uuid.uuid4())
|
||||
|
||||
model_a = body.models[0] if len(body.models) > 0 else ""
|
||||
model_b = body.models[1] if len(body.models) > 1 else ""
|
||||
|
||||
# For N>2 models, store the full list as JSON in blind_mapping
|
||||
if len(body.models) > 2:
|
||||
blind_mapping = json.dumps({"models": body.models})
|
||||
else:
|
||||
blind_mapping = None
|
||||
|
||||
db = SessionLocal()
|
||||
try:
|
||||
comp = Comparison(
|
||||
id=comp_id,
|
||||
prompt=body.prompt[:500],
|
||||
model_a=model_a,
|
||||
model_b=model_b,
|
||||
endpoint_a="",
|
||||
endpoint_b="",
|
||||
winner=body.winner,
|
||||
is_blind=body.is_blind,
|
||||
blind_mapping=blind_mapping,
|
||||
voted_at=datetime.utcnow(),
|
||||
owner=user,
|
||||
)
|
||||
db.add(comp)
|
||||
db.commit()
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
return {"status": "ok", "id": comp_id}
|
||||
|
||||
@router.get("/history")
|
||||
def list_comparisons(request: Request):
|
||||
"""List past comparisons."""
|
||||
user = get_current_user(request)
|
||||
db = SessionLocal()
|
||||
try:
|
||||
q = db.query(Comparison)
|
||||
if user:
|
||||
q = q.filter(Comparison.owner == user)
|
||||
comps = q.order_by(Comparison.created_at.desc()).limit(50).all()
|
||||
return [
|
||||
{
|
||||
"id": c.id,
|
||||
"prompt": c.prompt[:100],
|
||||
"model_a": c.model_a,
|
||||
"model_b": c.model_b,
|
||||
"winner": c.winner,
|
||||
"is_blind": c.is_blind,
|
||||
"voted_at": c.voted_at.isoformat() if c.voted_at else None,
|
||||
"created_at": c.created_at.isoformat() if c.created_at else None,
|
||||
}
|
||||
for c in comps
|
||||
]
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
@router.delete("/{comp_id}")
|
||||
def delete_comparison(request: Request, comp_id: str):
|
||||
"""Delete a comparison and its ephemeral sessions."""
|
||||
user = get_current_user(request)
|
||||
db = SessionLocal()
|
||||
try:
|
||||
comp = db.query(Comparison).filter(Comparison.id == comp_id).first()
|
||||
if not comp:
|
||||
raise HTTPException(404, "Comparison not found")
|
||||
# SECURITY: strict ownership — null-owner Comparisons were
|
||||
# accessible to every user.
|
||||
if user and comp.owner != user:
|
||||
raise HTTPException(404, "Comparison not found")
|
||||
db.delete(comp)
|
||||
db.commit()
|
||||
return {"status": "deleted"}
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
return router
|
||||
+14
-361
@@ -1,365 +1,18 @@
|
||||
# routes/compare_routes.py
|
||||
"""Model A/B comparison routes."""
|
||||
import json
|
||||
import uuid
|
||||
import random
|
||||
from datetime import datetime
|
||||
from fastapi import APIRouter, Form, HTTPException, Request
|
||||
from typing import List
|
||||
from pydantic import BaseModel
|
||||
import logging
|
||||
"""Backward-compat shim — canonical location is routes/compare/compare_routes.py.
|
||||
|
||||
from core.database import Comparison, SessionLocal
|
||||
from core.session_manager import SessionManager
|
||||
from src.auth_helpers import get_current_user
|
||||
from routes.session_routes import _reject_raw_endpoint_url_for_non_admin
|
||||
This module is replaced in ``sys.modules`` by the canonical module object so
|
||||
that ``import routes.compare_routes``, ``from routes.compare_routes import X``,
|
||||
``importlib.import_module("routes.compare_routes")``, and the
|
||||
``import ... as cr`` + ``monkeypatch.setattr(cr, "SessionLocal", ...)`` /
|
||||
``"_owned_endpoint_by_url"`` / ``"_owned_endpoint_by_id"`` pattern used by
|
||||
test_endpoint_owner_scope_followup.py all operate on the *same* object the
|
||||
application actually uses. Keeps existing import paths working after
|
||||
slice 2i (#4082/#4071). Source-introspection tests read the canonical file
|
||||
by path.
|
||||
"""
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
import sys as _sys
|
||||
|
||||
router = APIRouter(prefix="/api/compare", tags=["compare"])
|
||||
from routes.compare import compare_routes as _canonical # noqa: F401
|
||||
|
||||
|
||||
def _owned_endpoint_by_url(db, base_url, owner):
|
||||
"""ModelEndpoint whose base_url == `base_url` and is VISIBLE to `owner`
|
||||
(their own rows + legacy null-owner "shared" rows); None otherwise.
|
||||
|
||||
Owner-scoped on purpose. ModelEndpoint is per-user (core/database.py: non-null
|
||||
owner = private, "the model picker only shows the endpoint to that user") and
|
||||
holds a decrypted `api_key`. start_comparison copies the matched row's api_key
|
||||
into the caller-owned [CMP] session's headers, which then drives that session's
|
||||
/api/chat_stream calls — so an UNSCOPED base_url match would let a user mint a
|
||||
comparison bound to ANOTHER user's private endpoint and spend that owner's
|
||||
api_key / reach whatever base_url they configured. Mirrors
|
||||
session_routes._owned_endpoint. A null/empty owner is a no-op (single-user /
|
||||
legacy mode).
|
||||
"""
|
||||
from core.database import ModelEndpoint
|
||||
from src.auth_helpers import owner_filter
|
||||
q = db.query(ModelEndpoint).filter(ModelEndpoint.base_url == base_url)
|
||||
return owner_filter(q, ModelEndpoint, owner).first()
|
||||
|
||||
|
||||
def _owned_endpoint_by_id(db, endpoint_id, owner):
|
||||
"""ModelEndpoint whose id == `endpoint_id` and is VISIBLE to `owner` (their
|
||||
own rows + legacy null-owner "shared" rows); None otherwise.
|
||||
|
||||
Preferred over _owned_endpoint_by_url for credential resolution: two visible
|
||||
endpoints can share the same base_url but hold DIFFERENT api_keys (e.g. two
|
||||
accounts on the same provider). A base_url-only match returns whichever row
|
||||
sorts first, so it can copy the WRONG owner-scoped key into the [CMP] session.
|
||||
An id pins the exact registered endpoint, so /api/compare/start prefers it and
|
||||
only falls back to URL matching for legacy / admin raw-URL callers. Owner
|
||||
scoping is identical to _owned_endpoint_by_url (a null/empty owner is a no-op).
|
||||
"""
|
||||
from core.database import ModelEndpoint
|
||||
from src.auth_helpers import owner_filter
|
||||
q = db.query(ModelEndpoint).filter(ModelEndpoint.id == endpoint_id)
|
||||
return owner_filter(q, ModelEndpoint, owner).first()
|
||||
|
||||
|
||||
class RecordVoteRequest(BaseModel):
|
||||
prompt: str
|
||||
models: List[str]
|
||||
winner: str # model name or "tie"
|
||||
is_blind: bool = True
|
||||
|
||||
|
||||
def setup_compare_routes(session_manager: SessionManager):
|
||||
"""Setup comparison routes."""
|
||||
|
||||
@router.post("/start")
|
||||
def start_comparison(
|
||||
request: Request,
|
||||
prompt: str = Form(...),
|
||||
model_a: str = Form(...),
|
||||
model_b: str = Form(...),
|
||||
endpoint_a: str = Form(""),
|
||||
endpoint_b: str = Form(""),
|
||||
endpoint_a_id: str = Form(""),
|
||||
endpoint_b_id: str = Form(""),
|
||||
is_blind: str = Form("true"),
|
||||
):
|
||||
"""Create two ephemeral sessions and a comparison record.
|
||||
|
||||
Returns the comparison ID and the two session IDs so the client
|
||||
can fire two independent SSE streams to /api/chat_stream.
|
||||
"""
|
||||
user = getattr(request.state, 'current_user', None)
|
||||
comp_id = str(uuid.uuid4())
|
||||
sid_a = str(uuid.uuid4())
|
||||
sid_b = str(uuid.uuid4())
|
||||
|
||||
# Blind mapping: randomly assign left/right
|
||||
blind = str(is_blind).lower() == "true"
|
||||
if blind:
|
||||
mapping = {"left": "a", "right": "b"}
|
||||
if random.random() > 0.5:
|
||||
mapping = {"left": "b", "right": "a"}
|
||||
else:
|
||||
mapping = {"left": "a", "right": "b"}
|
||||
|
||||
# Map session IDs to left/right based on blind mapping
|
||||
session_left = sid_a if mapping["left"] == "a" else sid_b
|
||||
session_right = sid_a if mapping["right"] == "a" else sid_b
|
||||
|
||||
# In blind mode, name the helper sessions by their neutral slot
|
||||
# ("Model A" / "Model B") instead of the real model. Otherwise the
|
||||
# session name leaks the model in the sidebar and GET /api/sessions,
|
||||
# de-anonymizing the comparison before the user votes (issue #1285).
|
||||
slot_name = {session_left: "Model A", session_right: "Model B"}
|
||||
|
||||
# SECURITY: resolve and validate BOTH endpoints before creating any
|
||||
# session. Compare copies a registered endpoint's Authorization header
|
||||
# into the [CMP] session, so validating one endpoint while creating its
|
||||
# session, then rejecting the other, would leave a partial compare
|
||||
# session behind with that header attached. Doing all the owner-scope
|
||||
# resolution + raw-URL rejection up front means a 403 on either endpoint
|
||||
# aborts the whole request with nothing created and no header copied.
|
||||
from src.endpoint_resolver import build_chat_url, build_headers, normalize_base
|
||||
resolved = []
|
||||
db = SessionLocal()
|
||||
try:
|
||||
for sid, model, endpoint, endpoint_id in [
|
||||
(sid_a, model_a, endpoint_a, endpoint_a_id),
|
||||
(sid_b, model_b, endpoint_b, endpoint_b_id),
|
||||
]:
|
||||
# Prefer an explicit endpoint id: it pins the EXACT registered
|
||||
# endpoint (and its api_key), even when two endpoints visible to
|
||||
# the caller share a base_url with different keys — a URL-only
|
||||
# match would copy whichever row sorts first, i.e. possibly the
|
||||
# wrong key. Fall back to URL resolution only for legacy / admin
|
||||
# raw-URL callers that don't send an id.
|
||||
eid = endpoint_id.strip() if isinstance(endpoint_id, str) else ""
|
||||
if eid:
|
||||
ep = _owned_endpoint_by_id(db, eid, user)
|
||||
if ep is None:
|
||||
# An id the caller can't see (wrong owner / deleted) must
|
||||
# NOT silently fall back to a same-URL row with a different
|
||||
# key — that's exactly the mix-up ids exist to prevent.
|
||||
raise HTTPException(404, "Model endpoint not found")
|
||||
# The id already resolved the endpoint; ignore any raw URL the
|
||||
# caller also sent and dial the stored config instead.
|
||||
endpoint = ep.base_url
|
||||
elif not endpoint:
|
||||
raise HTTPException(
|
||||
422, "endpoint_a/endpoint_b or endpoint_a_id/endpoint_b_id is required"
|
||||
)
|
||||
else:
|
||||
# Resolve the supplied URL to a ModelEndpoint the caller owns
|
||||
# (their own rows + legacy null-owner shared rows), scoped so a
|
||||
# comparison can't borrow another user's private endpoint key.
|
||||
base = normalize_base(endpoint)
|
||||
ep = _owned_endpoint_by_url(db, base, user)
|
||||
# Reject *unregistered* raw URLs for signed-in non-admins; a
|
||||
# matched registered endpoint supplies an id so the caller can
|
||||
# still compare endpoints they own. Blanket-rejecting here (the
|
||||
# earlier `endpoint_id=None` call) locked non-admins out of
|
||||
# compare entirely, since compare resolves endpoints by URL with
|
||||
# no endpoint_id. Mirrors the gallery inpaint/harmonize checks.
|
||||
# Raised here (phase 1), before any session exists.
|
||||
_reject_raw_endpoint_url_for_non_admin(
|
||||
request, user, str(ep.id) if ep is not None else None, endpoint
|
||||
)
|
||||
# Bind the [CMP] session to the RESOLVED endpoint, not the raw
|
||||
# caller-supplied string. When the URL matches a registered
|
||||
# endpoint visible to the caller, use that row's own normalized
|
||||
# base URL (the same value owner scoping + endpoint validation
|
||||
# already vetted) so the session dials exactly where the stored
|
||||
# config points. The raw `endpoint` only survives for callers
|
||||
# allowed to pass one — admins / single-user mode, where
|
||||
# `_reject_raw_endpoint_url_for_non_admin` is a no-op and `ep`
|
||||
# is None. Mirrors the registered-endpoint path in session_routes.
|
||||
session_endpoint_url = (
|
||||
build_chat_url(normalize_base(ep.base_url)) if ep is not None else endpoint
|
||||
)
|
||||
# Headers come only from a matched endpoint's key; None when
|
||||
# `ep` is None (raw admin URL or no match), so a comparison can
|
||||
# never inherit another user's key/headers.
|
||||
headers = build_headers(ep.api_key, ep.base_url) if (ep and ep.api_key) else None
|
||||
resolved.append((sid, model, session_endpoint_url, headers))
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
# Both endpoints validated — only now create the ephemeral [CMP]
|
||||
# sessions and copy any resolved headers.
|
||||
for sid, model, session_endpoint_url, headers in resolved:
|
||||
name = f"[CMP] {slot_name[sid]}" if blind else f"[CMP] {model.split('/')[-1]}"
|
||||
session_manager.create_session(
|
||||
session_id=sid,
|
||||
name=name,
|
||||
endpoint_url=session_endpoint_url,
|
||||
model=model,
|
||||
rag=False,
|
||||
owner=user,
|
||||
)
|
||||
if headers:
|
||||
s = session_manager.sessions.get(sid)
|
||||
if s:
|
||||
s.headers = headers
|
||||
|
||||
# Store comparison record
|
||||
db = SessionLocal()
|
||||
try:
|
||||
comp = Comparison(
|
||||
id=comp_id,
|
||||
prompt=prompt,
|
||||
model_a=model_a,
|
||||
model_b=model_b,
|
||||
# Record the URL the session actually dials. For URL callers this
|
||||
# is their raw input; for id-only callers (empty endpoint_a/_b)
|
||||
# fall back to the resolved endpoint URL so the column stays
|
||||
# meaningful and non-null. resolved is in [a, b] order.
|
||||
endpoint_a=endpoint_a or resolved[0][2],
|
||||
endpoint_b=endpoint_b or resolved[1][2],
|
||||
is_blind=blind,
|
||||
blind_mapping=json.dumps(mapping),
|
||||
owner=user,
|
||||
)
|
||||
db.add(comp)
|
||||
db.commit()
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
# In blind mode, withhold the model identities AND the left/right
|
||||
# mapping from the response. The client already knows model_a/model_b
|
||||
# (it sent them), so returning either would defeat blind mode. They are
|
||||
# revealed by POST /api/compare/{id}/vote once the user has voted (#1285).
|
||||
return {
|
||||
"id": comp_id,
|
||||
"session_left": session_left,
|
||||
"session_right": session_right,
|
||||
"model_left": None if blind else (model_a if mapping["left"] == "a" else model_b),
|
||||
"model_right": None if blind else (model_a if mapping["right"] == "a" else model_b),
|
||||
"is_blind": blind,
|
||||
"mapping": None if blind else mapping,
|
||||
}
|
||||
|
||||
@router.post("/{comp_id}/vote")
|
||||
def vote_comparison(
|
||||
request: Request,
|
||||
comp_id: str,
|
||||
winner: str = Form(...), # "left", "right", or "tie"
|
||||
):
|
||||
"""Record the user's vote and reveal model names if blind."""
|
||||
user = get_current_user(request)
|
||||
db = SessionLocal()
|
||||
try:
|
||||
comp = db.query(Comparison).filter(Comparison.id == comp_id).first()
|
||||
if not comp:
|
||||
raise HTTPException(404, "Comparison not found")
|
||||
# SECURITY: strict ownership — null-owner Comparisons were
|
||||
# accessible to every user.
|
||||
if user and comp.owner != user:
|
||||
raise HTTPException(404, "Comparison not found")
|
||||
if comp.winner:
|
||||
raise HTTPException(400, "Already voted")
|
||||
|
||||
mapping = json.loads(comp.blind_mapping) if comp.blind_mapping else {"left": "a", "right": "b"}
|
||||
|
||||
if winner == "tie":
|
||||
comp.winner = "tie"
|
||||
elif winner == "left":
|
||||
comp.winner = mapping["left"]
|
||||
elif winner == "right":
|
||||
comp.winner = mapping["right"]
|
||||
else:
|
||||
raise HTTPException(400, "winner must be 'left', 'right', or 'tie'")
|
||||
|
||||
comp.voted_at = datetime.utcnow()
|
||||
db.commit()
|
||||
|
||||
return {
|
||||
"winner": comp.winner,
|
||||
"model_a": comp.model_a,
|
||||
"model_b": comp.model_b,
|
||||
"revealed": {
|
||||
"left": comp.model_a if mapping["left"] == "a" else comp.model_b,
|
||||
"right": comp.model_a if mapping["right"] == "a" else comp.model_b,
|
||||
},
|
||||
}
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
@router.post("/record")
|
||||
def record_comparison(request: Request, body: RecordVoteRequest):
|
||||
"""Lightweight endpoint to record a comparison vote from the frontend."""
|
||||
user = get_current_user(request)
|
||||
comp_id = str(uuid.uuid4())
|
||||
|
||||
model_a = body.models[0] if len(body.models) > 0 else ""
|
||||
model_b = body.models[1] if len(body.models) > 1 else ""
|
||||
|
||||
# For N>2 models, store the full list as JSON in blind_mapping
|
||||
if len(body.models) > 2:
|
||||
blind_mapping = json.dumps({"models": body.models})
|
||||
else:
|
||||
blind_mapping = None
|
||||
|
||||
db = SessionLocal()
|
||||
try:
|
||||
comp = Comparison(
|
||||
id=comp_id,
|
||||
prompt=body.prompt[:500],
|
||||
model_a=model_a,
|
||||
model_b=model_b,
|
||||
endpoint_a="",
|
||||
endpoint_b="",
|
||||
winner=body.winner,
|
||||
is_blind=body.is_blind,
|
||||
blind_mapping=blind_mapping,
|
||||
voted_at=datetime.utcnow(),
|
||||
owner=user,
|
||||
)
|
||||
db.add(comp)
|
||||
db.commit()
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
return {"status": "ok", "id": comp_id}
|
||||
|
||||
@router.get("/history")
|
||||
def list_comparisons(request: Request):
|
||||
"""List past comparisons."""
|
||||
user = get_current_user(request)
|
||||
db = SessionLocal()
|
||||
try:
|
||||
q = db.query(Comparison)
|
||||
if user:
|
||||
q = q.filter(Comparison.owner == user)
|
||||
comps = q.order_by(Comparison.created_at.desc()).limit(50).all()
|
||||
return [
|
||||
{
|
||||
"id": c.id,
|
||||
"prompt": c.prompt[:100],
|
||||
"model_a": c.model_a,
|
||||
"model_b": c.model_b,
|
||||
"winner": c.winner,
|
||||
"is_blind": c.is_blind,
|
||||
"voted_at": c.voted_at.isoformat() if c.voted_at else None,
|
||||
"created_at": c.created_at.isoformat() if c.created_at else None,
|
||||
}
|
||||
for c in comps
|
||||
]
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
@router.delete("/{comp_id}")
|
||||
def delete_comparison(request: Request, comp_id: str):
|
||||
"""Delete a comparison and its ephemeral sessions."""
|
||||
user = get_current_user(request)
|
||||
db = SessionLocal()
|
||||
try:
|
||||
comp = db.query(Comparison).filter(Comparison.id == comp_id).first()
|
||||
if not comp:
|
||||
raise HTTPException(404, "Comparison not found")
|
||||
# SECURITY: strict ownership — null-owner Comparisons were
|
||||
# accessible to every user.
|
||||
if user and comp.owner != user:
|
||||
raise HTTPException(404, "Comparison not found")
|
||||
db.delete(comp)
|
||||
db.commit()
|
||||
return {"status": "deleted"}
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
return router
|
||||
_sys.modules[__name__] = _canonical
|
||||
|
||||
@@ -0,0 +1,25 @@
|
||||
"""Regression test for the admin_wipe route shim (slice 2h, #4082/#4071).
|
||||
|
||||
The backward-compat shim at ``routes/admin_wipe_routes.py`` uses
|
||||
``sys.modules`` replacement so the legacy import path and the canonical
|
||||
``routes.admin_wipe.*`` path resolve to the *same* module object. This is
|
||||
required because ``test_admin_wipe_gallery.py`` does
|
||||
``import routes.admin_wipe_routes`` followed by
|
||||
``monkeypatch.setattr(routes.admin_wipe_routes, "SessionLocal", ...)`` and
|
||||
``"require_admin"`` — for those patches to take effect at runtime, the legacy
|
||||
module object and the canonical one must be identical.
|
||||
"""
|
||||
|
||||
import importlib
|
||||
|
||||
import routes.admin_wipe_routes as _shim_admin_wipe # noqa: F401
|
||||
|
||||
|
||||
def test_legacy_and_canonical_admin_wipe_module_are_same_object():
|
||||
"""``import routes.admin_wipe_routes`` must alias the canonical module."""
|
||||
legacy = importlib.import_module("routes.admin_wipe_routes")
|
||||
canonical = importlib.import_module("routes.admin_wipe.admin_wipe_routes")
|
||||
assert legacy is canonical, (
|
||||
"routes.admin_wipe_routes shim must resolve to the canonical "
|
||||
"routes.admin_wipe.admin_wipe_routes module object"
|
||||
)
|
||||
@@ -0,0 +1,25 @@
|
||||
"""Regression test for the cleanup route shim (slice 2g, #4082/#4071).
|
||||
|
||||
The backward-compat shim at ``routes/cleanup_routes.py`` uses ``sys.modules``
|
||||
replacement so the legacy import path and the canonical ``routes.cleanup.*``
|
||||
path resolve to the *same* module object. This is required because
|
||||
``test_cleanup_owner_scope.py`` uses string-targeted
|
||||
``monkeypatch.setattr("routes.cleanup_routes.get_cleanup_preview", ...)`` and
|
||||
``monkeypatch.delitem(sys.modules, "routes.cleanup_routes")`` + re-import —
|
||||
for those patches to take effect at runtime, the legacy module object and
|
||||
the canonical one must be identical.
|
||||
"""
|
||||
|
||||
import importlib
|
||||
|
||||
import routes.cleanup_routes as _shim_cleanup # noqa: F401
|
||||
|
||||
|
||||
def test_legacy_and_canonical_cleanup_module_are_same_object():
|
||||
"""``import routes.cleanup_routes`` must alias the canonical module."""
|
||||
legacy = importlib.import_module("routes.cleanup_routes")
|
||||
canonical = importlib.import_module("routes.cleanup.cleanup_routes")
|
||||
assert legacy is canonical, (
|
||||
"routes.cleanup_routes shim must resolve to the canonical "
|
||||
"routes.cleanup.cleanup_routes module object"
|
||||
)
|
||||
@@ -0,0 +1,25 @@
|
||||
"""Regression test for the compare route shim (slice 2i, #4082/#4071).
|
||||
|
||||
The backward-compat shim at ``routes/compare_routes.py`` uses ``sys.modules``
|
||||
replacement so the legacy import path and the canonical ``routes.compare.*``
|
||||
path resolve to the *same* module object. This is required because
|
||||
``test_endpoint_owner_scope_followup.py`` uses ``import routes.compare_routes
|
||||
as cr`` followed by ``monkeypatch.setattr(cr, "SessionLocal", ...)`` /
|
||||
``"_owned_endpoint_by_url"`` / ``"_owned_endpoint_by_id"`` — for those patches
|
||||
to take effect at runtime, the legacy module object and the canonical one
|
||||
must be identical.
|
||||
"""
|
||||
|
||||
import importlib
|
||||
|
||||
import routes.compare_routes as _shim_compare # noqa: F401
|
||||
|
||||
|
||||
def test_legacy_and_canonical_compare_module_are_same_object():
|
||||
"""``import routes.compare_routes`` must alias the canonical module."""
|
||||
legacy = importlib.import_module("routes.compare_routes")
|
||||
canonical = importlib.import_module("routes.compare.compare_routes")
|
||||
assert legacy is canonical, (
|
||||
"routes.compare_routes shim must resolve to the canonical "
|
||||
"routes.compare.compare_routes module object"
|
||||
)
|
||||
@@ -358,7 +358,7 @@ def test_compare_start_rejects_unowned_endpoint_id(monkeypatch):
|
||||
|
||||
|
||||
def test_compare_endpoint_key_lookup_is_owner_scoped():
|
||||
body = Path("routes/compare_routes.py").read_text(encoding="utf-8")
|
||||
body = Path("routes/compare/compare_routes.py").read_text(encoding="utf-8")
|
||||
start_body = body.split("def start_comparison", 1)[1].split("# Store comparison record", 1)[0]
|
||||
helper_body = body.split("def _owned_endpoint_by_url", 1)[1].split("class RecordVoteRequest", 1)[0]
|
||||
id_helper_body = body.split("def _owned_endpoint_by_id", 1)[1].split("class RecordVoteRequest", 1)[0]
|
||||
|
||||
Reference in New Issue
Block a user