diff --git a/.gitignore b/.gitignore index 13e92a9..b62b393 100644 --- a/.gitignore +++ b/.gitignore @@ -246,4 +246,7 @@ http-client.private.env.json charts/ test.yaml -src/app/baml_client \ No newline at end of file +src/app/baml_client +.DS_Store +src/.DS_Store +*.swp diff --git a/src/app/api/api_v1/endpoints/chat.py b/src/app/api/api_v1/endpoints/chat.py index 6a29da8..cffb7ac 100644 --- a/src/app/api/api_v1/endpoints/chat.py +++ b/src/app/api/api_v1/endpoints/chat.py @@ -6,7 +6,6 @@ import psycopg from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException, Request, status from fastapi.responses import StreamingResponse -from langchain_core.messages import ToolMessage from langgraph.checkpoint.postgres.aio import AsyncPostgresSaver from openai import RateLimitError from psycopg.rows import AsyncRowFactory, DictRow, dict_row @@ -20,7 +19,7 @@ from src.app.models import chat as models from src.app.search.services.search import SearchService, get_search_service from src.app.services.data_collection import get_data_collection_service -from src.app.services.helpers import linkify_missing_citations +from src.app.services.helpers import latest_tool_docs, linkify_missing_citations from src.app.shared.domain.constants import subjects as subjectsDict from src.app.shared.domain.exceptions import ( EmptyQueryError, @@ -351,7 +350,6 @@ async def agent_response( ): try: session_id = extract_session_cookie(request) - docs = [] thread_id = body.thread_id if body.thread_id else None @@ -400,11 +398,7 @@ async def agent_response( trace_context=trace_context, ) - all_docs = [] - for msg in res["messages"]: - if isinstance(msg, ToolMessage) and getattr(msg, "artifact", None): - all_docs.extend(msg.artifact) - docs = all_docs if all_docs else None + docs = latest_tool_docs(res["messages"]) content = linkify_missing_citations( cast(str, res["messages"][-1].content), docs or [] ) diff --git a/src/app/api/api_v1/endpoints/chat_utils.py b/src/app/api/api_v1/endpoints/chat_utils.py index 6f9db1b..143739c 100644 --- a/src/app/api/api_v1/endpoints/chat_utils.py +++ b/src/app/api/api_v1/endpoints/chat_utils.py @@ -107,9 +107,18 @@ async def _stream_agent_with_memory( trace_context=trace_context, ) + docs_found = False async for chunk in stream: + if chunk.get("status") == "processing" and chunk.get("docs"): + docs_found = True yield chunk + if not docs_found: + # No tool call this turn — fall back to the last known results + # instead of leaving the client with no docs at all. + docs = await chatfactory.agent_get_latest_docs(thread_id, memory) + yield {"status": "docs_final", "docs": docs} + def _build_final_stream_payload( *, @@ -173,6 +182,9 @@ async def _stream_agent_response( ) async for chunk in stream: + if chunk.get("status") == "docs_final": + docs = chunk.get("docs") + continue final_content, docs = _update_agent_stream_state(chunk, final_content, docs) if chunk.get("status") == "streaming" and chunk.get("content"): has_streamed_content = True diff --git a/src/app/services/data_collection.py b/src/app/services/data_collection.py index 2f7fcfa..462df76 100644 --- a/src/app/services/data_collection.py +++ b/src/app/services/data_collection.py @@ -72,7 +72,6 @@ async def register_search_data( corpora: list[str] | None = None, feature: str | None = "search", ) -> uuid.UUID | None: - if not self.should_collect: logger.info("data_collection is not enabled.") return None @@ -119,7 +118,6 @@ async def register_syllabus_data( agent_answer: str, feature: Literal["syllabus_creation", "syllabus_feedback"], ) -> uuid.UUID | None: - if not self.should_collect: logger.info("data_collection is not enabled.") return None @@ -246,7 +244,6 @@ async def register_chat_data( sources: list[Document], feature: str | None = "chat", ) -> tuple[uuid.UUID | None, uuid.UUID | None]: - if not self.should_collect: logger.info("data_collection is not enabled.") return None, None diff --git a/src/app/services/helpers.py b/src/app/services/helpers.py index cc1dde2..957a55d 100644 --- a/src/app/services/helpers.py +++ b/src/app/services/helpers.py @@ -6,6 +6,7 @@ import numpy from fastapi import HTTPException from json_repair import JSONReturnType +from langchain_core.messages import BaseMessage, ToolMessage from langdetect import detect_langs from qdrant_client.http.models import models from welearn_database.data.models import EmbeddingModel @@ -140,15 +141,26 @@ def stringify_docs_content(docs: List[Any]) -> str: return documents.strip() -_CITATION_RE = re.compile(r'(?)\[Doc\s*(\d+)\]') +_CITATION_RE = re.compile( + r'(?)\[Doc\s*(\d+)\](?!\()', re.IGNORECASE +) def linkify_missing_citations(text: str, docs: List[Any]) -> str: """ - Wraps any bare `[Doc N]` marker in `text` with the `` - tag for document N, using the URL from `docs[N-1]`. Markers already wrapped in an - tag are left untouched. Safety net for when the LLM forgets to format a - citation as a link itself. + Wraps any bare `[Doc N]` marker in `text` into a double-bracket Markdown link + `[[Doc N]](URL)` for document N, using the URL from `docs[N-1]`. The double + bracket is intentional: a plain Markdown link `[Doc N](URL)` renders with its + brackets stripped (shows just "Doc N"), so the visible label must itself + contain a literal "[Doc N]" for the brackets to survive rendering. + + Markers already wrapped this way, already a single-bracket Markdown link, or + already wrapped in an HTML `` tag, are left untouched. Safety net for when + the LLM forgets to format a citation as a link itself. + + Only matches a single document number per marker — a combined citation like + "[Docs 3 et 5]" is intentionally left as-is, since there's no single URL a + two-document marker could safely resolve to. Args: text: The assembled answer text. @@ -172,11 +184,32 @@ def _replace(match: "re.Match[str]") -> str: url = _url_for(n) if not url: return match.group(0) - return f'[Doc {n}]' + return f"[[Doc {n}]]({url})" return _CITATION_RE.sub(_replace, text) +def latest_tool_docs(messages: List[BaseMessage]) -> Optional[List[Any]]: + """ + Finds the most recent tool call's retrieved documents in a message list. + + Walks `messages` from the end, so a turn that made no new tool call still + resolves to the last tool call's results instead of nothing, and a turn + that did call the tool never picks up a stale, older call's results. + + Args: + messages: The full conversation message list (may span many turns). + + Returns: + The `artifact` of the most recent `ToolMessage` that has one, or + None if no tool call with results is found. + """ + for msg in reversed(messages): + if isinstance(msg, ToolMessage) and getattr(msg, "artifact", None): + return msg.artifact + return None + + def extract_json_from_response( response: str, ) -> JSONReturnType | tuple[JSONReturnType, list[dict[str, str]]] | str: diff --git a/src/app/services/prompts.py b/src/app/services/prompts.py index 9f02778..5b9b945 100644 --- a/src/app/services/prompts.py +++ b/src/app/services/prompts.py @@ -14,34 +14,57 @@ - Never pre-emptively output a full course structure, syllabus section, or multi-topic survey unless the user asked for exactly that. - If your draft answer is turning into a list of more than ~4 items or more than one paragraph, stop and cut it down. - Do not open with sycophantic phrases ("That sounds fascinating!", "Great question!", "What a fantastic starting point!"). Acknowledge context matter-of-factly and respond directly. +- When addressing the user, remain formal. - Always reply in the same language the user wrote in. **Ask before you answer at length (Socratic behavior)** - On the first substantive message of a new conversation, and whenever the user pivots to a new subject or topic mid-conversation, check whether you have enough context to give a genuinely useful answer: their discipline/course subject, level of study, and the kind of help they want (e.g. discussion prompts, a session plan, illustrative examples, background reading). - If that context is thin, do not produce a full answer yet. Ask only the clarifying questions you actually need, combined into a single short message rather than a numbered list — never more than 3 questions. -- A clarifying-question turn is a conversational meta-turn: do not call the retrieval tool on it. +- A clarifying-question turn is a conversational meta-turn: do not call `get_resources_about_sustainability` on it. - Once the user has answered, or their message already made intent and context clear, answer directly. Do not re-ask for context you already have, and do not interrogate the user turn after turn. -**No links beyond what was retrieved this turn** -- Never produce a link, URL, or `` tag for anything other than a document returned by `get_resources_about_sustainability` in this same conversation turn. This includes links you might otherwise produce from general/parametric knowledge (a well-known Wikipedia page, a UN SDG page, a journal homepage, etc.). If you want to reference something you did not retrieve, name it in plain text with no link and no fabricated URL. - -**Using the retrieval tool** +**Using the `get_resources_about_sustainability` retrieval tool** +- When preparing your response to the user, call the `get_resources_about_sustainability` tool as much as possible to get additional, relevantresources that will help you answer the user's question in a way that is more accurate and sourced. +- HOWEVER, do not call the tool for greetings, conversational meta-turns (e.g. "thanks", "can you explain that again"), clarifying-question turns (see above), or questions answerable from general knowledge where a cited source adds no value. - Call `get_resources_about_sustainability` at most once per response. Write a single comprehensive query that covers all aspects of the user's question. -- Call the tool for factual, SDG-specific, or topic-based questions where curated sources add value. -- Do not call the tool for greetings, conversational meta-turns (e.g. "thanks", "can you explain that again"), clarifying-question turns (see above), or questions answerable from general knowledge where a cited source adds no value. +- If the user's next question stays on the same topic as your most recent `get_resources_about_sustainability` call and those results still cover it, do not call it again — keep using and citing that same set of results. Call it again only once the topic shifts or those results no longer suffice. +- Make sure to use as many of the retrieved documents as relevant to answer the user's question, and cite them explicitly in your response next to the information that you used from their content. When citing a retrieved document, make sure to stay within the context of the document and not to make up information. - If the retrieved documents are insufficient to answer, say so in your response — do not make a second tool call. +**No sources beyond what was retrieved** +- Never name, describe, or link any source — an article, video, journal, dataset, or creator — other than a document returned by your most recent `get_resources_about_sustainability` call, either from this turn or from an earlier turn if you are reusing its results because the topic hasn't shifted. This applies even with no link attached: do not mention a title, journal name, or video you did not retrieve, not even in plain text. +- If you don't have a retrieved document to support a point, make the point in your own words with no source attribution at all. Never produce a link, URL, or fabricated citation from general/parametric knowledge (a well-known Wikipedia page, a UN SDG page, a journal homepage, etc.). + **Citing sources** -- The url of each document is on a dedicated line formatted as `url:`. Copy that URL character-for-character — never substitute a Wikipedia URL, construct a URL, or modify it in any way. -- Format every inline citation as: [Doc N] where URL is the verbatim value from the document's url line and N is the document number. Never write a bare `[Doc N]` without its surrounding `` tag — the tag is what makes the citation clickable. +- Every document's URL is on a dedicated line formatted as `url:`. Copy that URL character-for-character. Never substitute, construct, guess, or modify a URL in any way — not even a Wikipedia URL you believe is close enough. +- Markdown renders a link as only its label text — [Duck Duck Go](https://duckduckgo.com) displays as just "Duck Duck Go"; the brackets and the URL are consumed as syntax and never shown. We need the "[Doc N]" brackets to stay visible, so wrap the whole marker in an EXTRA pair of brackets: [[Doc N]](URL) — the outer brackets are Markdown link syntax (invisible once rendered), the inner "[Doc N]" is the literal label text that survives rendering. URL is the verbatim value from that document's url line and N is its document number. +- Wrong: "...pour modéliser des opérations industrielles Doc 2." (no brackets at all). Also wrong: "...opérations industrielles [Doc 2](URL)." (single brackets — renders with the brackets stripped, same problem). Correct: "...pour modéliser des opérations industrielles [[Doc 2]](URL)." (renders as the clickable text "[Doc 2]"). +- One document per citation marker. Never combine document numbers in a single bracket (never write "[[Doc 3 and 5]]" or "[[Docs 3 et 5]]" — a link can only point to one URL, so a combined marker is always broken). If a claim draws on two documents, place two separate markers next to each other: [[Doc 3]](url3) [[Doc 5]](url5). +- Before citing a document for a specific claim, confirm that exact claim is actually stated in that document's content — never attribute a fact, quote, or statistic to a document that doesn't contain it, even if a different retrieved document does. +- Only cite a document from your most recent `get_resources_about_sustainability` call — never a document number from before that call. Each call produces its own fresh Doc 1, Doc 2, etc.; once a newer call happens, the previous numbering is no longer valid, even if you cited it in an earlier response. - Do not invent examples, quotes, statistics, or facts not explicitly stated in the retrieved documents. If a document does not contain enough to support a claim, omit the claim. -- If no relevant documents are retrieved, say so explicitly before drawing on general knowledge. -- Do not cite any source that was not returned by the retrieval tool in this conversation turn. +- Do not cite any source besides what was returned by your most recent `get_resources_about_sustainability` call. -**Suggesting a next step** -- After giving a substantive answer (not on a clarifying-question turn), if a natural next step exists — going deeper on one aspect, moving from discussion to a concrete classroom activity, or connecting the topic to the user's own discipline or course — end with one focused question that helps them plan their teaching. Never ask more than one, and do not force it every turn. """ +AGENT_REMINDER_PROMPT = """Reminder of your standing instructions — re-checking every turn, especially in a long conversation: +- 3–4 sentences max unless the user asked for more; same language as the user, remain formal; no sycophantic openers. +- New topic + thin context → ask up to 3 clarifying questions instead of answering; no tool call on that turn. +- Call `get_resources_about_sustainability` at most once per response, only for factual/sourced questions — skip it if the current topic is already covered by your most recent call. +- Never name, describe, or link a source you did not retrieve — not even without a link, not even just a title. Zero exceptions. +- Cite only documents from your most recent `get_resources_about_sustainability` call, one document per marker, ALWAYS as [[Doc N]](URL) — double brackets, because a single-bracket Markdown link renders with its brackets stripped ("Doc 2" with no brackets at all), which is exactly the formatting error to avoid. Never combine numbers in one marker, never a URL you weren't given, never numbering from a superseded call. +- Every citation's claim must actually be stated in that specific document — never attribute it to the wrong document or invent it.""" + +## TAKEN OUT OF THE ABOVE PROMPT TO DEACTIVATE SUGGESTING A NEXT STEP AFTER AN ANSWER +# **Suggesting a next step** +# - After giving a substantive answer (not on a clarifying-question turn), if a natural next step exists — going deeper on one aspect, moving from discussion to a concrete classroom activity, or connecting the topic to the user's own discipline or course — end with one focused question that helps them plan their teaching. Never ask more than one, and do not force it every turn. +# **Response style** +# - Keep responses concise: 2–4 sentences by default. Expand only when the user explicitly asks for more detail. +# - When a follow-up question would genuinely help the user think deeper or clarify their intent, end with one focused question. Do not force a question on every turn. +# - Always reply in the same language the user wrote in. +## + + ########################################################### ### /qna/chat/answer and /qna/stream — legacy chat ######## ########################################################### diff --git a/src/app/services/sql_db/sql_service.py b/src/app/services/sql_db/sql_service.py index 1f908fb..ad0ab20 100644 --- a/src/app/services/sql_db/sql_service.py +++ b/src/app/services/sql_db/sql_service.py @@ -24,11 +24,9 @@ def __init__(self): self.session_maker = self._create_session() def _create_engine(self): - return create_engine(self.engine_url) def _create_session(self): - Session = sessionmaker(bind=self.engine) return Session diff --git a/src/app/shared/infra/abst_chat.py b/src/app/shared/infra/abst_chat.py index 7d22e56..8f33e0f 100644 --- a/src/app/shared/infra/abst_chat.py +++ b/src/app/shared/infra/abst_chat.py @@ -21,12 +21,18 @@ from fastapi import BackgroundTasks, Depends, Request from langchain.agents import create_agent # type: ignore -from langchain.agents.middleware import SummarizationMiddleware # type: ignore +from langchain.agents.middleware import ( # type: ignore + ClearToolUsesEdit, + SummarizationMiddleware, +) +from langchain.agents.middleware.types import AgentMiddleware # type: ignore from langchain.messages import HumanMessage # type: ignore -from langchain_core.messages import BaseMessage # type: ignore +from langchain_core.messages import BaseMessage, RemoveMessage # type: ignore +from langchain_core.messages.utils import count_tokens_approximately # type: ignore from langchain_core.runnables import RunnableConfig # type: ignore from langchain_mistralai import ChatMistralAI from langgraph.checkpoint.postgres.aio import AsyncPostgresSaver # type: ignore +from langgraph.graph.message import REMOVE_ALL_MESSAGES # type: ignore from langsmith import traceable from src.app.models.documents import Document @@ -36,6 +42,7 @@ from src.app.services.helpers import ( detect_language_from_entry, extract_json_from_response, + latest_tool_docs, stringify_docs_content, ) from src.app.shared.domain.exceptions import LanguageNotSupportedError @@ -52,6 +59,58 @@ # EcoLogits.init(["openai", "mistralai"]) +class _PersistClearedToolUses(AgentMiddleware): + """Applies a `ClearToolUsesEdit` permanently to the checkpointed state. + + `ContextEditingMiddleware` (the built-in `wrap_model_call` equivalent) only + edits a deepcopy for one model call, so old tool results stay in persisted + state forever and `SummarizationMiddleware` still has to wade through them. + Running this as a `before_model` hook instead persists the clearing, and + lets it run before summarization (both are `before_model`, so list order + is honored) instead of being architecturally stuck after it. + """ + + def __init__(self, edit: ClearToolUsesEdit) -> None: + super().__init__() + self._edit = edit + + def before_model(self, state, runtime): # noqa: ANN001, ARG002 + messages = [m.model_copy() for m in state["messages"]] + self._edit.apply(messages, count_tokens=count_tokens_approximately) + return {"messages": [RemoveMessage(id=REMOVE_ALL_MESSAGES), *messages]} + + async def abefore_model(self, state, runtime): # noqa: ANN001, ARG002 + return self.before_model(state, runtime) + + +class _ReinforceHardConstraints(AgentMiddleware): + """Re-states the full system prompt (condensed) on the latest turn, every + model call — long conversations dilute the system prompt's influence; + recency counters that by keeping the rules close to generation time. + """ + + def wrap_model_call(self, request, handler): # noqa: ANN001 + return handler(request.override(messages=self._with_reminder(request.messages))) + + async def awrap_model_call(self, request, handler): # noqa: ANN001 + return await handler( + request.override(messages=self._with_reminder(request.messages)) + ) + + @staticmethod + def _with_reminder(messages): # noqa: ANN001 + messages = list(messages) + if messages and isinstance(messages[-1], HumanMessage): + messages[-1] = messages[-1].model_copy( + update={ + "content": messages[-1].content + + "\n\n" + + prompts.AGENT_REMINDER_PROMPT + } + ) + return messages + + class _AgentInputState(TypedDict): messages: list[BaseMessage] @@ -440,10 +499,23 @@ async def _create_agent( get_resources_about_sustainability, ], middleware=[ + _PersistClearedToolUses( + ClearToolUsesEdit( + trigger=0, # no threshold: always enforce `keep`, not a token-overflow safety net + clear_at_least=0, # no minimum reclaim — clear every candidate outside `keep` + keep=1, # only the most recent tool call's results stay visible to the model + placeholder=( + "[Earlier search results cleared. Call " + "get_resources_about_sustainability again if you need " + "to cite something from them.]" + ), + ) + ), SummarizationMiddleware( model=agent_model, - trigger=("tokens", 64000), - ) + trigger=("tokens", 32000), + ), + _ReinforceHardConstraints(), ], checkpointer=memory, system_prompt=prompts.AGENT_SYSTEM_PROMPT, @@ -541,6 +613,23 @@ async def agent_get_history( if m.type in ("human", "ai") ] + async def agent_get_latest_docs( + self, + thread_id: uuid.UUID, + memory: AsyncPostgresSaver, + ) -> Optional[List[Any]]: + """ + Falls back to the persisted checkpoint to find the most recent tool + call's results for a thread — needed when the current turn made no + new tool call, since nothing streamed during it would otherwise carry + those docs to the caller. + """ + agent = await self._create_agent(memory=memory) + config = RunnableConfig(configurable={"thread_id": thread_id}) + + state = await agent.aget_state(config) + return latest_tool_docs(state.values.get("messages", [])) + @traceable( run_type=TRACE_RUN_TYPE_LLM, name=TraceName.RUN_LLM_WITH_JSON_PARSING.value, @@ -575,7 +664,9 @@ async def syllabus_feedback_completion( messages: list[dict], max_tokens: int, ) -> str: - result = await self.chat_client.completion(messages=messages, max_tokens=max_tokens) + result = await self.chat_client.completion( + messages=messages, max_tokens=max_tokens + ) if not isinstance(result, str): raise ValueError("Syllabus feedback response is not a string") return result diff --git a/src/app/tests/api/api_v1/test_chat.py b/src/app/tests/api/api_v1/test_chat.py index 2455394..0c55e08 100644 --- a/src/app/tests/api/api_v1/test_chat.py +++ b/src/app/tests/api/api_v1/test_chat.py @@ -2,9 +2,10 @@ import uuid from unittest import mock from unittest.mock import MagicMock - +from langgraph.checkpoint.memory import InMemorySaver import backoff from fastapi.testclient import TestClient +from langchain_core.messages import AIMessage, HumanMessage, ToolMessage from src.app.core.config import settings from src.app.models.documents import Document as DocumentModel @@ -144,7 +145,6 @@ async def test_chat_not_supported_lang(self, chat_mock, *mocks): self.assertEqual(response.status_code, 400) def test_new_questions_empty_query(self, *mocks): - with TestClient(app) as client: response = client.post( f"{settings.API_V1_STR}/qna/reformulate/questions", @@ -188,7 +188,6 @@ async def test_stream(self, *mocks): with mock.patch( "src.app.shared.infra.abst_chat.AbstractChat.chat_message", ) as stream_mock: - with TestClient(app) as client: response = client.post( f"{settings.API_V1_STR}/qna/stream", @@ -264,7 +263,92 @@ def test_chat_agent(self, agent_message_mock, *mocks): new=mock.MagicMock(return_value=True), ) @mock.patch("src.app.shared.infra.abst_chat.AbstractChat.agent_message") - def test_chat_agent_stream(self, agent_message_mock, *mocks): + def test_chat_agent_returns_only_latest_tool_call_docs( + self, agent_message_mock, *mocks + ): + earlier_docs = [{**source_example[0], "id": "earlierDoc"}] + latest_docs = [{**source_example[0], "id": "latestDoc"}] + agent_message_mock.return_value = { + "messages": [ + HumanMessage(content="first question"), + AIMessage(content=""), + ToolMessage(content="...", tool_call_id="1", artifact=earlier_docs), + AIMessage(content="answer using earlier docs"), + HumanMessage(content="second, unrelated question"), + AIMessage(content=""), + ToolMessage(content="...", tool_call_id="2", artifact=latest_docs), + AIMessage(content="answer using latest docs"), + ] + } + + with TestClient(app) as client: + response = client.post( + f"{settings.API_V1_STR}/qna/chat/agent", + json={ + "query": "a second, unrelated question", + "thread_id": str(uuid.uuid4()), + "corpora": ["corpus1"], + "sdg_filter": [1, 2, 3], + }, + headers={"X-API-Key": "test", "origin": "test"}, + ) + self.assertEqual(response.status_code, 200) + returned_ids = [doc["id"] for doc in response.json()["docs"]] + self.assertEqual(returned_ids, ["latestDoc"]) + + @mock.patch("psycopg.AsyncConnection.connect", new_callable=mock.AsyncMock) + @mock.patch( + "src.app.shared.infra.security.check_api_key_sync", + new=mock.MagicMock(return_value=True), + ) + @mock.patch("src.app.shared.infra.abst_chat.AbstractChat.agent_message") + def test_chat_agent_reuses_docs_when_no_new_search( + self, agent_message_mock, *mocks + ): + docs = [{**source_example[0], "id": "reusedDoc"}] + agent_message_mock.return_value = { + "messages": [ + HumanMessage(content="first question"), + AIMessage(content=""), + ToolMessage(content="...", tool_call_id="1", artifact=docs), + AIMessage(content="answer"), + HumanMessage(content="follow-up needing no new search"), + AIMessage(content="answer reusing the same docs"), + ] + } + + with TestClient(app) as client: + response = client.post( + f"{settings.API_V1_STR}/qna/chat/agent", + json={ + "query": "follow-up needing no new search", + "thread_id": str(uuid.uuid4()), + "corpora": ["corpus1"], + "sdg_filter": [1, 2, 3], + }, + headers={"X-API-Key": "test", "origin": "test"}, + ) + self.assertEqual(response.status_code, 200) + returned_ids = [doc["id"] for doc in response.json()["docs"]] + self.assertEqual(returned_ids, ["reusedDoc"]) + + @mock.patch( + "src.app.shared.infra.security.check_api_key_sync", + new=mock.MagicMock(return_value=True), + ) + @mock.patch( + "src.app.api.api_v1.endpoints.chat_utils.AsyncPostgresSaver", + return_value=InMemorySaver(), + ) + @mock.patch( + "src.app.api.api_v1.endpoints.chat_utils.psycopg.AsyncConnection.connect" + ) + @mock.patch("src.app.shared.infra.abst_chat.AbstractChat.agent_message") + def test_chat_agent_stream(self, agent_message_mock, connect_mock, *mocks): + fake_conn = mock.AsyncMock() + # async with await AsyncConnection.connect(...) as conn: + connect_mock.return_value.__aenter__.return_value = fake_conn + async def _fake_stream(): yield {"status": "test", "content": "fake content"} diff --git a/src/app/tests/api/api_v1/test_chat_utils.py b/src/app/tests/api/api_v1/test_chat_utils.py index 60f6d5e..0be359e 100644 --- a/src/app/tests/api/api_v1/test_chat_utils.py +++ b/src/app/tests/api/api_v1/test_chat_utils.py @@ -1,5 +1,6 @@ import unittest import uuid +from unittest import mock from src.app.api.api_v1.endpoints import chat_utils @@ -72,5 +73,105 @@ def test_format_sse_event(self): self.assertEqual(result, 'data: {"content": "abc"}\n\n') +class TestStreamAgentWithMemory(unittest.IsolatedAsyncioTestCase): + @mock.patch("src.app.api.api_v1.endpoints.chat_utils.AsyncPostgresSaver") + @mock.patch("psycopg.AsyncConnection.connect", new_callable=mock.AsyncMock) + async def test_skips_state_fetch_when_tool_call_happened_this_turn( + self, mock_connect, mock_saver + ): + async def fake_agent_stream(): + yield {"status": "processing", "docs": [{"id": "fresh-doc"}]} + yield {"status": "stop", "content": "answer"} + + chatfactory = mock.Mock() + chatfactory.agent_message = mock.AsyncMock(return_value=fake_agent_stream()) + chatfactory.agent_get_latest_docs = mock.AsyncMock() + + chunks = [ + chunk + async for chunk in chat_utils._stream_agent_with_memory( + db_uri="postgresql://test", + async_dict_row_factory=mock.Mock(), + chatfactory=chatfactory, + body=mock.Mock(query="q", corpora=None, sdg_filter=None), + sp=mock.Mock(), + background_tasks=mock.Mock(), + thread_id=uuid.uuid4(), + ) + ] + + chatfactory.agent_get_latest_docs.assert_not_called() + self.assertEqual(len(chunks), 2) + + @mock.patch("src.app.api.api_v1.endpoints.chat_utils.AsyncPostgresSaver") + @mock.patch("psycopg.AsyncConnection.connect", new_callable=mock.AsyncMock) + async def test_falls_back_to_latest_docs_when_no_tool_call_this_turn( + self, mock_connect, mock_saver + ): + async def fake_agent_stream(): + yield {"status": "stop", "content": "answer reusing prior docs"} + + chatfactory = mock.Mock() + chatfactory.agent_message = mock.AsyncMock(return_value=fake_agent_stream()) + chatfactory.agent_get_latest_docs = mock.AsyncMock( + return_value=[{"id": "reused-doc"}] + ) + + chunks = [ + chunk + async for chunk in chat_utils._stream_agent_with_memory( + db_uri="postgresql://test", + async_dict_row_factory=mock.Mock(), + chatfactory=chatfactory, + body=mock.Mock(query="q", corpora=None, sdg_filter=None), + sp=mock.Mock(), + background_tasks=mock.Mock(), + thread_id=uuid.uuid4(), + ) + ] + + chatfactory.agent_get_latest_docs.assert_called_once() + self.assertEqual( + chunks[-1], {"status": "docs_final", "docs": [{"id": "reused-doc"}]} + ) + + +class TestStreamAgentResponse(unittest.IsolatedAsyncioTestCase): + async def test_docs_final_chunk_overrides_docs_and_is_not_forwarded(self): + async def fake_stream(**kwargs): + yield {"status": "streaming", "content": "partial answer"} + yield {"status": "docs_final", "docs": [{"id": "fallback-doc"}]} + yield {"status": "stop", "content": "partial answer"} + + data_collection = mock.Mock() + data_collection.register_chat_data = mock.AsyncMock( + return_value=(None, uuid.uuid4()) + ) + + with mock.patch.object( + chat_utils, "_stream_agent_with_memory", side_effect=fake_stream + ): + events = [ + event + async for event in chat_utils._stream_agent_response( + db_uri="postgresql://test", + async_dict_row_factory=mock.Mock(), + body=mock.Mock(query="q"), + chatfactory=mock.Mock(), + sp=mock.Mock(), + background_tasks=mock.Mock(), + data_collection=data_collection, + session_id=None, + thread_id=uuid.uuid4(), + ) + ] + + self.assertFalse(any('"status": "docs_final"' in e for e in events)) + self.assertTrue(any('"id": "fallback-doc"' in e for e in events)) + data_collection.register_chat_data.assert_called_once() + _, kwargs = data_collection.register_chat_data.call_args + self.assertEqual(kwargs["sources"], [{"id": "fallback-doc"}]) + + if __name__ == "__main__": unittest.main() diff --git a/src/app/tests/api/api_v1/test_search.py b/src/app/tests/api/api_v1/test_search.py index e1185d4..fc9f7a0 100644 --- a/src/app/tests/api/api_v1/test_search.py +++ b/src/app/tests/api/api_v1/test_search.py @@ -310,7 +310,6 @@ async def test_search_all_slices_no_result(self, *mocks): new=mock.MagicMock(return_value=True), ) class SearchTestsAll(IsolatedAsyncioTestCase): - @patch( f"{search_pipeline_path}.get_collection_by_language", new=mock.AsyncMock( diff --git a/src/app/tests/api/api_v1/test_user.py b/src/app/tests/api/api_v1/test_user.py index 0108dc4..c0addf7 100644 --- a/src/app/tests/api/api_v1/test_user.py +++ b/src/app/tests/api/api_v1/test_user.py @@ -18,7 +18,6 @@ new=mock.MagicMock(return_value=True), ) class UserApiTests(unittest.IsolatedAsyncioTestCase): - @mock.patch("src.app.services.sql_db.queries_user.session_maker") async def test_create_user_when_not_exists(self, session_maker_mock, *mocks): """Si user_id non fourni, crée un nouvel utilisateur""" diff --git a/src/app/tests/services/test_abst_chat.py b/src/app/tests/services/test_abst_chat.py index f1a132b..8764bce 100644 --- a/src/app/tests/services/test_abst_chat.py +++ b/src/app/tests/services/test_abst_chat.py @@ -1,8 +1,16 @@ import unittest from unittest import mock +from langchain.agents.middleware import ClearToolUsesEdit, SummarizationMiddleware +from langchain_core.messages import AIMessage, HumanMessage, ToolMessage + +from src.app.services import prompts from src.app.shared.domain.exceptions import LanguageNotSupportedError -from src.app.shared.infra.abst_chat import AbstractChat +from src.app.shared.infra.abst_chat import ( + AbstractChat, + _PersistClearedToolUses, + _ReinforceHardConstraints, +) class TestAbstractChat(unittest.IsolatedAsyncioTestCase): @@ -87,18 +95,100 @@ async def test_chat_message(self): self.chat.chat_client.completion.assert_not_called() self.chat.chat_client.completion_stream.assert_called_once() - @mock.patch("src.app.shared.infra.abst_chat.create_agent") @mock.patch("src.app.shared.infra.abst_chat.ChatMistralAI") - async def test_create_agent_adds_summarization_middleware( - self, mock_chat_mistral, mock_create_agent + @mock.patch("src.app.shared.infra.abst_chat.get_settings") + @mock.patch("src.app.shared.infra.abst_chat.create_agent") + async def test_create_agent_adds_middleware_in_order( + self, mock_create_agent, mock_get_settings, mock_chat_mistral_ai ): mocked_model = mock.Mock() mocked_model._llm_type = "mistral-chat" # noqa: SLF001 - mock_chat_mistral.return_value = mocked_model + mock_chat_mistral_ai.return_value = mocked_model mock_create_agent.return_value = object() - await self.chat._create_agent(memory=None) + await self.chat._create_agent() + + _, kwargs = mock_create_agent.call_args + middleware = kwargs["middleware"] + self.assertEqual(len(middleware), 3) + + clear_uses, summarization, reinforcement = middleware + + self.assertIsInstance(clear_uses, _PersistClearedToolUses) + edit = clear_uses._edit + self.assertIsInstance(edit, ClearToolUsesEdit) + self.assertEqual(edit.keep, 1) + self.assertEqual(edit.trigger, 0) + self.assertEqual(edit.clear_at_least, 0) + + self.assertIsInstance(summarization, SummarizationMiddleware) + self.assertEqual(summarization.trigger, ("tokens", 32000)) + + self.assertIsInstance(reinforcement, _ReinforceHardConstraints) + + async def test_persist_cleared_tool_uses_evicts_older_results_only(self): + edit = ClearToolUsesEdit( + trigger=0, clear_at_least=0, keep=1, placeholder="[cleared]" + ) + middleware = _PersistClearedToolUses(edit) + + state = { + "messages": [ + HumanMessage(content="q1"), + AIMessage( + content="", + tool_calls=[ + { + "id": "1", + "name": "get_resources_about_sustainability", + "args": {}, + } + ], + ), + ToolMessage( + content="old result", tool_call_id="1", artifact=[{"id": "old"}] + ), + HumanMessage(content="q2"), + AIMessage( + content="", + tool_calls=[ + { + "id": "2", + "name": "get_resources_about_sustainability", + "args": {}, + } + ], + ), + ToolMessage( + content="new result", tool_call_id="2", artifact=[{"id": "new"}] + ), + ] + } + + result = middleware.before_model(state, runtime=mock.Mock()) + new_messages = result["messages"][1:] # [0] is the RemoveMessage marker + + cleared = next( + m for m in new_messages if getattr(m, "tool_call_id", None) == "1" + ) + kept = next(m for m in new_messages if getattr(m, "tool_call_id", None) == "2") + + self.assertEqual(cleared.content, "[cleared]") + self.assertIsNone(cleared.artifact) + self.assertEqual(kept.artifact, [{"id": "new"}]) + + def test_reinforce_hard_constraints_appends_reminder_to_last_human_message(self): + middleware = _ReinforceHardConstraints() + + result = middleware._with_reminder([HumanMessage(content="hello")]) + + self.assertTrue(result[-1].content.startswith("hello")) + self.assertIn(prompts.AGENT_REMINDER_PROMPT, result[-1].content) + + def test_reinforce_hard_constraints_leaves_non_human_last_message_alone(self): + middleware = _ReinforceHardConstraints() + messages = [HumanMessage(content="hello"), AIMessage(content="hi")] + + result = middleware._with_reminder(messages) - middleware = mock_create_agent.call_args.kwargs["middleware"] - assert len(middleware) == 1 - assert middleware[0].__class__.__name__ == "SummarizationMiddleware" + self.assertEqual(result[-1].content, "hi") diff --git a/src/app/tests/services/test_data_collection.py b/src/app/tests/services/test_data_collection.py index 1f0b51a..5b0d8a2 100644 --- a/src/app/tests/services/test_data_collection.py +++ b/src/app/tests/services/test_data_collection.py @@ -49,7 +49,6 @@ def test_non_workshop_origin(self, mock_get_campaign): class TestRegisterChatData(unittest.IsolatedAsyncioTestCase): - def setUp(self): _cache["is_campaign_active"] = True _cache["expires"] = None @@ -135,7 +134,6 @@ async def test_register_chat_data_user_not_found(self, mock_campaign, _, __): class TestRegisterDocumentClick(unittest.IsolatedAsyncioTestCase): - @patch( "src.app.services.data_collection.run_in_threadpool", side_effect=fake_run_in_threadpool, @@ -156,7 +154,6 @@ async def test_register_document_click(self, mock_campaign, mock_update, _): class TestRegisterDownloadSyllabus(unittest.IsolatedAsyncioTestCase): - @patch( "src.app.services.data_collection.run_in_threadpool", side_effect=fake_run_in_threadpool, @@ -189,7 +186,6 @@ async def test_register_syllabus_download( class TestRegisterSyllabus(unittest.IsolatedAsyncioTestCase): - @patch( "src.app.services.data_collection.run_in_threadpool", side_effect=fake_run_in_threadpool, diff --git a/src/app/tests/services/test_helpers.py b/src/app/tests/services/test_helpers.py index ce414f0..d629028 100644 --- a/src/app/tests/services/test_helpers.py +++ b/src/app/tests/services/test_helpers.py @@ -3,6 +3,7 @@ from unittest import TestCase, mock import numpy +from langchain_core.messages import AIMessage, HumanMessage, ToolMessage from langdetect.language import Language from src.app.bibliography.helpers.helpers import ( @@ -17,6 +18,7 @@ convert_embedding_bytes, detect_language_from_entry, extract_json_from_response, + latest_tool_docs, linkify_missing_citations, stringify_docs_content, ) @@ -113,13 +115,22 @@ def _make_doc(self, url: str) -> Document: def test_linkify_missing_citations_wraps_bare_marker(self): docs = [self._make_doc("https://example.org/1")] text = "Sustainability matters [Doc 1]." - expected = ( - 'Sustainability matters [Doc 1].' - ) + expected = "Sustainability matters [[Doc 1]](https://example.org/1)." self.assertEqual(linkify_missing_citations(text, docs), expected) - def test_linkify_missing_citations_leaves_existing_link_untouched(self): + def test_linkify_missing_citations_leaves_existing_double_bracket_link_untouched( + self, + ): + docs = [self._make_doc("https://example.org/1")] + text = "Already linked [[Doc 1]](https://example.org/1)." + self.assertEqual(linkify_missing_citations(text, docs), text) + + def test_linkify_missing_citations_leaves_existing_markdown_link_untouched(self): + docs = [self._make_doc("https://example.org/1")] + text = "Already linked [Doc 1](https://example.org/1)." + self.assertEqual(linkify_missing_citations(text, docs), text) + + def test_linkify_missing_citations_leaves_existing_html_link_untouched(self): docs = [self._make_doc("https://example.org/1")] text = 'Already linked [Doc 1].' self.assertEqual(linkify_missing_citations(text, docs), text) @@ -129,13 +140,10 @@ def test_linkify_missing_citations_mixed_bare_and_linked(self): self._make_doc("https://example.org/1"), self._make_doc("https://example.org/2"), ] - text = ( - 'See [Doc 1] ' - "and also [Doc 2]." - ) + text = "See [[Doc 1]](https://example.org/1) and also [Doc 2]." expected = ( - 'See [Doc 1] ' - 'and also [Doc 2].' + "See [[Doc 1]](https://example.org/1) " + "and also [[Doc 2]](https://example.org/2)." ) self.assertEqual(linkify_missing_citations(text, docs), expected) @@ -144,6 +152,15 @@ def test_linkify_missing_citations_out_of_range_untouched(self): text = "See [Doc 9] for more." self.assertEqual(linkify_missing_citations(text, docs), text) + def test_linkify_missing_citations_combined_marker_untouched(self): + docs = [ + self._make_doc("https://example.org/1"), + self._make_doc("https://example.org/2"), + self._make_doc("https://example.org/3"), + ] + text = "See [Docs 3 et 5] for more." + self.assertEqual(linkify_missing_citations(text, docs), text) + def test_linkify_missing_citations_empty_docs_or_text(self): docs = [self._make_doc("https://example.org/1")] self.assertEqual(linkify_missing_citations("", docs), "") @@ -152,11 +169,43 @@ def test_linkify_missing_citations_empty_docs_or_text(self): def test_linkify_missing_citations_dict_payload_fallback(self): docs = [{"document_url": "https://example.org/1"}] text = "See [Doc 1] for more." - expected = ( - 'See [Doc 1] for more.' - ) + expected = "See [[Doc 1]](https://example.org/1) for more." self.assertEqual(linkify_missing_citations(text, docs), expected) + def test_latest_tool_docs_returns_most_recent_call_only(self): + earlier_docs = [self._make_doc("https://example.org/earlier")] + latest_docs = [self._make_doc("https://example.org/latest")] + messages = [ + HumanMessage(content="first question"), + AIMessage(content=""), + ToolMessage(content="...", tool_call_id="1", artifact=earlier_docs), + AIMessage(content="answer using earlier docs"), + HumanMessage(content="follow-up question"), + AIMessage(content=""), + ToolMessage(content="...", tool_call_id="2", artifact=latest_docs), + AIMessage(content="answer using latest docs"), + ] + self.assertEqual(latest_tool_docs(messages), latest_docs) + + def test_latest_tool_docs_no_new_call_falls_back_to_last_one(self): + docs = [self._make_doc("https://example.org/only")] + messages = [ + HumanMessage(content="first question"), + AIMessage(content=""), + ToolMessage(content="...", tool_call_id="1", artifact=docs), + AIMessage(content="answer"), + HumanMessage(content="follow-up that needs no new search"), + AIMessage(content="answer reusing the same docs"), + ] + self.assertEqual(latest_tool_docs(messages), docs) + + def test_latest_tool_docs_no_tool_calls_returns_none(self): + messages = [ + HumanMessage(content="hello"), + AIMessage(content="hi there"), + ] + self.assertIsNone(latest_tool_docs(messages)) + def test_convert_embedding_bytes(self): x = numpy.random.rand( 5, diff --git a/src/app/tests/services/test_search.py b/src/app/tests/services/test_search.py index 0340ec5..2bd4adb 100644 --- a/src/app/tests/services/test_search.py +++ b/src/app/tests/services/test_search.py @@ -84,7 +84,6 @@ async def test_get_collection_by_language_with_collection(self): self.assertEqual(collection, exp_collection) def test_concatenate_same_doc_id_slices(self): - qdrant_docs: List[ScoredPoint] = [ ScoredPoint( id=1, diff --git a/src/app/tests/services/test_security.py b/src/app/tests/services/test_security.py index 8c59fe8..ec3e094 100644 --- a/src/app/tests/services/test_security.py +++ b/src/app/tests/services/test_security.py @@ -36,7 +36,6 @@ def test_check_api_key_false_when_inactive(self, session_maker_mock): class GetUserTests(unittest.IsolatedAsyncioTestCase): - @mock.patch( "src.app.shared.infra.security.check_api_key_sync", new=mock.MagicMock(return_value=True), diff --git a/src/app/tutor/service/agents.py b/src/app/tutor/service/agents.py index 33ee258..b046733 100644 --- a/src/app/tutor/service/agents.py +++ b/src/app/tutor/service/agents.py @@ -116,6 +116,7 @@ def __init__( async def generate(self, message: MessageWithResources) -> SyllabusResponseAgent: DISCIPLINARY_SKILLS = get_disciplinary_skills() + disciplinary_skills_sentences = "\n\nThe syllabus should also contribute to build the following disciplinary skills:\n-" contents = "summary :".join(message.summary) themes = ",".join([theme["theme"] for theme in message.themes]) prompt = ( @@ -124,7 +125,7 @@ async def generate(self, message: MessageWithResources) -> SyllabusResponseAgent f"The syllabus should be written in lang: {message.lang} the section names must also be written in {message.lang}, this is important \n\nTEXT CONTENTS:\n{contents}\n\n" f"THEMES:\n{themes} \n\nTake into account the users input courses title, level, duration and " f"description: {message.course_title}, {message.level}, {message.duration}, {message.description}." - f"{('\n\nThe syllabus should also contribute to build the following disciplinary skills:'+'\n- '.join(DISCIPLINARY_SKILLS[message.discipline])) if message.discipline in DISCIPLINARY_SKILLS.keys() else ''}" + f"{(disciplinary_skills_sentences.join(DISCIPLINARY_SKILLS[message.discipline])) if message.discipline in DISCIPLINARY_SKILLS.keys() else ''}" ) response = await self.run(prompt) return SyllabusResponseAgent(content=response, source=self.name)