diff --git a/.gitignore b/.gitignore index 8f7973cf..9d871b02 100644 --- a/.gitignore +++ b/.gitignore @@ -24,4 +24,7 @@ phd-advisor-frontend/firebase.json # Python virtual environments **/venv/ -.venv/ \ No newline at end of file +.venv/ + +# Cursor docs +.cursor/ \ No newline at end of file diff --git a/multi_llm_chatbot_backend/app/api/routes/chat.py b/multi_llm_chatbot_backend/app/api/routes/chat.py index 5e27b9b4..c6f6297c 100644 --- a/multi_llm_chatbot_backend/app/api/routes/chat.py +++ b/multi_llm_chatbot_backend/app/api/routes/chat.py @@ -12,8 +12,10 @@ from app.api.routes.chat_sessions import persist_message from app.api.utils import get_or_create_session_for_request_async from app.core.auth import get_current_active_user +from app.config import get_settings from app.core.bootstrap import chat_orchestrator from app.core.database import get_database +from app.core.persona_filter import get_available_persona_ids from app.core.session_manager import get_session_manager from app.models.user import User @@ -137,9 +139,17 @@ async def _event_generator(): ).to_ndjson() return + # Filter personas by system whitelist and user preferences + available = get_available_persona_ids( + registered_ids=chat_orchestrator.list_personas(), + system_allowed=get_settings().personas.allowed_advisors, + user_disabled=current_user.disabled_advisors, + ) + # Get personas most relevant to the current session top_personas = await chat_orchestrator.get_top_personas( session_id=sid, + allowed_ids=available, ) done_queue: asyncio.Queue = asyncio.Queue() diff --git a/multi_llm_chatbot_backend/app/api/routes/preferences.py b/multi_llm_chatbot_backend/app/api/routes/preferences.py new file mode 100644 index 00000000..08ec4593 --- /dev/null +++ b/multi_llm_chatbot_backend/app/api/routes/preferences.py @@ -0,0 +1,78 @@ +import logging +from typing import List, Optional + +from fastapi import APIRouter, Depends, HTTPException, status +from pydantic import BaseModel + +from app.config import get_settings +from app.core.auth import get_current_active_user +from app.core.bootstrap import chat_orchestrator +from app.core.database import get_database +from app.core.persona_filter import get_available_persona_ids +from app.models.user import User + +logger = logging.getLogger(__name__) + +router = APIRouter() + + +class AdvisorPreferencesRequest(BaseModel): + disabled_advisors: Optional[List[str]] = None + + +class AdvisorPreferencesResponse(BaseModel): + disabled_advisors: Optional[List[str]] = None + available_advisors: List[str] = [] + + +def _build_response(user: User) -> AdvisorPreferencesResponse: + available = get_available_persona_ids( + registered_ids=chat_orchestrator.list_personas(), + system_allowed=get_settings().personas.allowed_advisors, + ) + return AdvisorPreferencesResponse( + disabled_advisors=user.disabled_advisors, + available_advisors=available, + ) + + +@router.get("/me/advisor-preferences", response_model=AdvisorPreferencesResponse) +async def get_advisor_preferences( + current_user: User = Depends(get_current_active_user), +): + """ + Retrieve advisor preferences for the authenticated user. + @param current_user: Authenticated user from dependency injection + @return: AdvisorPreferencesResponse containing disabled and available advisor IDs + """ + return _build_response(current_user) + + +@router.put("/me/advisor-preferences", response_model=AdvisorPreferencesResponse) +async def update_advisor_preferences( + body: AdvisorPreferencesRequest, + current_user: User = Depends(get_current_active_user), +): + """ + Update advisor preferences for the authenticated user. + @param body: AdvisorPreferencesRequest containing the list of advisor IDs to disable + @param current_user: Authenticated user from dependency injection + @return: AdvisorPreferencesResponse with updated disabled and available advisor IDs + @raises HTTPException 400: If any provided advisor ID is not recognized + """ + if body.disabled_advisors is not None: + known_ids = set(chat_orchestrator.list_personas()) + unknown = [aid for aid in body.disabled_advisors if aid not in known_ids] + if unknown: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=f"Unknown advisor IDs: {unknown}", + ) + + db = get_database() + await db.users.update_one( + {"_id": current_user.id}, + {"$set": {"disabled_advisors": body.disabled_advisors}}, + ) + current_user.disabled_advisors = body.disabled_advisors + return _build_response(current_user) diff --git a/multi_llm_chatbot_backend/app/config.py b/multi_llm_chatbot_backend/app/config.py index 25c0a1a9..aca5a307 100644 --- a/multi_llm_chatbot_backend/app/config.py +++ b/multi_llm_chatbot_backend/app/config.py @@ -169,6 +169,16 @@ class PersonasConfig(BaseModel): personas_dir: str = "" config_dir: str = "" items: List[PersonaItemConfig] = [] + allowed_advisors: Optional[List[str]] = None + + @model_validator(mode='after') + def _validate_allowed_advisors(self): + if self.allowed_advisors is not None and len(self.allowed_advisors) == 0: + raise ValueError( + "allowed_advisors must not be an empty list; " + "omit the setting or set to null to allow all advisors" + ) + return self @model_validator(mode='after') def _load_personas_from_directory(self): @@ -320,13 +330,19 @@ class AppSettings(BaseModel): def get_frontend_config(self) -> dict: """Return the subset of configuration safe to expose to the frontend via ``GET /api/config``. Secrets are excluded.""" + allowed = self.personas.allowed_advisors + persona_items = self.personas.items + if allowed is not None: + allowed_set = set(allowed) + persona_items = [p for p in persona_items if p.id in allowed_set] + return { "app": self.app.dict(), "homepage": self.homepage.dict(), "login": self.login.dict(), "chat_page": self.chat_page.dict(), "personas": { - "items": [p.to_frontend_config() for p in self.personas.items], + "items": [p.to_frontend_config() for p in persona_items], }, } diff --git a/multi_llm_chatbot_backend/app/core/improved_orchestrator.py b/multi_llm_chatbot_backend/app/core/improved_orchestrator.py index b474dfd7..2191b125 100644 --- a/multi_llm_chatbot_backend/app/core/improved_orchestrator.py +++ b/multi_llm_chatbot_backend/app/core/improved_orchestrator.py @@ -871,20 +871,27 @@ async def chat_with_persona(self, user_input: str, persona_id: str, session_id: } - async def get_top_personas(self, session_id: str, k: int = 3) -> List[str]: + async def get_top_personas(self, session_id: str, k: int = 3, + allowed_ids: Optional[List[str]] = None) -> List[str]: """ Use the LLM to rank personas based on current session context. Falls back to default persona order if LLM fails or returns invalid data. + + When *allowed_ids* is provided, only those personas are considered + (for system-level and user-level filtering). """ + pool_ids = allowed_ids if allowed_ids is not None else list(self.personas.keys()) + pool = {pid: self.personas[pid] for pid in pool_ids if pid in self.personas} + try: session = self.session_manager.get_session(session_id) - if not self.personas: - logger.warning("No personas registered.") + if not pool: + logger.warning("No personas available after filtering.") return [] # Use the LLM from one of the existing persona objects - llm = next(iter(self.personas.values())).llm + llm = next(iter(pool.values())).llm # Use recent conversation context (last 5 messages) recent_context = "\n".join( @@ -894,11 +901,11 @@ async def get_top_personas(self, session_id: str, k: int = 3) -> List[str]: # Format available persona descriptions persona_descriptions = "\n".join([ f"- ID: {p.id}\n Name: {p.name}\n Prompt: {p.system_prompt.strip()}" - for p in self.personas.values() + for p in pool.values() ]) # Ensure k does not exceed the number of available personas - k = min(k, len(self.personas)) + k = min(k, len(pool)) app_title = get_settings().app.title @@ -935,15 +942,15 @@ async def get_top_personas(self, session_id: str, k: int = 3) -> List[str]: if isinstance(top_ids, dict): top_ids = next(iter(top_ids.values()), []) - # Step 3: Filter valid persona IDs - valid_ids = [pid for pid in top_ids if pid in self.personas] + # Step 3: Filter valid persona IDs against the allowed pool + valid_ids = [pid for pid in top_ids if pid in pool] if len(valid_ids) < k: logger.warning(f"LLM returned insufficient or invalid IDs. Got: {valid_ids}") - return list(self.personas.keys())[:k] + return list(pool.keys())[:k] return valid_ids[:k] except Exception as e: logger.error(f"Error selecting top personas: {e}") - return list(self.personas.keys())[:k] + return list(pool.keys())[:k] diff --git a/multi_llm_chatbot_backend/app/core/persona_filter.py b/multi_llm_chatbot_backend/app/core/persona_filter.py new file mode 100644 index 00000000..819883d7 --- /dev/null +++ b/multi_llm_chatbot_backend/app/core/persona_filter.py @@ -0,0 +1,29 @@ +from typing import List, Optional + + +def get_available_persona_ids( + registered_ids: List[str], + system_allowed: Optional[List[str]] = None, + user_disabled: Optional[List[str]] = None, +) -> List[str]: + """Return the persona IDs available after applying all filtering layers. + + Filtering is applied in order: + 1. System whitelist (``system_allowed``) — if not None, only IDs + present in this list survive. ``None`` means no restriction. + 2. User blocklist (``user_disabled``) — if not None, these IDs are + removed. ``None`` means no user overrides. + + The order of *registered_ids* is preserved in the result so that + downstream fallback logic (e.g. first-K when LLM ranking fails) + remains deterministic. + """ + ids = list(registered_ids) + + if system_allowed is not None: + ids = [pid for pid in ids if pid in system_allowed] + + if user_disabled is not None: + ids = [pid for pid in ids if pid not in user_disabled] + + return ids diff --git a/multi_llm_chatbot_backend/app/main.py b/multi_llm_chatbot_backend/app/main.py index 3f366cb0..ef62eb13 100644 --- a/multi_llm_chatbot_backend/app/main.py +++ b/multi_llm_chatbot_backend/app/main.py @@ -22,6 +22,7 @@ from app.api.routes.auth import router as auth_router from app.api.routes.chat_sessions import router as chat_sessions_router from app.api.routes.phd_canvas import router as phd_canvas_router +from app.api.routes.preferences import router as preferences_router import logging @@ -60,6 +61,7 @@ async def lifespan(app: FastAPI): app.include_router(auth_router, prefix="/auth", tags=["authentication"]) app.include_router(chat_sessions_router, prefix="/api", tags=["chat-sessions"]) app.include_router(phd_canvas_router, prefix="/api", tags=["phd-canvas"]) +app.include_router(preferences_router, prefix="/api", tags=["preferences"]) # Serve bundled avatar images _avatars_dir = Path(__file__).resolve().parent / "assets" / "avatars" diff --git a/multi_llm_chatbot_backend/app/models/user.py b/multi_llm_chatbot_backend/app/models/user.py index 95d1067c..2a3c9e16 100644 --- a/multi_llm_chatbot_backend/app/models/user.py +++ b/multi_llm_chatbot_backend/app/models/user.py @@ -47,6 +47,7 @@ class User(BaseModel): hashed_password: str academicStage: Optional[str] = None researchArea: Optional[str] = None + disabled_advisors: Optional[List[str]] = None created_at: datetime = Field(default_factory=datetime.utcnow) last_login: Optional[datetime] = None is_active: bool = True diff --git a/multi_llm_chatbot_backend/app/tests/unit/test_advisor_preferences.py b/multi_llm_chatbot_backend/app/tests/unit/test_advisor_preferences.py new file mode 100644 index 00000000..b8c6b79f --- /dev/null +++ b/multi_llm_chatbot_backend/app/tests/unit/test_advisor_preferences.py @@ -0,0 +1,186 @@ +import asyncio +import sys +import unittest +from datetime import datetime +from unittest.mock import AsyncMock, MagicMock, patch + +from bson import ObjectId +from fastapi import HTTPException + +# Stub heavy modules before importing the preferences route module. +_mock_bootstrap = MagicMock() +_mock_bootstrap.chat_orchestrator.list_personas.return_value = [ + "pragmatist", "theorist", "methodologist", +] +sys.modules.setdefault("app.core.bootstrap", _mock_bootstrap) +sys.modules.setdefault("app.core.rag_manager", MagicMock()) + +from fastapi import APIRouter # noqa: E402 + +_stub_router_module = MagicMock(router=APIRouter()) +for _name in ( + "app.api.routes.chat", + "app.api.routes.documents", + "app.api.routes.sessions", + "app.api.routes.provider", + "app.api.routes.debug", + "app.api.routes.root", + "app.api.routes.phd_canvas", +): + sys.modules.setdefault(_name, _stub_router_module) + +from app.api.routes.preferences import ( # noqa: E402 + AdvisorPreferencesRequest, + get_advisor_preferences, + update_advisor_preferences, +) +from app.models.user import User # noqa: E402 + +FAKE_USER_ID = ObjectId() +ALL_IDS = ["pragmatist", "theorist", "methodologist"] + + +def _make_fake_user(**overrides): + defaults = dict( + _id=FAKE_USER_ID, + firstName="Test", + lastName="User", + email="test@example.com", + hashed_password="$2b$12$fakehash", + is_active=True, + created_at=datetime(2025, 1, 1), + ) + defaults.update(overrides) + return User(**defaults) + + +def _mock_db(): + db = MagicMock() + db.users.update_one = AsyncMock() + return db + + +def _mock_settings(allowed_advisors=None): + settings = MagicMock() + settings.personas.allowed_advisors = allowed_advisors + return settings + + +# ------------------------------------------------------------------ +# GET /api/me/advisor-preferences +# ------------------------------------------------------------------ + + +@patch("app.api.routes.preferences.get_settings") +@patch("app.api.routes.preferences.chat_orchestrator") +class TestGetAdvisorPreferences(unittest.TestCase): + + def test_returns_none_when_no_prefs_set(self, mock_orch, mock_settings): + mock_orch.list_personas.return_value = ALL_IDS + mock_settings.return_value = _mock_settings() + + user = _make_fake_user() + result = asyncio.run(get_advisor_preferences(current_user=user)) + + self.assertIsNone(result.disabled_advisors) + self.assertEqual(result.available_advisors, ALL_IDS) + + def test_returns_disabled_list(self, mock_orch, mock_settings): + mock_orch.list_personas.return_value = ALL_IDS + mock_settings.return_value = _mock_settings() + + user = _make_fake_user(disabled_advisors=["theorist"]) + result = asyncio.run(get_advisor_preferences(current_user=user)) + + self.assertEqual(result.disabled_advisors, ["theorist"]) + + def test_available_reflects_system_whitelist(self, mock_orch, mock_settings): + mock_orch.list_personas.return_value = ALL_IDS + mock_settings.return_value = _mock_settings( + allowed_advisors=["pragmatist", "theorist"], + ) + + user = _make_fake_user() + result = asyncio.run(get_advisor_preferences(current_user=user)) + + self.assertEqual(result.available_advisors, ["pragmatist", "theorist"]) + + +# ------------------------------------------------------------------ +# PUT /api/me/advisor-preferences +# ------------------------------------------------------------------ + + +@patch("app.api.routes.preferences.get_database") +@patch("app.api.routes.preferences.get_settings") +@patch("app.api.routes.preferences.chat_orchestrator") +class TestUpdateAdvisorPreferences(unittest.TestCase): + + def test_valid_ids_persisted(self, mock_orch, mock_settings, mock_get_db): + mock_orch.list_personas.return_value = ALL_IDS + mock_settings.return_value = _mock_settings() + db = _mock_db() + mock_get_db.return_value = db + + user = _make_fake_user() + body = AdvisorPreferencesRequest(disabled_advisors=["theorist"]) + result = asyncio.run( + update_advisor_preferences(body=body, current_user=user) + ) + + db.users.update_one.assert_called_once_with( + {"_id": user.id}, + {"$set": {"disabled_advisors": ["theorist"]}}, + ) + self.assertEqual(result.disabled_advisors, ["theorist"]) + + def test_unknown_ids_rejected(self, mock_orch, mock_settings, mock_get_db): + mock_orch.list_personas.return_value = ALL_IDS + mock_settings.return_value = _mock_settings() + + user = _make_fake_user() + body = AdvisorPreferencesRequest(disabled_advisors=["fake_advisor"]) + + with self.assertRaises(HTTPException) as ctx: + asyncio.run( + update_advisor_preferences(body=body, current_user=user) + ) + + self.assertEqual(ctx.exception.status_code, 400) + self.assertIn("fake_advisor", ctx.exception.detail) + + def test_null_clears_preferences(self, mock_orch, mock_settings, mock_get_db): + mock_orch.list_personas.return_value = ALL_IDS + mock_settings.return_value = _mock_settings() + db = _mock_db() + mock_get_db.return_value = db + + user = _make_fake_user(disabled_advisors=["theorist"]) + body = AdvisorPreferencesRequest(disabled_advisors=None) + result = asyncio.run( + update_advisor_preferences(body=body, current_user=user) + ) + + db.users.update_one.assert_called_once_with( + {"_id": user.id}, + {"$set": {"disabled_advisors": None}}, + ) + self.assertIsNone(result.disabled_advisors) + + def test_empty_list_accepted(self, mock_orch, mock_settings, mock_get_db): + mock_orch.list_personas.return_value = ALL_IDS + mock_settings.return_value = _mock_settings() + db = _mock_db() + mock_get_db.return_value = db + + user = _make_fake_user(disabled_advisors=["theorist"]) + body = AdvisorPreferencesRequest(disabled_advisors=[]) + result = asyncio.run( + update_advisor_preferences(body=body, current_user=user) + ) + + db.users.update_one.assert_called_once_with( + {"_id": user.id}, + {"$set": {"disabled_advisors": []}}, + ) + self.assertEqual(result.disabled_advisors, []) diff --git a/multi_llm_chatbot_backend/app/tests/unit/test_persona_config.py b/multi_llm_chatbot_backend/app/tests/unit/test_persona_config.py index 1762bfa1..f44ba1d4 100644 --- a/multi_llm_chatbot_backend/app/tests/unit/test_persona_config.py +++ b/multi_llm_chatbot_backend/app/tests/unit/test_persona_config.py @@ -3,6 +3,7 @@ import tempfile import yaml import app.config +from pydantic import ValidationError from app.config import load_settings, load_personas_from_dir, PersonasConfig @@ -63,6 +64,71 @@ def test_uses_personas_dir(self): ids = {p.id for p in settings.personas.items} self.assertEqual(ids, {"one", "two"}) + def test_allowed_advisors_defaults_to_none(self): + cfg_path = _write_config(self.tmp_path, { + "personas": { + "items": [ + {"id": "a", "name": "A"}, + ] + } + }) + settings = load_settings(cfg_path) + self.assertIsNone(settings.personas.allowed_advisors) + + def test_allowed_advisors_populated(self): + cfg_path = _write_config(self.tmp_path, { + "personas": { + "allowed_advisors": ["one", "two"], + "items": [ + {"id": "one", "name": "One"}, + ] + } + }) + settings = load_settings(cfg_path) + self.assertEqual(settings.personas.allowed_advisors, ["one", "two"]) + + def test_allowed_advisors_empty_list_raises(self): + cfg_path = _write_config(self.tmp_path, { + "personas": { + "allowed_advisors": [], + "items": [ + {"id": "a", "name": "A"}, + ] + } + }) + with self.assertRaises(ValidationError): + load_settings(cfg_path) + + def test_frontend_config_includes_all_when_no_whitelist(self): + cfg_path = _write_config(self.tmp_path, { + "personas": { + "items": [ + {"id": "one", "name": "One"}, + {"id": "two", "name": "Two"}, + ] + } + }) + settings = load_settings(cfg_path) + frontend = settings.get_frontend_config() + ids = [p["id"] for p in frontend["personas"]["items"]] + self.assertEqual(ids, ["one", "two"]) + + def test_frontend_config_filters_by_whitelist(self): + cfg_path = _write_config(self.tmp_path, { + "personas": { + "allowed_advisors": ["two"], + "items": [ + {"id": "one", "name": "One"}, + {"id": "two", "name": "Two"}, + {"id": "three", "name": "Three"}, + ] + } + }) + settings = load_settings(cfg_path) + frontend = settings.get_frontend_config() + ids = [p["id"] for p in frontend["personas"]["items"]] + self.assertEqual(ids, ["two"]) + def test_bad_persona_does_not_crash_everything(self): """Validates that a bad persona in the inline items list causes a validation error -- the directory loader solves this for file-based configs.""" diff --git a/multi_llm_chatbot_backend/app/tests/unit/test_persona_filter.py b/multi_llm_chatbot_backend/app/tests/unit/test_persona_filter.py new file mode 100644 index 00000000..40dfe24b --- /dev/null +++ b/multi_llm_chatbot_backend/app/tests/unit/test_persona_filter.py @@ -0,0 +1,57 @@ +import unittest +from app.core.persona_filter import get_available_persona_ids + +ALL_IDS = ["pragmatist", "theorist", "methodologist", "mentor", "critic"] + + +class TestGetAvailablePersonaIds(unittest.TestCase): + + def test_no_filters_returns_all(self): + """None for both filters means no restrictions.""" + result = get_available_persona_ids(ALL_IDS) + self.assertEqual(result, ALL_IDS) + + def test_system_whitelist_filters(self): + result = get_available_persona_ids( + ALL_IDS, system_allowed=["theorist", "critic"] + ) + self.assertEqual(result, ["theorist", "critic"]) + + def test_user_disabled_filters(self): + result = get_available_persona_ids( + ALL_IDS, user_disabled=["mentor", "critic"] + ) + self.assertEqual(result, ["pragmatist", "theorist", "methodologist"]) + + def test_both_layers_cascade(self): + """System narrows first, then user narrows further.""" + result = get_available_persona_ids( + ALL_IDS, + system_allowed=["pragmatist", "theorist", "methodologist"], + user_disabled=["theorist"], + ) + self.assertEqual(result, ["pragmatist", "methodologist"]) + + def test_unknown_ids_in_user_disabled_ignored(self): + result = get_available_persona_ids( + ALL_IDS, user_disabled=["nonexistent", "also_fake"] + ) + self.assertEqual(result, ALL_IDS) + + def test_all_filtered_returns_empty(self): + result = get_available_persona_ids( + ALL_IDS, system_allowed=["theorist"], user_disabled=["theorist"] + ) + self.assertEqual(result, []) + + def test_order_preserved(self): + """Result order matches registered_ids, not system_allowed.""" + result = get_available_persona_ids( + ALL_IDS, system_allowed=["critic", "pragmatist"] + ) + self.assertEqual(result, ["pragmatist", "critic"]) + + def test_system_allowed_empty_list_allows_none(self): + """An explicit empty whitelist means no advisors are allowed.""" + result = get_available_persona_ids(ALL_IDS, system_allowed=[]) + self.assertEqual(result, []) diff --git a/phd-advisor-frontend/src/components/AdvisorStatusDropdown.js b/phd-advisor-frontend/src/components/AdvisorStatusDropdown.js index d1d5663d..49d7f2bd 100644 --- a/phd-advisor-frontend/src/components/AdvisorStatusDropdown.js +++ b/phd-advisor-frontend/src/components/AdvisorStatusDropdown.js @@ -1,11 +1,14 @@ import React, { useState, useEffect } from 'react'; import { Users, ChevronDown, Pencil } from 'lucide-react'; import AvatarPickerModal from './AvatarPickerModal'; +import Toggle from './Toggle'; +import { useAppConfig } from '../contexts/AppConfigContext'; const AdvisorStatusDropdown = ({ advisors, thinkingAdvisors, getAdvisorColors, isDark }) => { const [isOpen, setIsOpen] = useState(false); const [hoveredId, setHoveredId] = useState(null); const [pickerAdvisor, setPickerAdvisor] = useState(null); + const { isAdvisorEnabled, setAdvisorEnabled } = useAppConfig(); // Close dropdown when clicking outside useEffect(() => { @@ -24,10 +27,11 @@ const AdvisorStatusDropdown = ({ advisors, thinkingAdvisors, getAdvisorColors, i } const advisorEntries = Object.entries(advisors); - const thinkingCount = Array.isArray(thinkingAdvisors) - ? thinkingAdvisors.filter(id => id !== 'system').length + const thinkingCount = Array.isArray(thinkingAdvisors) + ? thinkingAdvisors.filter(id => id !== 'system').length : 0; const totalAdvisors = advisorEntries.length; + const enabledCount = advisorEntries.filter(([id]) => isAdvisorEnabled(id)).length; const handleToggle = () => { setIsOpen(!isOpen); @@ -42,7 +46,7 @@ const AdvisorStatusDropdown = ({ advisors, thinkingAdvisors, getAdvisorColors, i