From ed6de94c729563e32a47ba97d71cb42d01e4b269 Mon Sep 17 00:00:00 2001 From: Daniil Perkin Date: Fri, 25 Sep 2026 17:21:32 +0200 Subject: [PATCH] Keep Buddy's action and mentor requests out of the FAQ Every question in a project-scoped Buddy conversation will feed the FAQ once the chat is retired (Wiki#319, backend#214), in both capability modes, and only the AI classifier filters. Chat only ever received documentation questions, so both FAQ prompts described their input as questions "asked to a docs chatbot" and discarded nothing but greetings, smalltalk and thanks. Buddy is also a mentor: it is asked to act ("move my card to done", "remind my PM about the review"), to report on the asker ("what should I work on next?", "summarise my onboarding path") and to continue a conversation ("and the second one?"). Under the old rules each of those would have become an FAQ entry for the PM. Both prompts now discard four kinds of text: smalltalk (as before), requests to do something, questions about the asker's own onboarding state, and follow-ups that only make sense inside their conversation. The rule closes with the test that keeps a personal phrasing from being mistaken for a personal question: it stays a documentation question when the same documentation would answer anyone who asks it, so "how do I get VPN access?" still counts even though it says "I". The categories live in one constant, insights.faq.NON_FAQ_KINDS, that the live classifier (classify) and the full rebuild (group) both render. Two hand-kept copies would drift, and a rebuild that disagreed with the classifier would resurrect entries the live path had dropped, or drop entries it had kept, the next time a PM pressed refresh. "Documentation chatbot" / "docs chatbot" become "the project's assistant" in both prompts. The FaqClassifyRequest.question and FaqClassifyResponse.relevant descriptions, the classify route description, the module docstrings and docs/faq-grouping-concept.md say the same. No schema change: the backend keeps sending question text only, and strips quoted selections before sending (backend#214). Tests pin what a stub can prove. With a scripted model the verdict is whatever the script says, so asserting relevant=false for "move my card" would only test parsing. Instead the tests capture the system prompt each entry point actually sends and assert every category and example is in it, that the chatbot wording is gone, and that the classify prompt contains the shared constant verbatim. The parsing side is covered too: an irrelevant verdict yields no group, no documents and no second redaction call, and group drops discarded Buddy requests while keeping the mentor-toned documentation question. The capturing stubs override generate with its real signature, so they need no type: ignore. Verification: ruff format --check, ruff check and pyright src/ (0 errors) clean; pytest 980 passed, 8 skipped (963 before, +17 new). Reverting the two insights modules fails the new tests at collection. Closes #209 --- docs/faq-grouping-concept.md | 6 ++ src/api/routes/insights.py | 5 +- src/api/schemas.py | 9 ++- src/insights/faq.py | 53 ++++++++++---- src/insights/faq_classification.py | 24 ++++--- tests/insights/test_faq.py | 44 +++++++++++- tests/insights/test_faq_classification.py | 88 ++++++++++++++++++++++- 7 files changed, 199 insertions(+), 30 deletions(-) diff --git a/docs/faq-grouping-concept.md b/docs/faq-grouping-concept.md index 12556a0..9ef2cdc 100644 --- a/docs/faq-grouping-concept.md +++ b/docs/faq-grouping-concept.md @@ -73,6 +73,12 @@ Supporting decisions: - **Classification is idempotent** via the source message id. Events can be redelivered, and counting a message twice would quietly corrupt the frequency ranking the whole panel is ordered by. +- **The classifier, not the caller, decides what is an FAQ question.** Questions to Buddy (the + project's assistant, Wiki#319) include requests to act ("move my card to done"), questions about + the asker's own onboarding ("what should I work on next?") and context-only follow-ups ("and the + second one?") next to documentation questions. All of them are sent; `relevant = false` drops + them, as it drops smalltalk. Classify and the full rebuild share one list of these categories + (`insights.faq.NON_FAQ_KINDS`), so a rebuild keeps out exactly what the live path kept out. - **Retrieval only runs for a new entry.** An existing one already carries the documents that answer it. - **Redaction is folded into the same call**, rather than a second round-trip per message. diff --git a/src/api/routes/insights.py b/src/api/routes/insights.py index a73c243..093dd45 100644 --- a/src/api/routes/insights.py +++ b/src/api/routes/insights.py @@ -101,8 +101,9 @@ def group_faq_questions( "whether it is a real documentation question at all, and whether it " "joins an existing entry or opens a new one with its own title. The " "returned question text is PII-redacted. Called by the backend on " - "every chat question, which is why the prompt is bounded by a " - "candidate list of entries instead of the full question history." + "every question asked the project's assistant, which is why the prompt " + "is bounded by a candidate list of entries instead of the full question " + "history." ), ) def classify_faq_question( diff --git a/src/api/schemas.py b/src/api/schemas.py index ec2c4f6..7dd77ea 100644 --- a/src/api/schemas.py +++ b/src/api/schemas.py @@ -1560,7 +1560,9 @@ class FaqGroupRefSchema(BaseModel): class FaqClassifyRequest(ProjectScopedRequest): - question: str = Field(description="The question a user just asked in the chat.") + question: str = Field( + description="The question a user just asked the project's assistant." + ) groups: list[FaqGroupRefSchema] = Field( default_factory=list[FaqGroupRefSchema], description=( @@ -1595,7 +1597,10 @@ class FaqClassifyResponse(BaseModel): relevant: bool = Field( description=( - "False for greetings, smalltalk and other non-questions. The " + "False for anything that is not a documentation question: " + "greetings and smalltalk, requests for the assistant to act, " + "questions about the asker's own onboarding state, and follow-ups " + "that only make sense inside their conversation. The " "backend drops those instead of surfacing them as an FAQ." ) ) diff --git a/src/insights/faq.py b/src/insights/faq.py index da3ce1e..92741e7 100644 --- a/src/insights/faq.py +++ b/src/insights/faq.py @@ -1,9 +1,9 @@ """Semantic grouping of recurring user questions into FAQ clusters. Called by the backend's ``POST /insights/faq/refresh`` (pull-based, per -issue #66). ``/chat`` is stateless and this service does not retain question -history itself, so the backend sends the full set of questions to group on -every request. +issue #66). The project's assistant is stateless and this service does not +retain question history itself, so the backend sends the full set of questions +to group on every request. A *group* is one recurring question — the same thing asked in different words. Each carries a short generated **title** naming what is being asked (issue @@ -14,7 +14,7 @@ This full rebuild is the expensive path: its cost grows with the total number of questions, so it is the manual-refresh fallback rather than something to run -per chat message. The incremental counterpart lives in +per question asked. The incremental counterpart lives in :mod:`insights.faq_classification`. """ @@ -105,15 +105,42 @@ class _GroupPayload(BaseModel): "apart from a neighbouring one — 'Setup' or 'Access' alone is too generic" ) +# What is *not* an FAQ question, shared word for word by the live classifier +# (`insights.faq_classification`) and the full rebuild below, so a rebuild never +# resurrects what the classifier dropped, or the other way round. +# +# The questions come from the project's assistant (Buddy), which is a mentor +# as well as a documentation search: besides "how does X work" it is asked to +# act ("move my card"), to report on the asker ("what should I work on next?") +# and to continue a conversation ("and the second one?"). None of those is +# answered by documentation, and each would otherwise surface to the PM as a +# recurring question. The closing sentence keeps a personal *phrasing* from +# being mistaken for a personal *question*: "how do I get VPN access?" says "I" +# and is still the same question for every hire. +NON_FAQ_KINDS = ( + "(a) greetings, smalltalk, thanks or chit-chat (e.g. 'hey', 'hey there, " + "how you doing', 'thanks!'); " + "(b) requests for the assistant to do something rather than explain " + "something — board or task actions, reminders, messages to someone (e.g. " + "'move my card to done', 'remind my PM about the review'); " + "(c) questions about the asker's own onboarding state — their tasks, " + "progress or path (e.g. 'what should I work on next?', 'summarise my " + "onboarding path'); " + "(d) follow-ups that only make sense inside the conversation they were " + "asked in (e.g. 'and the second one?', 'can you explain that again?'). " + "A question stays a documentation question when the same documentation " + "would answer anyone who asks it, even when it is phrased personally " + "(e.g. 'how do I get VPN access?')" +) + _GROUPING_SYSTEM = ( - "You group recurring end-user questions asked to a docs chatbot into FAQ " - "entries for a PM-facing dashboard. Each input question is prefixed with " - "its id in square brackets.\n\n" + "You group recurring end-user questions asked to the project's assistant " + "into FAQ entries for a PM-facing dashboard. Each input question is " + "prefixed with its id in square brackets.\n\n" "Rules:\n" - "1. First set aside anything that is not a genuine, documentation-relevant " - "question — greetings, smalltalk, or chit-chat (e.g. 'hey', 'hey there, " - "how you doing', 'thanks!'). List their ids in discard_ids instead of a " - "group.\n" + "1. First set aside anything that is not a genuine documentation question " + "and list its ids in discard_ids instead of a group. That covers: " + f"{NON_FAQ_KINDS}.\n" "2. Group the remaining questions by what they are actually asking, not by " "surface sentence structure. Two questions belong together only if the " "same piece of documentation would answer both. Questions that name " @@ -149,8 +176,8 @@ def _cluster_questions( embedding-threshold clustering: it judges cluster membership by meaning (e.g. treating a named component like "frontend" vs "backend" as distinguishing) rather than a fixed cosine cutoff, isn't sensitive to - input order, and filters out non-questions (greetings/smalltalk) before - they can be surfaced as a group. + input order, and filters out what is not a documentation question + (see ``NON_FAQ_KINDS``) before it can be surfaced as a group. """ by_id = {q.id: q for q in questions if q.text.strip()} if not by_id: diff --git a/src/insights/faq_classification.py b/src/insights/faq_classification.py index 72c8754..a2190bc 100644 --- a/src/insights/faq_classification.py +++ b/src/insights/faq_classification.py @@ -3,8 +3,8 @@ The full rebuild in :mod:`insights.faq` costs one LLM pass over *every* question a project ever asked. That is fine as a manual fallback and impossible as a per-message operation, which is what issues #284/#285 need: the FAQ should -be current the moment someone asks a question in the chat, without a PM -pressing refresh. +be current the moment someone asks the project's assistant a question, without +a PM pressing refresh. This module is the cheap online counterpart. Two operations, each with a prompt whose size is bounded by the *structure* of the FAQ rather than by its history: @@ -34,7 +34,7 @@ from pydantic import BaseModel, ValidationError, model_validator from ingestion.metadata_store import IngestionMetadataStore -from insights.faq import TITLE_RULE, FaqDocument, documents_for +from insights.faq import NON_FAQ_KINDS, TITLE_RULE, FaqDocument, documents_for from insights.redaction import redact_pii, redact_structured from llm.base import LLMClient, Message from llm.parsing import extract_json_object @@ -63,9 +63,10 @@ class ExistingGroup: class Classification: """What to do with one incoming question. - ``relevant`` false means the text was smalltalk or otherwise not a - documentation question; the caller drops it and nothing else in this - object is meaningful. + ``relevant`` false means the text was not a documentation question — + smalltalk, a request to act, a question about the asker's own onboarding, + or a context-only follow-up (see ``insights.faq.NON_FAQ_KINDS``); the + caller drops it and nothing else in this object is meaningful. ``group_id`` set means "add to that existing entry"; ``None`` means "open a new one", in which case ``title`` names it and ``documents`` holds the @@ -119,14 +120,15 @@ class _MergePayload(BaseModel): _CLASSIFY_SYSTEM = ( - "You maintain the FAQ of a documentation chatbot for a PM-facing " - "dashboard. A user just asked one question. Decide where it belongs.\n\n" + "You maintain the FAQ of the project's assistant for a PM-facing " + "dashboard. A user just asked the assistant one question. Decide where it " + "belongs.\n\n" "You are given the FAQ's existing entries, each one a recurring question " "listed with its id, its title and the wording it is usually asked in.\n\n" "Rules:\n" - "1. If the text is not a genuine, documentation-relevant question — a " - "greeting, smalltalk, thanks, or chit-chat — set relevant to false and " - "stop. Everything else is then ignored.\n" + "1. If the text is not a genuine documentation question, set relevant to " + "false and stop. Everything else is then ignored. That covers: " + f"{NON_FAQ_KINDS}.\n" "2. Match the question to an existing entry ONLY if the same piece of " "documentation would answer both — i.e. it is the same request in " "different words. Questions naming different components, services, or " diff --git a/tests/insights/test_faq.py b/tests/insights/test_faq.py index ab438a6..4842a87 100644 --- a/tests/insights/test_faq.py +++ b/tests/insights/test_faq.py @@ -1,8 +1,10 @@ import json from collections.abc import Callable +from typing import cast from ingestion.metadata_store import ArtifactRecord, IngestionMetadataStore -from insights.faq import FaqDocument, FaqQuestionInput, group_faqs +from insights.faq import NON_FAQ_KINDS, FaqDocument, FaqQuestionInput, group_faqs +from llm.base import Message from rag.types import Chunk from tests.stubs.llm import StubLLMClient from tests.stubs.store import StubVectorStore @@ -420,3 +422,43 @@ def generate(self, messages: list[dict[str, object]]) -> str: # type: ignore[ov assert len(groups) == 1 assert groups[0].question == "Ask [NAME] for VPN access" assert [q.text for q in groups[0].questions] == ["Ask [NAME] for VPN access"] + + +def test_grouping_prompt_discards_every_kind_of_buddy_non_question() -> None: + """The rebuild names the same categories as the live classifier.""" + prompts: list[str] = [] + + class _CapturingLLM(_ScriptedFaqLLM): + def generate( + self, messages: list[Message], *, temperature: float | None = None + ) -> str: + if not prompts: + prompts.append(str(messages[0].get("content") or "")) + return super().generate(cast("list[dict[str, object]]", messages)) + + llm = _CapturingLLM(groups=[["q1"]]) + questions = [FaqQuestionInput(id="q1", text="How do I get VPN access?")] + + group_faqs(questions, llm, StubVectorStore(), _metadata_store(), _PROJECT) + + assert NON_FAQ_KINDS in prompts[0] + assert "discard_ids" in prompts[0] + assert "chatbot" not in prompts[0] + assert "the project's assistant" in prompts[0] + + +def test_group_faqs_drops_buddy_requests_and_keeps_mentor_toned_doc_questions() -> None: + llm = _ScriptedFaqLLM(groups=[["q1"]], discard_ids=["q2", "q3", "q4"]) + questions = [ + FaqQuestionInput(id="q1", text="How do I get VPN access?"), + FaqQuestionInput(id="q2", text="move my card to done"), + FaqQuestionInput(id="q3", text="what should I work on next?"), + FaqQuestionInput(id="q4", text="and the second one?"), + ] + + groups = group_faqs( + questions, llm, StubVectorStore(), _metadata_store(), project_id=_PROJECT + ) + + assert [g.question_ids for g in groups] == [["q1"]] + assert groups[0].question == "How do I get VPN access?" diff --git a/tests/insights/test_faq_classification.py b/tests/insights/test_faq_classification.py index 6b21cb1..2f0c79b 100644 --- a/tests/insights/test_faq_classification.py +++ b/tests/insights/test_faq_classification.py @@ -1,12 +1,16 @@ import json +from typing import cast + +import pytest from ingestion.metadata_store import IngestionMetadataStore -from insights.faq import FaqDocument +from insights.faq import NON_FAQ_KINDS, FaqDocument from insights.faq_classification import ( ExistingGroup, classify_question, merge_groups, ) +from llm.base import Message from rag.types import Chunk from tests.stubs.llm import StubLLMClient from tests.stubs.store import StubVectorStore @@ -516,3 +520,85 @@ def test_merge_groups_leaves_groups_alone_on_unparseable_output() -> None: llm = _ScriptedLLM("not json") assert merge_groups(_groups("g1", "g2", "g3"), target_max=2, llm=llm) == [] + + +class _PromptCapturingLLM(_ScriptedLLM): + """Records the system prompt of every `generate` call.""" + + def __init__(self, payload: object) -> None: + super().__init__(payload) + self.system_prompts: list[str] = [] + + def generate( + self, messages: list[Message], *, temperature: float | None = None + ) -> str: + self.system_prompts.append(str(messages[0].get("content") or "")) + return super().generate(cast("list[dict[str, object]]", messages)) + + +def _classify_prompt() -> str: + llm = _PromptCapturingLLM({"relevant": False}) + _classify(llm, question="move my card to done") + return llm.system_prompts[0] + + +@pytest.mark.parametrize( + "rule", + [ + # Each Buddy-only kind of non-question the classifier has to drop. + "requests for the assistant to do something", + "'move my card to done'", + "'remind my PM about the review'", + "questions about the asker's own onboarding state", + "'what should I work on next?'", + "'summarise my onboarding path'", + "follow-ups that only make sense inside the conversation", + "'and the second one?'", + # Smalltalk stays covered. + "greetings, smalltalk, thanks or chit-chat", + ], +) +def test_classify_prompt_names_every_kind_of_non_question(rule: str) -> None: + assert rule in _classify_prompt() + + +def test_classify_prompt_keeps_personally_phrased_doc_questions() -> None: + """'How do I get VPN access?' says "I" and is still everyone's question.""" + prompt = _classify_prompt() + + assert "same documentation would answer anyone who asks it" in prompt + assert "'how do I get VPN access?'" in prompt + + +def test_classify_prompt_no_longer_describes_a_docs_chatbot() -> None: + prompt = _classify_prompt() + + assert "chatbot" not in prompt + assert "the project's assistant" in prompt + + +def test_classify_and_rebuild_share_one_list_of_non_questions() -> None: + """A rebuild must keep out exactly what the live classifier kept out.""" + assert NON_FAQ_KINDS in _classify_prompt() + + +@pytest.mark.parametrize( + "question", + [ + "move my card to done", + "what should I work on next?", + "and the second one?", + ], +) +def test_classify_drops_a_buddy_request_the_model_marks_irrelevant( + question: str, +) -> None: + """The model's verdict is final: no title, no documents, no redaction.""" + llm = _ScriptedLLM({"relevant": False}) + + result = _classify(llm, question=question) + + assert not result.relevant + assert result.group_id is None + assert result.documents == [] + assert llm.calls == 1