From 92ef2f3811de314871df7ba5972d9b7c9622bdbf Mon Sep 17 00:00:00 2001 From: Abhinav Singh Date: Sat, 4 Apr 2026 17:18:28 -0400 Subject: [PATCH 1/2] Implement chat functionality with conversation management and message handling. Add new API endpoints for chat interactions, enhance data models for conversations and messages, and update UI components for chat thread display. Improve inspection data handling and integrate authentication for chat requests. --- app/api/chat_routes.py | 193 +++++++++++++++++++++ app/api/routes.py | 55 +++--- app/api/workspace.py | 6 +- app/auth/deps.py | 21 +++ app/main.py | 4 +- app/models/__init__.py | 4 +- app/models/conversation.py | 68 ++++++++ app/models/inspection_snapshot.py | 38 ++++ app/models/user.py | 12 +- app/schemas.py | 95 ++++++++++ app/services/__init__.py | 1 + app/services/analysis_run.py | 38 ++++ app/services/inspection_persistence.py | 27 +++ tests/test_api.py | 50 +++--- tests/test_conversations.py | 230 +++++++++++++++++++++++++ ui/src/api/chat.ts | 165 ++++++++++++++++-- ui/src/api/inspections.ts | 7 +- ui/src/api/types.ts | 51 ++++++ ui/src/components/app/ChatThread.tsx | 15 +- ui/src/hooks/useChat.ts | 169 ++++++++++++++---- ui/src/hooks/useInspectionPanel.ts | 6 +- ui/src/pages/AppPage.tsx | 14 +- 22 files changed, 1167 insertions(+), 102 deletions(-) create mode 100644 app/api/chat_routes.py create mode 100644 app/models/conversation.py create mode 100644 app/models/inspection_snapshot.py create mode 100644 app/services/__init__.py create mode 100644 app/services/analysis_run.py create mode 100644 app/services/inspection_persistence.py create mode 100644 tests/test_conversations.py diff --git a/app/api/chat_routes.py b/app/api/chat_routes.py new file mode 100644 index 0000000..64fece1 --- /dev/null +++ b/app/api/chat_routes.py @@ -0,0 +1,193 @@ +"""User-scoped conversation and authenticated chat orchestration.""" + +from __future__ import annotations + +from datetime import datetime, timezone + +from fastapi import APIRouter, Depends, HTTPException, status +from sqlalchemy import func, select +from sqlalchemy.orm import Session + +from app.auth.deps import get_current_user +from app.config import get_settings +from app.db.session import get_db +from app.models.conversation import Conversation, Message +from app.models.user import User +from app.schemas import ( + ChatAssistantMessagePublic, + ChatSubmitRequest, + ChatTurnResponse, + ConversationDetailResponse, + ConversationPublic, + ConversationsListResponse, + ConversationSummary, + MessagePublic, +) +from app.services.analysis_run import run_stored_analysis +from app.services.inspection_persistence import save_inspection_for_assistant_message +from app.utils.logging import get_logger + + +router = APIRouter(tags=["chat"]) +logger = get_logger(__name__) + + +def _conversation_title_from_query(query: str, max_len: int = 56) -> str: + compact = " ".join(query.split()) + if len(compact) <= max_len: + return compact + return f"{compact[: max_len - 3]}..." + + +def _preview_text(text: str | None, max_len: int = 120) -> str | None: + if text is None: + return None + single_line = " ".join(text.split()) + if len(single_line) <= max_len: + return single_line + return f"{single_line[: max_len - 1]}…" + + +@router.get("/conversations", response_model=ConversationsListResponse) +def list_conversations( + current_user: User = Depends(get_current_user), + db: Session = Depends(get_db), +) -> ConversationsListResponse: + last_id_sq = ( + select(Message.conversation_id.label("cid"), func.max(Message.id).label("mid")) + .group_by(Message.conversation_id) + .subquery() + ) + stmt = ( + select(Conversation, Message.content) + .outerjoin(last_id_sq, last_id_sq.c.cid == Conversation.id) + .outerjoin(Message, Message.id == last_id_sq.c.mid) + .where(Conversation.user_id == current_user.id) + .order_by(Conversation.updated_at.desc(), Conversation.id.desc()) + ) + rows = db.execute(stmt).all() + items = [ + ConversationSummary( + id=conv.id, + title=conv.title, + updated_at=conv.updated_at, + last_message_preview=_preview_text(last_content), + ) + for conv, last_content in rows + ] + return ConversationsListResponse(conversations=items) + + +@router.get("/conversations/{conversation_id}", response_model=ConversationDetailResponse) +def get_conversation( + conversation_id: int, + current_user: User = Depends(get_current_user), + db: Session = Depends(get_db), +) -> ConversationDetailResponse: + conv = db.get(Conversation, conversation_id) + if conv is None: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail={"message": "Conversation not found."}) + if conv.user_id != current_user.id: + raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail={"message": "Not allowed to access this conversation."}) + + msg_rows = db.execute( + select(Message) + .where(Message.conversation_id == conv.id) + .order_by(Message.created_at.asc(), Message.id.asc()) + ).scalars() + + return ConversationDetailResponse( + conversation=ConversationPublic.model_validate(conv), + messages=[MessagePublic.model_validate(m) for m in msg_rows], + ) + + +@router.post("/chat", response_model=ChatTurnResponse) +def chat_turn( + body: ChatSubmitRequest, + current_user: User = Depends(get_current_user), + db: Session = Depends(get_db), +) -> ChatTurnResponse: + now = datetime.now(timezone.utc) + if body.conversation_id is None: + conv = Conversation( + user_id=current_user.id, + title=_conversation_title_from_query(body.query), + created_at=now, + updated_at=now, + ) + db.add(conv) + db.flush() + else: + conv = db.get(Conversation, body.conversation_id) + if conv is None: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail={"message": "Conversation not found."}) + if conv.user_id != current_user.id: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail={"message": "Not allowed to access this conversation."}, + ) + + user_msg = Message(conversation_id=conv.id, role="user", content=body.query, created_at=now) + db.add(user_msg) + db.flush() + + try: + analysis_run = run_stored_analysis(body.query) + analysis_result = analysis_run.response + inspection_payload = analysis_run.inspection + except Exception as exc: # pragma: no cover - defensive API fallback + db.rollback() + logger.exception("Chat analyze failed", extra={"query": body.query, "conversation_id": conv.id}) + settings = get_settings() + detail: dict[str, str] = { + "message": "The analysis could not complete successfully. Inspect the server logs and retry.", + } + if settings.app_env.lower() != "production": + detail["error"] = str(exc) + raise HTTPException(status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail=detail) from exc + + assistant_meta: dict = { + "trace": [e.model_dump() for e in analysis_result.trace], + "executed_steps": [s.model_dump() for s in analysis_result.executed_steps], + "errors": [err.model_dump() for err in analysis_result.errors], + "inspection_id": analysis_result.inspection_id, + } + assistant_msg = Message( + conversation_id=conv.id, + role="assistant", + content=analysis_result.analysis, + metadata_json=assistant_meta, + created_at=datetime.now(timezone.utc), + ) + db.add(assistant_msg) + db.flush() + if analysis_result.inspection_id: + save_inspection_for_assistant_message( + db, + inspection_id=analysis_result.inspection_id, + payload=inspection_payload, + conversation_id=conv.id, + message_id=assistant_msg.id, + ) + conv.updated_at = assistant_msg.created_at + db.commit() + db.refresh(conv) + db.refresh(assistant_msg) + + return ChatTurnResponse( + conversation=ConversationPublic.model_validate(conv), + assistant_message=ChatAssistantMessagePublic( + id=assistant_msg.id, + role="assistant", + content=assistant_msg.content, + created_at=assistant_msg.created_at, + status="ready", + metadata_json=assistant_msg.metadata_json, + ), + analysis=analysis_result.analysis, + trace=analysis_result.trace, + executed_steps=analysis_result.executed_steps, + errors=analysis_result.errors, + inspection_id=analysis_result.inspection_id, + ) diff --git a/app/api/routes.py b/app/api/routes.py index 91a3ec6..b28e70e 100644 --- a/app/api/routes.py +++ b/app/api/routes.py @@ -2,12 +2,18 @@ from __future__ import annotations -from fastapi import APIRouter, File, HTTPException, UploadFile, status +from fastapi import APIRouter, Depends, File, HTTPException, UploadFile, status +from sqlalchemy.orm import Session -from app.agent.graph import run_analysis -from app.api.workspace import get_inspection, profile_upload, store_inspection +from app.api.workspace import get_inspection, profile_upload +from app.auth.deps import get_current_user_optional from app.config import get_settings -from app.schemas import AnalyzeRequest, AnalyzeResponse, HealthResponse, InspectionResponse, SampleQuestionsResponse, UploadResponse +from app.db.session import get_db +from app.models.conversation import Conversation +from app.models.inspection_snapshot import InspectionSnapshot +from app.models.user import User +from app.schemas import AnalyzeRequest, AnalyzeResponse, HealthResponse, InspectionData, InspectionResponse, SampleQuestionsResponse, UploadResponse +from app.services.analysis_run import run_stored_analysis from app.utils.constants import SAMPLE_QUESTIONS from app.utils.logging import get_logger @@ -44,8 +50,29 @@ async def upload_dataset(file: UploadFile = File(...)) -> UploadResponse: @router.get("/inspections/{inspection_id}", response_model=InspectionResponse) -def inspection_details(inspection_id: str) -> InspectionResponse: - """Return a stored inspection payload for the requested analysis.""" +def inspection_details( + inspection_id: str, + db: Session = Depends(get_db), + current_user: User | None = Depends(get_current_user_optional), +) -> InspectionResponse: + """Return a stored inspection (database snapshot for chat history, else in-memory from /analyze).""" + + row = db.get(InspectionSnapshot, inspection_id) + if row is not None: + if current_user is None: + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail={"message": "Authentication required to view this inspection."}, + headers={"WWW-Authenticate": "Bearer"}, + ) + conv = db.get(Conversation, row.conversation_id) + if conv is None or conv.user_id != current_user.id: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail={"message": "Not allowed to view this inspection."}, + ) + inspection = InspectionData.model_validate(row.payload_json) + return InspectionResponse(inspection=inspection, fallback=False) inspection = get_inspection(inspection_id) if inspection is None: @@ -58,21 +85,7 @@ def analyze(request: AnalyzeRequest) -> AnalyzeResponse: """Execute the analytics workflow for a user query.""" try: - state = run_analysis(request.query) - base_response = AnalyzeResponse( - analysis=state["analysis"], - trace=state.get("trace", []), - executed_steps=state.get("executed_steps", []), - errors=state.get("errors", []), - ) - inspection_id = store_inspection(request.query, base_response) - return AnalyzeResponse( - analysis=base_response.analysis, - trace=base_response.trace, - executed_steps=base_response.executed_steps, - errors=base_response.errors, - inspection_id=inspection_id, - ) + return run_stored_analysis(request.query).response except Exception as exc: # pragma: no cover - defensive API fallback logger.exception("Analyze request failed", extra={"query": request.query}) settings = get_settings() diff --git a/app/api/workspace.py b/app/api/workspace.py index 2613ac3..e9b47a2 100644 --- a/app/api/workspace.py +++ b/app/api/workspace.py @@ -84,13 +84,13 @@ def profile_upload(filename: str, content: bytes) -> UploadedAsset: return asset -def store_inspection(prompt: str, response: AnalyzeResponse) -> str: - """Build and store an inspection payload for later retrieval.""" +def store_inspection(prompt: str, response: AnalyzeResponse) -> tuple[str, InspectionData]: + """Build and store an inspection payload in memory for same-session retrieval.""" inspection = _build_inspection(_short_id("inspect"), prompt, response) with _STORE_LOCK: _INSPECTIONS[inspection.id] = inspection - return inspection.id + return inspection.id, inspection def get_inspection(inspection_id: str) -> InspectionData | None: diff --git a/app/auth/deps.py b/app/auth/deps.py index a884052..b4422dd 100644 --- a/app/auth/deps.py +++ b/app/auth/deps.py @@ -44,3 +44,24 @@ def get_current_user( headers={"WWW-Authenticate": "Bearer"}, ) return user + + +def get_current_user_optional( + creds: HTTPAuthorizationCredentials | None = Depends(_bearer), + db: Session = Depends(get_db), +) -> User | None: + """Same as `get_current_user` but returns None when unauthenticated (no 401).""" + + if creds is None or creds.scheme.lower() != "bearer": + return None + try: + payload = decode_access_token(creds.credentials) + user_id_str: str | None = payload.get("sub") + if user_id_str is None: + return None + user_id = int(user_id_str) + except Exception: + return None + + user = db.get(User, user_id) + return user diff --git a/app/main.py b/app/main.py index 058ff99..cdb7f5f 100644 --- a/app/main.py +++ b/app/main.py @@ -8,6 +8,7 @@ from fastapi.middleware.cors import CORSMiddleware from app.api.auth_routes import router as auth_router +from app.api.chat_routes import router as chat_router from app.api.routes import router from app.config import get_settings from app.utils.logging import configure_logging @@ -23,7 +24,7 @@ async def lifespan(_app: FastAPI): from app.db.base import Base from app.db.session import get_engine - from app.models import User # noqa: F401 + from app.models import Conversation, InspectionSnapshot, Message, User # noqa: F401 Base.metadata.create_all(bind=get_engine()) yield @@ -43,4 +44,5 @@ async def lifespan(_app: FastAPI): allow_headers=["*"], ) app.include_router(auth_router) +app.include_router(chat_router) app.include_router(router) diff --git a/app/models/__init__.py b/app/models/__init__.py index 788cc58..4649a41 100644 --- a/app/models/__init__.py +++ b/app/models/__init__.py @@ -1,5 +1,7 @@ """ORM models (import for metadata registration).""" +from app.models.conversation import Conversation, Message +from app.models.inspection_snapshot import InspectionSnapshot from app.models.user import User -__all__ = ["User"] +__all__ = ["Conversation", "InspectionSnapshot", "Message", "User"] diff --git a/app/models/conversation.py b/app/models/conversation.py new file mode 100644 index 0000000..cf0d46f --- /dev/null +++ b/app/models/conversation.py @@ -0,0 +1,68 @@ +"""Conversation and message ORM models.""" + +from __future__ import annotations + +from datetime import datetime, timezone +from typing import TYPE_CHECKING, Any + +from sqlalchemy import JSON, DateTime, ForeignKey, Integer, String, Text +from sqlalchemy.orm import Mapped, mapped_column, relationship + +from app.db.base import Base + +if TYPE_CHECKING: + from app.models.inspection_snapshot import InspectionSnapshot + from app.models.user import User + + +def _utc_now() -> datetime: + return datetime.now(timezone.utc) + + +class Conversation(Base): + __tablename__ = "conversations" + + id: Mapped[int] = mapped_column(Integer, primary_key=True, autoincrement=True) + user_id: Mapped[int] = mapped_column(ForeignKey("users.id", ondelete="CASCADE"), index=True, nullable=False) + title: Mapped[str] = mapped_column(String(512), nullable=False) + created_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), default=_utc_now, nullable=False) + updated_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), + default=_utc_now, + onupdate=_utc_now, + nullable=False, + ) + + user: Mapped[User] = relationship("User", back_populates="conversations") + messages: Mapped[list[Message]] = relationship( + "Message", + back_populates="conversation", + cascade="all, delete-orphan", + order_by="Message.id", + ) + inspection_snapshots: Mapped[list[InspectionSnapshot]] = relationship( + "InspectionSnapshot", + back_populates="conversation", + ) + + +class Message(Base): + __tablename__ = "messages" + + id: Mapped[int] = mapped_column(Integer, primary_key=True, autoincrement=True) + conversation_id: Mapped[int] = mapped_column( + ForeignKey("conversations.id", ondelete="CASCADE"), + index=True, + nullable=False, + ) + role: Mapped[str] = mapped_column(String(32), nullable=False) + content: Mapped[str] = mapped_column(Text, nullable=False) + metadata_json: Mapped[dict[str, Any] | None] = mapped_column(JSON, nullable=True) + created_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), default=_utc_now, nullable=False) + + conversation: Mapped[Conversation] = relationship("Conversation", back_populates="messages") + inspection_snapshot: Mapped[InspectionSnapshot | None] = relationship( + "InspectionSnapshot", + back_populates="message", + uselist=False, + ) diff --git a/app/models/inspection_snapshot.py b/app/models/inspection_snapshot.py new file mode 100644 index 0000000..45f7103 --- /dev/null +++ b/app/models/inspection_snapshot.py @@ -0,0 +1,38 @@ +"""Persisted inspection payload for chat history (survives process restart).""" + +from __future__ import annotations + +from datetime import datetime, timezone +from typing import TYPE_CHECKING, Any + +from sqlalchemy import JSON, DateTime, ForeignKey, String +from sqlalchemy.orm import Mapped, mapped_column, relationship + +from app.db.base import Base + +if TYPE_CHECKING: + from app.models.conversation import Conversation, Message + + +def _utc_now() -> datetime: + return datetime.now(timezone.utc) + + +class InspectionSnapshot(Base): + """Full `InspectionData` JSON keyed by public inspection id (e.g. inspect_abc12def).""" + + __tablename__ = "inspection_snapshots" + + id: Mapped[str] = mapped_column(String(64), primary_key=True) + conversation_id: Mapped[int] = mapped_column(ForeignKey("conversations.id", ondelete="CASCADE"), index=True, nullable=False) + message_id: Mapped[int] = mapped_column( + ForeignKey("messages.id", ondelete="CASCADE"), + unique=True, + index=True, + nullable=False, + ) + payload_json: Mapped[dict[str, Any]] = mapped_column(JSON, nullable=False) + created_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), default=_utc_now, nullable=False) + + conversation: Mapped[Conversation] = relationship("Conversation", back_populates="inspection_snapshots") + message: Mapped[Message] = relationship("Message", back_populates="inspection_snapshot") diff --git a/app/models/user.py b/app/models/user.py index 1c02a84..dcdd9df 100644 --- a/app/models/user.py +++ b/app/models/user.py @@ -3,12 +3,16 @@ from __future__ import annotations from datetime import datetime, timezone +from typing import TYPE_CHECKING from sqlalchemy import DateTime, String -from sqlalchemy.orm import Mapped, mapped_column +from sqlalchemy.orm import Mapped, mapped_column, relationship from app.db.base import Base +if TYPE_CHECKING: + from app.models.conversation import Conversation + def _utc_now() -> datetime: return datetime.now(timezone.utc) @@ -22,3 +26,9 @@ class User(Base): hashed_password: Mapped[str] = mapped_column(String(255), nullable=False) display_name: Mapped[str | None] = mapped_column(String(255), nullable=True) created_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), default=_utc_now, nullable=False) + + conversations: Mapped[list[Conversation]] = relationship( + "Conversation", + back_populates="user", + cascade="all, delete-orphan", + ) diff --git a/app/schemas.py b/app/schemas.py index 279b89c..61c27e5 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -2,6 +2,7 @@ from __future__ import annotations +from datetime import datetime from typing import Any, Literal from pydantic import BaseModel, ConfigDict, Field, field_validator @@ -214,6 +215,100 @@ class AnalyzeResponse(BaseModel): inspection_id: str | None = None +MessageRoleLiteral = Literal["user", "assistant"] + + +class ConversationSummary(BaseModel): + """Sidebar-ready conversation row.""" + + model_config = ConfigDict(from_attributes=True) + + id: int + title: str + updated_at: datetime + last_message_preview: str | None = None + + +class ConversationsListResponse(BaseModel): + conversations: list[ConversationSummary] + + +class MessagePublic(BaseModel): + """One persisted chat message.""" + + model_config = ConfigDict(from_attributes=True) + + id: int + role: MessageRoleLiteral + content: str + created_at: datetime + metadata_json: dict[str, Any] | None = None + + +class ConversationPublic(BaseModel): + model_config = ConfigDict(from_attributes=True) + + id: int + title: str + created_at: datetime + updated_at: datetime + + +class ConversationDetailResponse(BaseModel): + """Single conversation with ordered messages.""" + + conversation: ConversationPublic + messages: list[MessagePublic] + + +class ChatSubmitRequest(BaseModel): + """Authenticated chat turn: optional thread id plus user query.""" + + conversation_id: int | str | None = None + query: str = Field(..., min_length=3) + + @field_validator("conversation_id", mode="before") + @classmethod + def normalize_conversation_id(cls, v: Any) -> int | None: + if v is None: + return None + if isinstance(v, bool): + raise ValueError("conversation_id must be an integer") + if isinstance(v, int): + return v + if isinstance(v, str): + s = v.strip() + if not s: + return None + return int(s) + raise ValueError("conversation_id must be an integer or numeric string") + + +class ChatAssistantMessagePublic(BaseModel): + """Assistant row returned after a chat turn (persists alongside analysis fields).""" + + model_config = ConfigDict(from_attributes=True) + + id: int + role: MessageRoleLiteral = "assistant" + content: str + created_at: datetime + status: Literal["ready", "sending", "error"] = "ready" + metadata_json: dict[str, Any] | None = None + + +class ChatTurnResponse(BaseModel): + """POST /chat response: persisted thread + analysis payload.""" + + conversation: ConversationPublic + assistant_message: ChatAssistantMessagePublic + analysis: str + trace: list[TraceEvent] + executed_steps: list[ExecutedStep] + errors: list[ErrorItem] + inspection_id: str | None = None + + class HealthResponse(BaseModel): """Health endpoint response.""" diff --git a/app/services/__init__.py b/app/services/__init__.py new file mode 100644 index 0000000..ca739c5 --- /dev/null +++ b/app/services/__init__.py @@ -0,0 +1 @@ +"""Application services (orchestration helpers).""" diff --git a/app/services/analysis_run.py b/app/services/analysis_run.py new file mode 100644 index 0000000..087d413 --- /dev/null +++ b/app/services/analysis_run.py @@ -0,0 +1,38 @@ +"""Shared analysis execution + inspection storage (used by /analyze and /chat).""" + +from __future__ import annotations + +from dataclasses import dataclass + +from app.agent.graph import run_analysis +from app.api.workspace import store_inspection +from app.schemas import AnalyzeResponse, InspectionData + + +@dataclass(frozen=True) +class StoredAnalysisRun: + """Result of a full analysis run including the built inspection payload.""" + + response: AnalyzeResponse + inspection: InspectionData + + +def run_stored_analysis(query: str) -> StoredAnalysisRun: + """Run `run_analysis`, persist inspection to process memory, return API + inspection objects.""" + + state = run_analysis(query) + base = AnalyzeResponse( + analysis=state["analysis"], + trace=state.get("trace", []), + executed_steps=state.get("executed_steps", []), + errors=state.get("errors", []), + ) + inspection_id, inspection = store_inspection(query, base) + response = AnalyzeResponse( + analysis=base.analysis, + trace=base.trace, + executed_steps=base.executed_steps, + errors=base.errors, + inspection_id=inspection_id, + ) + return StoredAnalysisRun(response=response, inspection=inspection) diff --git a/app/services/inspection_persistence.py b/app/services/inspection_persistence.py new file mode 100644 index 0000000..d684345 --- /dev/null +++ b/app/services/inspection_persistence.py @@ -0,0 +1,27 @@ +"""Persist full inspection payloads for authenticated chat turns.""" + +from __future__ import annotations + +from sqlalchemy.orm import Session + +from app.models.inspection_snapshot import InspectionSnapshot +from app.schemas import InspectionData + + +def save_inspection_for_assistant_message( + db: Session, + *, + inspection_id: str, + payload: InspectionData, + conversation_id: int, + message_id: int, +) -> None: + """Store a JSON snapshot of `InspectionData` for later GET /inspections/{id}.""" + + snap = InspectionSnapshot( + id=inspection_id, + conversation_id=conversation_id, + message_id=message_id, + payload_json=payload.model_dump(mode="json"), + ) + db.add(snap) diff --git a/tests/test_api.py b/tests/test_api.py index e8a7368..c99c639 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -6,11 +6,21 @@ from fastapi.testclient import TestClient from app.api.workspace import clear_workspace_state +from app.config import get_settings +from app.db.session import reset_engine_and_session from app.main import app from app.schemas import AnalyzeResponse -client = TestClient(app) +@pytest.fixture +def client(tmp_path, monkeypatch): + monkeypatch.setenv("DATABASE_PATH", str(tmp_path / "api_test.sqlite")) + get_settings.cache_clear() + reset_engine_and_session() + with TestClient(app) as test_client: + yield test_client + get_settings.cache_clear() + reset_engine_and_session() @pytest.fixture(autouse=True) @@ -37,21 +47,21 @@ def test_analyze_response_accepts_skipped_trace() -> None: assert resp.trace[0].status == "skipped" -def test_health_endpoint() -> None: +def test_health_endpoint(client: TestClient) -> None: response = client.get("/health") assert response.status_code == 200 payload = response.json() assert payload["status"] == "ok" -def test_sample_questions_endpoint() -> None: +def test_sample_questions_endpoint(client: TestClient) -> None: response = client.get("/sample-questions") assert response.status_code == 200 payload = response.json() assert len(payload["questions"]) >= 3 -def test_analyze_endpoint_structure() -> None: +def test_analyze_endpoint_structure(client: TestClient) -> None: def fake_run_analysis(query: str) -> dict: # noqa: ARG001 return { "analysis": "## Summary\nPipeline velocity improved.\n", @@ -80,14 +90,14 @@ def fake_run_analysis(query: str) -> dict: # noqa: ARG001 } app.dependency_overrides = {} - import app.api.routes as routes + import app.services.analysis_run as analysis_run - original = routes.run_analysis - routes.run_analysis = fake_run_analysis + original = analysis_run.run_analysis + analysis_run.run_analysis = fake_run_analysis try: response = client.post("/analyze", json={"query": "Why did pipeline velocity drop this week?"}) finally: - routes.run_analysis = original + analysis_run.run_analysis = original assert response.status_code == 200 payload = response.json() assert {"analysis", "trace", "executed_steps", "errors", "inspection_id"} <= payload.keys() @@ -97,18 +107,18 @@ def fake_run_analysis(query: str) -> dict: # noqa: ARG001 assert isinstance(payload["inspection_id"], str) -def test_analyze_endpoint_returns_http_500_on_failure() -> None: +def test_analyze_endpoint_returns_http_500_on_failure(client: TestClient) -> None: def fake_run_analysis(query: str) -> dict: # noqa: ARG001 raise RuntimeError("planner exploded") - import app.api.routes as routes + import app.services.analysis_run as analysis_run - original = routes.run_analysis - routes.run_analysis = fake_run_analysis + original = analysis_run.run_analysis + analysis_run.run_analysis = fake_run_analysis try: response = client.post("/analyze", json={"query": "Why did pipeline velocity drop this week?"}) finally: - routes.run_analysis = original + analysis_run.run_analysis = original assert response.status_code == 500 payload = response.json() @@ -116,7 +126,7 @@ def fake_run_analysis(query: str) -> dict: # noqa: ARG001 assert payload["detail"]["error"] == "planner exploded" -def test_upload_endpoint_profiles_csv() -> None: +def test_upload_endpoint_profiles_csv(client: TestClient) -> None: response = client.post( "/uploads", files={"file": ("pipeline.csv", BytesIO(b"stage,amount\nopen,10\nwon,25\n"), "text/csv")}, @@ -132,7 +142,7 @@ def test_upload_endpoint_profiles_csv() -> None: assert payload["asset"]["status"] == "verified" -def test_inspection_endpoint_returns_stored_inspection() -> None: +def test_inspection_endpoint_returns_stored_inspection(client: TestClient) -> None: def fake_run_analysis(query: str) -> dict: # noqa: ARG001 return { "analysis": "## Summary\nPipeline velocity improved.\n", @@ -160,14 +170,14 @@ def fake_run_analysis(query: str) -> dict: # noqa: ARG001 "errors": [], } - import app.api.routes as routes + import app.services.analysis_run as analysis_run - original = routes.run_analysis - routes.run_analysis = fake_run_analysis + original = analysis_run.run_analysis + analysis_run.run_analysis = fake_run_analysis try: analyze_response = client.post("/analyze", json={"query": "Why did pipeline velocity drop this week?"}) finally: - routes.run_analysis = original + analysis_run.run_analysis = original inspection_id = analyze_response.json()["inspection_id"] response = client.get(f"/inspections/{inspection_id}") @@ -180,7 +190,7 @@ def fake_run_analysis(query: str) -> dict: # noqa: ARG001 assert payload["inspection"]["trace"][0]["label"] == "Query Planning" -def test_inspection_endpoint_returns_404_for_unknown_id() -> None: +def test_inspection_endpoint_returns_404_for_unknown_id(client: TestClient) -> None: response = client.get("/inspections/inspect_missing") assert response.status_code == 404 assert response.json()["detail"]["message"] == "Inspection not found." diff --git a/tests/test_conversations.py b/tests/test_conversations.py new file mode 100644 index 0000000..d731f9d --- /dev/null +++ b/tests/test_conversations.py @@ -0,0 +1,230 @@ +"""Conversation persistence and /chat orchestration tests.""" + +from __future__ import annotations + +import pytest +from fastapi.testclient import TestClient + +from app.config import get_settings +from app.db.session import reset_engine_and_session +from app.main import app + + +@pytest.fixture +def chat_client(tmp_path, monkeypatch): + monkeypatch.setenv("DATABASE_PATH", str(tmp_path / "chat_test.sqlite")) + get_settings.cache_clear() + reset_engine_and_session() + with TestClient(app) as client: + yield client + get_settings.cache_clear() + reset_engine_and_session() + + +def _signup(client: TestClient, email: str, password: str = "password123") -> str: + r = client.post("/auth/signup", json={"email": email, "password": password}) + assert r.status_code == 200, r.text + return r.json()["access_token"] + + +def _fake_analysis_state(query: str) -> dict: # noqa: ARG001 + return { + "analysis": "## Demo\nHello from fake analysis.\n", + "trace": [{"step": "planner_compiled_node", "status": "completed", "details": {}}], + "executed_steps": [], + "errors": [], + } + + +def test_chat_creates_conversation_on_first_prompt(chat_client: TestClient) -> None: + import app.services.analysis_run as analysis_run + + token = _signup(chat_client, "first@example.com") + original = analysis_run.run_analysis + analysis_run.run_analysis = _fake_analysis_state + try: + r = chat_client.post( + "/chat", + headers={"Authorization": f"Bearer {token}"}, + json={"query": "Why is pipeline velocity changing?"}, + ) + finally: + analysis_run.run_analysis = original + + assert r.status_code == 200, r.text + body = r.json() + assert "conversation" in body + assert body["conversation"]["id"] >= 1 + assert body["conversation"]["title"] == "Why is pipeline velocity changing?" + assert body["assistant_message"]["role"] == "assistant" + assert body["assistant_message"]["content"] == "## Demo\nHello from fake analysis.\n" + assert body["analysis"] == body["assistant_message"]["content"] + assert body["assistant_message"]["metadata_json"]["inspection_id"] == body["inspection_id"] + + lst = chat_client.get("/conversations", headers={"Authorization": f"Bearer {token}"}) + assert lst.status_code == 200 + assert len(lst.json()["conversations"]) == 1 + assert lst.json()["conversations"][0]["last_message_preview"] is not None + + detail = chat_client.get( + f"/conversations/{body['conversation']['id']}", + headers={"Authorization": f"Bearer {token}"}, + ) + assert detail.status_code == 200 + msgs = detail.json()["messages"] + assert len(msgs) == 2 + assert msgs[0]["role"] == "user" + assert msgs[0]["content"] == "Why is pipeline velocity changing?" + assert msgs[1]["role"] == "assistant" + + +def test_chat_appends_to_existing_conversation(chat_client: TestClient) -> None: + import app.services.analysis_run as analysis_run + + token = _signup(chat_client, "second@example.com") + original = analysis_run.run_analysis + analysis_run.run_analysis = _fake_analysis_state + try: + first = chat_client.post( + "/chat", + headers={"Authorization": f"Bearer {token}"}, + json={"query": "First question here"}, + ) + cid = first.json()["conversation"]["id"] + second = chat_client.post( + "/chat", + headers={"Authorization": f"Bearer {token}"}, + json={"conversation_id": cid, "query": "Second question here"}, + ) + finally: + analysis_run.run_analysis = original + + assert second.status_code == 200 + assert second.json()["conversation"]["id"] == cid + + detail = chat_client.get(f"/conversations/{cid}", headers={"Authorization": f"Bearer {token}"}) + assert len(detail.json()["messages"]) == 4 + + +def test_conversation_list_scoped_to_user(chat_client: TestClient) -> None: + import app.services.analysis_run as analysis_run + + original = analysis_run.run_analysis + analysis_run.run_analysis = _fake_analysis_state + try: + a = _signup(chat_client, "a_scoped@example.com") + b = _signup(chat_client, "b_scoped@example.com") + ca = chat_client.post( + "/chat", + headers={"Authorization": f"Bearer {a}"}, + json={"query": "User A thread"}, + ).json()["conversation"]["id"] + chat_client.post( + "/chat", + headers={"Authorization": f"Bearer {b}"}, + json={"query": "User B thread"}, + ) + finally: + analysis_run.run_analysis = original + + list_a = chat_client.get("/conversations", headers={"Authorization": f"Bearer {a}"}) + assert list_a.status_code == 200 + ids_a = {c["id"] for c in list_a.json()["conversations"]} + assert ids_a == {ca} + + list_b = chat_client.get("/conversations", headers={"Authorization": f"Bearer {b}"}) + ids_b = {c["id"] for c in list_b.json()["conversations"]} + assert ca not in ids_b + assert len(ids_b) == 1 + + +def test_conversations_require_auth(chat_client: TestClient) -> None: + r = chat_client.get("/conversations") + assert r.status_code == 401 + + r2 = chat_client.post("/chat", json={"query": "Needs auth and length"}) + assert r2.status_code == 401 + + +def test_cross_user_conversation_forbidden(chat_client: TestClient) -> None: + import app.services.analysis_run as analysis_run + + original = analysis_run.run_analysis + analysis_run.run_analysis = _fake_analysis_state + try: + owner = _signup(chat_client, "owner@example.com") + intruder = _signup(chat_client, "intruder@example.com") + cid = chat_client.post( + "/chat", + headers={"Authorization": f"Bearer {owner}"}, + json={"query": "Private thread"}, + ).json()["conversation"]["id"] + finally: + analysis_run.run_analysis = original + + forbidden = chat_client.get(f"/conversations/{cid}", headers={"Authorization": f"Bearer {intruder}"}) + assert forbidden.status_code == 403 + + post_forbidden = chat_client.post( + "/chat", + headers={"Authorization": f"Bearer {intruder}"}, + json={"conversation_id": cid, "query": "Malicious follow-up question here"}, + ) + assert post_forbidden.status_code == 403 + + +def test_unknown_conversation_returns_404(chat_client: TestClient) -> None: + token = _signup(chat_client, "solo@example.com") + r = chat_client.get("/conversations/99999", headers={"Authorization": f"Bearer {token}"}) + assert r.status_code == 404 + + +def test_persisted_inspection_requires_auth_after_memory_clear(chat_client: TestClient) -> None: + import app.services.analysis_run as analysis_run + from app.api.workspace import clear_workspace_state + + token = _signup(chat_client, "inspectpersist@example.com") + original = analysis_run.run_analysis + analysis_run.run_analysis = _fake_analysis_state + try: + r = chat_client.post( + "/chat", + headers={"Authorization": f"Bearer {token}"}, + json={"query": "Why is pipeline velocity changing?"}, + ) + finally: + analysis_run.run_analysis = original + + assert r.status_code == 200 + inspection_id = r.json()["inspection_id"] + clear_workspace_state() + + assert chat_client.get(f"/inspections/{inspection_id}").status_code == 401 + + loaded = chat_client.get(f"/inspections/{inspection_id}", headers={"Authorization": f"Bearer {token}"}) + assert loaded.status_code == 200 + assert loaded.json()["inspection"]["id"] == inspection_id + + +def test_persisted_inspection_forbidden_for_other_user(chat_client: TestClient) -> None: + import app.services.analysis_run as analysis_run + from app.api.workspace import clear_workspace_state + + owner = _signup(chat_client, "inspectowner@example.com") + intruder = _signup(chat_client, "inspectintruder@example.com") + original = analysis_run.run_analysis + analysis_run.run_analysis = _fake_analysis_state + try: + r = chat_client.post( + "/chat", + headers={"Authorization": f"Bearer {owner}"}, + json={"query": "Private inspection thread"}, + ) + finally: + analysis_run.run_analysis = original + + inspection_id = r.json()["inspection_id"] + clear_workspace_state() + + forbidden = chat_client.get(f"/inspections/{inspection_id}", headers={"Authorization": f"Bearer {intruder}"}) + assert forbidden.status_code == 403 diff --git a/ui/src/api/chat.ts b/ui/src/api/chat.ts index e0d61b4..ec6a369 100644 --- a/ui/src/api/chat.ts +++ b/ui/src/api/chat.ts @@ -1,34 +1,171 @@ -import { request } from "@/api/client"; +import { request, requestWithAuth, ApiError } from "@/api/client"; import { cacheInspection } from "@/api/inspections"; import { conversationTitleFromPrompt, mapAnalyzeResponseToUi } from "@/api/mappers"; -import type { AnalyzeApiRequest, AnalyzeApiResponse, ChatRequest, ChatResponse, ConversationsResponse } from "@/api/types"; +import type { + AnalyzeApiResponse, + ApiChatTurnResponse, + ApiConversationDetailResponse, + ApiConversationsListResponse, + ApiMessage, + ChatRequest, + ChatResponse, + ConversationsResponse, +} from "@/api/types"; import { seededConversations, buildAssistantMessage } from "@/data/mockChats"; import { isDemoOnlyMode, shouldFallbackToDemo } from "@/config/env"; import { shortId, sleep } from "@/lib/utils"; -import type { Conversation } from "@/types/chat"; +import type { ChatMessage, Conversation } from "@/types/chat"; -export async function fetchConversations(): Promise { +/** Numeric backend conversation id (local drafts use shortId prefixes). */ +export function isBackendConversationId(id: string): boolean { + return id !== "" && /^\d+$/.test(id); +} + +export async function fetchConversations(accessToken: string | null): Promise { await sleep(120); + + if (isDemoOnlyMode) { + return { + conversations: seededConversations, + fallback: true, + }; + } + + if (!accessToken) { + return { conversations: [], fallback: false }; + } + + const raw = await requestWithAuth("/conversations", accessToken); + const conversations = raw.conversations.map(mapApiSummaryToConversation); + return { conversations, fallback: false }; +} + +export async function fetchConversationDetail(accessToken: string, conversationId: string): Promise { + if (isDemoOnlyMode) { + const found = seededConversations.find((c) => c.id === conversationId); + if (found) return found; + throw new ApiError("Conversation not found in demo dataset.", 404); + } + + const raw = await requestWithAuth(`/conversations/${conversationId}`, accessToken); + return mapApiDetailToConversation(raw); +} + +function mapApiSummaryToConversation(row: ApiConversationsListResponse["conversations"][number]): Conversation { + return { + id: String(row.id), + title: row.title, + updatedAt: row.updated_at, + sourceLabel: "Connected backend", + mode: "live", + messages: [], + }; +} + +function mapApiDetailToConversation(raw: ApiConversationDetailResponse): Conversation { + const c = raw.conversation; + return { + id: String(c.id), + title: c.title, + updatedAt: c.updated_at, + sourceLabel: "Connected backend", + mode: "live", + messages: mapApiMessagesToChatMessages(raw.messages), + }; +} + +function mapApiMessagesToChatMessages(messages: ApiMessage[]): ChatMessage[] { + const out: ChatMessage[] = []; + for (let i = 0; i < messages.length; i++) { + const m = messages[i]; + if (m.role === "user") { + out.push({ + id: String(m.id), + role: "user", + content: m.content, + createdAt: m.created_at, + status: "ready", + }); + continue; + } + let userPrompt = ""; + for (let j = i - 1; j >= 0; j--) { + if (messages[j].role === "user") { + userPrompt = messages[j].content; + break; + } + } + out.push(mapPersistedAssistantToChatMessage(m, userPrompt || "Analysis")); + } + return out; +} + +function mapPersistedAssistantToChatMessage(m: ApiMessage, promptForInspection: string): ChatMessage { + const response: AnalyzeApiResponse = { + analysis: m.content, + trace: (m.metadata_json?.trace as AnalyzeApiResponse["trace"]) ?? [], + executed_steps: (m.metadata_json?.executed_steps as AnalyzeApiResponse["executed_steps"]) ?? [], + errors: (m.metadata_json?.errors as AnalyzeApiResponse["errors"]) ?? [], + inspection_id: + typeof m.metadata_json?.inspection_id === "string" ? m.metadata_json.inspection_id : undefined, + }; + const mapped = mapAnalyzeResponseToUi(promptForInspection, response); + cacheInspection(mapped.inspection); return { - conversations: isDemoOnlyMode ? seededConversations : [], - fallback: isDemoOnlyMode, + ...mapped.message, + id: String(m.id), + createdAt: m.created_at, }; } -export async function submitChatPrompt(payload: ChatRequest): Promise { +export async function submitChatPrompt(payload: ChatRequest, accessToken: string | null): Promise { + if (isDemoOnlyMode) { + await sleep(850); + const message = buildAssistantMessage(payload.prompt); + return { + conversationId: payload.conversationId ?? shortId("chat"), + title: conversationTitleFromPrompt(payload.prompt), + message, + mode: "demo", + fallback: true, + }; + } + + if (!accessToken) { + throw new ApiError("Not authenticated."); + } + try { - const requestBody: AnalyzeApiRequest = { query: payload.prompt }; - const response = await request("/analyze", { + const body: Record = { query: payload.prompt }; + if (payload.conversationId && isBackendConversationId(payload.conversationId)) { + body.conversation_id = Number.parseInt(payload.conversationId, 10); + } + + const raw = await requestWithAuth("/chat", accessToken, { method: "POST", - body: JSON.stringify(requestBody), + body: JSON.stringify(body), }); - const mapped = mapAnalyzeResponseToUi(payload.prompt, response); + + const analyzeLike: AnalyzeApiResponse = { + analysis: raw.analysis, + trace: raw.trace, + executed_steps: raw.executed_steps, + errors: raw.errors, + inspection_id: raw.inspection_id ?? undefined, + }; + const mapped = mapAnalyzeResponseToUi(payload.prompt, analyzeLike); cacheInspection(mapped.inspection); + const assistantMessage: ChatMessage = { + ...mapped.message, + id: String(raw.assistant_message.id), + createdAt: raw.assistant_message.created_at, + }; + return { - conversationId: payload.conversationId ?? shortId("chat"), - title: mapped.title, - message: mapped.message, + conversationId: String(raw.conversation.id), + title: raw.conversation.title, + message: assistantMessage, mode: "live", fallback: false, }; diff --git a/ui/src/api/inspections.ts b/ui/src/api/inspections.ts index e7af710..387abd1 100644 --- a/ui/src/api/inspections.ts +++ b/ui/src/api/inspections.ts @@ -11,7 +11,8 @@ export function cacheInspection(inspection: InspectionData) { inspectionCache.set(inspection.id, inspection); } -export async function fetchInspection(inspectionId: string): Promise { +/** Pass `accessToken` for persisted chat inspections (required after backend restart / cold cache). */ +export async function fetchInspection(inspectionId: string, accessToken: string | null = null): Promise { const cachedInspection = inspectionCache.get(inspectionId); if (cachedInspection) { return { @@ -28,6 +29,8 @@ export async function fetchInspection(inspectionId: string): Promise(`/inspections/${inspectionId}`); + const response = await request(`/inspections/${inspectionId}`, { + authToken: accessToken ?? undefined, + }); return { ...response, fallback: false }; } diff --git a/ui/src/api/types.ts b/ui/src/api/types.ts index 8153d72..8ec8847 100644 --- a/ui/src/api/types.ts +++ b/ui/src/api/types.ts @@ -76,3 +76,54 @@ export interface AnalyzeApiResponse { errors: AnalyzeErrorItem[]; inspection_id?: string; } + +/** GET /conversations row (backend snake_case). */ +export interface ApiConversationSummary { + id: number; + title: string; + updated_at: string; + last_message_preview: string | null; +} + +export interface ApiConversationsListResponse { + conversations: ApiConversationSummary[]; +} + +/** GET /conversations/{id} message (backend snake_case). */ +export interface ApiMessage { + id: number; + role: "user" | "assistant"; + content: string; + created_at: string; + metadata_json: Record | null; +} + +export interface ApiConversationPublic { + id: number; + title: string; + created_at: string; + updated_at: string; +} + +export interface ApiConversationDetailResponse { + conversation: ApiConversationPublic; + messages: ApiMessage[]; +} + +/** POST /chat response (backend snake_case). */ +export interface ApiChatTurnResponse { + conversation: ApiConversationPublic; + assistant_message: { + id: number; + role: "assistant"; + content: string; + created_at: string; + status: string; + metadata_json: Record | null; + }; + analysis: string; + trace: AnalyzeTraceEvent[]; + executed_steps: AnalyzeExecutedStep[]; + errors: AnalyzeErrorItem[]; + inspection_id: string | null; +} diff --git a/ui/src/components/app/ChatThread.tsx b/ui/src/components/app/ChatThread.tsx index 8170566..6186569 100644 --- a/ui/src/components/app/ChatThread.tsx +++ b/ui/src/components/app/ChatThread.tsx @@ -8,16 +8,29 @@ import type { InspectionTabId } from "@/types/inspection"; interface ChatThreadProps { messages: ChatMessageType[]; isSubmitting: boolean; + /** Loading full thread from the server (e.g. after selecting a saved conversation). */ + isLoadingThread?: boolean; onInspect: (inspectionId: string, preferredTab?: InspectionTabId) => void; } -export function ChatThread({ messages, isSubmitting, onInspect }: ChatThreadProps) { +export function ChatThread({ messages, isSubmitting, isLoadingThread = false, onInspect }: ChatThreadProps) { const bottomRef = useRef(null); useEffect(() => { bottomRef.current?.scrollIntoView({ behavior: "smooth" }); }, [messages, isSubmitting]); + if (isLoadingThread && !messages.length) { + return ( +
+
+ + Loading conversation… +
+
+ ); + } + if (!messages.length) { return ( ([]); const [activeConversationId, setActiveConversationId] = useState(""); const [loading, setLoading] = useState(true); + const [threadLoading, setThreadLoading] = useState(false); const [isSubmitting, setIsSubmitting] = useState(false); const [error, setError] = useState(null); + const hydrateThread = useCallback( + async (conversationId: string) => { + if (!token || !isBackendConversationId(conversationId)) return; + setThreadLoading(true); + setError(null); + try { + const detail = await fetchConversationDetail(token, conversationId); + setConversations((current) => current.map((c) => (c.id === conversationId ? detail : c))); + } catch (err) { + setError(err instanceof Error ? err.message : "Unable to load conversation."); + } finally { + setThreadLoading(false); + } + }, + [token], + ); + useEffect(() => { + if (!isReady || !isAuthenticated) return; + + let cancelled = false; + const load = async () => { setLoading(true); setError(null); try { - const response = await fetchConversations(); + const response = await fetchConversations(token); + if (cancelled) return; setConversations(response.conversations); const storedId = uiStore.getActiveConversation(); - const nextActiveId = response.conversations.find((item) => item.id === storedId)?.id ?? response.conversations[0]?.id ?? ""; - setActiveConversationId(nextActiveId); + const resolvedId = + response.conversations.find((item) => item.id === storedId)?.id ?? + response.conversations[0]?.id ?? + ""; + + setActiveConversationId(resolvedId); + + if (token && resolvedId && isBackendConversationId(resolvedId)) { + setThreadLoading(true); + try { + const detail = await fetchConversationDetail(token, resolvedId); + if (cancelled) return; + setConversations((current) => current.map((c) => (c.id === resolvedId ? detail : c))); + } catch (err) { + if (!cancelled) { + setError(err instanceof Error ? err.message : "Unable to load conversation."); + } + } finally { + if (!cancelled) setThreadLoading(false); + } + } } catch (err) { - setError(err instanceof Error ? err.message : "Unable to load conversations."); + if (!cancelled) { + setError(err instanceof Error ? err.message : "Unable to load conversations."); + } } finally { - setLoading(false); + if (!cancelled) setLoading(false); } }; void load(); - }, []); + return () => { + cancelled = true; + }; + }, [isReady, isAuthenticated, token]); useEffect(() => { if (activeConversationId) { @@ -77,6 +132,7 @@ export function useChat() { const selectConversation = (conversationId: string) => { setActiveConversationId(conversationId); setError(null); + void hydrateThread(conversationId); }; const sendPrompt = async (prompt: string, attachments: UploadedAsset[] = []) => { @@ -98,34 +154,76 @@ export function useChat() { setError(null); try { - const response = await submitChatPrompt({ - conversationId, - prompt: trimmed, - attachmentIds: attachments.map((item) => item.id), - }); - - setConversations((current) => - current.map((conversation) => - conversation.id === conversationId - ? { - ...conversation, - title: - conversation.messages.length === 0 || conversation.title === "New analysis" - ? response.title - : conversation.title, - updatedAt: new Date().toISOString(), - mode: response.mode, - sourceLabel: - response.mode === "live" - ? "Connected backend" - : attachments.length > 0 - ? "Uploaded dataset" - : conversation.sourceLabel, - messages: [...conversation.messages, response.message], - } - : conversation, - ), + const response = await submitChatPrompt( + { + conversationId, + prompt: trimmed, + attachmentIds: attachments.map((item) => item.id), + }, + token, ); + + const resolvedId = response.conversationId; + + if (token && isBackendConversationId(resolvedId)) { + try { + const detail = await fetchConversationDetail(token, resolvedId); + setConversations((current) => { + const rest = current.filter((c) => c.id !== conversationId && c.id !== detail.id); + return [detail, ...rest]; + }); + setActiveConversationId(detail.id); + } catch { + setConversations((current) => + current.map((conversation) => + conversation.id === conversationId + ? { + ...conversation, + id: resolvedId, + title: + conversation.messages.length <= 1 || conversation.title === "New analysis" + ? response.title + : conversation.title, + updatedAt: new Date().toISOString(), + mode: response.mode, + sourceLabel: + response.mode === "live" + ? "Connected backend" + : attachments.length > 0 + ? "Uploaded dataset" + : conversation.sourceLabel, + messages: [...conversation.messages, response.message], + } + : conversation, + ), + ); + setActiveConversationId(resolvedId); + } + } else { + setConversations((current) => + current.map((conversation) => + conversation.id === conversationId + ? { + ...conversation, + title: + conversation.messages.length === 0 || conversation.title === "New analysis" + ? response.title + : conversation.title, + updatedAt: new Date().toISOString(), + mode: response.mode, + sourceLabel: + response.mode === "live" + ? "Connected backend" + : attachments.length > 0 + ? "Uploaded dataset" + : conversation.sourceLabel, + messages: [...conversation.messages, response.message], + } + : conversation, + ), + ); + } + return true; } catch (err) { setError(err instanceof Error ? err.message : "Unable to reach Planera."); @@ -140,6 +238,7 @@ export function useChat() { activeConversation, activeConversationId, loading, + threadLoading, isSubmitting, error, startNewChat, diff --git a/ui/src/hooks/useInspectionPanel.ts b/ui/src/hooks/useInspectionPanel.ts index 07604a9..0e46d74 100644 --- a/ui/src/hooks/useInspectionPanel.ts +++ b/ui/src/hooks/useInspectionPanel.ts @@ -1,8 +1,10 @@ import { useCallback, useState } from "react"; import { fetchInspection } from "@/api/inspections"; +import { useAuth } from "@/hooks/useAuth"; import type { InspectionData, InspectionTabId } from "@/types/inspection"; export function useInspectionPanel() { + const { token } = useAuth(); const [open, setOpen] = useState(false); const [maximized, setMaximized] = useState(false); const [activeTab, setActiveTab] = useState("sql"); @@ -17,7 +19,7 @@ export function useInspectionPanel() { setError(null); try { - const response = await fetchInspection(inspectionId); + const response = await fetchInspection(inspectionId, token); setInspection(response.inspection); } catch (err) { setInspection(null); @@ -25,7 +27,7 @@ export function useInspectionPanel() { } finally { setLoading(false); } - }, []); + }, [token]); return { open, diff --git a/ui/src/pages/AppPage.tsx b/ui/src/pages/AppPage.tsx index f2ea14d..386fa6a 100644 --- a/ui/src/pages/AppPage.tsx +++ b/ui/src/pages/AppPage.tsx @@ -36,7 +36,18 @@ const dashboardMetrics = [ export function AppPage() { const { isMobile, collapsed, mobileOpen, closeMobileSidebar, toggleSidebar } = useResponsiveSidebar(); const { uploads, isUploading, error: uploadError, latestUploadMode, uploadFile } = useUpload(); - const { conversations, activeConversation, activeConversationId, loading, isSubmitting, error, startNewChat, selectConversation, sendPrompt } = useChat(); + const { + conversations, + activeConversation, + activeConversationId, + loading, + threadLoading, + isSubmitting, + error, + startNewChat, + selectConversation, + sendPrompt, + } = useChat(); const inspection = useInspectionPanel(); const [draft, setDraft] = useState(""); const [attachments, setAttachments] = useState([]); @@ -125,6 +136,7 @@ export function AppPage() { void inspection.openInspection(inspectionId, preferredTab)} /> )} From 5b2b8606a03c08c0e09cce1b3cabfe56a6c604e7 Mon Sep 17 00:00:00 2001 From: Abhinav Singh Date: Sat, 4 Apr 2026 17:30:30 -0400 Subject: [PATCH 2/2] Refine API documentation and structure for chat and analysis endpoints. Clarify the roles of `POST /chat` as the primary product API and `POST /analyze` as a deprecated debug endpoint. Update README and UI components to reflect these changes, ensuring clear guidance for usage and persistence behavior. --- README.md | 16 +++++++++++++--- app/api/chat_routes.py | 18 ++++++++++++++++-- app/api/routes.py | 25 ++++++++++++++++++++++--- app/main.py | 21 ++++++++++++++++++++- app/services/analysis_run.py | 10 +++++++++- ui/README.md | 18 ++++++++++++------ ui/src/api/chat.ts | 9 ++++++++- ui/src/api/mappers.ts | 4 ++++ ui/src/api/types.ts | 5 +++++ 9 files changed, 109 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index 601c309..3c86b3e 100644 --- a/README.md +++ b/README.md @@ -70,9 +70,17 @@ Core modules: - `app/agent/executor.py`: SQL execution engine (pandas helpers retained) - `app/agent/analysis.py`: single-pass narrative from query + steps - `app/agent/graph.py`: LangGraph orchestration -- `app/api/routes.py`: API surface +- `app/api/routes.py`: Shared API surface (health, uploads, inspections, **stateless** `POST /analyze`) +- `app/api/chat_routes.py`: **Primary product** chat API (`POST /chat`, conversation history) - `ui/`: React + Vite frontend +### API: primary chat vs stateless analyze + +| Path | Role | +|------|------| +| **`POST /chat`** (with JWT) | **Product path:** persists conversations, messages, and inspection snapshots; use for the React app and any integrated client. | +| **`POST /analyze`** (no auth) | **Debug / manual testing:** same analytics engine, but **no persistence** and inspection data only in server memory until restart. Marked **deprecated** in OpenAPI; do not treat it as a peer to `/chat`. | + ## Repo Structure ```text @@ -121,14 +129,16 @@ API endpoints: - `GET /sample-questions` - `POST /uploads` - `GET /inspections/{inspection_id}` -- `POST /analyze` +- **`POST /chat`** — **primary:** authenticated analysis turn; persists thread + inspection snapshot +- `GET /conversations`, `GET /conversations/{id}` — list/load chat history (authenticated) +- `POST /analyze` — **deprecated / debug only:** stateless run (see table above) - `POST /auth/signup` — create user (SQLite), returns JWT - `POST /auth/login` — issue JWT - `GET /auth/me` — current user (`Authorization: Bearer `) **Database:** On API startup the app creates SQLite tables if needed (no separate migration step for this demo). By default the DB file is `planera.db` in the project root (same directory as `requirements.txt`). Override with `DATABASE_PATH` in `.env`. Add a strong `JWT_SECRET_KEY` before any shared deployment; the repo default is for local dev only. -Example request: +Example (debug — stateless; prefer `/chat` with a JWT for real usage): ```bash curl -X POST http://localhost:8000/analyze \ diff --git a/app/api/chat_routes.py b/app/api/chat_routes.py index 64fece1..0c5f72c 100644 --- a/app/api/chat_routes.py +++ b/app/api/chat_routes.py @@ -1,4 +1,9 @@ -"""User-scoped conversation and authenticated chat orchestration.""" +"""User-scoped conversation and authenticated chat orchestration. + +This module defines the **primary product HTTP API** for analysis turns: ``POST /chat`` persists +messages and inspection snapshots. Stateless ``POST /analyze`` (see ``app.api.routes``) exists only +for debugging and is not a substitute for these routes. +""" from __future__ import annotations @@ -102,7 +107,16 @@ def get_conversation( ) -@router.post("/chat", response_model=ChatTurnResponse) +@router.post( + "/chat", + response_model=ChatTurnResponse, + summary="Submit one chat turn (primary product path)", + description=( + "Authenticated endpoint: saves the user message, runs the analytics workflow, stores the " + "assistant reply and links an inspection snapshot when present. " + "Use this for all normal app traffic instead of ``POST /analyze``." + ), +) def chat_turn( body: ChatSubmitRequest, current_user: User = Depends(get_current_user), diff --git a/app/api/routes.py b/app/api/routes.py index b28e70e..da1cd92 100644 --- a/app/api/routes.py +++ b/app/api/routes.py @@ -55,7 +55,12 @@ def inspection_details( db: Session = Depends(get_db), current_user: User | None = Depends(get_current_user_optional), ) -> InspectionResponse: - """Return a stored inspection (database snapshot for chat history, else in-memory from /analyze).""" + """Return inspection detail. + + Prefer rows loaded from ``inspection_snapshots`` (written by ``POST /chat``); those require auth. + Otherwise falls back to the in-memory store populated by a stateless ``POST /analyze`` run in + the same server process (debug/demo). + """ row = db.get(InspectionSnapshot, inspection_id) if row is not None: @@ -80,9 +85,23 @@ def inspection_details( return InspectionResponse(inspection=inspection, fallback=False) -@router.post("/analyze", response_model=AnalyzeResponse) +@router.post( + "/analyze", + response_model=AnalyzeResponse, + tags=["debug"], + deprecated=True, + summary="Stateless analysis (debug / manual testing only)", + description=( + "**Not the primary product API.** For normal use, authenticated clients should call " + "`POST /chat`, which persists conversations, messages, and inspection snapshots.\n\n" + "This endpoint runs the same analytics pipeline without auth, without database persistence, " + "and keeps the inspection payload only in process memory (lost on restart). Use it for " + "local debugging, Swagger/Postman checks, and quick stateless demos.\n\n" + "Response shape aligns with the analysis/trace/steps portion of `POST /chat`." + ), +) def analyze(request: AnalyzeRequest) -> AnalyzeResponse: - """Execute the analytics workflow for a user query.""" + """Run analytics without persistence (see OpenAPI ``description`` — prefer ``POST /chat`` for product flows).""" try: return run_stored_analysis(request.query).response diff --git a/app/main.py b/app/main.py index cdb7f5f..4085152 100644 --- a/app/main.py +++ b/app/main.py @@ -33,8 +33,27 @@ async def lifespan(_app: FastAPI): app = FastAPI( title=settings.app_name, version="0.1.0", - description="Natural-language analytics copilot for GTM teams.", + description=( + "Natural-language analytics copilot for GTM teams.\n\n" + "**Primary product API:** `POST /chat` (authenticated) persists conversations, assistant " + "replies, and inspection snapshots. **Debug / stateless:** `POST /analyze` is deprecated in " + "OpenAPI and reserved for manual testing — same pipeline, no auth and no DB persistence." + ), lifespan=lifespan, + openapi_tags=[ + { + "name": "chat", + "description": "Primary app contract: user-scoped conversations and analysis turns (`POST /chat`, conversation list/detail).", + }, + { + "name": "auth", + "description": "JWT signup, login, and current user.", + }, + { + "name": "debug", + "description": "Stateless debugging and local testing (`POST /analyze`). Not equivalent to the persisted chat product path.", + }, + ], ) app.add_middleware( CORSMiddleware, diff --git a/app/services/analysis_run.py b/app/services/analysis_run.py index 087d413..e779925 100644 --- a/app/services/analysis_run.py +++ b/app/services/analysis_run.py @@ -1,4 +1,12 @@ -"""Shared analysis execution + inspection storage (used by /analyze and /chat).""" +"""Shared analytics workflow invocation and in-process inspection materialization. + +``run_stored_analysis`` backs both HTTP entry points: + +- **Product:** ``POST /chat`` (auth, SQLite history, persisted inspection snapshots). +- **Debug:** ``POST /analyze`` (no auth, no DB; inspection id valid only in-memory for that process). + +Both return the same ``AnalyzeResponse`` shape for the HTTP layer; only routing and persistence differ. +""" from __future__ import annotations diff --git a/ui/README.md b/ui/README.md index 34df97d..0a1e814 100644 --- a/ui/README.md +++ b/ui/README.md @@ -5,7 +5,7 @@ Planera is a premium analytics copilot frontend built with React, TypeScript, Vi This app is designed to work against a separately hosted backend API and includes a dedicated service layer for: - authentication (JWT session against `POST /auth/login`, `POST /auth/signup`, `GET /auth/me`) -- chat submission +- **chat submission** via **`POST /chat`** (authenticated product path — the UI does not call `POST /analyze` for normal usage) - file uploads - inspection data - validation and trace metadata @@ -13,6 +13,13 @@ This app is designed to work against a separately hosted backend API and include When the backend is unavailable, the UI can fall back to seeded demo data so the product remains demo-ready. +### Primary vs debug API (backend) + +| Backend path | Used by this UI for normal signed-in flows? | +|--------------|---------------------------------------------| +| **`POST /chat`** | **Yes** — every real analysis turn goes here (`src/api/chat.ts`). | +| **`POST /analyze`** | **No** — stateless debug endpoint on the server only (Swagger/curl/Postman). Same analysis shape may appear embedded in `/chat` responses or mapped in TypeScript as `AnalyzeApiResponse` — that is a **payload type name**, not an HTTP call to `/analyze`. | + ## Getting Started 1. Install dependencies @@ -121,14 +128,13 @@ The frontend keeps request logic out of presentational components. Update endpoi Current live contract: - `POST /auth/signup`, `POST /auth/login`, `GET /auth/me` — session and route protection -- `POST /analyze` is used for real chat submissions +- `POST /chat` — real analysis turns (persisted conversations and assistant messages) +- `GET /conversations`, `GET /conversations/:id` — conversation list and thread hydration - `POST /uploads` profiles CSV and TSV workspace uploads -- `GET /inspections/:id` fetches a stored inspection payload when it is not already cached client-side +- `GET /inspections/:id` fetches a stored inspection payload when it is not already cached client-side (requires auth when the snapshot came from `/chat`) - `GET /sample-questions` can be wired for dynamic prompt suggestions -Current gaps in the backend contract: - -- conversation history is currently local/demo until a backend history endpoint is introduced +The backend still exposes **`POST /analyze`** for stateless debugging only; the **React app does not use it** for authenticated workspace usage. If you want the frontend to use only real backend data, set: diff --git a/ui/src/api/chat.ts b/ui/src/api/chat.ts index ec6a369..15daccf 100644 --- a/ui/src/api/chat.ts +++ b/ui/src/api/chat.ts @@ -1,4 +1,11 @@ -import { request, requestWithAuth, ApiError } from "@/api/client"; +/** + * Authenticated chat + conversation APIs. + * + * Real turns use **`POST /chat`** only. Types named `Analyze*` mirror the analysis payload shape + * returned inside chat responses (and by the server's stateless `POST /analyze`); this module + * does not call `/analyze` for product flows. + */ +import { requestWithAuth, ApiError } from "@/api/client"; import { cacheInspection } from "@/api/inspections"; import { conversationTitleFromPrompt, mapAnalyzeResponseToUi } from "@/api/mappers"; import type { diff --git a/ui/src/api/mappers.ts b/ui/src/api/mappers.ts index b81bf9d..e2544a8 100644 --- a/ui/src/api/mappers.ts +++ b/ui/src/api/mappers.ts @@ -1,3 +1,7 @@ +/** + * Maps analysis-shaped JSON (from `POST /chat` or demo mocks) into chat messages and inspection UI state. + * Not tied to HTTP `POST /analyze` — that endpoint is server-only debug; the UI uses `/chat`. + */ import type { AnalyzeApiResponse, AnalyzeArtifactSummary, diff --git a/ui/src/api/types.ts b/ui/src/api/types.ts index 8ec8847..282b910 100644 --- a/ui/src/api/types.ts +++ b/ui/src/api/types.ts @@ -31,6 +31,7 @@ export interface InspectionResponse { fallback: boolean; } +/** Body for the server's stateless `POST /analyze` (debug only — the UI sends `POST /chat` instead). */ export interface AnalyzeApiRequest { query: string; } @@ -69,6 +70,10 @@ export interface AnalyzeExecutedStep { error?: string | null; } +/** + * Analysis payload shape shared by `POST /chat` (and the debug `POST /analyze` response). + * Naming reflects backend fields, not a requirement to call `/analyze`. + */ export interface AnalyzeApiResponse { analysis: string; trace: AnalyzeTraceEvent[];