From c9686af834a72dd624dc501d04c50980a42a3c16 Mon Sep 17 00:00:00 2001 From: C Cheng <10414576+ccheng26@users.noreply.github.com> Date: Tue, 10 Dec 2024 12:19:54 -0500 Subject: [PATCH] feat: Literal ai feedback (#151) --- app/src/chat_api.py | 49 +++++++++++++++++++++++- app/tests/src/test_chat_api.py | 70 ++++++++++++++++++++++++++++++++-- docs/app/api_spec.md | 64 +++++++++++++++++++++++++++---- 3 files changed, 170 insertions(+), 13 deletions(-) diff --git a/app/src/chat_api.py b/app/src/chat_api.py index 943645df..9e5aef66 100644 --- a/app/src/chat_api.py +++ b/app/src/chat_api.py @@ -10,7 +10,7 @@ from typing import Optional, Sequence from asyncer import asyncify -from fastapi import APIRouter, HTTPException, Request +from fastapi import APIRouter, HTTPException, Request, Response from literalai import AsyncLiteralClient from pydantic import BaseModel from sqlalchemy import select @@ -127,6 +127,53 @@ async def engines(user_id: str) -> list[str]: return response +class FeedbackRequest(BaseModel): + session_id: str + is_positive: bool + response_id: str + comment: Optional[str] = None + user_id: Optional[str] = None + + +@router.post("/feedback") +async def feedback( + request: FeedbackRequest, +) -> Response: + """Endpoint for creating feedback for a chatbot response message + + Args: + request (FeedbackRequest): + session_id: the session id, used if user_id is None + is_positive: if chatbot response answer is helpful or not + response_id: the response_id of the chatbot response + comment: user comment for the feedback + user_id: the user's id + + Returns: + FeedbackResponse + user_id: the user's id + value: 1 if is_positive was "true" and 0 if is_positive was "false" + step_id: ID of the step associated with the score + comment: the initial user comment for the feedback + """ + user_session_id = request.user_id if request.user_id else request.session_id + + session = await _get_user_session(user_session_id) + # API endpoint to send feedback https://docs.literalai.com/guides/logs#add-a-score + response = await literalai().api.create_score( + step_id=request.response_id, + name=session.user.user_id, + type="HUMAN", + value=1 if request.is_positive else 0, + comment=request.comment, + ) + logger.info("Received feedback value: %s for response_id %s", response.value, response.step_id) + if response.comment: + logger.info("Received comment: %s", response.comment) + + return Response(status_code=200) + + # endregion # region: =================== API Endpoints =================== diff --git a/app/tests/src/test_chat_api.py b/app/tests/src/test_chat_api.py index 59059e1d..30fd6f86 100644 --- a/app/tests/src/test_chat_api.py +++ b/app/tests/src/test_chat_api.py @@ -1,10 +1,12 @@ import logging +from typing import Optional from unittest.mock import AsyncMock, MagicMock import pytest from fastapi import HTTPException from fastapi.exceptions import RequestValidationError from fastapi.testclient import TestClient +from literalai import Score from src import chat_api from src.chat_api import ( @@ -44,10 +46,38 @@ def client(monkeypatch): return TestClient(router) -def test_api_healthcheck(client): - response = client.get("/api/healthcheck") - assert response.status_code == 200 - assert response.json()["status"] == "OK" +class MockLiteralAIApi: + async def get_or_create_user(self, identifier, metadata): + self.id = identifier + return self + + async def create_score( + self, + name: str | None, + type: str, + value: float, + step_id: Optional[str] = None, + comment: Optional[str] = None, + ): + return Score( + name=name, + type=type, + value=value, + step_id=step_id, + comment=comment, + generation_id=None, + dataset_experiment_item_id=None, + tags=None, + ) + + +@pytest.fixture +def literalai_client(monkeypatch): + mock = mock_literalai() + monkeypatch.setattr(mock, "api", MockLiteralAIApi()) + monkeypatch.setattr(chat_api, "literalai", lambda: mock) + + return TestClient(router) def test_api_engines(client): @@ -202,3 +232,35 @@ def test_get_chat_engine_not_allowed(user_info): ) with pytest.raises(HTTPException, match="Unknown engine: bridges-eligibility-manual"): get_chat_engine(session) + + +def test_post_feedback_success(literalai_client): + response = literalai_client.post( + "/api/feedback", + json={ + "session_id": "Session2", + "is_positive": "true", + "response_id": "response_id0", + "comment": "great answer", + }, + ) + + assert response.status_code == 200 + + +def test_post_feedback_fail(monkeypatch, literalai_client): + try: + literalai_client.post( + "/api/feedback", + json={ + "session_id": "Session2", + "is_positive": "true", + "comment": "great answer", + }, + ) + raise AssertionError("Expected RequestValidationError") + except RequestValidationError as e: + error = e.errors()[0] + assert error["type"] == "missing" + assert error["msg"] == "Field required" + assert error["loc"] == ("body", "response_id") diff --git a/docs/app/api_spec.md b/docs/app/api_spec.md index b0fb700c..a26e4956 100644 --- a/docs/app/api_spec.md +++ b/docs/app/api_spec.md @@ -1,9 +1,11 @@ # API Specification: Assistive Chatbot Integration + This API enables the integration of an Assistive Chatbot with other products. ## Endpoint: `POST /query` ### Description + This endpoint accepts a query from a navigator and responds with a generated answer from the chatbot, along with relevant citations. Each response includes auditing fields to monitor usage by user, organization, and customer. --- @@ -12,11 +14,10 @@ This endpoint accepts a query from a navigator and responds with a generated ans **Headers** -- **Authorization**: Bearer token for authenticating API access. - **Parameters** Note: we will defer making any of these required for now + - **user_id** _(string, optional)_: Unique identifier for the navigator within ImagineLA - **agency_id** _(string, optional)_: Identifier for the organization the navigator belongs to - **beneficiary_id** _(string, optional)_: Anonymized ID for the customer or beneficiary associated with the query, if applicable and distinct from `user_id`. @@ -59,7 +60,7 @@ Note: we will defer making any of these required for now #### Server Error Responses (5xx) - **500 Internal Server Error**: Generic server error. - + ### Example #### Example request @@ -70,7 +71,7 @@ Note: we will defer making any of these required for now "org_id": "org789", "customer_id": "cust001", "session_id": "sess2024A", - "query": + "query": "content": "What benefits are available for single mothers in California?", } ``` @@ -102,10 +103,57 @@ Note: we will defer making any of these required for now } ``` +## Endpoint: `POST /feedback` + +### Description + +This endpoint accepts feedback from a navigator and responds with the user_id, value, step_id and comment given for the feedback request. + +### Request Parameters + +**Headers** + +**Parameters** + +- **session_id** _(string, required)_: Unique identifier for the current session, to track prior messages in the conversation. +- **response_id** _(string, required)_: Unique identifier for the chatbot response. +- **is_positive** _(bool, required)_: If the chatbot response is helpful to the navigator or not. + +- **user_id** _(string, optional)_: Unique identifier for the navigator within ImagineLA. +- **comment** _(string, optional)_: The user's feedback comment for the LLM response. + +--- + +### Responses + +#### Success Response (200 OK) + +#### Client Error Responses (4xx) + +- **400 Bad Request**: Missing or invalid parameters. + +#### Server Error Responses (5xx) + +- **500 Internal Server Error**: Generic server error. + +### Example + +#### Example request + +```json +{ + "user_id": "nav12345", + "is_positive": true, + "response_id": "resp6789", + "comment": "this response is great." +} +``` + # For discussion ## Questions (Nava) - - Do we need the security key? What are the risks to leaving the API open? - - What kind of user information would be helpful to track with each request? - Some possibilities we identified include user_id, org_id, customer_id, and session_id - - Does this endpoint schema make sense? Are there any changes we should make before implementing this as a v1? (Keeping in mind we can adapt over time as needed.) + +- Do we need the security key? What are the risks to leaving the API open? +- What kind of user information would be helpful to track with each request? + Some possibilities we identified include user_id, org_id, customer_id, and session_id +- Does this endpoint schema make sense? Are there any changes we should make before implementing this as a v1? (Keeping in mind we can adapt over time as needed.)