diff --git a/apps/ai-service/adapters/postgres.py b/apps/ai-service/adapters/postgres.py index 43525b9..efe267f 100644 --- a/apps/ai-service/adapters/postgres.py +++ b/apps/ai-service/adapters/postgres.py @@ -20,6 +20,7 @@ class RetrievalTrace: citations: tuple[dict[str, Any], ...] correlation_id: str | None = None otel_trace_id: str | None = None + conversation_id: str | None = None created_at: datetime | None = None @@ -63,6 +64,7 @@ class PostgresTraceRepository: citations: tuple[dict[str, Any], ...], correlation_id: str | None = None, otel_trace_id: str | None = None, + conversation_id: str | None = None, ) -> str: import psycopg @@ -73,13 +75,13 @@ class PostgresTraceRepository: INSERT INTO rag_retrieval_trace ( trace_id, query_text, subject_scope, query_intent, decision, reason, resolved_drug_id, citations, - correlation_id, otel_trace_id - ) VALUES (%s, %s, %s, %s, %s, %s, %s, %s::jsonb, %s, %s) + correlation_id, otel_trace_id, conversation_id + ) VALUES (%s, %s, %s, %s, %s, %s, %s, %s::jsonb, %s, %s, %s) """, ( trace_id, query, subject_scope, intent, decision, reason, resolved_drug_id, json.dumps(citations, ensure_ascii=False), - correlation_id, otel_trace_id, + correlation_id, otel_trace_id, conversation_id, ), ) return trace_id @@ -92,7 +94,7 @@ class PostgresTraceRepository: """ SELECT trace_id::text, query_text, subject_scope, query_intent, decision, reason, resolved_drug_id, citations, - correlation_id, otel_trace_id, created_at + correlation_id, otel_trace_id, conversation_id, created_at FROM rag_retrieval_trace WHERE trace_id = %s """, (trace_id,), @@ -103,9 +105,46 @@ class PostgresTraceRepository: trace_id=row[0], query=row[1], subject_scope=row[2], intent=row[3], decision=row[4], reason=row[5], resolved_drug_id=row[6], citations=tuple(row[7]), correlation_id=row[8], otel_trace_id=row[9], - created_at=row[10], + conversation_id=row[10], created_at=row[11], ) + def list_by_conversation( + self, conversation_id: str, limit: int + ) -> list[RetrievalTrace]: + """Past queries for one session — Feature-List #25 (lịch sử tra + cứu), most recent first. Scoped to `conversation_id` on purpose: + this system has no auth anywhere (`apps/api-gateway`/`auth-service` + are unbuilt — see README), so an unscoped listing would mix every + browser's/user's queries together. Citations/answer text are NOT + persisted here (only decision/reason/resolved_drug_id) — a history + entry is for re-running the same query, not replaying its old + answer verbatim. + """ + import psycopg + + with psycopg.connect(self._dsn, connect_timeout=5) as connection: + rows = connection.execute( + """ + SELECT trace_id::text, query_text, subject_scope, query_intent, + decision, reason, resolved_drug_id, citations, + correlation_id, otel_trace_id, conversation_id, created_at + FROM rag_retrieval_trace + WHERE conversation_id = %s + ORDER BY created_at DESC + LIMIT %s + """, + (conversation_id, limit), + ).fetchall() + return [ + RetrievalTrace( + trace_id=row[0], query=row[1], subject_scope=row[2], intent=row[3], + decision=row[4], reason=row[5], resolved_drug_id=row[6], + citations=tuple(row[7]), correlation_id=row[8], otel_trace_id=row[9], + conversation_id=row[10], created_at=row[11], + ) + for row in rows + ] + def save_feedback( self, *, diff --git a/apps/ai-service/adapters/qdrant.py b/apps/ai-service/adapters/qdrant.py index d6831e8..878937a 100644 --- a/apps/ai-service/adapters/qdrant.py +++ b/apps/ai-service/adapters/qdrant.py @@ -359,6 +359,52 @@ class QdrantRetriever: hits.append(SearchHit(_document(payload), 1.0)) return hits + def list_sections(self, drug_id: str) -> list[tuple[str, str]]: + """Every section this drug has ANY content for — prose or + quarantined — as `(section_key, section_display_name)` pairs, in + book order (`rag.sections.SECTION_ORDER`). Feature-List #4: the UI + needs the real per-drug checklist, not a generic 19-item list, since + coverage genuinely varies (confirmed corpus-wide: 7 to 19 sections + per drug). + + Deliberately NOT `find_by_drug`'s `chunk_kind == "prose"` filter — a + section that exists ONLY as a quarantined table (no prose chunk at + all) is still a real section of this monograph; the caller decides + how to present a request for it (`find_by_section` already handles + the quarantine notice). A projection scroll: only the two payload + fields this needs, never `text` — the checklist has no reason to + pull every chunk's full content over the wire. + """ + from qdrant_client.models import FieldCondition, Filter, MatchValue + + from rag.sections import SECTION_ORDER + + scroll_filter = Filter( + must=[FieldCondition(key="drug_id", match=MatchValue(value=drug_id))] + ) + found: dict[str, str] = {} + offset = None + while True: + points, offset = self._client.scroll( + collection_name=self._collection_name, + scroll_filter=scroll_filter, + limit=256, + offset=offset, + with_payload=["section_key", "section_display_name"], + ) + for point in points: + payload = dict(point.payload or {}) + key = payload.get("section_key") + if key and key not in found: + found[key] = payload.get("section_display_name") or key + if offset is None: + break + + order = {key: index for index, key in enumerate(SECTION_ORDER)} + return sorted( + found.items(), key=lambda item: order.get(item[0], len(order)) + ) + def find_by_indication(self, indication_text: str, limit: int) -> list[SearchHit]: """Reverse lookup: every drug whose `chi_dinh` text mentions the given symptom/indication, keyword-matched. Deterministic, no fabrication diff --git a/apps/ai-service/bootstrap.py b/apps/ai-service/bootstrap.py index 75e6082..e078a75 100644 --- a/apps/ai-service/bootstrap.py +++ b/apps/ai-service/bootstrap.py @@ -137,7 +137,7 @@ def build_runtime(settings: Settings): effective_metrics = metrics or NullMetrics() configure_telemetry(settings, effective_metrics) if settings.embedding_provider == "disabled": - return None, None, PostgresTraceRepository(settings.postgres_dsn), metrics + return None, None, PostgresTraceRepository(settings.postgres_dsn), metrics, None if settings.embedding_provider != "cohere-v4": raise ValueError( "No production query embedder is configured. Supported values: " @@ -169,8 +169,13 @@ def build_runtime(settings: Settings): # drug identity through `LlmQueryUnderstander` against the same catalog. resolver = CatalogDrugResolver(aliases) reranker = _build_reranker(settings) + # Returned on its own below (Feature-List #4/#23's section-list and + # verbatim-section-text endpoints) — both are plain payload-filtered + # Qdrant reads with no LLM/generation involved, so they read straight + # from this adapter rather than through `RetrievalService`/`RagAgent`. + qdrant_retriever = QdrantRetriever(client, settings.qdrant_collection, embedder) retrieval = InstrumentedRetrievalService( - QdrantRetriever(client, settings.qdrant_collection, embedder), + qdrant_retriever, QdrantParentStore(client, settings.qdrant_collection), EvidencePolicy(minimum_score=settings.evidence_minimum_score), section_resolver=section_resolver, @@ -195,7 +200,7 @@ def build_runtime(settings: Settings): # understanding either, so there is no conversational/agent # capability to offer. Answer-only (retrieval-verified, no # generation) still works through `answers` directly. - return answers, None, trace_writer, metrics + return answers, None, trace_writer, metrics, qdrant_retriever agent = InstrumentedRagAgent( understander=InstrumentedQueryUnderstander( LlmQueryUnderstander(generator, _catalog_names(aliases), resolver) @@ -208,4 +213,4 @@ def build_runtime(settings: Settings): store=PostgresConversationStore(settings.postgres_dsn), metrics=effective_metrics, ) - return answers, agent, trace_writer, metrics + return answers, agent, trace_writer, metrics, qdrant_retriever diff --git a/apps/ai-service/main.py b/apps/ai-service/main.py index 18ededa..996c307 100644 --- a/apps/ai-service/main.py +++ b/apps/ai-service/main.py @@ -28,6 +28,7 @@ def create_app( conversational: Any | None = None, trace_writer: PostgresTraceRepository | None = None, metrics: Any | None = None, + section_retriever: Any | None = None, ) -> FastAPI: configured = settings or get_settings() effective_metrics = metrics or NullMetrics() @@ -37,6 +38,12 @@ def create_app( app.state.conversational = conversational app.state.trace_writer = trace_writer app.state.metrics = metrics + # The raw Qdrant retriever, not routed through RagAgent — Feature-List + # #4/#23's section-list and verbatim-section-text endpoints are plain + # payload-filtered reads with no LLM/generation step, so nothing about + # them belongs on the answer/agent path. None whenever embedding is + # disabled (no Qdrant client exists at all in that mode). + app.state.section_retriever = section_retriever @app.middleware("http") async def correlate_and_trace(request: Request, call_next): @@ -128,11 +135,14 @@ def _route_label(path: str) -> str: _settings = get_settings() -_answer_service, _conversational, _trace_writer, _metrics = build_runtime(_settings) +_answer_service, _conversational, _trace_writer, _metrics, _section_retriever = ( + build_runtime(_settings) +) app = create_app( settings=_settings, answer_service=_answer_service, conversational=_conversational, trace_writer=_trace_writer, metrics=_metrics, + section_retriever=_section_retriever, ) diff --git a/apps/ai-service/migrations/005_rag_trace_conversation.sql b/apps/ai-service/migrations/005_rag_trace_conversation.sql new file mode 100644 index 0000000..870f9e0 --- /dev/null +++ b/apps/ai-service/migrations/005_rag_trace_conversation.sql @@ -0,0 +1,6 @@ +ALTER TABLE rag_retrieval_trace + ADD COLUMN IF NOT EXISTS conversation_id varchar(128); + +CREATE INDEX IF NOT EXISTS rag_retrieval_trace_conversation_idx + ON rag_retrieval_trace (conversation_id, created_at DESC) + WHERE conversation_id IS NOT NULL; diff --git a/apps/ai-service/rag/agent.py b/apps/ai-service/rag/agent.py index 0dbd885..7598c8e 100644 --- a/apps/ai-service/rag/agent.py +++ b/apps/ai-service/rag/agent.py @@ -28,6 +28,7 @@ from .clinical import ConditionRelation, MedicationCandidateAssessment from .models import EvidenceDecision, RetrievalResult from .policy import looks_non_human from .service import RetrievalService +from .text import normalize_name from .understanding import QueryFrame, QueryUnderstander logger = logging.getLogger(__name__) @@ -53,6 +54,27 @@ MAX_LLM_CALLS_PER_TURN = 8 # turns on the same conversation, force a hard stop instead of asking again. MAX_CONSECUTIVE_CLARIFY = 4 +# Feature-List #18/#19: "out_of_scope" used to cover two different things +# with one identical message that named neither — a question about data +# the formulary genuinely never contains for ANY drug (price, vendor, +# brand availability), and a question with nothing to do with drugs at +# all. Found live 2026-08-14 asking both "Paracetamol giá bao nhiêu?" and +# "Thời tiết Hà Nội hôm nay?" and getting the same vague "nằm ngoài phần +# chuyên luận... có thể thuộc phụ lục chưa được đưa vào" — which reads as +# "maybe added later" for data that will never be in a drug formulary. +# Deliberately a keyword heuristic, same trade-off as policy.py's +# looks_non_human: cheap, auditable, no model call on the abstain path. +_OUT_OF_SCOPE_DATA_PHRASES = ( + "gia bao nhieu", "gia ca", "gia tien", "bao nhieu tien", + "mua o dau", "ban o dau", "nha thuoc nao ban", + "thuong hieu nao", "hang san xuat", "nha san xuat", +) + + +def _looks_like_missing_data_question(query: str) -> bool: + normalized = normalize_name(query) + return any(phrase in normalized for phrase in _OUT_OF_SCOPE_DATA_PHRASES) + class ConversationStore(Protocol): """Durable, cross-worker alternative to the in-process history dict — @@ -404,11 +426,23 @@ class RagAgent: turn_type=tt) if tt == "out_of_scope": + if _looks_like_missing_data_question(turn): + return AgentReply( + "abstain", "out_of_scope", + answer="Dược thư Quốc gia Việt Nam KHÔNG chứa giá bán, nơi bán " + "hay thương hiệu/nhà sản xuất cụ thể của thuốc — đây " + "không phải nội dung sách này tra cứu (sách chỉ có chỉ " + "định, liều dùng, chống chỉ định, tương tác thuốc và các " + "mục chuyên môn khác). Vui lòng tra các nguồn khác cho " + "thông tin này.", + turn_type=tt) return AgentReply( "abstain", "out_of_scope", - answer="Nội dung này nằm ngoài phần chuyên luận thuốc của Dược thư " - "(có thể thuộc phần hướng dẫn chung/phụ lục chưa được đưa vào). " - "Tôi chưa có dữ liệu để trả lời chính xác.", + answer="Câu hỏi này nằm ngoài phạm vi hỗ trợ của hệ thống. Hệ " + "thống chỉ tra cứu thông tin thuốc theo Dược thư Quốc gia " + "Việt Nam 2018 — chỉ định, liều dùng, chống chỉ định, tác " + "dụng phụ, tương tác thuốc và các mục chuyên môn khác của " + "một thuốc cụ thể.", turn_type=tt) if not frame.drugs: diff --git a/apps/ai-service/rag/answer.py b/apps/ai-service/rag/answer.py index cb42dc1..5efd99f 100644 --- a/apps/ai-service/rag/answer.py +++ b/apps/ai-service/rag/answer.py @@ -30,7 +30,7 @@ from .routing import QueryRoutingService # be different prompts (or a different judge) to be independent evidence — # see this function's own reasoning above. -_QUICK_REPLY_MAX_ITEMS = 18 # one per monograph section (see rag/sections.py SECTION_ORDER) +_QUICK_REPLY_MAX_ITEMS = 19 # one per monograph section (see rag/sections.py SECTION_ORDER) _QUICK_REPLY_MAX_CHARS = 40 logger = logging.getLogger(__name__) @@ -368,6 +368,39 @@ def _presentation_for_sources(source_ids: tuple[str, ...]) -> tuple[str, str]: return "Trả lời", "fact_list" +# Feature-List #14: a condition/symptom -> drug reverse lookup +# (`list_mode=True`, `RagAgent._condition_to_drug`) states which drugs the +# formulary's `chi_dinh` sections name for this indication — a factual +# lookup, never a treatment ranking (see +# `[[feedback_no_recommendation_gate]]`) — but rendered as a bare drug list +# it reads exactly like a recommendation. Fixed, non-generated block, same +# guarantee as `DISCLAIMER` above: the model never sees or writes this text, +# so it cannot reword, shorten, or drop it. Always the first block; `kind` +# matches `_plan_answer`'s forced `needs_warning=True` for `list_mode`, so +# the UI renders it with the same visual separation as any other warning +# block, not a plain bullet. +_LIST_MODE_NOTICE_BLOCK = AnswerBlock( + title="Đọc cho đúng", + kind="warning", + claims=( + AnswerClaim( + text=( + "Đây là kết quả TRA CỨU các thuốc mà Dược thư Quốc gia Việt " + "Nam ghi nhận chỉ định cho triệu chứng/bệnh này, KHÔNG PHẢI " + "khuyến nghị điều trị hay chỉ định lâm sàng cho một người " + "bệnh cụ thể." + ), + ), + ), +) + + +def _with_list_mode_notice( + blocks: tuple[AnswerBlock, ...], list_mode: bool +) -> tuple[AnswerBlock, ...]: + return (_LIST_MODE_NOTICE_BLOCK, *blocks) if list_mode else blocks + + def _build_blocks( claims: tuple[tuple[str, tuple[int, ...]], ...], indexed: list[tuple[str, Citation]], @@ -437,7 +470,10 @@ def _plan_answer(query: str, result: RetrievalResult, list_mode: bool) -> Answer layout=layout, reasoning_mode="synthesis" if multi_source else "direct_lookup", show_heading=multi_source or verbosity == "detailed", - needs_warning=any(section in _WARNING_SECTIONS for section in sections), + # `list_mode` always carries `_LIST_MODE_NOTICE_BLOCK` (see below) — + # this is what gives that block its actual warning presentation in + # the UI, not just a plain untitled bullet. + needs_warning=list_mode or any(section in _WARNING_SECTIONS for section in sections), ) @@ -677,7 +713,7 @@ class GroundedAnswerService: (text, (index,)) for index, text in enumerate(evidence_texts, start=1) ) - blocks = _build_blocks(claims, indexed) + blocks = _with_list_mode_notice(_build_blocks(claims, indexed), list_mode) return GroundedAnswer( result, "\n".join(text for text, _ in claims), @@ -724,7 +760,7 @@ class GroundedAnswerService: if 1 <= index <= len(indexed) )) citations = tuple(indexed[index - 1][1] for index in cited_indices) - blocks = _build_blocks(outcome.claims, indexed) + blocks = _with_list_mode_notice(_build_blocks(outcome.claims, indexed), list_mode) clean_answer = "\n".join(text for text, _ in outcome.claims) self._metrics.increment(metric_names.GENERATION_SERVED) return GroundedAnswer( diff --git a/apps/ai-service/rag/ports.py b/apps/ai-service/rag/ports.py index fbb3465..3e20fb2 100644 --- a/apps/ai-service/rag/ports.py +++ b/apps/ai-service/rag/ports.py @@ -76,3 +76,12 @@ class SectionRetriever(Protocol): class ParentStore(Protocol): def get(self, parent_id: str) -> ParentDocument | None: ... + + +class SectionListRetriever(Protocol): + """The per-drug section checklist Feature-List #4 needs, cheap because + `drug_id`/`section_key` are already-indexed Qdrant payload fields — no + new indexing. Separate from `SectionRetriever` (interface segregation): + this returns labels only, never chunk text.""" + + def list_sections(self, drug_id: str) -> list[tuple[str, str]]: ... diff --git a/apps/ai-service/rag/sections.py b/apps/ai-service/rag/sections.py index 143de44..60c4fe8 100644 --- a/apps/ai-service/rag/sections.py +++ b/apps/ai-service/rag/sections.py @@ -147,12 +147,27 @@ SECTION_PHRASES: dict[str, tuple[str, ...]] = { } -# Book order of monograph sections (Hướng dẫn sử dụng, printed page 39). Used to -# present a whole-drug overview when the query names the drug but no attribute — -# typing "PARACETAMOL" should return the monograph, never a "specify an -# attribute" dead-end. +# Book order of monograph sections, verified 2026-08-14 against the actual +# PDF (physical page 38 = printed page 39, "HƯỚNG DẪN SỬ DỤNG DƯỢC THƯ QUỐC +# GIA VIỆT NAM"), not assumed from an earlier reading of this constant. The +# guide numbers 19 items; item 1, "Tên chuyên luận thuốc", is the monograph's +# own title/heading, not a content section with a `section_key` — items 2-19 +# are exactly these 18 keys, in exactly this order. Confirms this tuple was +# already complete and correctly ordered for the book's own stated template. +# +# `ten_thuong_mai` (trade name) is real, present in the corpus (492/684 +# drugs) but is NOT one of the guide's 19 numbered items — the book's own +# template never promises it, so there is no book-verified position to place +# it at. Inserted right after `ten_chung_quoc_te` (generic/INN name) as the +# most natural adjacency (same convention `understanding.py`'s `SECTION_KEYS` +# already uses) — a judgment call, not a sourced fact, unlike the 18 above. +# +# Used to present a whole-drug overview when the query names the drug but no +# attribute — typing "PARACETAMOL" should return the monograph, never a +# "specify an attribute" dead-end. SECTION_ORDER: tuple[str, ...] = ( "ten_chung_quoc_te", + "ten_thuong_mai", "ma_atc", "loai_thuoc", "dang_thuoc_va_ham_luong", @@ -208,3 +223,37 @@ class SectionResolver: if f" {normalized_phrase} " in padded: return SectionMatch(section_key, phrase) return None + + def resolve_all(self, query: str) -> tuple[SectionMatch, ...]: + """Every distinct section a question genuinely names, not just the + first. Same longest-first order as `resolve()`, and the same + span-claiming rule: once a phrase's occurrence is accepted, any + shorter phrase whose only occurrence falls inside that already- + claimed span is a substring of it, not a second section — e.g. + "chỉ định" inside "chống chỉ định của X" must NOT count as a second, + separate mention of `chi_dinh`. A phrase counts only when it has an + occurrence that does not overlap any span already claimed by a + longer, earlier-accepted phrase. Returns `()` for no match and + exactly one item when the question names only one section — this is + a superset of `resolve()`, not a replacement for it. + """ + normalized_query = normalize_name(query) + if not normalized_query: + return () + padded = f" {normalized_query} " + claimed: list[tuple[int, int]] = [] + seen_sections: set[str] = set() + matches: list[SectionMatch] = [] + for normalized_phrase, section_key, phrase in self._index: + needle = f" {normalized_phrase} " + idx = padded.find(needle) + if idx == -1: + continue + span = (idx, idx + len(needle)) + if any(span[0] < c_end and c_start < span[1] for c_start, c_end in claimed): + continue + claimed.append(span) + if section_key not in seen_sections: + seen_sections.add(section_key) + matches.append(SectionMatch(section_key, phrase)) + return tuple(matches) diff --git a/apps/ai-service/rag/understanding.py b/apps/ai-service/rag/understanding.py index a7c15a5..62ec249 100644 --- a/apps/ai-service/rag/understanding.py +++ b/apps/ai-service/rag/understanding.py @@ -118,6 +118,31 @@ SECTION_KEY_HINTS: dict[str, str] = { "thong_tin_quy_che": "thông tin quy chế/pháp lý", } +# Short, chip-sized Vietnamese section names — SECTION_KEY_HINTS above is +# full-sentence disambiguation text for the model, well over +# _QUICK_REPLY_MAX_CHARS, so it cannot double as a quick_reply label. +_SECTION_SHORT_LABEL: dict[str, str] = { + "ten_chung_quoc_te": "Tên chung quốc tế", + "ten_thuong_mai": "Tên thương mại", + "ma_atc": "Mã ATC", + "loai_thuoc": "Loại thuốc", + "dang_thuoc_va_ham_luong": "Dạng thuốc và hàm lượng", + "duoc_ly_va_co_che_tac_dung": "Dược lý, cơ chế tác dụng", + "chi_dinh": "Chỉ định", + "chong_chi_dinh": "Chống chỉ định", + "than_trong": "Thận trọng", + "thoi_ky_mang_thai": "Thời kỳ mang thai", + "thoi_ky_cho_con_bu": "Thời kỳ cho con bú", + "tac_dung_khong_mong_muon": "Tác dụng không mong muốn", + "huong_dan_xu_tri_adr": "Xử trí ADR", + "lieu_luong_va_cach_dung": "Liều lượng và cách dùng", + "tuong_tac_thuoc": "Tương tác thuốc", + "qua_lieu_va_xu_tri": "Quá liều và xử trí", + "do_on_dinh_va_bao_quan": "Độ ổn định, bảo quản", + "tuong_ky": "Tương kỵ", + "thong_tin_quy_che": "Thông tin quy chế", +} + # What kind of turn this is — the router branches on it. Deliberately explicit so a # symptom lookup is never silently treated as a failed drug lookup, and a two-drug # interaction never collapses to an "ambiguous drug" abstain. @@ -294,7 +319,7 @@ _ALLOWED_ROUTES = { "uong", "tiem_tinh_mach", "tiem_bap", "tiem_duoi_da", "dat_truc_trang", "boi_ngoai_da", "nho_mat", "nho_mui", "khac", } -_QUICK_REPLY_MAX_ITEMS = 18 # one per monograph section (see rag/sections.py SECTION_ORDER) +_QUICK_REPLY_MAX_ITEMS = 19 # one per monograph section (see rag/sections.py SECTION_ORDER) _QUICK_REPLY_MAX_CHARS = 40 _SYSTEM = """Bạn là bộ HIỂU CÂU HỎI cho một chatbot tra cứu Dược thư Quốc gia Việt Nam. @@ -577,6 +602,7 @@ class LlmQueryUnderstander: resolved_section_key=(section_match.section_key if section_match else None), resolved_section_phrase=(section_match.phrase if section_match else None), ) + frame = _apply_multi_section_clarify(frame, turn) return _merge_with_prior_frame(frame, prior_frame) @staticmethod @@ -856,6 +882,51 @@ def _apply_named_drug_cues( return frame +def _apply_multi_section_clarify(frame: QueryFrame, turn: str) -> QueryFrame: + """A turn naming two-or-more distinct monograph sections for one drug + ("chỉ định và chống chỉ định của X") cannot be served by narrowing + `attribute` to a single value: both the model's own JSON output and + `_apply_named_drug_cues` above only ever produce one `attribute`, so + retrieval silently fetches evidence for whichever section wins while the + question handed to generation still promises both. The completeness check + then correctly reports a real, quote-backed gap against the one section + actually retrieved, generation retries with the same mismatched scope, + and the turn fails closed as a confusing `incomplete_answer` abstain — + found live 2026-08-14, reproduced 2/2 on "Chỉ định và chống chỉ định của + Aspirin là gì?". Answering every named section at once is a larger, + separate change (multi-attribute retrieval); asking which one first is + the safe interim behavior, using the generic `needs_clarify`+ + `clarify_reason` clarify path `RagAgent` already has (checked ahead of + the narrower `attribute is None` -> `missing_attribute` branch, and the + only one of the two that forwards `quick_replies`). + + Scoped to turns already read as a single named drug's own attribute(s) + — `_apply_named_drug_cues` runs first, so by this point `turn_type` is + already "drug_attribute"/"drug_overview" for exactly the cases this + bug affects; interaction/condition/dosing turns are untouched. + """ + if frame.turn_type not in ("drug_attribute", "drug_overview") or not frame.drugs: + return frame + matches = _SECTION_RESOLVER.resolve_all(turn) + distinct_sections = tuple(dict.fromkeys(match.section_key for match in matches)) + if len(distinct_sections) < 2: + return frame + options = tuple( + _SECTION_SHORT_LABEL.get(key, key) for key in distinct_sections + )[:_QUICK_REPLY_MAX_ITEMS] + return replace( + frame, + turn_type="drug_attribute", + attribute=None, + needs_clarify=True, + clarify_reason=( + "Câu hỏi nêu nhiều mục cùng lúc — anh/chị muốn xem mục nào trước?" + ), + quick_replies=options, + system_error=None, + ) + + _KNOWN_FACT_LABELS: tuple[tuple[str, str], ...] = ( ("population", "Đối tượng"), ("age_text", "Tuổi"), diff --git a/apps/ai-service/routers/rag.py b/apps/ai-service/routers/rag.py index 0531ed9..115ad75 100644 --- a/apps/ai-service/routers/rag.py +++ b/apps/ai-service/routers/rag.py @@ -10,6 +10,7 @@ from rag.answer import DISCLAIMER, GroundedAnswerService from rag.metrics import DECISION, TRACE_WRITE_FAILED, Metrics, NullMetrics from rag.models import QueryIntent, SubjectScope from rag.policy import resolve_subject_scope +from rag.ports import SectionListRetriever, SectionRetriever from rag.telemetry import ( annotate_current_span, current_correlation_id, @@ -24,6 +25,8 @@ class TraceWriter(Protocol): def save_feedback(self, **fields: Any) -> str: ... + def list_by_conversation(self, conversation_id: str, limit: int) -> list[Any]: ... + class RagQueryRequest(BaseModel): query: str = Field(min_length=1, max_length=4000) @@ -145,6 +148,15 @@ def _metrics(request: Request) -> Metrics: return getattr(request.app.state, "metrics", None) or NullMetrics() +def _section_retriever(request: Request) -> SectionListRetriever: + retriever = getattr(request.app.state, "section_retriever", None) + if retriever is None: + raise HTTPException( + status_code=503, detail="Section retrieval backend is not configured" + ) + return retriever + + router = APIRouter(prefix="/v1/rag", tags=["rag"]) @@ -168,6 +180,57 @@ def save_feedback( return RagFeedbackResponse(feedback_id=feedback_id) +class HistoryItem(BaseModel): + trace_id: str + query: str + decision: str + reason: str + resolved_drug_id: str | None = None + created_at: str + + +class HistoryResponse(BaseModel): + items: list[HistoryItem] + + +_HISTORY_LIMIT = 50 + + +@router.get("/history", response_model=HistoryResponse) +def list_history( + conversation_id: str, + traces: Annotated[TraceWriter, Depends(_trace_writer)], +) -> HistoryResponse: + """Feature-List #25: past queries for one session, most recent first, so + the UI can list them and let the user click one to re-run — not to + replay the old answer verbatim, which isn't persisted (see + `PostgresTraceRepository.list_by_conversation`'s docstring). Scoped + strictly to `conversation_id`: this system has no auth anywhere, so an + unscoped listing would leak every session's queries to every caller. + An empty/missing `conversation_id` returns no rows rather than every + trace in the database.""" + trimmed = conversation_id.strip() + if not trimmed: + return HistoryResponse(items=[]) + try: + rows = traces.list_by_conversation(trimmed, limit=_HISTORY_LIMIT) + except Exception as exc: + raise HTTPException(status_code=503, detail="trace_store_unavailable") from exc + return HistoryResponse( + items=[ + HistoryItem( + trace_id=row.trace_id, + query=row.query, + decision=row.decision, + reason=row.reason, + resolved_drug_id=row.resolved_drug_id, + created_at=row.created_at.isoformat() if row.created_at else "", + ) + for row in rows + ] + ) + + class SuggestResponse(BaseModel): suggestions: list[str] @@ -181,6 +244,98 @@ def suggest_drugs(q: str, request: Request) -> SuggestResponse: return SuggestResponse(suggestions=agent.complete(q.strip())) +class SectionListItem(BaseModel): + section_key: str + section_title: str + + +class SectionListResponse(BaseModel): + sections: list[SectionListItem] + + +@router.get("/sections", response_model=SectionListResponse) +def list_drug_sections( + drug_id: str, + retriever: Annotated[SectionListRetriever, Depends(_section_retriever)], +) -> SectionListResponse: + """Feature-List #4: the real per-drug section checklist, not a generic + fixed list — coverage genuinely varies (measured corpus-wide: 7 to 19 + sections per drug). No LLM/generation involved, a plain indexed-payload + read, so an unknown or unresolved `drug_id` returns an empty list rather + than a 404 — the caller (UI attribute picker) already knows which + `drug_id` it resolved before calling this.""" + sections = retriever.list_sections(drug_id.strip()) + return SectionListResponse( + sections=[ + SectionListItem(section_key=key, section_title=title) + for key, title in sections + ] + ) + + +class SectionTextPart(BaseModel): + part_index: int | None = None + text: str + # True for a quarantined table/formula chunk — `text` is then the + # chunker's own descriptor sentence ("bảng, trang N..."), not the + # table's content; never a paraphrase, per the quarantine contract + # (docs-legacy/adr/0006). The caller must not present this the same way + # as a real verbatim excerpt. + is_quarantined: bool + printed_page_start: int | None = None + printed_page_end: int | None = None + physical_page: int | None = None + + +class SectionTextResponse(BaseModel): + drug_id: str + section_key: str + section_title: str | None = None + parts: list[SectionTextPart] + + +def _section_text_part(hit) -> SectionTextPart: + doc = hit.document + ref = doc.source_refs[0] if doc.source_refs else None + printed_range = ref.printed_page_range if ref else None + printed_start = printed_range[0] if printed_range else (ref.printed_page if ref else None) + printed_end = printed_range[1] if printed_range else (ref.printed_page if ref else None) + return SectionTextPart( + part_index=doc.part_index, + text=doc.text, + is_quarantined=doc.requires_visual_check, + printed_page_start=printed_start, + printed_page_end=printed_end, + physical_page=ref.physical_page if ref else None, + ) + + +@router.get("/section-text", response_model=SectionTextResponse) +def get_section_text( + drug_id: str, + section_key: str, + retriever: Annotated[SectionRetriever, Depends(_section_retriever)], +) -> SectionTextResponse: + """Feature-List #23: the verbatim source of one section, on demand — no + LLM/generation/entailment involved, so there is nothing to verify; + `evidence_text` on a `/query` citation is the same underlying text but + only for chunks the model actually cited, never a guaranteed whole + section. `find_by_section` already returns every part in book order + (never truncated), which this just joins into an ordered part list — + curation of WHICH sections a UI offers this for (e.g. a "6 mục an + toàn" default) is a client concern; this endpoint is generic to any + real `section_key`, same as `find_by_section` itself.""" + hits = retriever.find_by_section(drug_id.strip(), section_key.strip()) + parts = [_section_text_part(hit) for hit in hits] + section_title = hits[0].document.section_title if hits else None + return SectionTextResponse( + drug_id=drug_id, + section_key=section_key, + section_title=section_title, + parts=parts, + ) + + def _map_citations(items) -> list[CitationResponse]: return [ CitationResponse( @@ -343,6 +498,7 @@ def query_rag( citations=tuple(item.model_dump() for item in citations), correlation_id=correlation_id, otel_trace_id=otel_trace_id, + conversation_id=payload.conversation_id, ) except Exception: metrics.increment(TRACE_WRITE_FAILED) diff --git a/apps/ai-service/tests/test_agent.py b/apps/ai-service/tests/test_agent.py index 2a226c9..6eec780 100644 --- a/apps/ai-service/tests/test_agent.py +++ b/apps/ai-service/tests/test_agent.py @@ -99,6 +99,27 @@ def test_out_of_scope_turn_type_abstains(): assert reply.reason == "out_of_scope" +def test_out_of_scope_price_question_states_the_book_does_not_have_it(): + """Regression: found live 2026-08-14 that a price question and a + genuinely off-topic question ("thời tiết Hà Nội hôm nay?") got the exact + same vague message, which never actually says Dược thư has no pricing + data at all — it read as "maybe in an appendix not digitized yet".""" + agent = _agent(QueryFrame(turn_type="out_of_scope")) + reply = agent.handle("Paracetamol giá bao nhiêu tiền một hộp?") + assert reply.decision == "abstain" + assert reply.reason == "out_of_scope" + assert "không chứa" in reply.answer.lower() + + +def test_out_of_scope_offtopic_question_states_supported_scope(): + agent = _agent(QueryFrame(turn_type="out_of_scope")) + reply = agent.handle("Thời tiết Hà Nội hôm nay thế nào?") + assert reply.decision == "abstain" + assert reply.reason == "out_of_scope" + assert "giá bán" not in reply.answer + assert "Dược thư" in reply.answer + + def test_veterinary_phrase_abstains_even_if_the_model_missed_it(): # The model returned an ordinary-looking frame; the keyword backstop # (`rag.policy.looks_non_human`, the same one F-02 wired server-side) diff --git a/apps/ai-service/tests/test_api.py b/apps/ai-service/tests/test_api.py index 775b677..aac4670 100644 --- a/apps/ai-service/tests/test_api.py +++ b/apps/ai-service/tests/test_api.py @@ -1,13 +1,23 @@ +from datetime import datetime, timezone + from fastapi.testclient import TestClient from adapters.prometheus import PrometheusMetrics -from adapters.postgres import FeedbackTraceNotFound +from adapters.postgres import FeedbackTraceNotFound, RetrievalTrace from config import Settings from main import create_app from rag.agent import AgentReply from rag.answer import DISCLAIMER, Citation, GroundedAnswerService from rag.metrics import TRACE_WRITE_FAILED, InMemoryMetrics -from rag.models import EvidenceDecision, QueryIntent, RetrievalResult, SubjectScope +from rag.models import ( + EvidenceDecision, + QueryIntent, + RetrievalDocument, + RetrievalResult, + SearchHit, + SourceRef, + SubjectScope, +) class FixedRouting: @@ -31,6 +41,9 @@ class MemoryTraceWriter: self.feedback.append(fields) return "feedback-1" + def list_by_conversation(self, conversation_id, limit): + return [] + def test_feedback_is_linked_to_the_answer_trace(): traces = MemoryTraceWriter() @@ -67,6 +80,76 @@ def test_feedback_rejects_an_unpersisted_trace(): assert response.json() == {"detail": "trace_not_found"} +class FakeHistoryTraceWriter(MemoryTraceWriter): + def __init__(self, by_conversation): + super().__init__() + self._by_conversation = by_conversation + self.calls = [] + + def list_by_conversation(self, conversation_id, limit): + self.calls.append((conversation_id, limit)) + return self._by_conversation.get(conversation_id, []) + + +def test_history_lists_past_queries_for_a_conversation_most_recent_first(): + when = datetime(2026, 8, 14, 10, 0, tzinfo=timezone.utc) + traces = FakeHistoryTraceWriter({ + "case-1": [ + RetrievalTrace( + trace_id="t2", query="Chống chỉ định metformin?", + subject_scope="human", intent="fact_lookup", + decision="answerable", reason="grounded_evidence_available", + resolved_drug_id="metformin", citations=(), + conversation_id="case-1", created_at=when, + ), + RetrievalTrace( + trace_id="t1", query="Chỉ định metformin?", + subject_scope="human", intent="fact_lookup", + decision="answerable", reason="grounded_evidence_available", + resolved_drug_id="metformin", citations=(), + conversation_id="case-1", created_at=when, + ), + ], + }) + app = create_app(settings=Settings(), trace_writer=traces) + + response = TestClient(app).get("/v1/rag/history", params={"conversation_id": "case-1"}) + + assert response.status_code == 200 + body = response.json() + assert [item["query"] for item in body["items"]] == [ + "Chống chỉ định metformin?", "Chỉ định metformin?", + ] + assert body["items"][0]["trace_id"] == "t2" + assert body["items"][0]["decision"] == "answerable" + assert traces.calls == [("case-1", 50)] + + +def test_history_with_empty_conversation_id_returns_no_rows_and_does_not_query(): + """An empty/missing id must not silently fall through to an unscoped + listing — there is no auth anywhere in this system to make that safe.""" + traces = FakeHistoryTraceWriter({}) + app = create_app(settings=Settings(), trace_writer=traces) + + response = TestClient(app).get("/v1/rag/history", params={"conversation_id": " "}) + + assert response.status_code == 200 + assert response.json() == {"items": []} + assert traces.calls == [] + + +def test_history_for_unknown_conversation_is_empty_not_an_error(): + traces = FakeHistoryTraceWriter({}) + app = create_app(settings=Settings(), trace_writer=traces) + + response = TestClient(app).get( + "/v1/rag/history", params={"conversation_id": "never-seen"} + ) + + assert response.status_code == 200 + assert response.json() == {"items": []} + + def test_health_and_fail_closed_rag_response_are_traced(): traces = MemoryTraceWriter() app = create_app( @@ -248,6 +331,137 @@ def test_suggest_with_no_agent_configured_returns_empty(): assert response.json() == {"suggestions": []} +class FakeSectionRetriever: + def __init__(self, sections=None, section_texts=None): + self._sections = sections or {} + self._section_texts = section_texts or {} + self.calls = [] + + def list_sections(self, drug_id): + self.calls.append(drug_id) + return self._sections.get(drug_id, []) + + def find_by_section(self, drug_id, section_key): + self.calls.append((drug_id, section_key)) + return self._section_texts.get((drug_id, section_key), []) + + +def test_list_sections_returns_the_real_per_drug_checklist(): + retriever = FakeSectionRetriever({ + "metformin": [ + ("chi_dinh", "Chỉ định"), + ("chong_chi_dinh", "Chống chỉ định"), + ], + }) + app = create_app( + settings=Settings(), trace_writer=MemoryTraceWriter(), section_retriever=retriever + ) + response = TestClient(app).get("/v1/rag/sections", params={"drug_id": "metformin"}) + + assert response.status_code == 200 + assert response.json() == { + "sections": [ + {"section_key": "chi_dinh", "section_title": "Chỉ định"}, + {"section_key": "chong_chi_dinh", "section_title": "Chống chỉ định"}, + ] + } + assert retriever.calls == ["metformin"] + + +def test_list_sections_with_no_retriever_configured_is_503_not_an_empty_list(): + """Distinct from `/suggest`'s empty-list fallback on purpose: an empty + list here would read as "this drug has zero sections", which is false — + it means the backend isn't configured at all.""" + app = create_app(settings=Settings(), trace_writer=MemoryTraceWriter()) + response = TestClient(app).get("/v1/rag/sections", params={"drug_id": "metformin"}) + + assert response.status_code == 503 + + +def _hit(text, *, part_index=0, printed_page=200, physical_page=195, quarantined=False): + return SearchHit( + document=RetrievalDocument( + doc_id=f"metformin__chong_chi_dinh__{part_index}", + drug_id="metformin", + kind="block_descriptor" if quarantined else "prose", + text=text, + section_key="chong_chi_dinh", + section_title="Chống chỉ định", + source_refs=(SourceRef( + physical_page=physical_page, precision="exact", printed_page=printed_page, + ),), + part_index=part_index, + requires_visual_check=quarantined, + ), + score=1.0, + ) + + +def test_section_text_joins_parts_in_order_with_page_provenance(): + retriever = FakeSectionRetriever(section_texts={ + ("metformin", "chong_chi_dinh"): [ + _hit("Phần một.", part_index=0, printed_page=200), + _hit("Phần hai.", part_index=1, printed_page=201), + ], + }) + app = create_app( + settings=Settings(), trace_writer=MemoryTraceWriter(), section_retriever=retriever + ) + response = TestClient(app).get( + "/v1/rag/section-text", params={"drug_id": "metformin", "section_key": "chong_chi_dinh"} + ) + + assert response.status_code == 200 + body = response.json() + assert body["section_title"] == "Chống chỉ định" + assert [p["text"] for p in body["parts"]] == ["Phần một.", "Phần hai."] + assert body["parts"][0]["printed_page_start"] == 200 + assert body["parts"][0]["is_quarantined"] is False + assert retriever.calls == [("metformin", "chong_chi_dinh")] + + +def test_section_text_flags_quarantined_parts_instead_of_treating_them_as_verbatim(): + retriever = FakeSectionRetriever(section_texts={ + ("metformin", "chong_chi_dinh"): [ + _hit( + "METFORMIN — Chống chỉ định — bảng, trang 200. Nội dung chỉ " + "tra cứu được trên ảnh trang gốc.", + quarantined=True, + ), + ], + }) + app = create_app( + settings=Settings(), trace_writer=MemoryTraceWriter(), section_retriever=retriever + ) + response = TestClient(app).get( + "/v1/rag/section-text", params={"drug_id": "metformin", "section_key": "chong_chi_dinh"} + ) + + assert response.json()["parts"][0]["is_quarantined"] is True + + +def test_section_text_with_unknown_drug_returns_empty_parts_not_an_error(): + retriever = FakeSectionRetriever() + app = create_app( + settings=Settings(), trace_writer=MemoryTraceWriter(), section_retriever=retriever + ) + response = TestClient(app).get( + "/v1/rag/section-text", params={"drug_id": "khong_ton_tai", "section_key": "chi_dinh"} + ) + + assert response.status_code == 200 + assert response.json()["parts"] == [] + + +def test_section_text_with_no_retriever_configured_is_503(): + app = create_app(settings=Settings(), trace_writer=MemoryTraceWriter()) + response = TestClient(app).get( + "/v1/rag/section-text", params={"drug_id": "metformin", "section_key": "chi_dinh"} + ) + + assert response.status_code == 503 + + # --- F-09: trace persistence is fail-open ------------------------------------ diff --git a/apps/ai-service/tests/test_citation_and_intro.py b/apps/ai-service/tests/test_citation_and_intro.py index 6ed57c8..e45332d 100644 --- a/apps/ai-service/tests/test_citation_and_intro.py +++ b/apps/ai-service/tests/test_citation_and_intro.py @@ -227,6 +227,62 @@ def test_list_mode_skips_the_sufficiency_clarify(): assert g.generated is True +def test_list_mode_prepends_a_lookup_not_recommendation_notice_block(): + """Feature-List #14: a condition/symptom -> drug list reads like a + treatment recommendation unless it is explicitly labelled as a lookup. + The notice must be a fixed block the model never writes (so it can't be + reworded or dropped), first in `blocks`, and carry the plan's + `needs_warning` flag so the UI actually renders it set apart.""" + result = _answerable(_evidence(0, 100), _evidence(1, 200)) + gen = _Generator( + {"claims": [ + {"text": "Đoạn bằng chứng 0", "citations": [1]}, + {"text": "Đoạn bằng chứng 1", "citations": [2]}, + ], "evidence_sufficient": True, "clarifying_question": None}, + ) + service = GroundedAnswerService(_Routing(result), gen) + + g = service.answer_from_result("thuốc gì trị sốt", result, list_mode=True) + + assert g.blocks[0].title == "Đọc cho đúng" + assert g.blocks[0].kind == "warning" + assert "TRA CỨU" in g.blocks[0].claims[0].text + assert "KHÔNG PHẢI" in g.blocks[0].claims[0].text + assert g.plan is not None and g.plan.needs_warning is True + # The generated claims still follow, untouched, after the fixed notice. + assert len(g.blocks) == 2 + + +def test_single_drug_answer_has_no_list_mode_notice(): + """The notice is specific to `list_mode` (multi-drug reverse lookup) — + an ordinary single-drug attribute answer must not carry it.""" + evidence = Evidence( + evidence_id="a__chong_chi_dinh__0", + matched_doc_id="a__chong_chi_dinh__0", + kind="prose", + text="Chống chỉ định của thuốc A.", + score=1.0, + source_refs=(SourceRef(physical_page=100, precision="exact", printed_page=101),), + hydrated_from_parent=False, + requires_visual_check=False, + drug_id="a", + drug_name="A", + section_key="chong_chi_dinh", + ) + result = _answerable(evidence) + gen = _Generator({ + "claims": [{"text": "Không dùng thuốc A khi mẫn cảm.", "citations": [1]}], + "evidence_sufficient": True, + "clarifying_question": None, + "quick_replies": [], + }) + service = GroundedAnswerService(_Routing(result), gen) + + g = service.answer_from_result("Chống chỉ định của A?", result, list_mode=False) + + assert all(block.title != "Đọc cho đúng" for block in g.blocks) + + def test_list_mode_rejects_a_generated_drug_outside_candidate_set(): evidence = Evidence( evidence_id="a__chi_dinh__0", diff --git a/apps/ai-service/tests/test_live_datastores.py b/apps/ai-service/tests/test_live_datastores.py index f2c11df..f307322 100644 --- a/apps/ai-service/tests/test_live_datastores.py +++ b/apps/ai-service/tests/test_live_datastores.py @@ -24,6 +24,9 @@ CONVERSATION_MIGRATION = ( FEEDBACK_MIGRATION = ( Path(__file__).resolve().parents[1] / "migrations/004_rag_answer_feedback.sql" ) +HISTORY_MIGRATION = ( + Path(__file__).resolve().parents[1] / "migrations/005_rag_trace_conversation.sql" +) class _PlumbingEmbedder: @@ -117,6 +120,7 @@ def test_real_postgres_migration_insert_and_read_back(): "postgresql://duoc_thu:duoc_thu@localhost:5432/duoc_thu" ) repository.migrate(MIGRATION) + repository.migrate(HISTORY_MIGRATION) trace_id = repository.save( query="Liều abacavir?", subject_scope="human", @@ -145,6 +149,7 @@ def test_real_postgres_feedback_upserts_against_a_persisted_trace(): ) repository.migrate(MIGRATION) repository.migrate(FEEDBACK_MIGRATION) + repository.migrate(HISTORY_MIGRATION) trace_id = repository.save( query="Gút dùng thuốc gì?", subject_scope="human", @@ -171,6 +176,43 @@ def test_real_postgres_feedback_upserts_against_a_persisted_trace(): assert second == first +def test_real_postgres_history_lists_by_conversation_most_recent_first(): + """Feature-List #25 against a real Postgres: proves the migration, + save()'s new conversation_id column, and list_by_conversation's + filter+order all actually work together, not just against fakes.""" + from adapters.postgres import PostgresTraceRepository + + repository = PostgresTraceRepository( + "postgresql://duoc_thu:duoc_thu@localhost:5432/duoc_thu" + ) + repository.migrate(MIGRATION) + repository.migrate(HISTORY_MIGRATION) + conversation_id = f"history-integration-{uuid.uuid4()}" + + first_id = repository.save( + query="Chỉ định của metformin?", subject_scope="human", intent="fact_lookup", + decision="answerable", reason="grounded_evidence_available", + resolved_drug_id="metformin", citations=(), conversation_id=conversation_id, + ) + second_id = repository.save( + query="Chống chỉ định của metformin?", subject_scope="human", intent="fact_lookup", + decision="answerable", reason="grounded_evidence_available", + resolved_drug_id="metformin", citations=(), conversation_id=conversation_id, + ) + # A different conversation must never leak into this one's history. + repository.save( + query="Liều aspirin?", subject_scope="human", intent="fact_lookup", + decision="answerable", reason="grounded_evidence_available", + resolved_drug_id="aspirin", citations=(), + conversation_id=f"other-{uuid.uuid4()}", + ) + + rows = repository.list_by_conversation(conversation_id, limit=10) + + assert [row.trace_id for row in rows] == [second_id, first_id] + assert all(row.conversation_id == conversation_id for row in rows) + + def test_real_postgres_conversation_store_round_trip(): """F-08's durable conversation history against a real Postgres, not a fake — proves `append`/`recent` actually persist and window correctly, @@ -264,6 +306,7 @@ def test_real_rag_agent_end_to_end_through_the_http_api(): "postgresql://duoc_thu:duoc_thu@localhost:5432/duoc_thu" ) traces.migrate(MIGRATION) + traces.migrate(HISTORY_MIGRATION) try: qdrant.create_collection( collection_name=collection, @@ -363,6 +406,7 @@ def test_api_round_trip_uses_qdrant_and_persists_postgres_trace(): "postgresql://duoc_thu:duoc_thu@localhost:5432/duoc_thu" ) traces.migrate(MIGRATION) + traces.migrate(HISTORY_MIGRATION) try: qdrant.create_collection( collection_name=collection, diff --git a/apps/ai-service/tests/test_qdrant_adapter.py b/apps/ai-service/tests/test_qdrant_adapter.py index d9ab52e..b3abf7c 100644 --- a/apps/ai-service/tests/test_qdrant_adapter.py +++ b/apps/ai-service/tests/test_qdrant_adapter.py @@ -243,3 +243,79 @@ def test_search_lexical_excludes_non_matching_sections(): hits = retriever.search_lexical("loét dạ dày", "aspirin", limit=5) assert hits == [] + + +def _section_meta_payload( + drug_id: str, section_key: str, display_name: str, part_index: int = 0 +) -> dict: + return { + "chunk_id": f"{drug_id}__{section_key}__{part_index}", "drug_id": drug_id, + "section_key": section_key, "section_display_name": display_name, + "part_index": part_index, "chunk_kind": "prose", "text": "nội dung", + } + + +def test_list_sections_returns_book_order_not_scroll_order(): + client = _FakeScrollClient([ + _section_meta_payload("aspirin", "qua_lieu_va_xu_tri", "Quá liều và xử trí"), + _section_meta_payload("aspirin", "chi_dinh", "Chỉ định"), + _section_meta_payload("aspirin", "ten_chung_quoc_te", "Tên chung quốc tế"), + ]) + retriever = QdrantRetriever(client, "duocthu_v1", embedder=None) + + sections = retriever.list_sections("aspirin") + + assert [key for key, _ in sections] == [ + "ten_chung_quoc_te", "chi_dinh", "qua_lieu_va_xu_tri", + ] + assert dict(sections)["chi_dinh"] == "Chỉ định" + + +def test_list_sections_dedupes_multi_part_sections(): + client = _FakeScrollClient([ + _section_meta_payload("aspirin", "chi_dinh", "Chỉ định", part_index=0), + _section_meta_payload("aspirin", "chi_dinh", "Chỉ định", part_index=1), + _section_meta_payload("aspirin", "chi_dinh", "Chỉ định", part_index=2), + ]) + retriever = QdrantRetriever(client, "duocthu_v1", embedder=None) + + sections = retriever.list_sections("aspirin") + + assert sections == [("chi_dinh", "Chỉ định")] + + +def test_list_sections_includes_quarantined_only_sections(): + """A section with no prose chunk at all (only a block_descriptor) is + still a real section of the monograph — must not be filtered out the + way `find_by_drug`'s prose-only overview deliberately is.""" + client = _FakeScrollClient([ + { + "chunk_id": "aspirin__lieu_luong_va_cach_dung__block__p1_t0", + "drug_id": "aspirin", "section_key": "lieu_luong_va_cach_dung", + "section_display_name": "Liều lượng và cách dùng", + "chunk_kind": "block_descriptor", "text": "bảng, trang 1.", + }, + ]) + retriever = QdrantRetriever(client, "duocthu_v1", embedder=None) + + sections = retriever.list_sections("aspirin") + + assert sections == [("lieu_luong_va_cach_dung", "Liều lượng và cách dùng")] + + +def test_list_sections_places_ten_thuong_mai_after_generic_name(): + """`ten_thuong_mai` isn't one of the book's own 19 numbered fields + (verified against the actual PDF, printed page 39) — placed right after + the generic name as the most natural adjacency.""" + client = _FakeScrollClient([ + _section_meta_payload("aspirin", "chi_dinh", "Chỉ định"), + _section_meta_payload("aspirin", "ten_thuong_mai", "Tên thương mại"), + _section_meta_payload("aspirin", "ten_chung_quoc_te", "Tên chung quốc tế"), + ]) + retriever = QdrantRetriever(client, "duocthu_v1", embedder=None) + + sections = retriever.list_sections("aspirin") + + assert [key for key, _ in sections] == [ + "ten_chung_quoc_te", "ten_thuong_mai", "chi_dinh", + ] diff --git a/apps/ai-service/tests/test_section_routing.py b/apps/ai-service/tests/test_section_routing.py index ef956b3..df932f1 100644 --- a/apps/ai-service/tests/test_section_routing.py +++ b/apps/ai-service/tests/test_section_routing.py @@ -177,6 +177,49 @@ class TestSectionResolver: ) +class TestSectionResolverResolveAll: + """`resolve_all` — the fix for `abstain/incomplete_answer` reproduced + live 2026-08-14 on "Chỉ định và chống chỉ định của Aspirin là gì?": + `resolve()` silently picked one section, retrieval only fetched that + one, and the still-broad question failed the completeness check against + it. These pin that a genuine two-section question reports both, while a + substring collision (the exact case `resolve()` itself guards against) + still reports only one. + """ + + def test_two_genuinely_named_sections_both_reported(self) -> None: + resolver = SectionResolver() + matches = resolver.resolve_all( + "Chỉ định và chống chỉ định của Aspirin là gì?" + ) + assert {m.section_key for m in matches} == {"chi_dinh", "chong_chi_dinh"} + + def test_substring_collision_is_not_double_counted(self) -> None: + """"chỉ định" is a literal substring of "chống chỉ định" — this must + stay a single match, exactly like `resolve()` already guarantees.""" + resolver = SectionResolver() + matches = resolver.resolve_all("Chống chỉ định của aspirin là gì?") + assert [m.section_key for m in matches] == ["chong_chi_dinh"] + + def test_three_sections_named_at_once(self) -> None: + resolver = SectionResolver() + matches = resolver.resolve_all( + "Liều dùng, chống chỉ định và tương tác thuốc của Metformin?" + ) + assert {m.section_key for m in matches} == { + "lieu_luong_va_cach_dung", "chong_chi_dinh", "tuong_tac_thuoc", + } + + def test_single_section_question_still_returns_one(self) -> None: + resolver = SectionResolver() + matches = resolver.resolve_all("Liều dùng metformin?") + assert [m.section_key for m in matches] == ["lieu_luong_va_cach_dung"] + + def test_unrecognised_question_returns_empty(self) -> None: + assert SectionResolver().resolve_all("thuốc này giá bao nhiêu") == () + assert SectionResolver().resolve_all("") == () + + class TestSectionRouting: def test_named_section_bypasses_similarity_entirely(self) -> None: retriever = SectionAwareRetriever(ALL_DOCS) diff --git a/apps/ai-service/tests/test_understanding.py b/apps/ai-service/tests/test_understanding.py index 12bfa92..c573ae7 100644 --- a/apps/ai-service/tests/test_understanding.py +++ b/apps/ai-service/tests/test_understanding.py @@ -110,6 +110,58 @@ def test_golden_named_drug_section_overrides_a_misclassified_relation_frame(): assert frame.needs_clarify is False +def test_two_sections_named_at_once_clarifies_instead_of_silently_narrowing(): + """Regression for `abstain/incomplete_answer` reproduced live 2026-08-14 + on "Chỉ định và chống chỉ định của Aspirin là gì?": the turn used to + silently collapse to whichever one section `_apply_named_drug_cues` + picked (chống chỉ định, being the longer phrase), so retrieval only + fetched that section's evidence while the question handed to generation + still promised both — a real, quote-backed completeness gap the + generator could never close. Must now clarify instead.""" + understander = LlmQueryUnderstander(_FixedLlm({ + "turn_type": "drug_attribute", + "drugs": ["paracetamol_acetaminophen"], + "unknown_drugs": [], + "attribute": "chong_chi_dinh", + "population": None, + "weight_kg": None, + "age_text": None, + "indication": None, + "needs_clarify": False, + "clarify_reason": None, + }), CATALOG, RESOLVER) + + frame = understander.understand( + "Chỉ định và chống chỉ định của Paracetamol là gì?" + ) + + assert frame.turn_type == "drug_attribute" + assert frame.drugs == ("paracetamol_acetaminophen",) + assert frame.attribute is None + assert frame.needs_clarify is True + assert set(frame.quick_replies) == {"Chỉ định", "Chống chỉ định"} + + +def test_single_section_named_is_unaffected_by_the_multi_section_clarify(): + understander = LlmQueryUnderstander(_FixedLlm({ + "turn_type": "drug_attribute", + "drugs": ["paracetamol_acetaminophen"], + "unknown_drugs": [], + "attribute": "chong_chi_dinh", + "population": None, + "weight_kg": None, + "age_text": None, + "indication": None, + "needs_clarify": False, + "clarify_reason": None, + }), CATALOG, RESOLVER) + + frame = understander.understand("Chống chỉ định của Paracetamol là gì?") + + assert frame.attribute == "chong_chi_dinh" + assert frame.needs_clarify is False + + def test_exact_candidate_does_not_repeat_the_catalog_wide_fuzzy_scan(): resolver = _FakeResolver({"metformin": "metformin"}) understander = LlmQueryUnderstander(_FixedLlm({ @@ -244,11 +296,11 @@ def test_quick_replies_are_parsed_when_the_model_offers_them(): def test_quick_replies_are_dynamic_but_bounded_before_becoming_ui_chips(): - # 19 distinct valid entries (after " Người lớn " / "người lớn" dedup - # and the non-string 12 are dropped) so the 18-item cap — one per + # 20 distinct valid entries (after " Người lớn " / "người lớn" dedup + # and the non-string 12 are dropped) so the 19-item cap — one per # monograph section, see rag/sections.py SECTION_ORDER — still trims # the last one, not just the old 4-item cap. - extra = [f"Lựa chọn {i}" for i in range(6, 20)] + extra = [f"Lựa chọn {i}" for i in range(6, 21)] understander = LlmQueryUnderstander(_FixedLlm({ "turn_type": "dosing_calc", "drugs": ["paracetamol_acetaminophen"], "unknown_drugs": [], "attribute": None, "population": None, @@ -263,11 +315,11 @@ def test_quick_replies_are_dynamic_but_bounded_before_becoming_ui_chips(): frame = understander.understand("liều paracetamol") - assert len(frame.quick_replies) == 18 + assert len(frame.quick_replies) == 19 assert frame.quick_replies[:5] == ( "Người lớn", "Trẻ em", "Phụ nữ có thai", "Người cao tuổi", "Lựa chọn thứ năm" ) - assert "Lựa chọn 19" not in frame.quick_replies + assert "Lựa chọn 20" not in frame.quick_replies def test_string_false_does_not_turn_into_a_clarification(): diff --git a/apps/web/app/_components/Sidebar.tsx b/apps/web/app/_components/Sidebar.tsx index 123a8d1..7679661 100644 --- a/apps/web/app/_components/Sidebar.tsx +++ b/apps/web/app/_components/Sidebar.tsx @@ -1,6 +1,6 @@ "use client"; -import React, { useState } from "react"; +import React, { useEffect, useState } from "react"; import { Plus, MessageSquare, @@ -11,6 +11,7 @@ import { ChevronLeft, ChevronRight, ShieldCheck, + History, Zap, } from "lucide-react"; import { cn } from "@duoc-thu/ui"; @@ -59,6 +60,12 @@ const QUICK_PROMPTS = [ }, ]; +interface HistoryItem { + trace_id: string; + query: string; + decision: string; +} + export function Sidebar({ currentSessionId, sessions, @@ -70,6 +77,32 @@ export function Sidebar({ }: SidebarProps) { const [searchTerm, setSearchTerm] = useState(""); const [isCollapsed, setIsCollapsed] = useState(false); + const [historyItems, setHistoryItems] = useState([]); + + // Feature-List #25: past queries FOR THIS SESSION, most recent first — + // click one to re-run it (via `onQuickQuery`, same path the hardcoded + // quick-prompts below already use). Deliberately re-fetched whenever the + // active session changes, not lifted to page.tsx: `Composer.tsx`'s own + // autocomplete fetch already establishes the pattern of a component + // owning its own small read, rather than everything prop-drilled down. + useEffect(() => { + if (!currentSessionId) { + setHistoryItems([]); + return; + } + let cancelled = false; + fetch(`/api/history?conversation_id=${encodeURIComponent(currentSessionId)}`) + .then((res) => (res.ok ? res.json() : { items: [] })) + .then((data) => { + if (!cancelled) setHistoryItems(Array.isArray(data?.items) ? data.items : []); + }) + .catch(() => { + if (!cancelled) setHistoryItems([]); + }); + return () => { + cancelled = true; + }; + }, [currentSessionId]); const filteredSessions = sessions.filter((s) => s.title.toLowerCase().includes(searchTerm.toLowerCase()) @@ -204,6 +237,33 @@ export function Sidebar({ }) )} + {/* Query History — real past questions for this session (Feature- + List #25), click to re-run the same question. Empty when the + session has no persisted queries yet (a brand-new session, or + history backend unavailable) — the quick-prompt templates below + still work as a starting point either way. */} + {historyItems.length > 0 && ( +
+
+ + Lịch sử câu hỏi +
+
+ {historyItems.map((item) => ( + + ))} +
+
+ )} + {/* Quick Prompts Section */}
diff --git a/apps/web/app/api/history/route.ts b/apps/web/app/api/history/route.ts new file mode 100644 index 0000000..986b889 --- /dev/null +++ b/apps/web/app/api/history/route.ts @@ -0,0 +1,38 @@ +import { NextResponse } from "next/server"; + +export const runtime = "nodejs"; + +const API_GATEWAY_URL = + process.env.API_GATEWAY_URL ?? process.env.AI_SERVICE_URL ?? "http://localhost:8000"; + +export async function GET(request: Request) { + const { searchParams } = new URL(request.url); + const conversationId = searchParams.get("conversation_id")?.trim() || ""; + + if (!conversationId) { + return NextResponse.json({ items: [] }); + } + + try { + const targetUrl = API_GATEWAY_URL.includes("/v1/rag") + ? `${API_GATEWAY_URL.replace(/\/query$/, "/history")}?conversation_id=${encodeURIComponent(conversationId)}` + : `${API_GATEWAY_URL}/v1/rag/history?conversation_id=${encodeURIComponent(conversationId)}`; + + const upstream = await fetch(targetUrl, { + method: "GET", + headers: { + "X-Client-Version": "1.0.0", + }, + cache: "no-store", + }); + + if (!upstream.ok) { + return NextResponse.json({ items: [] }); + } + + const data = await upstream.json(); + return NextResponse.json(data); + } catch { + return NextResponse.json({ items: [] }); + } +} diff --git a/apps/web/app/page.tsx b/apps/web/app/page.tsx index 54ee768..8dc09e4 100644 --- a/apps/web/app/page.tsx +++ b/apps/web/app/page.tsx @@ -12,6 +12,28 @@ function createSessionId() { return `session-${crypto.randomUUID()}`; } +// Feature-List #25: the session id is the key `/v1/rag/history` scopes its +// listing by, so persisting it is a prerequisite for history surviving a +// refresh at all — not just a UX nicety. Chat MESSAGE CONTENT is +// deliberately NOT persisted here (server never stores generated answer +// text/blocks either, only decision/reason metadata — see +// `PostgresTraceRepository.list_by_conversation`'s docstring): a resumed +// session's transcript starts empty, matching the spec's own "re-run the +// query" wording rather than "replay the old answer". +const SESSIONS_STORAGE_KEY = "dt_sessions"; +const CURRENT_SESSION_STORAGE_KEY = "dt_current_session_id"; + +function loadStoredSessions(): ChatSession[] | null { + try { + const raw = localStorage.getItem(SESSIONS_STORAGE_KEY); + if (!raw) return null; + const parsed = JSON.parse(raw); + return Array.isArray(parsed) && parsed.length > 0 ? parsed : null; + } catch { + return null; + } +} + export default function ChatPage() { const [sessions, setSessions] = useState([]); const [currentSessionId, setCurrentSessionId] = useState(""); @@ -19,12 +41,49 @@ export default function ChatPage() { const [queryOverride, setQueryOverride] = useState<{ text: string; token: number } | null>(null); useEffect(() => { + const stored = loadStoredSessions(); + if (stored) { + const storedCurrent = (() => { + try { + return localStorage.getItem(CURRENT_SESSION_STORAGE_KEY); + } catch { + return null; + } + })(); + setSessions(stored); + setMessagesBySession(Object.fromEntries(stored.map((s) => [s.id, []]))); + setCurrentSessionId(stored.find((s) => s.id === storedCurrent)?.id ?? stored[0].id); + return; + } const id = createSessionId(); setSessions([{ id, title: "Phiên tra cứu mới", updatedAt: new Date().toISOString() }]); setMessagesBySession({ [id]: [] }); setCurrentSessionId(id); }, []); + // Persist whenever the session list / active session changes — covers + // new/deleted sessions and switching between them. Guarded on non-empty + // so the pre-mount-effect empty state never overwrites a real stored + // list with `[]`. + useEffect(() => { + if (sessions.length === 0) return; + try { + localStorage.setItem(SESSIONS_STORAGE_KEY, JSON.stringify(sessions)); + } catch { + // Storage full or unavailable (e.g. private browsing) — the session + // list simply won't survive a refresh this time. + } + }, [sessions]); + + useEffect(() => { + if (!currentSessionId) return; + try { + localStorage.setItem(CURRENT_SESSION_STORAGE_KEY, currentSessionId); + } catch { + // ignore — same fallback as above + } + }, [currentSessionId]); + // Citation & Evidence Panel State const [citations, setCitations] = useState([]); const [activeCitationIndex, setActiveCitationIndex] = useState(null); diff --git a/coordination/CLAUDE_CLAIM_2026-08-14.md b/coordination/CLAUDE_CLAIM_2026-08-14.md new file mode 100644 index 0000000..ac71c1d --- /dev/null +++ b/coordination/CLAUDE_CLAIM_2026-08-14.md @@ -0,0 +1,64 @@ +# Claude ownership claim — 2026-08-14 + +Working from `Feature-List-AI-Duoc-thu-V1.md` (new file, owner-added this +session): a live 17-query audit against production found 5/26 features fully +passing, plus one reproducible bug (`incomplete_answer` on any two-section +question, e.g. "Chỉ định và chống chỉ định của Aspirin"). Full audit + plan +were reviewed and approved by the owner before starting +(`C:\Users\vuxba\.claude\plans\snug-sparking-goose.md`, not in-repo). + +Checked `WORK_SPLIT_2026-08-10.md` (Codex owns `rag/**`) against the more +recent per-session claims: `CLAUDE_CLAIM_2026-08-11.md` and +`CLAUDE_CLAIM_2026-08-12.md` both show Claude editing `rag/answer.py` / +`rag/agent.py` directly after that split, each time with an explicit claim +and an explicit "not touching" list rather than treating the 08-10 split as +still absolute. Following that same practice here. + +`git log` confirms **production is on current HEAD** (`4f867aa`, "Deploy to +production" succeeded 2026-08-14T04:59:27Z) — three same-day/previous-day +commits (`3c6262e`, `623fd62`, `f662835`) already fixed adjacent bugs; none +of them touch the scope below. + +## Work split across 3 branches/PRs this session + +- **PR1 (this claim, branch `agent/fix-clarify-and-safety-messaging`)**: the + `incomplete_answer` bug fix + 3 small response-text/UI fixes. +- **PR2 (next)**: two new read-only endpoints — list sections per drug, + verbatim section text. +- **PR3 (last)**: query-history persistence (new Postgres column via + additive migration, new endpoint, frontend wiring). + +Each will get its own claim update / follow-up entry here as it starts. + +## PR1 — files claimed now + +- `apps/ai-service/rag/sections.py` — add a multi-match helper only; + `SectionResolver.resolve()`'s existing single-match behavior is + unchanged (other callers depend on "or nothing at all"). +- `apps/ai-service/rag/understanding.py` — detect when a turn names ≥2 + distinct sections, route to the existing `missing_attribute` clarify + instead of silently collapsing to one and generating a wrong-scope answer. +- `apps/ai-service/rag/agent.py` — vary the `out_of_scope` message + (price/vendor/brand vs. genuinely off-topic) instead of one shared string. +- `apps/ai-service/rag/answer.py` — fixed, non-generated notice block on + `list_mode` (condition→drug) answers, same "module constant" pattern as + the existing `DISCLAIMER`. +- `apps/ai-service/tests/test_understanding.py`, + `apps/ai-service/tests/test_section_routing.py`, + `apps/ai-service/tests/test_agent.py`, + `apps/ai-service/tests/test_grounded_generation.py` — new/updated tests + for the above. +- `packages/ui/src/ChatBubble.tsx` — render the existing + `message.disclaimer` field per-message (data already flows end-to-end, + just never rendered). + +**Not touching**: `rag/service.py`, `rag/routing.py`, retrieval adapters, +`ingestion/`, or anything under the pre-existing dirty `docs/` deletion +block already in the working tree (unrelated restructuring, left alone). + +## Local-only, no deploy + +Per standing instruction: build + test locally only. No push, no PR open/merge, +no deploy without the owner's explicit go for each PR. Postgres backup + +`rollback.yml` awareness apply to PR3 (the only one with a schema change), +noted in the plan file. diff --git a/packages/ui/src/ChatBubble.tsx b/packages/ui/src/ChatBubble.tsx index 27d1e03..68ca0fe 100644 --- a/packages/ui/src/ChatBubble.tsx +++ b/packages/ui/src/ChatBubble.tsx @@ -306,6 +306,19 @@ export function ChatBubble({ : renderStructuredContent(message.content, message.citations)}
+ {/* Per-message disclaimer — `message.disclaimer` is populated by the + backend on every message (including abstain/clarify), never by + the model itself (`rag/answer.py`'s DISCLAIMER constant); this was + already flowing end-to-end but nothing rendered it, so it only + ever surfaced as one page-level banner (`DisclaimerBanner`), not + per answer as F-HT #22 ("mọi câu trả lời y tế đều có") requires. */} + {message.disclaimer && ( +

+ + {message.disclaimer} +

+ )} + {/* Quick-reply chips — only for a clarify turn the model gave a few natural discrete answers to; free text always still works. */} {onQuickReply && message.quickReplies && message.quickReplies.length > 0 && (